2022-09-25 18:15:57

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH 0/4] riscv: entry: further clean up and VMAP_STACK fix

I planed to do similar generic entry transaction as Guo Ren did[1], and
I have some commits in local. Since Guo has sent out the series, so I
dropped my version and just provide those in my local repo but missing
in Guo's series. However, this doesn't mean this series depends on
Guo's series, in fact except the first one, the remaining three patches
are independent on generic entry.

NOTE: patch4 can also clean up arch/riscv/kernel/mcount-dyn.S as well
but there's a trivial difference in the context saving, I dunno whether
is it better to clean up mcount-dyn.S too, any suggestions are welcome.

[1]https://lore.kernel.org/linux-riscv/[email protected]/T/#t

Jisheng Zhang (4):
riscv: remove extra level wrappers of trace_hardirqs_{on,off}
riscv: consolidate ret_from_kernel_thread into ret_from_fork
riscv: fix race when vmap stack overflow and remove shadow_stack
riscv: entry: consolidate general regs saving into save_gp

arch/riscv/include/asm/asm-prototypes.h | 1 -
arch/riscv/include/asm/thread_info.h | 4 +-
arch/riscv/kernel/Makefile | 2 -
arch/riscv/kernel/asm-offsets.c | 1 +
arch/riscv/kernel/entry.S | 150 +++++++-----------------
arch/riscv/kernel/process.c | 5 +-
arch/riscv/kernel/trace_irq.c | 27 -----
arch/riscv/kernel/trace_irq.h | 11 --
arch/riscv/kernel/traps.c | 15 +--
9 files changed, 47 insertions(+), 169 deletions(-)
delete mode 100644 arch/riscv/kernel/trace_irq.c
delete mode 100644 arch/riscv/kernel/trace_irq.h

--
2.34.1


2022-09-25 18:46:17

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH 4/4] riscv: entry: consolidate general regs saving into save_gp

