On Tue, Nov 27, 2018 at 12:09 PM Rick Chen rickchen36@gmail.com wrote:
Subject: [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot in S-mode
This patch adds kconfig option RISCV_SMODE to run U-Boot in S-mode. When this opition is enabled we use s<xyz> CSRs instead of m<xyz> CSRs.
It is important to note that there is no equivalent S-mode CSR for misa and mhartid CSRs so we expect M-mode runtime firmware (BBL or equivalent) to emulate misa and mhartid CSR read.
In-future, we will have more patches to avoid accessing misa and mhartid CSRs from S-mode.
Signed-off-by: Anup Patel anup@brainfault.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
arch/riscv/Kconfig | 5 +++++ arch/riscv/cpu/start.S | 33 ++++++++++++++++++++++++++++ arch/riscv/include/asm/encoding.h | 2 ++ arch/riscv/lib/interrupts.c | 36 +++++++++++++++++++++++-------- 4 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 3e0af55e71..732a357a99 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -55,6 +55,11 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
+config RISCV_SMODE
bool "Run in S-Mode"helpEnable this option to build U-Boot for RISC-V S-Modeconfig 32BIT bool
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 15e1b8199a..704190f946 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -41,10 +41,18 @@ _start: li t0, CONFIG_SYS_SDRAM_BASE SREG a2, 0(t0) la t0, trap_entry +#ifdef CONFIG_RISCV_SMODE
csrw stvec, t0+#else csrw mtvec, t0 +#endif
/* mask all interrupts */+#ifdef CONFIG_RISCV_SMODE
csrw sie, zero+#else csrw mie, zero +#endif
/* Enable cache */ jal icache_enable@@ -166,7 +174,11 @@ fix_rela_dyn: */ la t0, trap_entry add t0, t0, t6 +#ifdef CONFIG_RISCV_SMODE
csrw stvec, t0+#else csrw mtvec, t0 +#endif
clear_bss: la t0, __bss_start /* t0 <- rel __bss_start in FLASH */ @@ -238,17 +250,34 @@ trap_entry: SREG x29, 29*REGBYTES(sp) SREG x30, 30*REGBYTES(sp) SREG x31, 31*REGBYTES(sp) +#ifdef CONFIG_RISCV_SMODE
csrr a0, scausecsrr a1, sepc+#else csrr a0, mcause csrr a1, mepc +#endif mv a2, sp jal handle_trap +#ifdef CONFIG_RISCV_SMODE
csrw sepc, a0+#else csrw mepc, a0 +#endif
+#ifdef CONFIG_RISCV_SMODE +/*
- Remain in S-mode after sret
- */
li t0, SSTATUS_SPPcsrs sstatus, t0+#else /*
- Remain in M-mode after mret
*/ li t0, MSTATUS_MPP csrs mstatus, t0 +#endif
It only the difference between s and m in csr. The code seem to be duplicate too much. Can you implement it in macro ? or consider to separate it in different .S file
It may too complex to maintain in the future.
Here are few reasons why not to do this: 1. We don't have same set of CSRs for M-mode and S-mode. For certain, M-mode CSRs we don't have any corresponding S-mode CSRs. 2. Number of CSRs for each mode are really few so there won't be much #ifdef in code. Having separate .S will be total overkill and too much code duplication. 3. Using macro would mean we use incomplete CSR names examples: "status" for "mstatus"/"sstatus", "epc" for "mepc"/"sepc", etc. This means we cannot grep code for use of a CSR. I think this is less readable compared to using #ifdef
Despite above reasons, above can always be attempted as separate patch.
Regards, Anup