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

> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
> index 29c281bef28ef..d193293a0fd25 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;

This isn't a bug, but does the updated comment still accurately describe
what happens in both cases?

When is_struct_ops is true, stack_args_off is 0, so the code loads stack
arguments directly from FP+0 without skipping anything. The indirect
trampoline prologue saves RA and FP below FP, not above it, so there are
no saved slots to skip in that case.

The comment now says "Skip the saved T0 and FP slots for a traced
function," but it sits on a line that executes for both traced functions
(where the skip happens) and indirect trampolines (where it doesn't).

Could the comment be moved or reworded to reflect that the skipping only
applies to the fentry case? For instance, a comment on the stack_args_off
initialization could explain both frame layouts.

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32538351576