All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64
@ 2022-06-15 23:03 Yonghong Song
  2022-06-15 23:03 ` [PATCH dwarves v2 1/2] libbpf: Sync with latest libbpf repo Yonghong Song
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Yonghong Song @ 2022-06-15 23:03 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team

Add support for enum64. For 64-bit enumerator value,
previously, the value is truncated into 32bit, e.g.,
for the following enum in linux uapi bpf.h,
  enum {
        BPF_F_INDEX_MASK                = 0xffffffffULL,
        BPF_F_CURRENT_CPU               = BPF_F_INDEX_MASK,
  /* BPF_FUNC_perf_event_output for sk_buff input context. */
        BPF_F_CTXLEN_MASK               = (0xfffffULL << 32),
  };

BPF_F_CTXLEN_MASK will be encoded with 0 with BTF_KIND_ENUM
after pahole dwarf-to-btf conversion.
With this patch, the BPF_F_CTXLEN_MASK will be encoded properly
with BTF_KIND_ENUM64.

This patch is on top of tmp.master since tmp.master has not
been sync'ed with master branch yet.

Changelogs:
  v1 -> v2:
    - Add flag --skip_encoding_btf_enum64 to disable newly-added functionality.

Yonghong Song (2):
  libbpf: Sync with latest libbpf repo
  btf: Support BTF_KIND_ENUM64

 btf_encoder.c     | 65 +++++++++++++++++++++++++++++++++++------------
 btf_encoder.h     |  2 +-
 dwarf_loader.c    | 12 +++++++++
 dwarves.h         |  4 ++-
 dwarves_fprintf.c |  6 ++++-
 lib/bpf           |  2 +-
 pahole.c          | 10 +++++++-
 7 files changed, 80 insertions(+), 21 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH dwarves v2 1/2] libbpf: Sync with latest libbpf repo
  2022-06-15 23:03 [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Yonghong Song
@ 2022-06-15 23:03 ` Yonghong Song
  2022-06-15 23:03 ` [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64 Yonghong Song
  2022-06-28  9:51 ` [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Jiri Olsa
  2 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2022-06-15 23:03 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team

Sync up to commit
  645500dd7d2d ci: blacklist mptcp test on s390x

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 lib/bpf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bpf b/lib/bpf
index 87dff0a..645500d 160000
--- a/lib/bpf
+++ b/lib/bpf
@@ -1 +1 @@
-Subproject commit 87dff0a2c775c5943ca9233e69c81a25f2ed1a77
+Subproject commit 645500dd7d2d6b5bb76e4c0375d597d4f0c4814e
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64
  2022-06-15 23:03 [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Yonghong Song
  2022-06-15 23:03 ` [PATCH dwarves v2 1/2] libbpf: Sync with latest libbpf repo Yonghong Song
@ 2022-06-15 23:03 ` Yonghong Song
  2022-06-27 22:30   ` Andrii Nakryiko
  2022-06-28  9:51 ` [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Jiri Olsa
  2 siblings, 1 reply; 10+ messages in thread
From: Yonghong Song @ 2022-06-15 23:03 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team

BTF_KIND_ENUM64 is supported with latest libbpf, which
supports 64-bit enum values. Latest libbpf also supports
signedness for enum values. Add enum64 support in
dwarf-to-btf conversion.

The following is an example of new encoding which covers
signed/unsigned enum64/enum variations.

  $cat t.c
  enum { /* signed, enum64 */
    A = -1,
    B = 0xffffffff,
  } g1;
  enum { /* unsigned, enum64 */
    C = 1,
    D = 0xfffffffff,
  } g2;
  enum { /* signed, enum */
    E = -1,
    F = 0xfffffff,
  } g3;
  enum { /* unsigned, enum */
    G = 1,
    H = 0xfffffff,
  } g4;
  $ clang -g -c t.c
  $ pahole -JV t.o
  btf_encoder__new: 't.o' doesn't have '.data..percpu' section
  Found 0 per-CPU variables!
  File t.o:
  [1] ENUM64 (anon) size=8
          A val=-1
          B val=4294967295
  [2] INT long size=8 nr_bits=64 encoding=SIGNED
  [3] ENUM64 (anon) size=8
          C val=1
          D val=68719476735
  [4] INT unsigned long size=8 nr_bits=64 encoding=(none)
  [5] ENUM (anon) size=4
          E val=-1
          F val=268435455
  [6] INT int size=4 nr_bits=32 encoding=SIGNED
  [7] ENUM (anon) size=4
          G val=1
          H val=268435455
  [8] INT unsigned int size=4 nr_bits=32 encoding=(none)

With the flag to skip enum64 encoding,

  $ pahole -JV t.o --skip_encoding_btf_enum64
  btf_encoder__new: 't.o' doesn't have '.data..percpu' section
  Found 0 per-CPU variables!
  File t.o:
  [1] ENUM (anon) size=8
        A val=4294967295
        B val=4294967295
  [2] INT long size=8 nr_bits=64 encoding=SIGNED
  [3] ENUM (anon) size=8
        C val=1
        D val=4294967295
  [4] INT unsigned long size=8 nr_bits=64 encoding=(none)
  [5] ENUM (anon) size=4
        E val=4294967295
        F val=268435455
  [6] INT int size=4 nr_bits=32 encoding=SIGNED
  [7] ENUM (anon) size=4
        G val=1
        H val=268435455
  [8] INT unsigned int size=4 nr_bits=32 encoding=(none)

In the above btf encoding without enum64, all enum types
with the same type size as the corresponding enum64. All these
enum types have unsigned type (kflag = 0) which is required
before kernel enum64 support.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 btf_encoder.c     | 65 +++++++++++++++++++++++++++++++++++------------
 btf_encoder.h     |  2 +-
 dwarf_loader.c    | 12 +++++++++
 dwarves.h         |  4 ++-
 dwarves_fprintf.c |  6 ++++-
 pahole.c          | 10 +++++++-
 6 files changed, 79 insertions(+), 20 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 9e708e4..96de54c 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -144,6 +144,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
 	[BTF_KIND_FLOAT]        = "FLOAT",
 	[BTF_KIND_DECL_TAG]     = "DECL_TAG",
 	[BTF_KIND_TYPE_TAG]     = "TYPE_TAG",
+	[BTF_KIND_ENUM64]	= "ENUM64",
 };
 
 static const char *btf__printable_name(const struct btf *btf, uint32_t offset)
@@ -490,34 +491,64 @@ static int32_t btf_encoder__add_struct(struct btf_encoder *encoder, uint8_t kind
 	return id;
 }
 
-static int32_t btf_encoder__add_enum(struct btf_encoder *encoder, const char *name, uint32_t bit_size)
+static int32_t btf_encoder__add_enum(struct btf_encoder *encoder, const char *name, uint32_t bit_size,
+				     bool is_signed, bool no_enum64)
 {
 	struct btf *btf = encoder->btf;
 	const struct btf_type *t;
 	int32_t id, size;
+	bool is_enum32;
 
 	size = BITS_ROUNDUP_BYTES(bit_size);
-	id = btf__add_enum(btf, name, size);
+	is_enum32 = size <= 4 || no_enum64;
+	if (is_enum32)
+		id = btf__add_enum(btf, name, size);
+	else
+		id = btf__add_enum64(btf, name, size, is_signed);
 	if (id > 0) {
 		t = btf__type_by_id(btf, id);
 		btf_encoder__log_type(encoder, t, false, true, "size=%u", t->size);
 	} else {
-		btf__log_err(btf, BTF_KIND_ENUM, name, true,
+		btf__log_err(btf, is_enum32 ? BTF_KIND_ENUM : BTF_KIND_ENUM64, name, true,
 			      "size=%u Error emitting BTF type", size);
 	}
 	return id;
 }
 
-static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int32_t value)
+static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int64_t value,
+				     bool is_signed, bool is_enum64, bool no_enum64)
 {
-	int err = btf__add_enum_value(encoder->btf, name, value);
+	const char *fmt_str;
+	int err;
+
+	/* If enum64 is not allowed, generate enum32 with unsigned int value. In enum64-supported
+	 * libbpf library, btf__add_enum_value() will set the kflag (sign bit) in common_type
+	 * if the value is negative.
+	 */
+	if (no_enum64)
+		err = btf__add_enum_value(encoder->btf, name, (uint32_t)value);
+	else if (is_enum64)
+		err = btf__add_enum64_value(encoder->btf, name, value);
+	else
+		err = btf__add_enum_value(encoder->btf, name, value);
 
 	if (!err) {
-		if (encoder->verbose)
-			printf("\t%s val=%d\n", name, value);
+		if (encoder->verbose) {
+			if (no_enum64) {
+				printf("\t%s val=%u\n", name, (uint32_t)value);
+			} else {
+				fmt_str = is_signed ? "\t%s val=%lld\n" : "\t%s val=%llu\n";
+				printf(fmt_str, name, (unsigned long long)value);
+			}
+		}
 	} else {
-		fprintf(stderr, "\t%s val=%d Error emitting BTF enum value\n",
-			name, value);
+		if (no_enum64) {
+			fprintf(stderr, "\t%s val=%u Error emitting BTF enum value\n", name, (uint32_t)value);
+		} else {
+			fmt_str = is_signed ? "\t%s val=%lld Error emitting BTF enum value\n"
+					    : "\t%s val=%llu Error emitting BTF enum value\n";
+			fprintf(stderr, fmt_str, name, (unsigned long long)value);
+		}
 	}
 	return err;
 }
@@ -844,27 +875,29 @@ static uint32_t array_type__nelems(struct tag *tag)
 	return nelem;
 }
 
