All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32
@ 2026-06-03  3:53 ` Rui Qi
  0 siblings, 0 replies; 6+ messages in thread
From: Rui Qi @ 2026-06-03  3:53 UTC (permalink / raw)
  To: palmer, pjw, aou, alex; +Cc: Rui Qi, linux-riscv, linux-kernel

The frame pointer (s0/fp) in call_on_irq_stack is set using
STACKFRAME_SIZE_ON_STACK, which equals ALIGN(sizeof(struct stackframe),
STACK_ALIGN). On RV64, sizeof(struct stackframe) is 16 and the aligned
size is also 16, so there is no difference. However, on RV32,
sizeof(struct stackframe) is 8 while STACKFRAME_SIZE_ON_STACK is 16 due
to 8 bytes of alignment padding.

The stack unwinder does 'frame = (struct stackframe *)fp - 1', which
reads from 'fp - sizeof(struct stackframe)'. On RV32, with fp set to
sp + STACKFRAME_SIZE_ON_STACK (sp + 16), the unwinder reads from
sp + 8, which falls in the alignment padding rather than where the
saved fp/ra are actually stored (sp + 0 and sp + 4).

Fix this by introducing STACKFRAME_SIZE (the unaligned sizeof) and using
it for frame pointer setup and restoration, while keeping
STACKFRAME_SIZE_ON_STACK for the stack pointer allocation/deallocation
which must remain 16-byte aligned.

Signed-off-by: Rui Qi <qirui.001@bytedance.com>
---
 arch/riscv/kernel/asm-offsets.c | 1 +
 arch/riscv/kernel/entry.S       | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
index af827448a609..c1b5f7eb03fd 100644
--- a/arch/riscv/kernel/asm-offsets.c
+++ b/arch/riscv/kernel/asm-offsets.c
@@ -500,6 +500,7 @@ void asm_offsets(void)
 	OFFSET(SBI_HART_BOOT_TASK_PTR_OFFSET, sbi_hart_boot_data, task_ptr);
 	OFFSET(SBI_HART_BOOT_STACK_PTR_OFFSET, sbi_hart_boot_data, stack_ptr);
 
+	DEFINE(STACKFRAME_SIZE, sizeof(struct stackframe));
 	DEFINE(STACKFRAME_SIZE_ON_STACK, ALIGN(sizeof(struct stackframe), STACK_ALIGN));
 	OFFSET(STACKFRAME_FP, stackframe, fp);
 	OFFSET(STACKFRAME_RA, stackframe, ra);
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index d011fb51c59a..e8b654e2b7b5 100644
--- 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
 
 	/* Switch to the per-CPU shadow call stack */
 	scs_save_current
@@ -399,7 +399,7 @@ SYM_FUNC_START(call_on_irq_stack)
 	scs_load_current
 
 	/* Switch back to the thread stack and restore ra and s0 */
-	addi	sp, s0, -STACKFRAME_SIZE_ON_STACK
+	addi	sp, s0, -STACKFRAME_SIZE
 	REG_L	ra, STACKFRAME_RA(sp)
 	REG_L	s0, STACKFRAME_FP(sp)
 	addi	sp, sp, STACKFRAME_SIZE_ON_STACK
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32
@ 2026-06-03  3:53 ` Rui Qi
  0 siblings, 0 replies; 6+ messages in thread
From: Rui Qi @ 2026-06-03  3:53 UTC (permalink / raw)
  To: palmer, pjw, aou, alex; +Cc: Rui Qi, linux-riscv, linux-kernel

The frame pointer (s0/fp) in call_on_irq_stack is set using
STACKFRAME_SIZE_ON_STACK, which equals ALIGN(sizeof(struct stackframe),
STACK_ALIGN). On RV64, sizeof(struct stackframe) is 16 and the aligned
size is also 16, so there is no difference. However, on RV32,
sizeof(struct stackframe) is 8 while STACKFRAME_SIZE_ON_STACK is 16 due
to 8 bytes of alignment padding.

The stack unwinder does 'frame = (struct stackframe *)fp - 1', which
reads from 'fp - sizeof(struct stackframe)'. On RV32, with fp set to
sp + STACKFRAME_SIZE_ON_STACK (sp + 16), the unwinder reads from
sp + 8, which falls in the alignment padding rather than where the
saved fp/ra are actually stored (sp + 0 and sp + 4).

Fix this by introducing STACKFRAME_SIZE (the unaligned sizeof) and using
it for frame pointer setup and restoration, while keeping
STACKFRAME_SIZE_ON_STACK for the stack pointer allocation/deallocation
which must remain 16-byte aligned.

Signed-off-by: Rui Qi <qirui.001@bytedance.com>
---
 arch/riscv/kernel/asm-offsets.c | 1 +
 arch/riscv/kernel/entry.S       | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
index af827448a609..c1b5f7eb03fd 100644
--- a/arch/riscv/kernel/asm-offsets.c
+++ b/arch/riscv/kernel/asm-offsets.c
@@ -500,6 +500,7 @@ void asm_offsets(void)
 	OFFSET(SBI_HART_BOOT_TASK_PTR_OFFSET, sbi_hart_boot_data, task_ptr);
 	OFFSET(SBI_HART_BOOT_STACK_PTR_OFFSET, sbi_hart_boot_data, stack_ptr);
 
