[PATCH 3/4] riscv: entry: Do not clobber the frame pointer

From: Samuel Holland
Date: Wed May 29 2024 - 20:18:14 EST


s0 is reserved for the frame pointer, so it should not be used as a
temporary register. Clobbering the frame pointer breaks stack traces.

- In handle_exception() and ret_from_exception(), use a2 for the saved
stack pointer. a2 is chosen because r2 is the stack pointer register.
- In ret_from_exception(), use s1 for the saved status CSR value. Avoid
clobbering s1 in the privilege mode check so it does not need to be
reloaded later in the function.
- Use s1 and s2 in ret_from_fork() instead of s0 and s1. The entire
p->thread.s array is zeroed at the beginning of copy_thread(), so the
registers do not need to be zeroed separately for kernel threads.

Signed-off-by: Samuel Holland <samuel.holland@xxxxxxxxxx>
---

arch/riscv/kernel/entry.S | 29 ++++++++++++++---------------
arch/riscv/kernel/process.c | 5 ++---
2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index d13d1aad7649..bd1c5621df45 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -58,13 +58,13 @@ SYM_CODE_START(handle_exception)
*/
li t0, SR_SUM | SR_FS_VS

- REG_L s0, TASK_TI_USER_SP(tp)
+ REG_L a2, TASK_TI_USER_SP(tp)
csrrc s1, CSR_STATUS, t0
csrr s2, CSR_EPC
csrr s3, CSR_TVAL
csrr s4, CSR_CAUSE
csrr s5, CSR_SCRATCH
- REG_S s0, PT_SP(sp)
+ REG_S a2, PT_SP(sp)
REG_S s1, PT_STATUS(sp)
REG_S s2, PT_EPC(sp)
REG_S s3, PT_BADADDR(sp)
@@ -125,19 +125,19 @@ SYM_CODE_START_NOALIGN(ret_from_exception)
call riscv_v_context_nesting_end
#endif

- REG_L s0, PT_STATUS(sp)
+ REG_L s1, PT_STATUS(sp)
#ifdef CONFIG_RISCV_M_MODE
/* the MPP value is too large to be used as an immediate arg for addi */
li t0, SR_MPP
- and s0, s0, t0
+ and t0, s1, t0
#else
- andi s0, s0, SR_SPP
+ andi t0, s1, SR_SPP
#endif
- bnez s0, 1f
+ bnez t0, 1f

/* Save unwound kernel stack pointer in thread_info */
- addi s0, sp, PT_SIZE_ON_STACK
- REG_S s0, TASK_TI_KERNEL_SP(tp)
+ addi t0, sp, PT_SIZE_ON_STACK
+ REG_S t0, TASK_TI_KERNEL_SP(tp)

/* Save the kernel shadow call stack pointer */
scs_save_current
@@ -148,7 +148,6 @@ SYM_CODE_START_NOALIGN(ret_from_exception)
*/
csrw CSR_SCRATCH, tp
1:
- REG_L a0, PT_STATUS(sp)
/*
* The current load reservation is effectively part of the processor's
* state, in the sense that load reservations cannot be shared between
@@ -169,7 +168,7 @@ SYM_CODE_START_NOALIGN(ret_from_exception)
REG_L a2, PT_EPC(sp)
REG_SC x0, a2, PT_EPC(sp)

- csrw CSR_STATUS, a0
+ csrw CSR_STATUS, s1
csrw CSR_EPC, a2

REG_L x1, PT_RA(sp)
@@ -207,13 +206,13 @@ SYM_CODE_START_LOCAL(handle_kernel_stack_overflow)
REG_S x5, PT_T0(sp)
save_from_x6_to_x31

- REG_L s0, TASK_TI_KERNEL_SP(tp)
+ REG_L a2, TASK_TI_KERNEL_SP(tp)
csrr s1, CSR_STATUS
csrr s2, CSR_EPC
csrr s3, CSR_TVAL
csrr s4, CSR_CAUSE
csrr s5, CSR_SCRATCH
- REG_S s0, PT_SP(sp)
+ REG_S a2, PT_SP(sp)
REG_S s1, PT_STATUS(sp)
REG_S s2, PT_EPC(sp)
REG_S s3, PT_BADADDR(sp)
@@ -227,10 +226,10 @@ ASM_NOKPROBE(handle_kernel_stack_overflow)

SYM_CODE_START(ret_from_fork)
call schedule_tail
- beqz s0, 1f /* not from kernel thread */
+ beqz s1, 1f /* not from kernel thread */
/* Call fn(arg) */
- move a0, s1
- jalr s0
+ move a0, s2
+ jalr s1
1:
move a0, sp /* pt_regs */
la ra, ret_from_exception
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index e4bc61c4e58a..5512c31e1256 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -208,8 +208,8 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
/* Supervisor/Machine, irqs on: */
childregs->status = SR_PP | SR_PIE;

- p->thread.s[0] = (unsigned long)args->fn;
- p->thread.s[1] = (unsigned long)args->fn_arg;
+ p->thread.s[1] = (unsigned long)args->fn;
+ p->thread.s[2] = (unsigned long)args->fn_arg;
} else {
*childregs = *(current_pt_regs());
/* Turn off status.VS */
@@ -219,7 +219,6 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
if (clone_flags & CLONE_SETTLS)
childregs->tp = tls;
childregs->a0 = 0; /* Return value of fork() */
- p->thread.s[0] = 0;
}
p->thread.riscv_v_flags = 0;
if (has_vector())
--
2.44.1