Message-ID: <8b800c09-eade-4dcf-90f6-2f5a78170bc4@huaweicloud.com>
Date: Mon, 21 Apr 2025 10:14:43 +0800
From: Xu Kuohai <xukuohai@...weicloud.com>
To: Alexis Lothoré <alexis.lothore@...tlin.com>,
 Xu Kuohai <xukuohai@...weicloud.com>,
 Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>,
 John Fastabend <john.fastabend@...il.com>,
 Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
 <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>,
 Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
 KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
 Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
 Puranjay Mohan <puranjay@...nel.org>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Florent Revest <revest@...omium.org>,
 Bastien Curutchet <bastien.curutchet@...tlin.com>, ebpf@...uxfoundation.org,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>, bpf@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-kselftest@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com
Subject: Re: [PATCH RFC bpf-next 1/4] bpf: add struct largest member size in
 func model

On 4/21/2025 12:02 AM, Alexis Lothoré wrote:
> Hi Xu,
> 
> On Thu Apr 17, 2025 at 4:10 PM CEST, Xu Kuohai wrote:
>> On 4/17/2025 3:14 PM, Alexis Lothoré wrote:
>>> 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@...tlin.com> wrote:
> 
> [...]
> 
>>>> 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.
>>>
>>
>> Not exactly. The compiler's "packed" and "alignment" attributes cause a
>> structure to be aligned differently from its natural alignment.
>>
>> For example, with the following three structures:
>>
>> struct s0 {
>>       __int128 x;
>> };
>>
>> struct s1 {
>>       __int128 x;
>> } __attribute__((packed));
>>
>> struct s2 {
>>       __int128 x;
>> } __attribute__((aligned(64)));
>>
>> Even though the largest member size is the same, s0 will be aligned to 16
>> bytes, s1 and s2 are not aligned the same way. s1 has no alignment due to
>> the "packed" attribute, while s2 will be aligned to 64 bytes.
>>
>> When these three structures are passed as function arguments, they will be
>> located on different positions on the stack.
>>
>> For the following three functions:
>>
>> int f0(__int128 a, __int128 b, __int128 c, int64_t d, __int128 e, int64_t f, struct s0 g);
>> int f1(__int128 a, __int128 b, __int128 c, int64_t d, __int128 e, int64_t f, struct s1 g);
>> int f2(__int128 a, __int128 b, __int128 c, int64_t d, __int128 e, int64_t f, struct s2 g);
>>
>> g will be located at sp+32 in f0, sp + 24 in f1, and some 64-byte aligned
>> stack address in f2.
> 
> Ah, thanks for those clear examples, I completely overlooked this
> possibility. And now that you mention it, I feel a bit dumb because I now
> remember that you mentioned this in Puranjay's series...
> 
> I took a quick look at the x86 JIT compiler for reference, and saw no code
> related to this specific case neither. So I searched in the kernel for
> actual functions taking struct arguments by value AND being declared with some
> packed or aligned attribute. I only found a handful of those, and none
> seems to take enough arguments to have the corresponding struct passed on the
> stack. So rather than supporting this very specific case, I am tempted
> to just return an error for now during trampoline creation if we detect such
> structure (and then the JIT compiler can keep using data size to compute
> alignment, now that it is sure not to receive custom alignments). Or am I
> missing some actual cases involving those very specific alignments ?
> 

How can we reliably 'detect' the case? If a function has such a parameter
but we fail to detect it, the BPF trampoline will pass an incorrect value
to the function, which is also unacceptable.

> Thanks,
> 
> Alexis
> 

Powered by blists - more mailing lists