From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anup Patel Date: Tue, 27 Nov 2018 18:08:06 +0530 Subject: [U-Boot] [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot in S-mode In-Reply-To: References: <20181126103910.14457-1-anup@brainfault.org> <20181126103910.14457-2-anup@brainfault.org> <752D002CFF5D0F4FA35C0100F1D73F3FA3A50B90@ATCPCS16.andestech.com>

Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Tue, Nov 27, 2018 at 4:17 PM Alexander Graf wrote: > > > > On 27.11.18 07:52, Anup Patel wrote: > > On Tue, Nov 27, 2018 at 12:09 PM Rick Chen 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 CSRs instead of m 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 > >>>> Reviewed-by: Bin Meng > >>>> Tested-by: Bin Meng > >>>> Reviewed-by: Lukas Auer > >>>> --- > >>>> 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? As-per my understand, there is no direct way of knowing current execution mode in RISC-V at runtime. Of course, software can step from M to S/U using mret OR from S to U using sret. Further software running in U-mode can do ecall to S-mode and S-mode can do ecall to M-mode. Software is generally written for a particular execution mode (M, S, or U). Regards, Anup