Re: [PATCH dwarves v1] pahole: generate "bpf_fastcall" decl tags for eligible kfuncs

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

On 16/09/2024 10:19, Eduard Zingerman wrote:
> For kfuncs marked with KF_FASTCALL flag generate the following pair of
> decl tags:
> 
>     $ bpftool btf dump file vmlinux
>     ...
>     [A] FUNC 'bpf_rdonly_cast' type_id=...
>     ...
>     [B] DECL_TAG 'bpf_kfunc' type_id=A component_idx=-1
>     [C] DECL_TAG 'bpf_fastcall' type_id=A component_idx=-1
> 
> So that bpftool could find 'bpf_fastcall' decl tag and generate
> appropriate C declarations for such kfuncs, e.g.:
> 
>     #ifndef __VMLINUX_H__
>     #define __VMLINUX_H__
>     ...
>     #define __bpf_fastcall __attribute__((bpf_fastcall))
>     ...
>     __bpf_fastcall extern void *bpf_rdonly_cast(...) ...;
> 
> For additional information about 'bpf_fastcall' attribute,
> see the following commit in the LLVM source tree:
> 
> 64e464349bfc ("[BPF] introduce __attribute__((bpf_fastcall))")
> 
> And the following Linux kernel commit:
> 
> 52839f31cece ("Merge branch 'no_caller_saved_registers-attribute-for-helper-calls'")
> 
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>

hi Eduard,

you've added support for multiple declaration tags as part of this, but
I wonder if we could go slightly further to simplify any additional
future KF_* flags -> decl tag needs?

Specifically if we had an array of <set8 flags, tag name> mappings such
that we can add support for new declaration tags by simply adding a new
flag and declaration tag string. When checking flags value in
btf_encoder__tag_kfunc(), we'd just walk the array entries, and for each
matching flag add the associated decl tag. Would that work?

> ---
>  btf_encoder.c | 59 +++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 43 insertions(+), 16 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 8a2d92e..ae059e0 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -39,15 +39,19 @@
>  #define BTF_ID_SET8_PFX		"__BTF_ID__set8__"
>  #define BTF_SET8_KFUNCS		(1 << 0)
>  #define BTF_KFUNC_TYPE_TAG	"bpf_kfunc"
> +#define BTF_FASTCALL_TAG	"bpf_fastcall"
> +#define KF_FASTCALL		(1 << 12)
> +

probably need an #ifndef KF_FASTCALL/#endif here once this makes it into
uapi.


> +struct btf_id_and_flag {
> +        uint32_t id;
> +        uint32_t flags;
> +};
>  
>  /* Adapted from include/linux/btf_ids.h */
>  struct btf_id_set8 {
>          uint32_t cnt;
>          uint32_t flags;
> -        struct {
> -                uint32_t id;
> -                uint32_t flags;
> -        } pairs[];
> +	struct btf_id_and_flag pairs[];
>  };
>  
>  /* state used to do later encoding of saved functions */
> @@ -1517,21 +1521,34 @@ out:
>  	return err;
>  }
>  
> -static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *funcs, const char *kfunc)
> +static int add_kfunc_decl_tag(struct btf *btf, const char *tag, __u32 id, const char *kfunc)
> +{
> +	int err;
> +
> +	err = btf__add_decl_tag(btf, tag, id, -1);
> +	if (err < 0) {
> +		fprintf(stderr, "%s: failed to insert kfunc decl tag for '%s': %d\n",
> +			__func__, kfunc, err);
> +		return err;
> +	}
> +	return 0;
> +}
> +
> +static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *funcs, const char *kfunc, __u32 flags)
>  {
>  	struct btf_func key = { .name = kfunc };
>  	struct btf *btf = encoder->btf;
>  	struct btf_func *target;
>  	const void *base;
>  	unsigned int cnt;
> -	int err = -1;
> +	int err;
>  
>  	base = gobuffer__entries(funcs);
>  	cnt = gobuffer__nr_entries(funcs);
>  	target = bsearch(&key, base, cnt, sizeof(key), btf_func_cmp);
>  	if (!target) {
>  		fprintf(stderr, "%s: failed to find kfunc '%s' in BTF\n", __func__, kfunc);
> -		goto out;
> +		return -1;
>  	}
>  
>  	/* Note we are unconditionally adding the btf_decl_tag even
> @@ -1539,16 +1556,16 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *
>  	 * We are ok to do this b/c we will later btf__dedup() to remove
>  	 * any duplicates.
>  	 */
> -	err = btf__add_decl_tag(btf, BTF_KFUNC_TYPE_TAG, target->type_id, -1);
> -	if (err < 0) {
> -		fprintf(stderr, "%s: failed to insert kfunc decl tag for '%s': %d\n",
> -			__func__, kfunc, err);
> -		goto out;
> +	err = add_kfunc_decl_tag(btf, BTF_KFUNC_TYPE_TAG, target->type_id, kfunc);
> +	if (err < 0)
> +		return err;
> +	if (flags & KF_FASTCALL) {
> +		err = add_kfunc_decl_tag(btf, BTF_FASTCALL_TAG, target->type_id, kfunc);
> +		if (err < 0)
> +			return err;
>  	}
>  
> -	err = 0;
> -out:
> -	return err;
> +	return 0;
>  }
>  
>  static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
> @@ -1675,8 +1692,10 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  	/* Now inject BTF with kfunc decl tag for detected kfuncs */
>  	for (i = 0; i < nr_syms; i++) {
>  		const struct btf_kfunc_set_range *ranges;
> +		const struct btf_id_and_flag *pair;
>  		unsigned int ranges_cnt;
>  		char *func, *name;
> +		ptrdiff_t off;
>  		GElf_Sym sym;
>  		bool found;
>  		int err;
> @@ -1704,6 +1723,14 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  
>  			if (ranges[j].start <= addr && addr < ranges[j].end) {
>  				found = true;
> +				off = addr - idlist_addr;
> +				if (off < 0 || off + sizeof(*pair) > idlist->d_size) {
> +					fprintf(stderr, "%s: kfunc '%s' offset outside section '%s'\n",
> +						__func__, func, BTF_IDS_SECTION);
> +					free(func);
> +					goto out;
> +				}
> +				pair = idlist->d_buf + off;
>  				break;
>  			}
>  		}
> @@ -1712,7 +1739,7 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  			continue;
>  		}
>  
> -		err = btf_encoder__tag_kfunc(encoder, &btf_funcs, func);
> +		err = btf_encoder__tag_kfunc(encoder, &btf_funcs, func, pair->flags);
>  		if (err) {
>  			fprintf(stderr, "%s: failed to tag kfunc '%s'\n", __func__, func);
>  			free(func);