Re: [PATCH bpf-next v1 07/14] bpf, loongarch: Fix stack arguments for indirect trampolines

On Fri Aug 28, 2026 at 6:33 AM CEST, Tiezhu Yang wrote:
> Cc: [email protected]
> Cc: Hengqi Chen <[email protected]>
>
> On 2026/8/22 上午7:35, Kumar Kartikeya Dwivedi wrote:
>> LoongArch passes arguments beyond a0-a7 at the caller stack pointer. The
>> trampoline store_args() helper always reads those arguments at FP + 16,
>> which is correct for an fentry trampoline: its prologue leaves FP 16 bytes
>> below the stack pointer at trampoline entry after accounting for the saved
>> parent and traced-function frames.
>>
>> A struct_ops indirect trampoline is entered through a function pointer and
>> only saves its own RA and FP before setting FP to the entry stack pointer.
>> Its stack arguments therefore start at FP, not FP + 16. As a result, every
>> stack-passed struct_ops argument is currently read two slots late.
>>
>> Select the source offset based on whether the trampoline is indirect. This
>> also prepares the stack-passed arena argument path to consume the actual
>> pointer slot.
>>
>> Fixes: c9ebe2016de9 ("LoongArch: BPF: Support up to 12 function arguments for trampoline")
>> Cc: Tiezhu Yang <[email protected]>
>> Cc: Huacai Chen <[email protected]>
>> Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
>> ---
>>   arch/loongarch/net/bpf_jit.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
>> index 29c281bef28e..d193293a0fd2 100644
>> --- a/arch/loongarch/net/bpf_jit.c
>> +++ b/arch/loongarch/net/bpf_jit.c
>> @@ -1662,17 +1662,18 @@ int bpf_arch_text_invalidate(void *dst, size_t len)
>>   	return ret;
>>   }
>>
>> -static void store_args(struct jit_ctx *ctx, int nr_arg_slots, int args_off)
>> +static void store_args(struct jit_ctx *ctx, int nr_arg_slots, int args_off, bool is_struct_ops)
>>   {
>> +	int stack_args_off = is_struct_ops ? 0 : 16;
>>   	int i;
>>
>>   	for (i = 0; i < nr_arg_slots; i++) {
>>   		if (i < LOONGARCH_MAX_REG_ARGS)
>>   			emit_insn(ctx, std, LOONGARCH_GPR_A0 + i, LOONGARCH_GPR_FP, -args_off);
>>   		else {
>> -			/* Skip slots for T0 and FP of traced function */
>> +			/* Skip the saved T0 and FP slots for a traced function. */
>>   			emit_insn(ctx, ldd, LOONGARCH_GPR_T1, LOONGARCH_GPR_FP,
>> -				  16 + (i - LOONGARCH_MAX_REG_ARGS) * 8);
>> +				  stack_args_off + (i - LOONGARCH_MAX_REG_ARGS) * 8);
>>   			emit_insn(ctx, std, LOONGARCH_GPR_T1, LOONGARCH_GPR_FP, -args_off);
>>   		}
>>   		args_off -= 8;
>> @@ -1995,7 +1996,7 @@ static int __arch_prepare_bpf_trampoline(struct jit_ctx *ctx, struct bpf_tramp_i
>>   	func_meta = nr_arg_slots;
>>   	emit_store_stack_imm64(ctx, LOONGARCH_GPR_T1, -func_meta_off, func_meta);
>>
>> -	store_args(ctx, nr_arg_slots, args_off);
>> +	store_args(ctx, nr_arg_slots, args_off, is_struct_ops);
>>
>>   	if (bpf_fsession_cnt(tnodes)) {
>>   		/* clear all session cookies' value */
>
> According to the suggestion from bot+bpf-ci, adding the following
> comment before "int stack_args_off = is_struct_ops ? 0 : 16;" and
> removing the pre-existing comment inside the else branch would be
> much better:
>
> /*
>   * Regular fentry trampolines need to skip the 16-byte saved T0 and FP
>   * slots of the traced function. Indirect struct_ops trampolines place
>   * incoming stack arguments directly at FP.
>   */
>
> This clarifies both frame layouts without causing any ambiguity
> inside the loop.
>
> Other than that, the code looks good to me:
>
> Acked-by: Tiezhu Yang <[email protected]>
>
> By the way, I wrote a selftest for this patch:
>
>    selftests/bpf: Add test for indirect struct_ops trampoline
>
> https://lore.kernel.org/bpf/[email protected]/
>
> Without this patch:
>
>    $ sudo ./test_progs -t struct_ops_trampoline
>    ...
>    run_struct_ops_trampoline:FAIL:check_stack_passed_arg9 unexpected
> check_stack_passed_arg9: actual 69570864 != expected 9999
>    #471     struct_ops_trampoline:FAIL
>    Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED
>
> With this patch:
>
>    $ sudo ./test_progs -t struct_ops_trampoline
>    #471     struct_ops_trampoline:OK
>    Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
>
> Tested-by: Tiezhu Yang <[email protected]>

Thanks a lot for the test. Please also take a look at the argument rebase
patches once you folks have more cycles. Will definitely need your help and
reviews since they are mostly vibe coded.

>
> Thanks,
> Tiezhu