Consolidate the saving GPs(except sp and tp into save_gp macro.
No functional change.

Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/riscv/kernel/entry.S | 85 ++++++++++++++-------------------------
1 file changed, 31 insertions(+), 54 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 442d93beffcf..04e11d257ad6 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -14,31 +14,8 @@
#include <asm/asm-offsets.h>
#include <asm/errata_list.h>

-ENTRY(handle_exception)
- /*
- * If coming from userspace, preserve the user thread pointer and load
- * the kernel thread pointer. If we came from the kernel, the scratch
- * register will contain 0, and we should continue on the current TP.
- */
- csrrw tp, CSR_SCRATCH, tp
- bnez tp, _save_context
-
-_restore_kernel_tpsp:
- csrr tp, CSR_SCRATCH
- REG_S sp, TASK_TI_KERNEL_SP(tp)
-
-#ifdef CONFIG_VMAP_STACK
- addi sp, sp, -(PT_SIZE_ON_STACK)
- srli sp, sp, THREAD_SHIFT
- andi sp, sp, 0x1
- bnez sp, handle_kernel_stack_overflow
- REG_L sp, TASK_TI_KERNEL_SP(tp)
-#endif
-
-_save_context:
- REG_S sp, TASK_TI_USER_SP(tp)
- REG_L sp, TASK_TI_KERNEL_SP(tp)
- addi sp, sp, -(PT_SIZE_ON_STACK)
+ /* save all GPs except sp and tp */
+ .macro save_gp
REG_S x1, PT_RA(sp)
REG_S x3, PT_GP(sp)
REG_S x5, PT_T0(sp)
@@ -68,6 +45,34 @@ _save_context:
REG_S x29, PT_T4(sp)
REG_S x30, PT_T5(sp)
REG_S x31, PT_T6(sp)
+ .endm
+
+ENTRY(handle_exception)
+ /*
+ * If coming from userspace, preserve the user thread pointer and load
+ * the kernel thread pointer. If we came from the kernel, the scratch
+ * register will contain 0, and we should continue on the current TP.
+ */
+ csrrw tp, CSR_SCRATCH, tp
+ bnez tp, _save_context
+
+_restore_kernel_tpsp:
+ csrr tp, CSR_SCRATCH
+ REG_S sp, TASK_TI_KERNEL_SP(tp)
+
+#ifdef CONFIG_VMAP_STACK
+ addi sp, sp, -(PT_SIZE_ON_STACK)
+ srli sp, sp, THREAD_SHIFT
+ andi sp, sp, 0x1
+ bnez sp, handle_kernel_stack_overflow
+ REG_L sp, TASK_TI_KERNEL_SP(tp)
+#endif
+
+_save_context:
+ REG_S sp, TASK_TI_USER_SP(tp)
+ REG_L sp, TASK_TI_KERNEL_SP(tp)
+ addi sp, sp, -(PT_SIZE_ON_STACK)
+ save_gp

/*
* Disable user-mode memory access as it should only be set in the
@@ -234,35 +239,7 @@ ENTRY(handle_kernel_stack_overflow)
addi sp, sp, -(PT_SIZE_ON_STACK)

//save context to overflow stack
- REG_S x1, PT_RA(sp)
- REG_S x3, PT_GP(sp)
- REG_S x5, PT_T0(sp)
- REG_S x6, PT_T1(sp)
- REG_S x7, PT_T2(sp)
- REG_S x8, PT_S0(sp)
- REG_S x9, PT_S1(sp)
- REG_S x10, PT_A0(sp)
- REG_S x11, PT_A1(sp)
- REG_S x12, PT_A2(sp)
- REG_S x13, PT_A3(sp)
- REG_S x14, PT_A4(sp)
- REG_S x15, PT_A5(sp)
- REG_S x16, PT_A6(sp)
- REG_S x17, PT_A7(sp)
- REG_S x18, PT_S2(sp)
- REG_S x19, PT_S3(sp)
- REG_S x20, PT_S4(sp)
- REG_S x21, PT_S5(sp)
- REG_S x22, PT_S6(sp)
- REG_S x23, PT_S7(sp)
- REG_S x24, PT_S8(sp)
- REG_S x25, PT_S9(sp)
- REG_S x26, PT_S10(sp)
- REG_S x27, PT_S11(sp)
- REG_S x28, PT_T3(sp)
- REG_S x29, PT_T4(sp)
- REG_S x30, PT_T5(sp)
- REG_S x31, PT_T6(sp)
+ save_gp

REG_L s0, TASK_TI_KERNEL_SP(tp)
csrr s1, CSR_STATUS
--
2.34.1

2022-09-26 00:11:32

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 4/4] riscv: entry: consolidate general regs saving into save_gp

Acked-by: Guo Ren <kernel.org>

For the save & restore pair, I also suggest adding restore_gp.

On Mon, Sep 26, 2022 at 2:03 AM Jisheng Zhang <[email protected]> wrote:
>
> Consolidate the saving GPs(except sp and tp into save_gp macro.
> No functional change.
>
> Signed-off-by: Jisheng Zhang <[email protected]>
> ---
> arch/riscv/kernel/entry.S | 85 ++++++++++++++-------------------------
> 1 file changed, 31 insertions(+), 54 deletions(-)
>
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index 442d93beffcf..04e11d257ad6 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -14,31 +14,8 @@
> #include <asm/asm-offsets.h>
> #include <asm/errata_list.h>
>
> -ENTRY(handle_exception)
> - /*
> - * If coming from userspace, preserve the user thread pointer and load
> - * the kernel thread pointer. If we came from the kernel, the scratch
> - * register will contain 0, and we should continue on the current TP.
> - */
> - csrrw tp, CSR_SCRATCH, tp
> - bnez tp, _save_context
> -
> -_restore_kernel_tpsp:
> - csrr tp, CSR_SCRATCH
> - REG_S sp, TASK_TI_KERNEL_SP(tp)
> -
> -#ifdef CONFIG_VMAP_STACK
> - addi sp, sp, -(PT_SIZE_ON_STACK)
> - srli sp, sp, THREAD_SHIFT
> - andi sp, sp, 0x1
> - bnez sp, handle_kernel_stack_overflow
> - REG_L sp, TASK_TI_KERNEL_SP(tp)
> -#endif
> -
> -_save_context:
> - REG_S sp, TASK_TI_USER_SP(tp)
> - REG_L sp, TASK_TI_KERNEL_SP(tp)
> - addi sp, sp, -(PT_SIZE_ON_STACK)
> + /* save all GPs except sp and tp */
> + .macro save_gp
> REG_S x1, PT_RA(sp)
> REG_S x3, PT_GP(sp)
> REG_S x5, PT_T0(sp)
> @@ -68,6 +45,34 @@ _save_context:
> REG_S x29, PT_T4(sp)
> REG_S x30, PT_T5(sp)
> REG_S x31, PT_T6(sp)
> + .endm
> +
> +ENTRY(handle_exception)
> + /*
> + * If coming from userspace, preserve the user thread pointer and load
> + * the kernel thread pointer. If we came from the kernel, the scratch
> + * register will contain 0, and we should continue on the current TP.
> + */
> + csrrw tp, CSR_SCRATCH, tp
> + bnez tp, _save_context
> +
> +_restore_kernel_tpsp:
> + csrr tp, CSR_SCRATCH
> + REG_S sp, TASK_TI_KERNEL_SP(tp)
> +
> +#ifdef CONFIG_VMAP_STACK
> + addi sp, sp, -(PT_SIZE_ON_STACK)
> + srli sp, sp, THREAD_SHIFT
> + andi sp, sp, 0x1
> + bnez sp, handle_kernel_stack_overflow
> + REG_L sp, TASK_TI_KERNEL_SP(tp)
> +#endif
> +
> +_save_context:
> + REG_S sp, TASK_TI_USER_SP(tp)
> + REG_L sp, TASK_TI_KERNEL_SP(tp)
> + addi sp, sp, -(PT_SIZE_ON_STACK)
> + save_gp
>
> /*
> * Disable user-mode memory access as it should only be set in the
> @@ -234,35 +239,7 @@ ENTRY(handle_kernel_stack_overflow)
> addi sp, sp, -(PT_SIZE_ON_STACK)
>
> //save context to overflow stack
> - REG_S x1, PT_RA(sp)
> - REG_S x3, PT_GP(sp)
> - REG_S x5, PT_T0(sp)
> - REG_S x6, PT_T1(sp)
> - REG_S x7, PT_T2(sp)
> - REG_S x8, PT_S0(sp)
> - REG_S x9, PT_S1(sp)
> - REG_S x10, PT_A0(sp)
> - REG_S x11, PT_A1(sp)
> - REG_S x12, PT_A2(sp)
> - REG_S x13, PT_A3(sp)
> - REG_S x14, PT_A4(sp)
> - REG_S x15, PT_A5(sp)
> - REG_S x16, PT_A6(sp)
> - REG_S x17, PT_A7(sp)
> - REG_S x18, PT_S2(sp)
> - REG_S x19, PT_S3(sp)
> - REG_S x20, PT_S4(sp)
> - REG_S x21, PT_S5(sp)
> - REG_S x22, PT_S6(sp)
> - REG_S x23, PT_S7(sp)
> - REG_S x24, PT_S8(sp)
> - REG_S x25, PT_S9(sp)
> - REG_S x26, PT_S10(sp)
> - REG_S x27, PT_S11(sp)
> - REG_S x28, PT_T3(sp)
> - REG_S x29, PT_T4(sp)
> - REG_S x30, PT_T5(sp)
> - REG_S x31, PT_T6(sp)
> + save_gp
>
> REG_L s0, TASK_TI_KERNEL_SP(tp)
> csrr s1, CSR_STATUS
> --
> 2.34.1
>


--
Best Regards
Guo Ren