Re: [PATCH bpf-next v6 1/3] riscv, bpf: Add 12-argument support for RV64 bpf trampoline

Pu Lehui
2024-07-06T02:28:32Z

On 2024/7/5 20:51, Puranjay Mohan wrote:
> Pu Lehui <pulehui@huaweicloud.com> writes:
>
>> From: Pu Lehui <pulehui@huawei.com>
>>
>> This patch adds 12 function arguments support for riscv64 bpf
>> trampoline. The current bpf trampoline supports <= sizeof(u64) bytes
>> scalar arguments [0] and <= 16 bytes struct arguments [1]. Therefore, we
>> focus on the situation where scalars are at most XLEN bits and
>> aggregates whose total size does not exceed 2×XLEN bits in the riscv
>> calling convention [2].
[SNIP]
>> @@ -854,7 +875,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
>> retval_off = stack_size;
>> }
>>
>> - stack_size += nregs * 8;
>> + stack_size += nr_arg_slots * 8;
>> args_off = stack_size;
>>
>> stack_size += 8;
>> @@ -871,8 +892,14 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
>> stack_size += 8;
>> sreg_off = stack_size;
>>
>> + if (nr_arg_slots - RV_MAX_REG_ARGS > 0)
>> + stack_size += (nr_arg_slots - RV_MAX_REG_ARGS) * 8;
>
> Hi Pu,
> Although this is merged now, while working on this for arm64 I realised
> that the above doesn't check for BPF_TRAMP_F_CALL_ORIG and can waste
> some stack space, we should change this to:
>
> if ((flags & BPF_TRAMP_F_CALL_ORIG) && (nr_arg_slots - RV_MAX_REG_ARGS > 0))
> stack_size += (nr_arg_slots - RV_MAX_REG_ARGS) * 8;
>
> It will save some stack space when BPF_TRAMP_F_CALL_ORIG is not set?

Nice catch. It will be better. Feel free to patch it. Thanks!

>
> I can send a patch if you think this is worth fixing.
>
>
> Thanks,
> Puranjay