> bpf, riscv: Use resolved pointer for timed may_goto
>
> The timed may_goto fixup now passes the resolved counter pointer through
> BPF_REG_AX instead of a stack offset.
>
> Copy the pointer directly into the first argument register rather than
> adding it to the BPF frame pointer again.
>
> Fixes: 6ef8ff20c30b ("bpf, riscv: Add support for timed may_goto")
Should the Fixes: tag point to d8319a04dafc ("bpf: Fix timed may_goto
stack pointer for private stacks") instead? That commit changed the
calling convention from passing a stack offset to passing a resolved
pointer through BPF_REG_AX, which is what broke the riscv trampoline.
The original riscv implementation (6ef8ff20c30b) worked correctly with
the original calling convention. riscv64 cannot hit the private-stack bug
that d8319a04dafc targeted: it does not implement
bpf_jit_supports_private_stack(), so the __weak default in
kernel/bpf/core.c returns false, and private stacks are gated on it in
kernel/bpf/verifier.c. On riscv, regmap[BPF_REG_FP] = RV_REG_S5 is the
one and only BPF frame pointer, and the pre-patch 'add a0, t0, s5'
computed exactly the right address.
What this patch actually does on riscv is adapt to the new BPF_REG_AX
calling convention introduced by d8319a04dafc, which is patch 1 of the
same series. If stable/AUTOSEL tooling picks this commit up on the
strength of its Fixes: tag without also taking d8319a04dafc, BPF_REG_AX
still holds the raw immediate from the old fixup (BPF_MOV64_IMM(BPF_REG_AX,
stack_off_cnt), i.e. -stack_depth-16, up to -528):
mv a0, t0 /* a0 = -528, not a pointer */
call bpf_check_timed_may_goto
bpf_check_timed_may_goto() then dereferences p->timestamp
(kernel/bpf/core.c) at 0xfffffffffffffdf0, oopsing on every timed
may_goto loop.
Should the changelog also declare that d8319a04dafc is a prerequisite
for this patch?
Because d8319a04dafc lands first and each architecture is converted in a
later commit, riscv64 (and arm64, powerpc64, s390) BPF timed may_goto is
broken at every intermediate commit of the series. Does the series need to
be structured differently to remain bisectable for those architectures?
> diff --git a/arch/riscv/net/bpf_timed_may_goto.S
> b/arch/riscv/net/bpf_timed_may_goto.S
> index 02c637d87420..8c03e272e0bf 100644
> --- a/arch/riscv/net/bpf_timed_may_goto.S
> +++ b/arch/riscv/net/bpf_timed_may_goto.S
> @@ -6,12 +6,11 @@
>
> /*
> * Trampoline for the BPF timed may_goto loop bound. Custom calling
> convention:
> - * - input: stack offset in BPF_REG_AX (t0)
> + * - input: count and timestamp pointer in BPF_REG_AX (t0)
> * - output: updated count in BPF_REG_AX (t0)
> *
> - * Calls bpf_check_timed_may_goto(ptr) with the standard RISC-V ABI, where
> - * ptr = BPF_REG_FP (s5) + BPF_REG_AX (t0). BPF R0-R5 (a5, a0-a4) are saved
> - * across the call; BPF_REG_FP (s5) is callee-saved and needs no saving.
> + * Calls bpf_check_timed_may_goto(ptr) with the standard RISC-V ABI.
> + * BPF R0-R5 (a5, a0-a4) are saved across the call.
> */
>
> SYM_FUNC_START(arch_bpf_timed_may_goto)
> @@ -28,7 +27,11 @@ SYM_FUNC_START(arch_bpf_timed_may_goto)
> REG_S a3, 1*SZREG(sp)
> REG_S a4, 0*SZREG(sp)
>
> - add a0, t0, s5
> + /*
> + * BPF_REG_AX (t0) contains the count and timestamp pointer; pass it as
> + * the first argument in a0.
> + */
> + mv a0, t0
> call bpf_check_timed_may_goto
> mv t0, a0
>
---
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/33914557238