+	DEFINE(STACKFRAME_SIZE, sizeof(struct stackframe));
 	DEFINE(STACKFRAME_SIZE_ON_STACK, ALIGN(sizeof(struct stackframe), STACK_ALIGN));
 	OFFSET(STACKFRAME_FP, stackframe, fp);
 	OFFSET(STACKFRAME_RA, stackframe, ra);
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index d011fb51c59a..e8b654e2b7b5 100644
--- 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
 
 	/* Switch to the per-CPU shadow call stack */
 	scs_save_current
@@ -399,7 +399,7 @@ SYM_FUNC_START(call_on_irq_stack)
 	scs_load_current
 
 	/* Switch back to the thread stack and restore ra and s0 */
-	addi	sp, s0, -STACKFRAME_SIZE_ON_STACK
+	addi	sp, s0, -STACKFRAME_SIZE
 	REG_L	ra, STACKFRAME_RA(sp)
 	REG_L	s0, STACKFRAME_FP(sp)
 	addi	sp, sp, STACKFRAME_SIZE_ON_STACK
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32
  2026-06-03  3:53 ` Rui Qi
@ 2026-06-22  2:54   ` Rui Qi
  -1 siblings, 0 replies; 6+ messages in thread
From: Rui Qi @ 2026-06-22  2:54 UTC (permalink / raw)
  To: palmer, pjw, aou, alex, Conor Dooley; +Cc: linux-riscv, linux-kernel

Hi Palmer, Paul, Albert, Alexandre,

Gentle ping on this patch. It has been posted for over two weeks
now and I would appreciate any feedback.

This fixes a frame pointer bug in call_on_irq_stack on RV32 where
the unwinder reads from the alignment padding instead of the
actual saved fp/ra, causing broken stack traces on 32-bit RISC-V.

Please let me know if there are any concerns or if further changes
are needed.

+Cc: Conor Dooley -- adding RISC-V subsystem reviewer.


