On 5/1/22 3:00 PM, Yonghong Song wrote:
> Currently, BTF only supports upto 32bit enum value with BTF_KIND_ENUM.
> But in kernel, some enum indeed has 64bit values, e.g.,
> in uapi bpf.h, we have
> enum {
> BPF_F_INDEX_MASK = 0xffffffffULL,
> BPF_F_CURRENT_CPU = BPF_F_INDEX_MASK,
> BPF_F_CTXLEN_MASK = (0xfffffULL << 32),
> };
> In this case, BTF_KIND_ENUM will encode the value of BPF_F_CTXLEN_MASK
> as 0, which certainly is incorrect.
>
> This patch added a new btf kind, BTF_KIND_ENUM64, which permits
> 64bit value to cover the above use case. The BTF_KIND_ENUM64 has
> the following three bytes followed by the common type:
> struct bpf_enum64 {
> __u32 nume_off;
> __u32 hi32;
> __u32 lo32;
> };
> Currently, btf type section has an alignment of 4 as all element types
> are u32. Representing the value with __u64 will introduce a pad
> for bpf_enum64 and may also introduce misalignment for the 64bit value.
> Hence, two members of hi32 and lo32 are chosen to avoid these issues.
>
> The kflag is also introduced for BTF_KIND_ENUM and BTF_KIND_ENUM64
> to indicate whether the value is signed or unsigned. The kflag intends
> to provide consistent output of BTF C fortmat with the original
> source code. For example, the original BTF_KIND_ENUM bit value is 0xffffffff.
> The format C has two choices, print out 0xffffffff or -1 and current libbpf
> prints out as unsigned value. But if the signedness is preserved in btf,
> the value can be printed the same as the original source code.
>
> The new BTF_KIND_ENUM64 is intended to support the enum value represented as
> 64bit value. But it can represent all BTF_KIND_ENUM values as well.
> The value size of BTF_KIND_ENUM64 is encoded to 8 to represent its intent.
> The compiler ([1]) and pahole will generate BTF_KIND_ENUM64 only if the value has
> to be represented with 64 bits.
>
> [1] https://reviews.llvm.org/D124641
>
> Signed-off-by: Yonghong Song <yhs@xxxxxx>
> ---
> include/linux/btf.h | 18 ++++-
> include/uapi/linux/btf.h | 17 ++++-
> kernel/bpf/btf.c | 132 ++++++++++++++++++++++++++++++---
> tools/include/uapi/linux/btf.h | 17 ++++-
> 4 files changed, 168 insertions(+), 16 deletions(-)
[...]
> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
> index a9162a6c0284..2aac226a27b2 100644
> --- a/include/uapi/linux/btf.h
> +++ b/include/uapi/linux/btf.h
> @@ -36,10 +36,10 @@ struct btf_type {
> * bits 24-28: kind (e.g. int, ptr, array...etc)
> * bits 29-30: unused
> * bit 31: kind_flag, currently used by
> - * struct, union and fwd
> + * struct, union, enum, fwd and enum64
> */
> __u32 info;
> - /* "size" is used by INT, ENUM, STRUCT, UNION and DATASEC.
> + /* "size" is used by INT, ENUM, STRUCT, UNION, DATASEC and ENUM64.
> * "size" tells the size of the type it is describing.
> *
> * "type" is used by PTR, TYPEDEF, VOLATILE, CONST, RESTRICT,
> @@ -63,7 +63,7 @@ enum {
> BTF_KIND_ARRAY = 3, /* Array */
> BTF_KIND_STRUCT = 4, /* Struct */
> BTF_KIND_UNION = 5, /* Union */
> - BTF_KIND_ENUM = 6, /* Enumeration */
> + BTF_KIND_ENUM = 6, /* Enumeration for int/unsigned int values */
nit: Maybe it would be more clear to say something like "Enumeration
representable in <= 32 bits", and something similar for ENUM64? This applies to
docs/bpf patch as well. I don't feel strongly about it.
> BTF_KIND_FWD = 7, /* Forward */
> BTF_KIND_TYPEDEF = 8, /* Typedef */
> BTF_KIND_VOLATILE = 9, /* Volatile */
> @@ -76,6 +76,7 @@ enum {
> BTF_KIND_FLOAT = 16, /* Floating point */
> BTF_KIND_DECL_TAG = 17, /* Decl Tag */
> BTF_KIND_TYPE_TAG = 18, /* Type Tag */
> + BTF_KIND_ENUM64 = 19, /* Enumeration for long/unsigned long values */
>
> NR_BTF_KINDS,
> BTF_KIND_MAX = NR_BTF_KINDS - 1,
> @@ -186,4 +187,14 @@ struct btf_decl_tag {
> __s32 component_idx;
> };
[...]
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 2f0b0440131c..17e24b362d3d 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -307,6 +307,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",
> };
>
> const char *btf_type_str(const struct btf_type *t)
> @@ -664,6 +665,7 @@ static bool btf_type_has_size(const struct btf_type *t)
> case BTF_KIND_ENUM:
> case BTF_KIND_DATASEC:
> case BTF_KIND_FLOAT:
> + case BTF_KIND_ENUM64:
> return true;
> }
>
[...]
> @@ -1832,6 +1840,7 @@ __btf_resolve_size(const struct btf *btf, const struct btf_type *type,
> case BTF_KIND_UNION:
> case BTF_KIND_ENUM:
> case BTF_KIND_FLOAT:
> + case BTF_KIND_ENUM64:
> size = type->size;
> goto resolved;
Is it possible to replace this check w/ btf_type_has_size that you also updated?
Looks like case's match, aside from BTF_KIND_DATASEC.
