Re: [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32

[Rui Qi]

On 6/23/26 2:08 AM, Nam Cao wrote:
> "Rui Qi" <qirui.001@xxxxxxxxxxxxx> writes:
>> --- a/arch/riscv/kernel/entry.S
>> +++ b/arch/riscv/kernel/entry.S
>> @@ -383,7 +383,7 @@ SYM_FUNC_START(call_on_irq_stack)
>>  	addi	sp, sp, -STACKFRAME_SIZE_ON_STACK
>>  	REG_S	ra, STACKFRAME_RA(sp)
>>  	REG_S	s0, STACKFRAME_FP(sp)
>> -	addi	s0, sp, STACKFRAME_SIZE_ON_STACK
>> +	addi	s0, sp, STACKFRAME_SIZE
> 
> Doesn't this break the calling convention? The ABI specifies that "After
> the prologue, the frame pointer register will point to [...] the stack
> pointer value on entry to the current procedure". The frame pointer is
> already correct here.
> 
> It is the storage locations of ra and fp that are broken. The ABI says
> "This puts the return address at fp - XLEN/8, and the previous frame
> pointer at fp - 2 * XLEN/8".
> 
> Or am I confused?
> 
> Nam

Hi Nam,

You are right, thank you for the correction. My patch makes fp
point to sp + STACKFRAME_SIZE instead of the entry stack pointer,
which violates the RISC-V calling convention.

The real issue is that on RV32, STACKFRAME_SIZE (8) differs from
STACKFRAME_SIZE_ON_STACK (16) due to alignment padding. The
current code stores ra and prev_fp at STACKFRAME_RA/STACKFRAME_FP
offsets from sp, which puts them at the wrong positions relative
to fp.

The correct fix, as you suggested, is to keep fp = entry_sp and
store ra and prev_fp at the ABI-mandated locations (fp - SZ and
fp - 2*SZ).

Samuel Holland actually proposed this same fix back in May 2024
[1] using exactly this approach: redefining STACKFRAME_FP and
STACKFRAME_RA as negative offsets from the frame pointer, so
that the storage locations automatically match the ABI on both
RV32 and RV64. His patch was never merged and the bug still
exists today.

I will drop my patch and resubmit Samuel's fix instead.

[1]
https://lore.kernel.org/all/20240530001733.1407654-2-samuel.holland@xxxxxxxxxx/