Re: [PATCH bpf-next v3 0/6] Add BPF Exceptions support for RISC-V

["Björn Töpel" <bjorn@kernel.org>]

Varun R Mallya <varunrmallya@gmail.com> writes:

> This patchset aims to add BPF exceptions supports for riscv64 by
> implementing the arch_bpf_stack_walk function and then updating the
> prologue and epilogue for BPF JIT on riscv. Also,
> bpf_jit_supports_exceptions() returns true now so that the verifier does
> not reject programs containing BPF exceptions on riscv64.
>
> On riscv the unwinder used by arch_bpf_stack_walk() is the frame-pointer
> unwinder, so exception support is gated on CONFIG_FRAME_POINTER.
>
> In the prologue and epilogue of the RISC-V JIT, I saved the
> return address and then the frame-pointer according to [1]. Also,
> according to [2], s0 to s11 are callee saved registers, which is why a
> new array (rv_exception_csave_regs) has been created to save these
> registers which contains all the required registers along with the frame
> pointer as well as return address.

...

> Since all exception tests pass now, the denylist file has been removed
> as it is empty now.
>
> Changes in v1->v2:
> v1: https://lore.kernel.org/bpf/20260621144259.288135-1-varunrmallya@gmail.com/
> - resolve the ftrace trampoline address with ftrace_graph_ret_addr()
>   inside arch_bpf_stack_walk()
> - Remove duplicated code in bpf_jit_build_prologue()
>
> Changes in v2->v3:
> v2: https://lore.kernel.org/bpf/20260628081710.113333-1-varunrmallya@gmail.com/
> - extend walk_stackframe to use sp/fp similar to how arm64 does it. (Björn Töpel)
> - refactor prologue/epilogue into helpers. (Björn Töpel)
> - fix comments to make them netdev style.
> - save/restore the tail-call counter in the exception prologue/epilogue
> - exception_tail_call now passes since bpf2bpf+tailcall mixing landed [3], so
>   drop the (now-empty) DENYLIST.riscv64
> - (!aux->exception_cb) has been simplified to only one condition (Pu Lehui)

Varun!

Thanks for the rework. The overall structure is much better now, and the
main v2 comments around reusing the stack walker and splitting the
prologue/epilogue have been addressed!

I tried to make sense of Sashiko's reports as well...

Sashiko seems correct about sp being stale after crossing an exception
frame. We restore pc and fp from pt_regs, so sp should be restored from
there as well. Would you agree?

The full pt_regs should also be validated before dereferencing it...?

I *think* the instrumentation concern also looks valid. The new
stack-walk adapter callbacks should not be instrumentable.

I don't think the !CONFIG_FRAME_POINTER report is an exception-support
bug. Exceptions require a valid boundary frame pointer, so the current
gating looks correct.

Sashiko is also correct that deleting DENYLIST.riscv64 breaks the
documented selftest command. Please update the README or retain an empty
file.

Minor wording issue: ra is not callee-saved. Please say “RA and the
callee-saved registers s0-s11.”


Thanks,
Björn