Re: [PATCH bpf-next 2/3] riscv, bpf: Add support for BPF exceptions

[Varun R Mallya <varunrmallya@gmail.com>]

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
> > > > +}

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv