On Mon, Jun 29, 2026 at 10:53:26AM +0800, Pu Lehui wrote:
>
> On 2026/6/28 15:34, Varun R Mallya wrote:
> > On Tue, Jun 23, 2026 at 10:11:57AM +0800, Pu Lehui wrote:
> > >
> > > We don't need to duplicate code. Please merge it.
> >
> > Making this change in the next version!
> >
> > > > +
> > > > + if (!aux->exception_cb && aux->exception_boundary) {
> > > > + /*
> > > > + * Boundary program: allocate the frame and
> > > > save the
> > > > + * full callee-saved set, capturing the
> > > > caller's values.
> > > > + */
> > > > + emit_addi(RV_REG_SP, RV_REG_SP, -stack_adjust,
> > > > ctx);
> > > > + for (i = 0; i <
> > > > ARRAY_SIZE(rv_exception_csave_regs); i++) {
> > > > + emit_sd(RV_REG_SP, store_offset,
> > > > + rv_exception_csave_regs[i],
> > > > ctx);
> > > > + store_offset -= 8;
> > > > + }
> > > > + emit_addi(RV_REG_FP, RV_REG_SP, stack_adjust,
> > > > ctx);
> > > > + } else {
> > > > + /*
> > > > + * Exception callback, reuse the boundary
> > > > program's
> > > > + * frame, whose frame pointer is passed in a2.
> > > > Setting
> > >
> > > something confused—why is it A2? I feel like I missed something.
> >
> > bpf_throw() invokes the exception callback as
> > bpf_exception_cb(cookie, sp, bp, 0, 0) , whose 3rd argument (which,
> > according to RISC-V's calling convention resides in A2) is the boundary
> > prog's frame pointer. Since this else branch handles
> > the callback, it expects A2 to have the frame pointer. The arm
> > implementation does something very similar with emit(A64_MOV(1, A64_FP,
> > A64_R(2)), ctx)
> > where A64_R(2) is the third arg.
>
> ok, so it would be better to clear in the comments that it is the third
> parameter.
>
Will do that in v3. Sorry for being very late to reply to this. There
are BTF issues I am encountering while running RISC-V kernels which I
have found the cause for and will send a report and propose a fix soon.
> >
> > > > + * SP = FP - stack_adjust lines the epilogue's
> > > > loads up
> > > > + * with the registers the boundary saved.
> > > > + */
> > > > + emit_mv(RV_REG_FP, RV_REG_A2, ctx);
> > > > + emit_addi(RV_REG_SP, RV_REG_FP, -stack_adjust,
> > > > ctx);
> > > > + }
> > > > +
> > > > + goto tail_setup;
> > > > + }
> > > > +
> > > > if (seen_reg(RV_REG_RA, ctx))
> > > > stack_adjust += 8;
> > > > stack_adjust += 8; /* RV_REG_FP */
> > > > @@ -2082,6 +2173,7 @@ void bpf_jit_build_prologue(struct rv_jit_context
> > > > *ctx, bool is_subprog)
> > > > emit_addi(RV_REG_FP, RV_REG_SP, stack_adjust, ctx);
> > > > +tail_setup:
> > > > if (bpf_stack_adjust)
> > > > emit_addi(RV_REG_S5, RV_REG_SP, bpf_stack_adjust, ctx);
> > > > @@ -2157,3 +2249,13 @@ bool bpf_jit_supports_fsession(void)
> > > > {
> > > > return true;
> > > > }
> > > > +
> > > > +bool bpf_jit_supports_exceptions(void)
> > > > +{
> > > > + /*
> > > > + * bpf_throw() unwinds by walking the frame-pointer chain from
> > > > inside
> > > > + * the kernel back into the BPF frames (see
> > > > arch_bpf_stack_walk()), so
> > > > + * exceptions require the frame-pointer unwinder to be enabled.
> > > > + */
> > > > + return IS_ENABLED(CONFIG_FRAME_POINTER);
> > >
> > > riscv select ARCH_WANT_FRAME_POINTERS, so this will always true
> >
> > I checked that the kernel compiled even when I turned
> > CONFIG_FRAME_POINTER explicitly off, so not gating this
> > would be a mistake, right ? ARCH_WANT_FRAME_POINTERS makes
> > CONFIG_FRAME_POINTER user selectable and makes it default to
> > true, but it's not always true. What does force it is PERF_EVENTS=y but if
> > that too is turned off, then CONFIG_FRAME_POINTER can also be turned
> > off.
>
> alright, lgtm
> >
> > Thanks for the review!!
> > - Varun
> > > > +}
