On 27.11.18 07:52, Anup Patel wrote:
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:
- 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.
On AArch64, we determine the current EL during runtime and then branch to respective labels for different ELs (see the switch_el macro for asm as well as current_el() for C).
Wouldn't it make sense to attempt something similar with S-mode, so that the same binary can run either in M or in S depending on how it was started?
Alex