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

From: Varun R Mallya <varunrmallya@gmail.com>
To: Pu Lehui <pulehui@huawei.com>
Cc: pjw@kernel.org, palmer@dabbelt.com, aou@eecs.berkeley.edu,
	ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	eddyz87@gmail.com, memxor@gmail.com, bjorn@kernel.org,
	alex@ghiti.fr, martin.lau@linux.dev, song@kernel.org,
	yonghong.song@linux.dev, jolsa@kernel.org, emil@etsalapatis.com,
	puranjay@kernel.org, linux-riscv@lists.infradead.org,
	linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH bpf-next 2/3] riscv, bpf: Add support for BPF exceptions
Date: Sun, 28 Jun 2026 13:04:42 +0530	[thread overview]
Message-ID: <akDOkptnqJt9LDtj@computer> (raw)
In-Reply-To: <b6e8c677-5fff-4de1-8483-fc23361898dc@huawei.com>

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.

> > +			 * 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.

Thanks for the review!!
- Varun
> > +}

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