-static int32_t btf_encoder__add_enum_type(struct btf_encoder *encoder, struct tag *tag)
+static int32_t btf_encoder__add_enum_type(struct btf_encoder *encoder, struct tag *tag, bool no_enum64)
 {
 	struct type *etype = tag__type(tag);
 	struct enumerator *pos;
 	const char *name = type__name(etype);
 	int32_t type_id;
 
-	type_id = btf_encoder__add_enum(encoder, name, etype->size);
+	type_id = btf_encoder__add_enum(encoder, name, etype->size, etype->is_signed_enum, no_enum64);
 	if (type_id < 0)
 		return type_id;
 
 	type__for_each_enumerator(etype, pos) {
 		name = enumerator__name(pos);
-		if (btf_encoder__add_enum_val(encoder, name, pos->value))
+		if (btf_encoder__add_enum_val(encoder, name, pos->value, etype->is_signed_enum,
+					      etype->size > 32, no_enum64))
 			return -1;
 	}
 
 	return type_id;
 }
 
-static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag, uint32_t type_id_off)
+static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag, uint32_t type_id_off,
+				   struct conf_load *conf_load)
 {
 	/* single out type 0 as it represents special type "void" */
 	uint32_t ref_type_id = tag->type == 0 ? 0 : type_id_off + tag->type;
@@ -903,7 +936,7 @@ static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag,
 		encoder->need_index_type = true;
 		return btf_encoder__add_array(encoder, ref_type_id, encoder->array_index_id, array_type__nelems(tag));
 	case DW_TAG_enumeration_type:
-		return btf_encoder__add_enum_type(encoder, tag);
+		return btf_encoder__add_enum_type(encoder, tag, conf_load->skip_encoding_btf_enum64);
 	case DW_TAG_subroutine_type:
 		return btf_encoder__add_func_proto(encoder, tag__ftype(tag), type_id_off);
 	default:
@@ -1422,7 +1455,7 @@ void btf_encoder__delete(struct btf_encoder *encoder)
 	free(encoder);
 }
 
-int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu)
+int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct conf_load *conf_load)
 {
 	uint32_t type_id_off = btf__type_cnt(encoder->btf) - 1;
 	struct llvm_annotation *annot;
@@ -1446,7 +1479,7 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu)
 	}
 
 	cu__for_each_type(cu, core_id, pos) {
-		btf_type_id = btf_encoder__encode_tag(encoder, pos, type_id_off);
+		btf_type_id = btf_encoder__encode_tag(encoder, pos, type_id_off, conf_load);
 
 		if (btf_type_id < 0 ||
 		    tag__check_id_drift(pos, core_id, btf_type_id, type_id_off)) {
diff --git a/btf_encoder.h b/btf_encoder.h
index 339fae2..a65120c 100644
--- a/btf_encoder.h
+++ b/btf_encoder.h
@@ -21,7 +21,7 @@ void btf_encoder__delete(struct btf_encoder *encoder);
 
 int btf_encoder__encode(struct btf_encoder *encoder);
 
-int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu);
+int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct conf_load *conf_load);
 
 void btf_encoders__add(struct list_head *encoders, struct btf_encoder *encoder);
 
diff --git a/dwarf_loader.c b/dwarf_loader.c
index a0d964b..4767602 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -632,6 +632,18 @@ static void type__init(struct type *type, Dwarf_Die *die, struct cu *cu, struct
 	type->resized		 = 0;
 	type->nr_members	 = 0;
 	type->nr_static_members	 = 0;
+	type->is_signed_enum	 = 0;
+
+	Dwarf_Attribute attr;
+	if (dwarf_attr(die, DW_AT_type, &attr) != NULL) {
+		Dwarf_Die type_die;
+		if (dwarf_formref_die(&attr, &type_die) != NULL) {
+			uint64_t encoding = attr_numeric(&type_die, DW_AT_encoding);
+
+			if (encoding == DW_ATE_signed || encoding == DW_ATE_signed_char)
+				type->is_signed_enum = 1;
+		}
+	}
 }
 
 static struct type *type__new(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
diff --git a/dwarves.h b/dwarves.h
index 4d0e4b6..bec9f08 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -65,6 +65,7 @@ struct conf_load {
 	bool			skip_encoding_btf_decl_tag;
 	bool			skip_missing;
 	bool			skip_encoding_btf_type_tag;
+	bool			skip_encoding_btf_enum64;
 	uint8_t			hashtable_bits;
 	uint8_t			max_hashtable_bits;
 	uint16_t		kabi_prefix_len;
@@ -1046,6 +1047,7 @@ struct type {
 	uint8_t		 definition_emitted:1;
 	uint8_t		 fwd_decl_emitted:1;
 	uint8_t		 resized:1;
+	uint8_t		 is_signed_enum:1;
 };
 
 void __type__init(struct type *type);
@@ -1365,7 +1367,7 @@ static inline struct string_type *tag__string_type(const struct tag *tag)
 struct enumerator {
 	struct tag	 tag;
 	const char	 *name;
-	uint32_t	 value;
+	uint64_t	 value;
 	struct tag_cu	 type_enum; // To cache the type_enum searches
 };
 
diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
index 2cec584..ce64c79 100644
--- a/dwarves_fprintf.c
+++ b/dwarves_fprintf.c
@@ -437,7 +437,11 @@ size_t enumeration__fprintf(const struct tag *tag, const struct conf_fprintf *co
 	type__for_each_enumerator(type, pos) {
 		printed += fprintf(fp, "%.*s\t%-*s = ", indent, tabs,
 				   max_entry_name_len, enumerator__name(pos));
-		printed += fprintf(fp, conf->hex_fmt ?  "%#x" : "%u", pos->value);
+		if (conf->hex_fmt)
+			printed += fprintf(fp, "%#llx", (unsigned long long)pos->value);
+		else
+			printed += fprintf(fp, type->is_signed_enum ?  "%lld" : "%llu",
+					   (unsigned long long)pos->value);
 		printed += fprintf(fp, ",\n");
 	}
 
diff --git a/pahole.c b/pahole.c
index 78caa08..e87d9a4 100644
--- a/pahole.c
+++ b/pahole.c
@@ -1220,6 +1220,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version;
 #define ARGP_compile		   334
 #define ARGP_languages		   335
 #define ARGP_languages_exclude	   336
+#define ARGP_skip_encoding_btf_enum64 337
 
 static const struct argp_option pahole__options[] = {
 	{
@@ -1622,6 +1623,11 @@ static const struct argp_option pahole__options[] = {
 		.arg  = "LANGUAGES",
 		.doc  = "Don't consider compilation units written in these languages"
 	},
+	{
+		.name = "skip_encoding_btf_enum64",
+		.key  = ARGP_skip_encoding_btf_enum64,
+		.doc  = "Do not encode ENUM64sin BTF."
+	},
 	{
 		.name = NULL,
 	}
@@ -1787,6 +1793,8 @@ static error_t pahole__options_parser(int key, char *arg,
 		/* fallthru */
 	case ARGP_languages:
 		languages.str = arg;			break;
+	case ARGP_skip_encoding_btf_enum64:
+		conf_load.skip_encoding_btf_enum64 = true;	break;
 	default:
 		return ARGP_ERR_UNKNOWN;
 	}
@@ -3067,7 +3075,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
 			encoder = btf_encoder;
 		}
 
-		if (btf_encoder__encode_cu(encoder, cu)) {
+		if (btf_encoder__encode_cu(encoder, cu, conf_load)) {
 			fprintf(stderr, "Encountered error while encoding BTF.\n");
 			exit(1);
 		}
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64
  2022-06-15 23:03 ` [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64 Yonghong Song
@ 2022-06-27 22:30   ` Andrii Nakryiko
  2022-06-28  6:19     ` Yonghong Song
  0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2022-06-27 22:30 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Andrii Nakryiko, bpf, Daniel Borkmann, Kernel Team

On Wed, Jun 15, 2022 at 4:03 PM Yonghong Song <yhs@fb.com> wrote:
>
> BTF_KIND_ENUM64 is supported with latest libbpf, which
> supports 64-bit enum values. Latest libbpf also supports
> signedness for enum values. Add enum64 support in
> dwarf-to-btf conversion.
>
> The following is an example of new encoding which covers
> signed/unsigned enum64/enum variations.
>
>   $cat t.c
>   enum { /* signed, enum64 */
>     A = -1,
>     B = 0xffffffff,
>   } g1;
>   enum { /* unsigned, enum64 */
>     C = 1,
>     D = 0xfffffffff,
>   } g2;
>   enum { /* signed, enum */
>     E = -1,
>     F = 0xfffffff,
>   } g3;
>   enum { /* unsigned, enum */
>     G = 1,
>     H = 0xfffffff,
>   } g4;
>   $ clang -g -c t.c
>   $ pahole -JV t.o
>   btf_encoder__new: 't.o' doesn't have '.data..percpu' section
>   Found 0 per-CPU variables!
>   File t.o:
>   [1] ENUM64 (anon) size=8
>           A val=-1
>           B val=4294967295
>   [2] INT long size=8 nr_bits=64 encoding=SIGNED
>   [3] ENUM64 (anon) size=8
>           C val=1
>           D val=68719476735
>   [4] INT unsigned long size=8 nr_bits=64 encoding=(none)
>   [5] ENUM (anon) size=4
>           E val=-1
>           F val=268435455
>   [6] INT int size=4 nr_bits=32 encoding=SIGNED
>   [7] ENUM (anon) size=4
>           G val=1
>           H val=268435455
>   [8] INT unsigned int size=4 nr_bits=32 encoding=(none)
>
> With the flag to skip enum64 encoding,
>
>   $ pahole -JV t.o --skip_encoding_btf_enum64
>   btf_encoder__new: 't.o' doesn't have '.data..percpu' section
>   Found 0 per-CPU variables!
>   File t.o:
>   [1] ENUM (anon) size=8
>         A val=4294967295
>         B val=4294967295
>   [2] INT long size=8 nr_bits=64 encoding=SIGNED
>   [3] ENUM (anon) size=8
>         C val=1
>         D val=4294967295
>   [4] INT unsigned long size=8 nr_bits=64 encoding=(none)
>   [5] ENUM (anon) size=4
>         E val=4294967295
>         F val=268435455
>   [6] INT int size=4 nr_bits=32 encoding=SIGNED
>   [7] ENUM (anon) size=4
>         G val=1
>         H val=268435455
>   [8] INT unsigned int size=4 nr_bits=32 encoding=(none)
>
> In the above btf encoding without enum64, all enum types
> with the same type size as the corresponding enum64. All these
> enum types have unsigned type (kflag = 0) which is required
> before kernel enum64 support.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  btf_encoder.c     | 65 +++++++++++++++++++++++++++++++++++------------
>  btf_encoder.h     |  2 +-
>  dwarf_loader.c    | 12 +++++++++
>  dwarves.h         |  4 ++-
>  dwarves_fprintf.c |  6 ++++-
>  pahole.c          | 10 +++++++-
>  6 files changed, 79 insertions(+), 20 deletions(-)
>

Sorry for late review, I don't always catch up on emails from older
emails first :(

[...]

>         size = BITS_ROUNDUP_BYTES(bit_size);
> -       id = btf__add_enum(btf, name, size);
> +       is_enum32 = size <= 4 || no_enum64;
> +       if (is_enum32)
> +               id = btf__add_enum(btf, name, size);
> +       else
> +               id = btf__add_enum64(btf, name, size, is_signed);
>         if (id > 0) {
>                 t = btf__type_by_id(btf, id);
>                 btf_encoder__log_type(encoder, t, false, true, "size=%u", t->size);
>         } else {
> -               btf__log_err(btf, BTF_KIND_ENUM, name, true,
> +               btf__log_err(btf, is_enum32 ? BTF_KIND_ENUM : BTF_KIND_ENUM64, name, true,
>                               "size=%u Error emitting BTF type", size);
>         }
>         return id;
>  }
>
> -static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int32_t value)
> +static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int64_t value,
> +                                    bool is_signed, bool is_enum64, bool no_enum64)

It was quite confusing to see "is_enum64" and "no_enum64" as arguments
to the same function :)

I'll let Arnaldo decide for himself, but I think it would be cleaner
to pass such configuration switches as fields in struct btf_encoder
itself and just check such flags from relevant btf_encoder__add_xxx()
functions. Such flags are global by nature, so it seems fitting.

But other than that looks good to me.

Acked-by: Andrii Nakryiko <andrii@kernel.org>

>  {
> -       int err = btf__add_enum_value(encoder->btf, name, value);
> +       const char *fmt_str;
> +       int err;
> +
> +       /* If enum64 is not allowed, generate enum32 with unsigned int value. In enum64-supported
> +        * libbpf library, btf__add_enum_value() will set the kflag (sign bit) in common_type
> +        * if the value is negative.
> +        */
> +       if (no_enum64)
> +               err = btf__add_enum_value(encoder->btf, name, (uint32_t)value);
> +       else if (is_enum64)
> +               err = btf__add_enum64_value(encoder->btf, name, value);
> +       else
> +               err = btf__add_enum_value(encoder->btf, name, value);

[...]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64
  2022-06-27 22:30   ` Andrii Nakryiko
@ 2022-06-28  6:19     ` Yonghong Song
  0 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2022-06-28  6:19 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Andrii Nakryiko, bpf, Daniel Borkmann, Kernel Team



On 6/27/22 3:30 PM, Andrii Nakryiko wrote:
> On Wed, Jun 15, 2022 at 4:03 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> BTF_KIND_ENUM64 is supported with latest libbpf, which
>> supports 64-bit enum values. Latest libbpf also supports
>> signedness for enum values. Add enum64 support in
>> dwarf-to-btf conversion.
>>
>> The following is an example of new encoding which covers
>> signed/unsigned enum64/enum variations.
>>
>>    $cat t.c
>>    enum { /* signed, enum64 */
>>      A = -1,
>>      B = 0xffffffff,
>>    } g1;
>>    enum { /* unsigned, enum64 */
>>      C = 1,
>>      D = 0xfffffffff,
>>    } g2;
>>    enum { /* signed, enum */
>>      E = -1,
>>      F = 0xfffffff,
>>    } g3;
>>    enum { /* unsigned, enum */
>>      G = 1,
>>      H = 0xfffffff,
>>    } g4;
>>    $ clang -g -c t.c
>>    $ pahole -JV t.o
>>    btf_encoder__new: 't.o' doesn't have '.data..percpu' section
>>    Found 0 per-CPU variables!
>>    File t.o:
>>    [1] ENUM64 (anon) size=8
>>            A val=-1
>>            B val=4294967295
>>    [2] INT long size=8 nr_bits=64 encoding=SIGNED
>>    [3] ENUM64 (anon) size=8
>>            C val=1
>>            D val=68719476735
>>    [4] INT unsigned long size=8 nr_bits=64 encoding=(none)
>>    [5] ENUM (anon) size=4
>>            E val=-1
>>            F val=268435455
>>    [6] INT int size=4 nr_bits=32 encoding=SIGNED
>>    [7] ENUM (anon) size=4
>>            G val=1
>>            H val=268435455
>>    [8] INT unsigned int size=4 nr_bits=32 encoding=(none)
>>
>> With the flag to skip enum64 encoding,
>>
>>    $ pahole -JV t.o --skip_encoding_btf_enum64
>>    btf_encoder__new: 't.o' doesn't have '.data..percpu' section
>>    Found 0 per-CPU variables!
>>    File t.o:
>>    [1] ENUM (anon) size=8
>>          A val=4294967295
>>          B val=4294967295
>>    [2] INT long size=8 nr_bits=64 encoding=SIGNED
>>    [3] ENUM (anon) size=8
>>          C val=1
>>          D val=4294967295
>>    [4] INT unsigned long size=8 nr_bits=64 encoding=(none)
>>    [5] ENUM (anon) size=4
>>          E val=4294967295
>>          F val=268435455
>>    [6] INT int size=4 nr_bits=32 encoding=SIGNED
>>    [7] ENUM (anon) size=4
>>          G val=1
>>          H val=268435455
>>    [8] INT unsigned int size=4 nr_bits=32 encoding=(none)
>>
>> In the above btf encoding without enum64, all enum types
>> with the same type size as the corresponding enum64. All these
>> enum types have unsigned type (kflag = 0) which is required
>> before kernel enum64 support.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   btf_encoder.c     | 65 +++++++++++++++++++++++++++++++++++------------
>>   btf_encoder.h     |  2 +-
>>   dwarf_loader.c    | 12 +++++++++
>>   dwarves.h         |  4 ++-
>>   dwarves_fprintf.c |  6 ++++-
>>   pahole.c          | 10 +++++++-
>>   6 files changed, 79 insertions(+), 20 deletions(-)
>>
> 
> Sorry for late review, I don't always catch up on emails from older
> emails first :(
> 
> [...]
> 
>>          size = BITS_ROUNDUP_BYTES(bit_size);
>> -       id = btf__add_enum(btf, name, size);
>> +       is_enum32 = size <= 4 || no_enum64;
>> +       if (is_enum32)
>> +               id = btf__add_enum(btf, name, size);
>> +       else
>> +               id = btf__add_enum64(btf, name, size, is_signed);
>>          if (id > 0) {
>>                  t = btf__type_by_id(btf, id);
>>                  btf_encoder__log_type(encoder, t, false, true, "size=%u", t->size);
>>          } else {
>> -               btf__log_err(btf, BTF_KIND_ENUM, name, true,
>> +               btf__log_err(btf, is_enum32 ? BTF_KIND_ENUM : BTF_KIND_ENUM64, name, true,
>>                                "size=%u Error emitting BTF type", size);
>>          }
>>          return id;
>>   }
>>
>> -static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int32_t value)
>> +static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int64_t value,
>> +                                    bool is_signed, bool is_enum64, bool no_enum64)
> 
> It was quite confusing to see "is_enum64" and "no_enum64" as arguments
> to the same function :)
> 
> I'll let Arnaldo decide for himself, but I think it would be cleaner
> to pass such configuration switches as fields in struct btf_encoder
> itself and just check such flags from relevant btf_encoder__add_xxx()
> functions. Such flags are global by nature, so it seems fitting.

This will require an additional struct definition. It should work but I 
will wait for Arnaldo's comments as well.

> 
> But other than that looks good to me.
> 
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> 
>>   {
>> -       int err = btf__add_enum_value(encoder->btf, name, value);
>> +       const char *fmt_str;
>> +       int err;
>> +
>> +       /* If enum64 is not allowed, generate enum32 with unsigned int value. In enum64-supported
>> +        * libbpf library, btf__add_enum_value() will set the kflag (sign bit) in common_type
>> +        * if the value is negative.
>> +        */
>> +       if (no_enum64)
>> +               err = btf__add_enum_value(encoder->btf, name, (uint32_t)value);
>> +       else if (is_enum64)
>> +               err = btf__add_enum64_value(encoder->btf, name, value);
>> +       else
>> +               err = btf__add_enum_value(encoder->btf, name, value);
> 
> [...]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64
  2022-06-15 23:03 [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Yonghong Song
  2022-06-15 23:03 ` [PATCH dwarves v2 1/2] libbpf: Sync with latest libbpf repo Yonghong Song
  2022-06-15 23:03 ` [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64 Yonghong Song
@ 2022-06-28  9:51 ` Jiri Olsa
  2022-06-29 16:30   ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 10+ messages in thread
From: Jiri Olsa @ 2022-06-28  9:51 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team

On Wed, Jun 15, 2022 at 04:03:06PM -0700, Yonghong Song wrote:
> Add support for enum64. For 64-bit enumerator value,
> previously, the value is truncated into 32bit, e.g.,
> for the following enum in linux uapi bpf.h,
>   enum {
>         BPF_F_INDEX_MASK                = 0xffffffffULL,
>         BPF_F_CURRENT_CPU               = BPF_F_INDEX_MASK,
>   /* BPF_FUNC_perf_event_output for sk_buff input context. */
>         BPF_F_CTXLEN_MASK               = (0xfffffULL << 32),
>   };
> 
> BPF_F_CTXLEN_MASK will be encoded with 0 with BTF_KIND_ENUM
> after pahole dwarf-to-btf conversion.
> With this patch, the BPF_F_CTXLEN_MASK will be encoded properly
> with BTF_KIND_ENUM64.

yep, tried this on latest vmlinux and got:

[705813] ENUM64 (anon) size=8
        BPF_F_INDEX_MASK val=4294967295
        BPF_F_CURRENT_CPU val=4294967295
        BPF_F_CTXLEN_MASK val=4503595332403200

which is correct

Tested-by: Jiri Olsa <jolsa@kernel.org>

jirka

> 
> This patch is on top of tmp.master since tmp.master has not
> been sync'ed with master branch yet.
> 
> Changelogs:
>   v1 -> v2:
>     - Add flag --skip_encoding_btf_enum64 to disable newly-added functionality.
> 
> Yonghong Song (2):
>   libbpf: Sync with latest libbpf repo
>   btf: Support BTF_KIND_ENUM64
> 
>  btf_encoder.c     | 65 +++++++++++++++++++++++++++++++++++------------
>  btf_encoder.h     |  2 +-
>  dwarf_loader.c    | 12 +++++++++
>  dwarves.h         |  4 ++-
>  dwarves_fprintf.c |  6 ++++-
>  lib/bpf           |  2 +-
>  pahole.c          | 10 +++++++-
>  7 files changed, 80 insertions(+), 21 deletions(-)
> 
> -- 
> 2.30.2
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64
  2022-06-28  9:51 ` [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Jiri Olsa
@ 2022-06-29 16:30   ` Arnaldo Carvalho de Melo
  2022-06-29 16:56     ` [PATCH] btf_loader: support BTF_KIND_ENUM64 was " Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-06-29 16:30 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Yonghong Song, Arnaldo Carvalho de Melo, dwarves,
	Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann,
	kernel-team

Em Tue, Jun 28, 2022 at 11:51:52AM +0200, Jiri Olsa escreveu:
> On Wed, Jun 15, 2022 at 04:03:06PM -0700, Yonghong Song wrote:
> > Add support for enum64. For 64-bit enumerator value,
> > previously, the value is truncated into 32bit, e.g.,
> > for the following enum in linux uapi bpf.h,
> >   enum {
> >         BPF_F_INDEX_MASK                = 0xffffffffULL,
> >         BPF_F_CURRENT_CPU               = BPF_F_INDEX_MASK,
> >   /* BPF_FUNC_perf_event_output for sk_buff input context. */
> >         BPF_F_CTXLEN_MASK               = (0xfffffULL << 32),
> >   };
> > 
> > BPF_F_CTXLEN_MASK will be encoded with 0 with BTF_KIND_ENUM
> > after pahole dwarf-to-btf conversion.
> > With this patch, the BPF_F_CTXLEN_MASK will be encoded properly
> > with BTF_KIND_ENUM64.
> 
> yep, tried this on latest vmlinux and got:
> 
> [705813] ENUM64 (anon) size=8
>         BPF_F_INDEX_MASK val=4294967295
>         BPF_F_CURRENT_CPU val=4294967295
>         BPF_F_CTXLEN_MASK val=4503595332403200
> 
> which is correct
> 
> Tested-by: Jiri Olsa <jolsa@kernel.org>

I'm testing with v3:

⬢[acme@toolbox pahole]$ pahole -JV vmlinux-v5.18-rc7+ | grep -B10 -A5 BPF_F_CTXLEN_MASK
	BPF_FUNC_xdp_get_buff_len val=188
	BPF_FUNC_xdp_load_bytes val=189
	BPF_FUNC_xdp_store_bytes val=190
	BPF_FUNC_copy_from_user_task val=191
	BPF_FUNC_skb_set_tstamp val=192
	BPF_FUNC_ima_file_hash val=193
	__BPF_FUNC_MAX_ID val=194
[672880] ENUM64 (anon) size=8
	BPF_F_INDEX_MASK val=4294967295
	BPF_F_CURRENT_CPU val=4294967295
	BPF_F_CTXLEN_MASK val=4503595332403200
[672881] ENUM (anon) size=4
	BPF_F_GET_BRANCH_RECORDS_SIZE val=1
[672882] ARRAY (anon) type_id=672304 index_type_id=18 nr_elems=4
[672883] STRUCT (anon) size=16
	tp_name type_id=672266 bits_offset=0

But:

⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B10 -A5 BPF_F_CTXLEN_MASK
BTF: idx: 4173, Unknown kind 19
BTF: idx: 4975, Unknown kind 19
BTF: idx: 6673, Unknown kind 19
BTF: idx: 27413, Unknown kind 19
BTF: idx: 30626, Unknown kind 19
BTF: idx: 30829, Unknown kind 19
BTF: idx: 38040, Unknown kind 19
BTF: idx: 56969, Unknown kind 19
BTF: idx: 83004, Unknown kind 19
⬢[acme@toolbox pahole]$

Ok, I need to update pahole's BTF loader to support:

lib/bpf/src/btf.h:#define BTF_KIND_ENUM64		19	/* Enum for up-to 64bit values */


Working on it now.

Jiri, can I keep your Tested-by for v3?

- Arnaldo

> jirka
> 
> > 
> > This patch is on top of tmp.master since tmp.master has not
> > been sync'ed with master branch yet.
> > 
> > Changelogs:
> >   v1 -> v2:
> >     - Add flag --skip_encoding_btf_enum64 to disable newly-added functionality.
> > 
> > Yonghong Song (2):
> >   libbpf: Sync with latest libbpf repo
> >   btf: Support BTF_KIND_ENUM64
> > 
> >  btf_encoder.c     | 65 +++++++++++++++++++++++++++++++++++------------
> >  btf_encoder.h     |  2 +-
> >  dwarf_loader.c    | 12 +++++++++
> >  dwarves.h         |  4 ++-
> >  dwarves_fprintf.c |  6 ++++-
> >  lib/bpf           |  2 +-
> >  pahole.c          | 10 +++++++-
> >  7 files changed, 80 insertions(+), 21 deletions(-)
> > 
> > -- 
> > 2.30.2
> > 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] btf_loader: support BTF_KIND_ENUM64 was Re: [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64
  2022-06-29 16:30   ` Arnaldo Carvalho de Melo
@ 2022-06-29 16:56     ` Arnaldo Carvalho de Melo
  2022-07-06  4:28       ` Andrii Nakryiko
  0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-06-29 16:56 UTC (permalink / raw)
  To: Yonghong Song, Jiri Olsa
  Cc: dwarves, Alexei Starovoitov, Andrii Nakryiko, bpf,
	Daniel Borkmann, kernel-team

Em Wed, Jun 29, 2022 at 01:30:10PM -0300, Arnaldo Carvalho de Melo escreveu:
> ⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B10 -A5 BPF_F_CTXLEN_MASK
> BTF: idx: 4173, Unknown kind 19
> BTF: idx: 4975, Unknown kind 19
> BTF: idx: 6673, Unknown kind 19
> BTF: idx: 27413, Unknown kind 19
> BTF: idx: 30626, Unknown kind 19
> BTF: idx: 30829, Unknown kind 19
> BTF: idx: 38040, Unknown kind 19
> BTF: idx: 56969, Unknown kind 19
> BTF: idx: 83004, Unknown kind 19
> ⬢[acme@toolbox pahole]$
> 
> Ok, I need to update pahole's BTF loader to support:
> 
> lib/bpf/src/btf.h:#define BTF_KIND_ENUM64		19	/* Enum for up-to 64bit values */
> 
> 
> Working on it now.

⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B5 -A5 BPF_F_CTXLEN_MASK

/* 27413 */
enum {
	BPF_F_INDEX_MASK  = 4294967295,
	BPF_F_CURRENT_CPU = 4294967295,
	BPF_F_CTXLEN_MASK = 4503595332403200,
} __attribute__((__packed__)); /* size: 8 */

/* 27414 */
enum {
	BPF_F_GET_BRANCH_RECORDS_SIZE = 1,
⬢[acme@toolbox pahole]$

Quick patch here, please Ack, if possible:

diff --git a/btf_loader.c b/btf_loader.c
index b5d444643adf30b1..e57ecce2cde26e4e 100644
--- a/btf_loader.c
+++ b/btf_loader.c
@@ -312,6 +312,49 @@ out_free:
 	return -ENOMEM;
 }
 
+static struct enumerator *enumerator__new64(const char *name, uint64_t value)
+{
+	struct enumerator *en = tag__alloc(sizeof(*en));
+
+	if (en != NULL) {
+		en->name = name;
+		en->value = value; // Value is already 64-bit, as this is used with DWARF as well
+		en->tag.tag = DW_TAG_enumerator;
+	}
+
+	return en;
+}
+
+static int create_new_enumeration64(struct cu *cu, const struct btf_type *tp, uint32_t id)
+{
+	struct btf_enum64 *ep = btf_enum64(tp);
+	uint16_t i, vlen = btf_vlen(tp);
+	struct type *enumeration = type__new(DW_TAG_enumeration_type,
+					     cu__btf_str(cu, tp->name_off),
+					     tp->size ? tp->size * 8 : (sizeof(int) * 8));
+
+	if (enumeration == NULL)
+		return -ENOMEM;
+
+	for (i = 0; i < vlen; i++) {
+		const char *name = cu__btf_str(cu, ep[i].name_off);
+		uint64_t value = ((uint64_t)ep[i].val_hi32) << 32 | ep[i].val_lo32;
+		struct enumerator *enumerator = enumerator__new64(name, value);
+
+		if (enumerator == NULL)
+			goto out_free;
+
+		enumeration__add(enumeration, enumerator);
+	}
+
+	cu__add_tag_with_id(cu, &enumeration->namespace.tag, id);
+
+	return 0;
+out_free:
+	enumeration__delete(enumeration);
+	return -ENOMEM;
+}
+
 static int create_new_subroutine_type(struct cu *cu, const struct btf_type *tp, uint32_t id)
 {
 	struct ftype *proto = tag__alloc(sizeof(*proto));
@@ -419,6 +462,9 @@ static int btf__load_types(struct btf *btf, struct cu *cu)
 		case BTF_KIND_ENUM:
 			err = create_new_enumeration(cu, type_ptr, type_index);
 			break;
+		case BTF_KIND_ENUM64:
+			err = create_new_enumeration64(cu, type_ptr, type_index);
+			break;
 		case BTF_KIND_FWD:
 			err = create_new_forward_decl(cu, type_ptr, type_index);
 			break;

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] btf_loader: support BTF_KIND_ENUM64 was Re: [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64
  2022-06-29 16:56     ` [PATCH] btf_loader: support BTF_KIND_ENUM64 was " Arnaldo Carvalho de Melo
@ 2022-07-06  4:28       ` Andrii Nakryiko
  2022-07-06 15:51         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2022-07-06  4:28 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Yonghong Song, Jiri Olsa, dwarves, Alexei Starovoitov,
	Andrii Nakryiko, bpf, Daniel Borkmann, Kernel Team

On Wed, Jun 29, 2022 at 9:56 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Wed, Jun 29, 2022 at 01:30:10PM -0300, Arnaldo Carvalho de Melo escreveu:
> > ⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B10 -A5 BPF_F_CTXLEN_MASK
> > BTF: idx: 4173, Unknown kind 19
> > BTF: idx: 4975, Unknown kind 19
> > BTF: idx: 6673, Unknown kind 19
> > BTF: idx: 27413, Unknown kind 19
> > BTF: idx: 30626, Unknown kind 19
> > BTF: idx: 30829, Unknown kind 19
> > BTF: idx: 38040, Unknown kind 19
> > BTF: idx: 56969, Unknown kind 19
> > BTF: idx: 83004, Unknown kind 19
> > ⬢[acme@toolbox pahole]$
> >
> > Ok, I need to update pahole's BTF loader to support:
> >
> > lib/bpf/src/btf.h:#define BTF_KIND_ENUM64             19      /* Enum for up-to 64bit values */
> >
> >
> > Working on it now.
>
> ⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B5 -A5 BPF_F_CTXLEN_MASK
>
> /* 27413 */
> enum {
>         BPF_F_INDEX_MASK  = 4294967295,
>         BPF_F_CURRENT_CPU = 4294967295,
>         BPF_F_CTXLEN_MASK = 4503595332403200,
> } __attribute__((__packed__)); /* size: 8 */
>
> /* 27414 */
> enum {
>         BPF_F_GET_BRANCH_RECORDS_SIZE = 1,
> ⬢[acme@toolbox pahole]$
>
> Quick patch here, please Ack, if possible:
>

two minor nits, but looks good overall:

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> diff --git a/btf_loader.c b/btf_loader.c
> index b5d444643adf30b1..e57ecce2cde26e4e 100644
> --- a/btf_loader.c
> +++ b/btf_loader.c
> @@ -312,6 +312,49 @@ out_free:
>         return -ENOMEM;
>  }
>
> +static struct enumerator *enumerator__new64(const char *name, uint64_t value)
> +{
> +       struct enumerator *en = tag__alloc(sizeof(*en));
> +
> +       if (en != NULL) {
> +               en->name = name;
> +               en->value = value; // Value is already 64-bit, as this is used with DWARF as well
> +               en->tag.tag = DW_TAG_enumerator;
> +       }
> +
> +       return en;
> +}
> +
> +static int create_new_enumeration64(struct cu *cu, const struct btf_type *tp, uint32_t id)
> +{
> +       struct btf_enum64 *ep = btf_enum64(tp);
> +       uint16_t i, vlen = btf_vlen(tp);
> +       struct type *enumeration = type__new(DW_TAG_enumeration_type,
> +                                            cu__btf_str(cu, tp->name_off),
> +                                            tp->size ? tp->size * 8 : (sizeof(int) * 8));

tp->size should always be valid, so this fall back to sizeof(int)
isn't necessary

> +
> +       if (enumeration == NULL)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < vlen; i++) {
> +               const char *name = cu__btf_str(cu, ep[i].name_off);
> +               uint64_t value = ((uint64_t)ep[i].val_hi32) << 32 | ep[i].val_lo32;

use btf_enum64_value() defined in libbpf's btf.h header

> +               struct enumerator *enumerator = enumerator__new64(name, value);
> +
> +               if (enumerator == NULL)
> +                       goto out_free;
> +
> +               enumeration__add(enumeration, enumerator);
> +       }
> +
> +       cu__add_tag_with_id(cu, &enumeration->namespace.tag, id);
> +
> +       return 0;
> +out_free:
> +       enumeration__delete(enumeration);
> +       return -ENOMEM;
> +}
> +
>  static int create_new_subroutine_type(struct cu *cu, const struct btf_type *tp, uint32_t id)
>  {
>         struct ftype *proto = tag__alloc(sizeof(*proto));
> @@ -419,6 +462,9 @@ static int btf__load_types(struct btf *btf, struct cu *cu)
>                 case BTF_KIND_ENUM:
>                         err = create_new_enumeration(cu, type_ptr, type_index);
>                         break;
> +               case BTF_KIND_ENUM64:
> +                       err = create_new_enumeration64(cu, type_ptr, type_index);
> +                       break;
>                 case BTF_KIND_FWD:
>                         err = create_new_forward_decl(cu, type_ptr, type_index);
>                         break;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] btf_loader: support BTF_KIND_ENUM64 was Re: [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64
  2022-07-06  4:28       ` Andrii Nakryiko
@ 2022-07-06 15:51         ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-07-06 15:51 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Yonghong Song, Jiri Olsa, dwarves, Alexei Starovoitov,
	Andrii Nakryiko, bpf, Daniel Borkmann, Kernel Team

Em Tue, Jul 05, 2022 at 09:28:05PM -0700, Andrii Nakryiko escreveu:
> On Wed, Jun 29, 2022 at 9:56 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Wed, Jun 29, 2022 at 01:30:10PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > ⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B10 -A5 BPF_F_CTXLEN_MASK
> > > BTF: idx: 4173, Unknown kind 19
> > > BTF: idx: 4975, Unknown kind 19
> > > BTF: idx: 6673, Unknown kind 19
> > > BTF: idx: 27413, Unknown kind 19
> > > BTF: idx: 30626, Unknown kind 19
> > > BTF: idx: 30829, Unknown kind 19
> > > BTF: idx: 38040, Unknown kind 19
> > > BTF: idx: 56969, Unknown kind 19
> > > BTF: idx: 83004, Unknown kind 19
> > > ⬢[acme@toolbox pahole]$
> > >
> > > Ok, I need to update pahole's BTF loader to support:
> > >
> > > lib/bpf/src/btf.h:#define BTF_KIND_ENUM64             19      /* Enum for up-to 64bit values */
> > >
> > >
> > > Working on it now.
> >
> > ⬢[acme@toolbox pahole]$ pdwtags -F btf vmlinux-v5.18-rc7+ | grep -B5 -A5 BPF_F_CTXLEN_MASK
> >
> > /* 27413 */
> > enum {
> >         BPF_F_INDEX_MASK  = 4294967295,
> >         BPF_F_CURRENT_CPU = 4294967295,
> >         BPF_F_CTXLEN_MASK = 4503595332403200,
> > } __attribute__((__packed__)); /* size: 8 */
> >
> > /* 27414 */
> > enum {
> >         BPF_F_GET_BRANCH_RECORDS_SIZE = 1,
> > ⬢[acme@toolbox pahole]$
> >
> > Quick patch here, please Ack, if possible:
> >
> 
> two minor nits, but looks good overall:
> 
> Acked-by: Andrii Nakryiko <andrii@kernel.org>

Thanks for reviewing it!
 
> > diff --git a/btf_loader.c b/btf_loader.c
> > index b5d444643adf30b1..e57ecce2cde26e4e 100644
> > --- a/btf_loader.c
> > +++ b/btf_loader.c
> > @@ -312,6 +312,49 @@ out_free:
> >         return -ENOMEM;
> >  }
> >
> > +static struct enumerator *enumerator__new64(const char *name, uint64_t value)
> > +{
> > +       struct enumerator *en = tag__alloc(sizeof(*en));
> > +
> > +       if (en != NULL) {
> > +               en->name = name;
> > +               en->value = value; // Value is already 64-bit, as this is used with DWARF as well
> > +               en->tag.tag = DW_TAG_enumerator;
> > +       }
> > +
> > +       return en;
> > +}
> > +
> > +static int create_new_enumeration64(struct cu *cu, const struct btf_type *tp, uint32_t id)
> > +{
> > +       struct btf_enum64 *ep = btf_enum64(tp);
> > +       uint16_t i, vlen = btf_vlen(tp);
> > +       struct type *enumeration = type__new(DW_TAG_enumeration_type,
> > +                                            cu__btf_str(cu, tp->name_off),
> > +                                            tp->size ? tp->size * 8 : (sizeof(int) * 8));
> 
> tp->size should always be valid, so this fall back to sizeof(int)
> isn't necessary

This was just a copy from the create_new_enumeration() one, will remove
it there as well as a prep patch, then not have it in the 64-bit
version.
 
> > +
> > +       if (enumeration == NULL)
> > +               return -ENOMEM;
> > +
> > +       for (i = 0; i < vlen; i++) {
> > +               const char *name = cu__btf_str(cu, ep[i].name_off);
> > +               uint64_t value = ((uint64_t)ep[i].val_hi32) << 32 | ep[i].val_lo32;
> 
> use btf_enum64_value() defined in libbpf's btf.h header

Great, will pick that
 
> > +               struct enumerator *enumerator = enumerator__new64(name, value);
> > +
> > +               if (enumerator == NULL)
> > +                       goto out_free;
> > +
> > +               enumeration__add(enumeration, enumerator);
> > +       }
> > +
> > +       cu__add_tag_with_id(cu, &enumeration->namespace.tag, id);
> > +
> > +       return 0;
> > +out_free:
> > +       enumeration__delete(enumeration);
> > +       return -ENOMEM;
> > +}
> > +
> >  static int create_new_subroutine_type(struct cu *cu, const struct btf_type *tp, uint32_t id)
> >  {
> >         struct ftype *proto = tag__alloc(sizeof(*proto));
> > @@ -419,6 +462,9 @@ static int btf__load_types(struct btf *btf, struct cu *cu)
> >                 case BTF_KIND_ENUM:
> >                         err = create_new_enumeration(cu, type_ptr, type_index);
> >                         break;
> > +               case BTF_KIND_ENUM64:
> > +                       err = create_new_enumeration64(cu, type_ptr, type_index);
> > +                       break;
> >                 case BTF_KIND_FWD:
> >                         err = create_new_forward_decl(cu, type_ptr, type_index);
> >                         break;

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-07-06 15:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-15 23:03 [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Yonghong Song
2022-06-15 23:03 ` [PATCH dwarves v2 1/2] libbpf: Sync with latest libbpf repo Yonghong Song
2022-06-15 23:03 ` [PATCH dwarves v2 2/2] btf: Support BTF_KIND_ENUM64 Yonghong Song
2022-06-27 22:30   ` Andrii Nakryiko
2022-06-28  6:19     ` Yonghong Song
2022-06-28  9:51 ` [PATCH dwarves v2 0/2] btf: support BTF_KIND_ENUM64 Jiri Olsa
2022-06-29 16:30   ` Arnaldo Carvalho de Melo
2022-06-29 16:56     ` [PATCH] btf_loader: support BTF_KIND_ENUM64 was " Arnaldo Carvalho de Melo
2022-07-06  4:28       ` Andrii Nakryiko
2022-07-06 15:51         ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.