Re: [PATCH v6 bpf-next 6/9] bpf: Add BTF_KIND_FLOAT support

[Yonghong Song <yhs@fb.com>]

On 2/24/21 3:45 PM, Ilya Leoshkevich wrote:
> On the kernel side, introduce a new btf_kind_operations. It is
> similar to that of BTF_KIND_INT, however, it does not need to
> handle encodings and bit offsets. Do not implement printing, since
> the kernel does not know how to format floating-point values.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   kernel/bpf/btf.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 77 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 2efeb5f4b343..c405edc8e615 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -173,7 +173,7 @@
>   #define BITS_ROUNDUP_BYTES(bits) \
>   	(BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits))
>   
> -#define BTF_INFO_MASK 0x8f00ffff
> +#define BTF_INFO_MASK 0x9f00ffff
>   #define BTF_INT_MASK 0x0fffffff
>   #define BTF_TYPE_ID_VALID(type_id) ((type_id) <= BTF_MAX_TYPE)
>   #define BTF_STR_OFFSET_VALID(name_off) ((name_off) <= BTF_MAX_NAME_OFFSET)
> @@ -280,6 +280,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
>   	[BTF_KIND_FUNC_PROTO]	= "FUNC_PROTO",
>   	[BTF_KIND_VAR]		= "VAR",
>   	[BTF_KIND_DATASEC]	= "DATASEC",
> +	[BTF_KIND_FLOAT]	= "FLOAT",
>   };
>   
>   static const char *btf_type_str(const struct btf_type *t)
> @@ -574,6 +575,7 @@ static bool btf_type_has_size(const struct btf_type *t)
>   	case BTF_KIND_UNION:
>   	case BTF_KIND_ENUM:
>   	case BTF_KIND_DATASEC:
> +	case BTF_KIND_FLOAT:
>   		return true;
>   	}
>   
> @@ -1704,6 +1706,7 @@ __btf_resolve_size(const struct btf *btf, const struct btf_type *type,
>   		case BTF_KIND_STRUCT:
>   		case BTF_KIND_UNION:
>   		case BTF_KIND_ENUM:
> +		case BTF_KIND_FLOAT:
>   			size = type->size;
>   			goto resolved;
>   
> @@ -1849,7 +1852,7 @@ static int btf_df_check_kflag_member(struct btf_verifier_env *env,
>   	return -EINVAL;
>   }
>   
> -/* Used for ptr, array and struct/union type members.
> +/* Used for ptr, array struct/union and float type members.
>    * int, enum and modifier types have their specific callback functions.
>    */
>   static int btf_generic_check_kflag_member(struct btf_verifier_env *env,
> @@ -3675,6 +3678,77 @@ static const struct btf_kind_operations datasec_ops = {
>   	.show			= btf_datasec_show,
>   };
>   
> +static s32 btf_float_check_meta(struct btf_verifier_env *env,
> +				const struct btf_type *t,
> +				u32 meta_left)
> +{
> +	if (btf_type_vlen(t)) {
> +		btf_verifier_log_type(env, t, "vlen != 0");
> +		return -EINVAL;
> +	}
> +
> +	if (btf_type_kflag(t)) {
> +		btf_verifier_log_type(env, t, "Invalid btf_info kind_flag");
> +		return -EINVAL;
> +	}
> +
> +	if (t->size != 2 && t->size != 4 && t->size != 8 && t->size != 12 &&
> +	    t->size != 16) {
> +		btf_verifier_log_type(env, t, "Invalid type_size");
> +		return -EINVAL;
> +	}
> +
> +	btf_verifier_log_type(env, t, NULL);
> +
> +	return 0;
> +}
> +
> +static int btf_float_check_member(struct btf_verifier_env *env,
> +				  const struct btf_type *struct_type,
> +				  const struct btf_member *member,
> +				  const struct btf_type *member_type)
> +{
> +	u64 start_offset_bytes;
> +	u64 end_offset_bytes;
> +	u64 misalign_bits;
> +	u64 align_bytes;
> +	u64 align_bits;
> +
> +	align_bytes = min_t(u64, sizeof(void *), member_type->size);

I listed the following possible (size, align) pairs:
     size     x86_32 align_bytes   x86_64 align bytes
      2        2                    2
      4        4                    4
      8        4                    8
      12       4                    8
      16       4                    8

A few observations.
   1. I don't know, just want to confirm, for double, the alignment 
could be 4 (for a member) on 32bit system, is that right?
   2. for size 12, alignment will be 8 for x86_64 system, this is 
strange, not sure whether it is true or not. Or size 12 cannot be
on x86_64 and we should error out if sizeof(void *) is 8.

> +	align_bits = align_bytes * BITS_PER_BYTE;
> +	div64_u64_rem(member->offset, align_bits, &misalign_bits);
> +	if (misalign_bits) {
> +		btf_verifier_log_member(env, struct_type, member,
> +					"Member is not properly aligned");
> +		return -EINVAL;
> +	}
> +
> +	start_offset_bytes = member->offset / BITS_PER_BYTE;
> +	end_offset_bytes = start_offset_bytes + member_type->size;
> +	if (end_offset_bytes > struct_type->size) {
> +		btf_verifier_log_member(env, struct_type, member,
> +					"Member exceeds struct_size");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void btf_float_log(struct btf_verifier_env *env,
> +			  const struct btf_type *t)
> +{
> +	btf_verifier_log(env, "size=%u", t->size);
> +}
> +
> +static const struct btf_kind_operations float_ops = {
> +	.check_meta = btf_float_check_meta,
> +	.resolve = btf_df_resolve,
> +	.check_member = btf_float_check_member,
> +	.check_kflag_member = btf_generic_check_kflag_member,
> +	.log_details = btf_float_log,
> +	.show = btf_df_show,
> +};
> +
>   static int btf_func_proto_check(struct btf_verifier_env *env,
>   				const struct btf_type *t)
>   {
> @@ -3808,6 +3882,7 @@ static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS] = {
>   	[BTF_KIND_FUNC_PROTO] = &func_proto_ops,
>   	[BTF_KIND_VAR] = &var_ops,
>   	[BTF_KIND_DATASEC] = &datasec_ops,
> +	[BTF_KIND_FLOAT] = &float_ops,
>   };
>   
>   static s32 btf_check_meta(struct btf_verifier_env *env,
>