Re: [RFC bpf-next 01/15] bpf: Extend UAPI to support location information

[Alan Maguire <alan.maguire@oracle.com>]

On 23/10/2025 01:56, Eduard Zingerman wrote:
> On Wed, 2025-10-08 at 18:34 +0100, Alan Maguire wrote:
> 
> [...]
> 
>> diff --git a/include/linux/btf.h b/include/linux/btf.h
>> index f06976ffb63f..65091c6aff4b 100644
>> --- a/include/linux/btf.h
>> +++ b/include/linux/btf.h
> 
> [...]
> 
>>> @@ -552,7 +579,7 @@ struct btf_field_desc {
>>  	/* member struct size, or zero, if no members */
>>  	int m_sz;
>>  	/* repeated per-member offsets */
>> -	int m_off_cnt, m_offs[1];
>> +	int m_off_cnt, m_offs[2];
>>  };
> 
> Should this be a part of patch #2?
> Commit message of the patch #2 explains why its needed.
>

Probably should be; I tried to keep the kernel stuff in patch 1 but this
particular change is more conceptually related to the changes in patch 2
alright given that we share field iteration. Can move all the field
iterator changes into a distinct patch for clarity.
 >> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
>> index 266d4ffa6c07..a74b9d202847 100644
>> --- a/include/uapi/linux/btf.h
>> +++ b/include/uapi/linux/btf.h
> 
> [...]
> 
>> +/* BTF_KIND_LOC_PARAM consists a btf_type specifying a vlen of 0, name_off is 0
>> + * and is followed by a singular "struct btf_loc_param". type/size specifies
>> + * the size of the associated location value.  The size value should be
>> + * cast to a __s32 as negative sizes can be specified; -8 to indicate a signed
>> + * 8 byte value for example.
> 
> Not sure it matters after Andrii's suggestion, but I find this
> description a bit cryptic. Maybe just note that (s32)(t)->size
> can be -8, -4, -2 for signed values, 2, 4, 8 for unsigned values,
> and its absolute value denotes the size of the value in bytes?
> 
> +1 to Andrii's suggestion to use enum to represent btf_loc_param "tag".
> 
> Also, what register numbering scheme is used?
> Probably should be mentioned in the docstring.
> 

Good point; it's basically the register numbering we get from DWARF to
ensure it's arch-agnostic. Regs 0-31 map the same way they do for DWARF
and reg 33 is fp.

>> + *
>> + * If kind_flag is 1 the btf_loc is a constant value, otherwise it represents
>> + * a register, possibly dereferencing it with the specified offset.
>> + *
>> + * "struct btf_type" is followed by a "struct btf_loc_param" which consists
>> + * of either the 64-bit value or the register number, offset etc.
>> + * Interpretation depends on whether the kind_flag is set as described above.
>> + */
> 
> [...]
> 
>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>> index 0de8fc8a0e0b..29cec549f119 100644
>> --- a/kernel/bpf/btf.c
>> +++ b/kernel/bpf/btf.c
> 
> [...]
> 
>> +static void btf_loc_param_log(struct btf_verifier_env *env,
>> +			 const struct btf_type *t)
>> +{
>> +	const struct btf_loc_param *p = btf_loc_param(t);
>> +
>> +	if (btf_type_kflag(t))
>> +		btf_verifier_log(env, "type=%u const=%lld", t->type, btf_loc_param_value(t));
>> +	else
>> +		btf_verifier_log(env, "type=%u reg=%u flags=%d offset %u",
> 
> Nit: print flags in hex?
>

yep good idea. Thanks!

Alan