diff mbox series

[RFC,bpf-next,1/4] bpf: add struct largest member size in func model

Message ID 20250411-many_args_arm64-v1-1-0a32fe72339e@bootlin.com (mailing list archive)
State New
Headers show
Series bpf, arm64: support up to 12 arguments | expand

Commit Message

Alexis Lothoré April 11, 2025, 8:32 p.m. UTC
In order to properly JIT the trampolines needed to attach BPF programs
to functions, some architectures like ARM64 need to know about the
alignment needed for the function arguments. Such alignment can
generally be deduced from the argument size, but that's not completely
true for composite types. In the specific case of ARM64, the AAPCS64 ABI
defines that a composite type which needs to be passed through stack
must be aligned on the maximum between 8 and the largest alignment
constraint of its first-level members. So the JIT compiler needs more
information about the arguments to make sure to generate code that
respects those alignment constraints.

For struct arguments, add information about the size of the largest
first-level member in the struct btf_func_model to allow the JIT
compiler to guess the needed alignment. The information is quite
specific, but it allows to keep arch-specific concerns (ie: guessing the
final needed alignment for an argument) isolated in each JIT compiler.

Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
 include/linux/bpf.h |  1 +
 kernel/bpf/btf.c    | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

Comments

Jiri Olsa April 14, 2025, 11:04 a.m. UTC | #1
On Fri, Apr 11, 2025 at 10:32:10PM +0200, Alexis Lothoré (eBPF Foundation) wrote:
> In order to properly JIT the trampolines needed to attach BPF programs
> to functions, some architectures like ARM64 need to know about the
> alignment needed for the function arguments. Such alignment can
> generally be deduced from the argument size, but that's not completely
> true for composite types. In the specific case of ARM64, the AAPCS64 ABI
> defines that a composite type which needs to be passed through stack
> must be aligned on the maximum between 8 and the largest alignment
> constraint of its first-level members. So the JIT compiler needs more
> information about the arguments to make sure to generate code that
> respects those alignment constraints.
> 
> For struct arguments, add information about the size of the largest
> first-level member in the struct btf_func_model to allow the JIT
> compiler to guess the needed alignment. The information is quite
> specific, but it allows to keep arch-specific concerns (ie: guessing the
> final needed alignment for an argument) isolated in each JIT compiler.
> 
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
> ---
>  include/linux/bpf.h |  1 +
>  kernel/bpf/btf.c    | 25 +++++++++++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 3f0cc89c0622cb1a097999afb78c17102593b6bb..8b34dcf60a0ce09228ff761b962ab67b6a3e2263 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1106,6 +1106,7 @@ struct btf_func_model {
>  	u8 nr_args;
>  	u8 arg_size[MAX_BPF_FUNC_ARGS];
>  	u8 arg_flags[MAX_BPF_FUNC_ARGS];
> +	u8 arg_largest_member_size[MAX_BPF_FUNC_ARGS];
>  };
>  
>  /* Restore arguments before returning from trampoline to let original function
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 16ba36f34dfab7531babf5753cab9f368cddefa3..5d40911ec90210086a6175d569abb6e52d75ad17 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7318,6 +7318,29 @@ static int __get_type_size(struct btf *btf, u32 btf_id,
>  	return -EINVAL;
>  }
>  
> +static u8 __get_largest_member_size(struct btf *btf, const struct btf_type *t)
> +{
> +	const struct btf_member *member;
> +	const struct btf_type *mtype;
> +	u8 largest_member_size = 0;
> +	int i;
> +
> +	if (!__btf_type_is_struct(t))
> +		return largest_member_size;
> +
> +	for_each_member(i, t, member) {
> +		mtype = btf_type_by_id(btf, member->type);
> +		while (mtype && btf_type_is_modifier(mtype))
> +			mtype = btf_type_by_id(btf, mtype->type);
> +		if (!mtype)
> +			return -EINVAL;

should we use __get_type_size for member->type instead ?

jirka

> +		if (mtype->size > largest_member_size)
> +			largest_member_size = mtype->size;
> +	}
> +
> +	return largest_member_size;
> +}
> +
>  static u8 __get_type_fmodel_flags(const struct btf_type *t)
>  {
>  	u8 flags = 0;
> @@ -7396,6 +7419,8 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
>  		}
>  		m->arg_size[i] = ret;
>  		m->arg_flags[i] = __get_type_fmodel_flags(t);
> +		m->arg_largest_member_size[i] =
> +			__get_largest_member_size(btf, t);
>  	}
>  	m->nr_args = nargs;
>  	return 0;
> 
> -- 
> 2.49.0
>
Alexis Lothoré April 14, 2025, 8:27 p.m. UTC | #2
Hello Jiri,

On Mon Apr 14, 2025 at 1:04 PM CEST, Jiri Olsa wrote:
> On Fri, Apr 11, 2025 at 10:32:10PM +0200, Alexis Lothoré (eBPF Foundation) wrote:

[...]

>> +	for_each_member(i, t, member) {
>> +		mtype = btf_type_by_id(btf, member->type);
>> +		while (mtype && btf_type_is_modifier(mtype))
>> +			mtype = btf_type_by_id(btf, mtype->type);
>> +		if (!mtype)
>> +			return -EINVAL;
>
> should we use __get_type_size for member->type instead ?

Ah, yes, thanks for the hint, that will allow to get rid of the manual
modifiers skip.

Alexis
> jirka
Andrii Nakryiko April 16, 2025, 9:24 p.m. UTC | #3
On Fri, Apr 11, 2025 at 1:32 PM Alexis Lothoré (eBPF Foundation)
<alexis.lothore@bootlin.com> wrote:
>
> In order to properly JIT the trampolines needed to attach BPF programs
> to functions, some architectures like ARM64 need to know about the
> alignment needed for the function arguments. Such alignment can
> generally be deduced from the argument size, but that's not completely
> true for composite types. In the specific case of ARM64, the AAPCS64 ABI
> defines that a composite type which needs to be passed through stack
> must be aligned on the maximum between 8 and the largest alignment
> constraint of its first-level members. So the JIT compiler needs more
> information about the arguments to make sure to generate code that
> respects those alignment constraints.
>
> For struct arguments, add information about the size of the largest
> first-level member in the struct btf_func_model to allow the JIT
> compiler to guess the needed alignment. The information is quite

I might be missing something, but how can the *size* of the field be
used to calculate that argument's *alignment*? i.e., I don't
understand why arg_largest_member_size needs to be calculated instead
of arg_largest_member_alignment...

> specific, but it allows to keep arch-specific concerns (ie: guessing the
> final needed alignment for an argument) isolated in each JIT compiler.

couldn't all this information be calculated in the JIT compiler (if
JIT needs that) from BTF?

>
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
> ---
>  include/linux/bpf.h |  1 +
>  kernel/bpf/btf.c    | 25 +++++++++++++++++++++++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 3f0cc89c0622cb1a097999afb78c17102593b6bb..8b34dcf60a0ce09228ff761b962ab67b6a3e2263 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1106,6 +1106,7 @@ struct btf_func_model {
>         u8 nr_args;
>         u8 arg_size[MAX_BPF_FUNC_ARGS];
>         u8 arg_flags[MAX_BPF_FUNC_ARGS];
> +       u8 arg_largest_member_size[MAX_BPF_FUNC_ARGS];
>  };
>
>  /* Restore arguments before returning from trampoline to let original function
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 16ba36f34dfab7531babf5753cab9f368cddefa3..5d40911ec90210086a6175d569abb6e52d75ad17 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7318,6 +7318,29 @@ static int __get_type_size(struct btf *btf, u32 btf_id,
>         return -EINVAL;
>  }
>
> +static u8 __get_largest_member_size(struct btf *btf, const struct btf_type *t)
> +{
> +       const struct btf_member *member;
> +       const struct btf_type *mtype;
> +       u8 largest_member_size = 0;
> +       int i;
> +
> +       if (!__btf_type_is_struct(t))
> +               return largest_member_size;
> +
> +       for_each_member(i, t, member) {
> +               mtype = btf_type_by_id(btf, member->type);
> +               while (mtype && btf_type_is_modifier(mtype))
> +                       mtype = btf_type_by_id(btf, mtype->type);
> +               if (!mtype)
> +                       return -EINVAL;
> +               if (mtype->size > largest_member_size)
> +                       largest_member_size = mtype->size;
> +       }
> +
> +       return largest_member_size;
> +}
> +
>  static u8 __get_type_fmodel_flags(const struct btf_type *t)
>  {
>         u8 flags = 0;
> @@ -7396,6 +7419,8 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
>                 }
>                 m->arg_size[i] = ret;
>                 m->arg_flags[i] = __get_type_fmodel_flags(t);
> +               m->arg_largest_member_size[i] =
> +                       __get_largest_member_size(btf, t);
>         }
>         m->nr_args = nargs;
>         return 0;
>
> --
> 2.49.0
>
Alexis Lothoré April 17, 2025, 7:14 a.m. UTC | #4
Hi Andrii,

On Wed Apr 16, 2025 at 11:24 PM CEST, Andrii Nakryiko wrote:
> On Fri, Apr 11, 2025 at 1:32 PM Alexis Lothoré (eBPF Foundation)
> <alexis.lothore@bootlin.com> wrote:
>>
>> In order to properly JIT the trampolines needed to attach BPF programs
>> to functions, some architectures like ARM64 need to know about the
>> alignment needed for the function arguments. Such alignment can
>> generally be deduced from the argument size, but that's not completely
>> true for composite types. In the specific case of ARM64, the AAPCS64 ABI
>> defines that a composite type which needs to be passed through stack
>> must be aligned on the maximum between 8 and the largest alignment
>> constraint of its first-level members. So the JIT compiler needs more
>> information about the arguments to make sure to generate code that
>> respects those alignment constraints.
>>
>> For struct arguments, add information about the size of the largest
>> first-level member in the struct btf_func_model to allow the JIT
>> compiler to guess the needed alignment. The information is quite
>
> I might be missing something, but how can the *size* of the field be
> used to calculate that argument's *alignment*? i.e., I don't
> understand why arg_largest_member_size needs to be calculated instead
> of arg_largest_member_alignment...

Indeed I initially checked whether I could return directly some alignment
info from btf, but it then involves the alignment computation in the btf
module. Since there could be minor differences between architectures about
alignment requirements, I though it would be better to in fact keep alignment
computation out of the btf module. For example, I see that 128 bits values
are aligned on 16 bytes on ARM64, while being aligned on 8 bytes on S390. 

And since for ARM64, all needed alignments are somehow derived from size
(it is either directly size for fundamental types, or alignment of the
largest member for structs, which is then size of largest member),
returning the size seems to be enough to allow the JIT side to compute
alignments.

>> specific, but it allows to keep arch-specific concerns (ie: guessing the
>> final needed alignment for an argument) isolated in each JIT compiler.
>
> couldn't all this information be calculated in the JIT compiler (if
> JIT needs that) from BTF?

From what I understand, the JIT compiler does not have access to BTF info,
only a substract from it, arranged in a struct btf_func_model ? This
struct btf_func_model already has size info for standard types, but for
structs we need some additional info about the members, hence this
arg_largest_member_alignment addition in btf_func_model.

Thanks,

Alexis
diff mbox series

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 3f0cc89c0622cb1a097999afb78c17102593b6bb..8b34dcf60a0ce09228ff761b962ab67b6a3e2263 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1106,6 +1106,7 @@  struct btf_func_model {
 	u8 nr_args;
 	u8 arg_size[MAX_BPF_FUNC_ARGS];
 	u8 arg_flags[MAX_BPF_FUNC_ARGS];
+	u8 arg_largest_member_size[MAX_BPF_FUNC_ARGS];
 };
 
 /* Restore arguments before returning from trampoline to let original function
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 16ba36f34dfab7531babf5753cab9f368cddefa3..5d40911ec90210086a6175d569abb6e52d75ad17 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7318,6 +7318,29 @@  static int __get_type_size(struct btf *btf, u32 btf_id,
 	return -EINVAL;
 }
 
+static u8 __get_largest_member_size(struct btf *btf, const struct btf_type *t)
+{
+	const struct btf_member *member;
+	const struct btf_type *mtype;
+	u8 largest_member_size = 0;
+	int i;
+
+	if (!__btf_type_is_struct(t))
+		return largest_member_size;
+
+	for_each_member(i, t, member) {
+		mtype = btf_type_by_id(btf, member->type);
+		while (mtype && btf_type_is_modifier(mtype))
+			mtype = btf_type_by_id(btf, mtype->type);
+		if (!mtype)
+			return -EINVAL;
+		if (mtype->size > largest_member_size)
+			largest_member_size = mtype->size;
+	}
+
+	return largest_member_size;
+}
+
 static u8 __get_type_fmodel_flags(const struct btf_type *t)
 {
 	u8 flags = 0;
@@ -7396,6 +7419,8 @@  int btf_distill_func_proto(struct bpf_verifier_log *log,
 		}
 		m->arg_size[i] = ret;
 		m->arg_flags[i] = __get_type_fmodel_flags(t);
+		m->arg_largest_member_size[i] =
+			__get_largest_member_size(btf, t);
 	}
 	m->nr_args = nargs;
 	return 0;