On 27.11.18 07:52, Anup Patel wrote: > On Tue, Nov 27, 2018 at 12:09 PM Rick Chen <rickche...@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 <a...@brainfault.org> >>>> Reviewed-by: Bin Meng <bmeng...@gmail.com> >>>> Tested-by: Bin Meng <bmeng...@gmail.com> >>>> Reviewed-by: Lukas Auer <lukas.a...@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" >>>> + help >>>> + Enable this option to build U-Boot for RISC-V S-Mode >>>> + >>>> config 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, scause >>>> + csrr 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_SPP >>>> + csrs 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.
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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
