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

[Pu Lehui <pulehui@huawei.com>]

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.

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