| Message ID | 20181126103910.14457-2-anup@brainfault.org |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | [U-Boot,v5,1/4] riscv: Add kconfig option to run U-Boot in S-mode | expand |
> > 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" > > + 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. B.R Rick > > LREG x1, 1*REGBYTES(sp) > > LREG x2, 2*REGBYTES(sp) > > LREG x3, 3*REGBYTES(sp) > > @@ -281,4 +310,8 @@ trap_entry: > > LREG x30, 30*REGBYTES(sp) > > LREG x31, 31*REGBYTES(sp) > > addi sp, sp, 32*REGBYTES > > +#ifdef CONFIG_RISCV_SMODE > > + sret > > +#else > > mret > > +#endif > > diff --git a/arch/riscv/include/asm/encoding.h > > b/arch/riscv/include/asm/encoding.h > > index 9ea50ce640..153f5af2ff 100644 > > --- a/arch/riscv/include/asm/encoding.h > > +++ b/arch/riscv/include/asm/encoding.h > > @@ -143,6 +143,8 @@ > > # define MCAUSE_CAUSE MCAUSE32_CAUSE > > #endif > > > > +#define SCAUSE_INT MCAUSE_INT > > + > > #define RISCV_PGSHIFT 12 > > #define RISCV_PGSIZE BIT(RISCV_PGSHIFT) > > > > diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c index > > 903a1c4cd5..8793f233d0 100644 > > --- a/arch/riscv/lib/interrupts.c > > +++ b/arch/riscv/lib/interrupts.c > > @@ -34,17 +34,35 @@ int disable_interrupts(void) > > return 0; > > } > > > > -ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs) > > +ulong handle_trap(ulong cause, ulong epc, struct pt_regs *regs) > > { > > - ulong is_int; > > + ulong is_irq, irq; > > > > - is_int = (mcause & MCAUSE_INT); > > - if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_EXT)) > > - external_interrupt(0); /* handle_m_ext_interrupt */ > > - else if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_TIMER)) > > - timer_interrupt(0); /* handle_m_timer_interrupt */ > > - else > > - _exit_trap(mcause, epc, regs); > > +#ifdef CONFIG_RISCV_SMODE > > + is_irq = (cause & SCAUSE_INT); > > + irq = (cause & ~SCAUSE_INT); > > +#else > > + is_irq = (cause & MCAUSE_INT); > > + irq = (cause & ~MCAUSE_INT); > > +#endif > > + > > + if (is_irq) { > > + switch (irq) { > > + case IRQ_M_EXT: > > + case IRQ_S_EXT: > > + external_interrupt(0); /* handle external interrupt */ > > + break; > > + case IRQ_M_TIMER: > > + case IRQ_S_TIMER: > > + timer_interrupt(0); /* handle timer interrupt */ > > + break; > > + default: > > + _exit_trap(cause, epc, regs); > > + break; > > + }; > > + } else { > > + _exit_trap(cause, epc, regs); > > + } > > > > return epc; > > } > > -- > > 2.17.1 >
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" > > > + 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. Regards, Anup
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" >>>> + 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
On Tue, Nov 27, 2018 at 4:17 PM Alexander Graf <agraf@suse.de> wrote: > > > > 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" > >>>> + 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
Anup Patel <anup@brainfault.org> 於 2018年11月27日 週二 下午8:38寫道： > > On Tue, Nov 27, 2018 at 4:17 PM Alexander Graf <agraf@suse.de> wrote: > > > > > > > > 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" > > >>>> + 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. > > Hi Anup I mean it is not a good way to switch s and m mode like that #ifdef CONFIG_RISCV_SMODE csrw sepc, a0 #else csrw mepc, a0 #endif You can use ## to get stvec ot mtvec in different CONFIG It can be refered to some kernel code as below: glue.h #define ____glue(name,fn) name##fn #define __glue(name,fn) ____glue(name,fn) glue-proc.h #ifdef CONFIG_CPU_ARM720T #define CPU_NAME cpu_arm720 #else #define CPU_NAME cpu_arm7tdmi #endif #define cpu_proc_init __glue(CPU_NAME,_proc_init) Then It will compile as cpu_arm720_proc_init or cpu_arm7tdmi_proc_init in different configuration. stvec and mtvce can be implement by the same way. B.R Rick > > 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
On Fri, Nov 30, 2018 at 12:34 PM Rick Chen <rickchen36@gmail.com> wrote: > > Anup Patel <anup@brainfault.org> 於 2018年11月27日 週二 下午8:38寫道： > > > > On Tue, Nov 27, 2018 at 4:17 PM Alexander Graf <agraf@suse.de> wrote: > > > > > > > > > > > > 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" > > > >>>> + 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. > > > > > Hi Anup > > I mean it is not a good way to switch s and m mode like that > #ifdef CONFIG_RISCV_SMODE > csrw sepc, a0 > #else > csrw mepc, a0 > #endif > > > You can use ## to get stvec ot mtvec in different CONFIG > It can be refered to some kernel code as below: > glue.h > #define ____glue(name,fn) name##fn > #define __glue(name,fn) ____glue(name,fn) > > glue-proc.h > #ifdef CONFIG_CPU_ARM720T > #define CPU_NAME cpu_arm720 > #else > #define CPU_NAME cpu_arm7tdmi > #endif > > #define cpu_proc_init __glue(CPU_NAME,_proc_init) > > Then It will compile as cpu_arm720_proc_init or cpu_arm7tdmi_proc_init > in different configuration. > stvec and mtvce can be implement by the same way. I had understood your suggestion previously. Though I am not fan of this approach, I will change it anyway. Thanks, Anup
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 LREG x1, 1*REGBYTES(sp) LREG x2, 2*REGBYTES(sp) LREG x3, 3*REGBYTES(sp) @@ -281,4 +310,8 @@ trap_entry: LREG x30, 30*REGBYTES(sp) LREG x31, 31*REGBYTES(sp) addi sp, sp, 32*REGBYTES +#ifdef CONFIG_RISCV_SMODE + sret +#else mret +#endif diff --git a/arch/riscv/include/asm/encoding.h b/arch/riscv/include/asm/encoding.h index 9ea50ce640..153f5af2ff 100644 --- a/arch/riscv/include/asm/encoding.h +++ b/arch/riscv/include/asm/encoding.h @@ -143,6 +143,8 @@ # define MCAUSE_CAUSE MCAUSE32_CAUSE #endif +#define SCAUSE_INT MCAUSE_INT + #define RISCV_PGSHIFT 12 #define RISCV_PGSIZE BIT(RISCV_PGSHIFT) diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c index 903a1c4cd5..8793f233d0 100644 --- a/arch/riscv/lib/interrupts.c +++ b/arch/riscv/lib/interrupts.c @@ -34,17 +34,35 @@ int disable_interrupts(void) return 0; } -ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs) +ulong handle_trap(ulong cause, ulong epc, struct pt_regs *regs) { - ulong is_int; + ulong is_irq, irq; - is_int = (mcause & MCAUSE_INT); - if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_EXT)) - external_interrupt(0); /* handle_m_ext_interrupt */ - else if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_TIMER)) - timer_interrupt(0); /* handle_m_timer_interrupt */ - else - _exit_trap(mcause, epc, regs); +#ifdef CONFIG_RISCV_SMODE + is_irq = (cause & SCAUSE_INT); + irq = (cause & ~SCAUSE_INT); +#else + is_irq = (cause & MCAUSE_INT); + irq = (cause & ~MCAUSE_INT); +#endif + + if (is_irq) { + switch (irq) { + case IRQ_M_EXT: + case IRQ_S_EXT: + external_interrupt(0); /* handle external interrupt */ + break; + case IRQ_M_TIMER: + case IRQ_S_TIMER: + timer_interrupt(0); /* handle timer interrupt */ + break; + default: + _exit_trap(cause, epc, regs); + break; + }; + } else { + _exit_trap(cause, epc, regs); + } return epc; }