On 6/3/26 11:53 AM, Rui Qi wrote:
> The frame pointer (s0/fp) in call_on_irq_stack is set using
> STACKFRAME_SIZE_ON_STACK, which equals ALIGN(sizeof(struct stackframe),
> STACK_ALIGN). On RV64, sizeof(struct stackframe) is 16 and the aligned
> size is also 16, so there is no difference. However, on RV32,
> sizeof(struct stackframe) is 8 while STACKFRAME_SIZE_ON_STACK is 16 due
> to 8 bytes of alignment padding.
> 
> The stack unwinder does 'frame = (struct stackframe *)fp - 1', which
> reads from 'fp - sizeof(struct stackframe)'. On RV32, with fp set to
> sp + STACKFRAME_SIZE_ON_STACK (sp + 16), the unwinder reads from
> sp + 8, which falls in the alignment padding rather than where the
> saved fp/ra are actually stored (sp + 0 and sp + 4).
> 
> Fix this by introducing STACKFRAME_SIZE (the unaligned sizeof) and using
> it for frame pointer setup and restoration, while keeping
> STACKFRAME_SIZE_ON_STACK for the stack pointer allocation/deallocation
> which must remain 16-byte aligned.
> 
> Signed-off-by: Rui Qi <qirui.001@bytedance.com>
> ---
>  arch/riscv/kernel/asm-offsets.c | 1 +
>  arch/riscv/kernel/entry.S       | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
> index af827448a609..c1b5f7eb03fd 100644
> --- a/arch/riscv/kernel/asm-offsets.c
> +++ b/arch/riscv/kernel/asm-offsets.c
> @@ -500,6 +500,7 @@ void asm_offsets(void)
>  	OFFSET(SBI_HART_BOOT_TASK_PTR_OFFSET, sbi_hart_boot_data, task_ptr);
>  	OFFSET(SBI_HART_BOOT_STACK_PTR_OFFSET, sbi_hart_boot_data, stack_ptr);
>  
> +	DEFINE(STACKFRAME_SIZE, sizeof(struct stackframe));
>  	DEFINE(STACKFRAME_SIZE_ON_STACK, ALIGN(sizeof(struct stackframe), STACK_ALIGN));
>  	OFFSET(STACKFRAME_FP, stackframe, fp);
>  	OFFSET(STACKFRAME_RA, stackframe, ra);
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index d011fb51c59a..e8b654e2b7b5 100644
> --- 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
>  
>  	/* Switch to the per-CPU shadow call stack */
>  	scs_save_current
> @@ -399,7 +399,7 @@ SYM_FUNC_START(call_on_irq_stack)
>  	scs_load_current
>  
>  	/* Switch back to the thread stack and restore ra and s0 */
> -	addi	sp, s0, -STACKFRAME_SIZE_ON_STACK
> +	addi	sp, s0, -STACKFRAME_SIZE
>  	REG_L	ra, STACKFRAME_RA(sp)
>  	REG_L	s0, STACKFRAME_FP(sp)
>  	addi	sp, sp, STACKFRAME_SIZE_ON_STACK

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32
@ 2026-06-22  2:54   ` Rui Qi
  0 siblings, 0 replies; 6+ messages in thread
From: Rui Qi @ 2026-06-22  2:54 UTC (permalink / raw)
  To: palmer, pjw, aou, alex, Conor Dooley; +Cc: linux-riscv, linux-kernel

Hi Palmer, Paul, Albert, Alexandre,

Gentle ping on this patch. It has been posted for over two weeks
now and I would appreciate any feedback.

This fixes a frame pointer bug in call_on_irq_stack on RV32 where
the unwinder reads from the alignment padding instead of the
actual saved fp/ra, causing broken stack traces on 32-bit RISC-V.

Please let me know if there are any concerns or if further changes
are needed.

+Cc: Conor Dooley -- adding RISC-V subsystem reviewer.


On 6/3/26 11:53 AM, Rui Qi wrote:
> The frame pointer (s0/fp) in call_on_irq_stack is set using
> STACKFRAME_SIZE_ON_STACK, which equals ALIGN(sizeof(struct stackframe),
> STACK_ALIGN). On RV64, sizeof(struct stackframe) is 16 and the aligned
> size is also 16, so there is no difference. However, on RV32,
> sizeof(struct stackframe) is 8 while STACKFRAME_SIZE_ON_STACK is 16 due
> to 8 bytes of alignment padding.
> 
> The stack unwinder does 'frame = (struct stackframe *)fp - 1', which
> reads from 'fp - sizeof(struct stackframe)'. On RV32, with fp set to
> sp + STACKFRAME_SIZE_ON_STACK (sp + 16), the unwinder reads from
> sp + 8, which falls in the alignment padding rather than where the
> saved fp/ra are actually stored (sp + 0 and sp + 4).
> 
> Fix this by introducing STACKFRAME_SIZE (the unaligned sizeof) and using
> it for frame pointer setup and restoration, while keeping
> STACKFRAME_SIZE_ON_STACK for the stack pointer allocation/deallocation
> which must remain 16-byte aligned.
> 
> Signed-off-by: Rui Qi <qirui.001@bytedance.com>
> ---
>  arch/riscv/kernel/asm-offsets.c | 1 +
>  arch/riscv/kernel/entry.S       | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
> index af827448a609..c1b5f7eb03fd 100644
> --- a/arch/riscv/kernel/asm-offsets.c
> +++ b/arch/riscv/kernel/asm-offsets.c
> @@ -500,6 +500,7 @@ void asm_offsets(void)
>  	OFFSET(SBI_HART_BOOT_TASK_PTR_OFFSET, sbi_hart_boot_data, task_ptr);
>  	OFFSET(SBI_HART_BOOT_STACK_PTR_OFFSET, sbi_hart_boot_data, stack_ptr);
>  
> +	DEFINE(STACKFRAME_SIZE, sizeof(struct stackframe));
>  	DEFINE(STACKFRAME_SIZE_ON_STACK, ALIGN(sizeof(struct stackframe), STACK_ALIGN));
>  	OFFSET(STACKFRAME_FP, stackframe, fp);
>  	OFFSET(STACKFRAME_RA, stackframe, ra);
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index d011fb51c59a..e8b654e2b7b5 100644
> --- 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
>  
>  	/* Switch to the per-CPU shadow call stack */
>  	scs_save_current
> @@ -399,7 +399,7 @@ SYM_FUNC_START(call_on_irq_stack)
>  	scs_load_current
>  
>  	/* Switch back to the thread stack and restore ra and s0 */
> -	addi	sp, s0, -STACKFRAME_SIZE_ON_STACK
> +	addi	sp, s0, -STACKFRAME_SIZE
>  	REG_L	ra, STACKFRAME_RA(sp)
>  	REG_L	s0, STACKFRAME_FP(sp)
>  	addi	sp, sp, STACKFRAME_SIZE_ON_STACK

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32
  2026-06-03  3:53 ` Rui Qi
@ 2026-06-22 18:08   ` Nam Cao
  -1 siblings, 0 replies; 6+ messages in thread
From: Nam Cao @ 2026-06-22 18:08 UTC (permalink / raw)
  To: Rui Qi, palmer, pjw, aou, alex; +Cc: Rui Qi, linux-riscv, linux-kernel

"Rui Qi" <qirui.001@bytedance.com> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32
@ 2026-06-22 18:08   ` Nam Cao
  0 siblings, 0 replies; 6+ messages in thread
From: Nam Cao @ 2026-06-22 18:08 UTC (permalink / raw)
  To: Rui Qi, palmer, pjw, aou, alex; +Cc: Rui Qi, linux-riscv, linux-kernel

"Rui Qi" <qirui.001@bytedance.com> 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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-06-22 18:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-03  3:53 [PATCH] riscv: fix frame pointer in call_on_irq_stack for RV32 Rui Qi
2026-06-03  3:53 ` Rui Qi
2026-06-22  2:54 ` Rui Qi
2026-06-22  2:54   ` Rui Qi
2026-06-22 18:08 ` Nam Cao
2026-06-22 18:08   ` Nam Cao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.