Message ID | 20181121034112.7136-2-anup@brainfault.org |
---|---|
State | Superseded |
Delegated to: | Andes |
Headers | show |
Series | [U-Boot,v3,1/3] riscv: Add kconfig option to run u-boot in S-mode | expand |
Hi Anup, On Wed, 2018-11-21 at 09:11 +0530, Anup Patel wrote: > This patch adds kconfig option RISCV_SMODE to run u-boot in nit: U-Boot (also in the first line of the commit message) > 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> > --- > arch/riscv/Kconfig | 5 +++++ > arch/riscv/cpu/start.S | 33 +++++++++++++++++++++++++++++++++ > 2 files changed, 38 insertions(+) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 3e0af55e71..8f2139ff60 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 an U-Boot for RISC-V S-Mode nit: there should be no "an" here > + > config 32BIT > bool > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > index 5af189b338..e4276e8e19 100644 > --- a/arch/riscv/cpu/start.S > +++ b/arch/riscv/cpu/start.S > @@ -39,10 +39,18 @@ _start: > mv s1, a1 > > la t0, trap_entry > +#ifdef CONFIG_RISCV_SMODE It might make the code more readable by using defines for the CSRs, which are set to the correct values at the start of start.S depending on if CONFIG_RISCV_SMODE is set (similar to what we do for the load and store instructions). This is just a thought, I think this way is perfectly fine as well. :) Reviewed-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> Thanks, Lukas > + 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 > @@ -164,7 +172,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 */ > @@ -236,17 +248,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) > @@ -279,4 +308,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
On Tue, 20 Nov 2018 19:41:10 PST (-0800), anup@brainfault.org wrote: > 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. Ya, I don't like this. Our current boot protocol puts mhartid in a1 upon entering the supervisor, and allows the detection of misa via a device tree pointer provided in a0. As long as everyone agrees this isn't what we're actually looking for then I'm fine with the patch, I just don't want to end up requiring this "M mode emulates some CSRs for S mode" interface. Since we don't have an actual platform spec we've got to be careful to avoid a bunch of defacto interfaces that we'll need to support later. > > Signed-off-by: Anup Patel <anup@brainfault.org> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > Tested-by: Bin Meng <bmeng.cn@gmail.com> > --- > arch/riscv/Kconfig | 5 +++++ > arch/riscv/cpu/start.S | 33 +++++++++++++++++++++++++++++++++ > 2 files changed, 38 insertions(+) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 3e0af55e71..8f2139ff60 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 an 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 5af189b338..e4276e8e19 100644 > --- a/arch/riscv/cpu/start.S > +++ b/arch/riscv/cpu/start.S > @@ -39,10 +39,18 @@ _start: > mv s1, a1 > > 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 > @@ -164,7 +172,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 */ > @@ -236,17 +248,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) > @@ -279,4 +308,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
Hi Palmer, On Wed, 2018-11-21 at 08:28 -0800, Palmer Dabbelt wrote: > On Tue, 20 Nov 2018 19:41:10 PST (-0800), anup@brainfault.org wrote: > > 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. > > Ya, I don't like this. Our current boot protocol puts mhartid in a1 > upon > entering the supervisor, and allows the detection of misa via a > device tree > pointer provided in a0. > > As long as everyone agrees this isn't what we're actually looking for > then I'm > fine with the patch, I just don't want to end up requiring this "M > mode > emulates some CSRs for S mode" interface. Since we don't have an > actual > platform spec we've got to be careful to avoid a bunch of defacto > interfaces > that we'll need to support later. > I agree with you. We actually won't need the CSR emulation of misa and mhartid for long. With Bin's recent patch series [1], the ISA string is read from the device tree, replacing the use of misa. mhartid is only used to pass the hart id to the kernel. I am going to replace it in a patch series I am working on. So the interface should just be temporary. Thanks, Lukas [1]: https://patchwork.ozlabs.org/project/uboot/list/?series=75644 > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > Tested-by: Bin Meng <bmeng.cn@gmail.com> > > --- > > arch/riscv/Kconfig | 5 +++++ > > arch/riscv/cpu/start.S | 33 +++++++++++++++++++++++++++++++++ > > 2 files changed, 38 insertions(+) > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index 3e0af55e71..8f2139ff60 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 an 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 5af189b338..e4276e8e19 100644 > > --- a/arch/riscv/cpu/start.S > > +++ b/arch/riscv/cpu/start.S > > @@ -39,10 +39,18 @@ _start: > > mv s1, a1 > > > > 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 > > @@ -164,7 +172,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 > > */ > > @@ -236,17 +248,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) > > @@ -279,4 +308,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
Hi Anup, On Wed, 2018-11-21 at 13:47 +0000, Auer, Lukas wrote: > Hi Anup, > > On Wed, 2018-11-21 at 09:11 +0530, Anup Patel wrote: > > This patch adds kconfig option RISCV_SMODE to run u-boot in > > nit: U-Boot (also in the first line of the commit message) > > > S-mode. When this opition is enabled we use s<xyz> CSRs instead > > of m<xyz> CSRs. > > Can you also modify arch/riscv/lib/interrupts.c to test for the supervisor instead of the machine timer and external interrupts when CONFIG_RISCV_SMODE is set? Thanks, Lukas > > 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> > > --- > > arch/riscv/Kconfig | 5 +++++ > > arch/riscv/cpu/start.S | 33 +++++++++++++++++++++++++++++++++ > > 2 files changed, 38 insertions(+) > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index 3e0af55e71..8f2139ff60 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 an U-Boot for RISC-V S-Mode > > nit: there should be no "an" here > > > + > > config 32BIT > > bool > > > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > index 5af189b338..e4276e8e19 100644 > > --- a/arch/riscv/cpu/start.S > > +++ b/arch/riscv/cpu/start.S > > @@ -39,10 +39,18 @@ _start: > > mv s1, a1 > > > > la t0, trap_entry > > +#ifdef CONFIG_RISCV_SMODE > > It might make the code more readable by using defines for the CSRs, > which are set to the correct values at the start of start.S depending > on if CONFIG_RISCV_SMODE is set (similar to what we do for the load > and > store instructions). > This is just a thought, I think this way is perfectly fine as well. > :) > > Reviewed-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> > > Thanks, > Lukas > > > + 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 > > @@ -164,7 +172,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 */ > > @@ -236,17 +248,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) > > @@ -279,4 +308,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 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On Wed, 21 Nov 2018 14:06:00 PST (-0800), lukas.auer@aisec.fraunhofer.de wrote: > Hi Palmer, > > On Wed, 2018-11-21 at 08:28 -0800, Palmer Dabbelt wrote: >> On Tue, 20 Nov 2018 19:41:10 PST (-0800), anup@brainfault.org wrote: >> > 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. >> >> Ya, I don't like this. Our current boot protocol puts mhartid in a1 >> upon >> entering the supervisor, and allows the detection of misa via a >> device tree >> pointer provided in a0. >> >> As long as everyone agrees this isn't what we're actually looking for >> then I'm >> fine with the patch, I just don't want to end up requiring this "M >> mode >> emulates some CSRs for S mode" interface. Since we don't have an >> actual >> platform spec we've got to be careful to avoid a bunch of defacto >> interfaces >> that we'll need to support later. >> > > I agree with you. We actually won't need the CSR emulation of misa and > mhartid for long. With Bin's recent patch series [1], the ISA string is > read from the device tree, replacing the use of misa. mhartid is only > used to pass the hart id to the kernel. I am going to replace it in a > patch series I am working on. > So the interface should just be temporary. Great, thanks! > > Thanks, > Lukas > > [1]: https://patchwork.ozlabs.org/project/uboot/list/?series=75644 > >> > >> > Signed-off-by: Anup Patel <anup@brainfault.org> >> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> >> > Tested-by: Bin Meng <bmeng.cn@gmail.com> >> > --- >> > arch/riscv/Kconfig | 5 +++++ >> > arch/riscv/cpu/start.S | 33 +++++++++++++++++++++++++++++++++ >> > 2 files changed, 38 insertions(+) >> > >> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >> > index 3e0af55e71..8f2139ff60 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 an 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 5af189b338..e4276e8e19 100644 >> > --- a/arch/riscv/cpu/start.S >> > +++ b/arch/riscv/cpu/start.S >> > @@ -39,10 +39,18 @@ _start: >> > mv s1, a1 >> > >> > 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 >> > @@ -164,7 +172,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 >> > */ >> > @@ -236,17 +248,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) >> > @@ -279,4 +308,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
On Wed, Nov 21, 2018 at 9:58 PM Palmer Dabbelt <palmer@sifive.com> wrote: > > On Tue, 20 Nov 2018 19:41:10 PST (-0800), anup@brainfault.org wrote: > > 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. > > Ya, I don't like this. Our current boot protocol puts mhartid in a1 upon > entering the supervisor, and allows the detection of misa via a device tree > pointer provided in a0. > > As long as everyone agrees this isn't what we're actually looking for then I'm > fine with the patch, I just don't want to end up requiring this "M mode > emulates some CSRs for S mode" interface. Since we don't have an actual > platform spec we've got to be careful to avoid a bunch of defacto interfaces > that we'll need to support later. Yes, it's a temporary thingy to emulate misa and mhartid in BBL. Eventually, U-Boot will be fixed to avoid direct access of misa and mhartid. Regards, Anup
On Thu, Nov 22, 2018 at 3:41 AM Auer, Lukas <lukas.auer@aisec.fraunhofer.de> wrote: > > Hi Anup, > > On Wed, 2018-11-21 at 13:47 +0000, Auer, Lukas wrote: > > Hi Anup, > > > > On Wed, 2018-11-21 at 09:11 +0530, Anup Patel wrote: > > > This patch adds kconfig option RISCV_SMODE to run u-boot in > > > > nit: U-Boot (also in the first line of the commit message) > > > > > S-mode. When this opition is enabled we use s<xyz> CSRs instead > > > of m<xyz> CSRs. > > > > > Can you also modify arch/riscv/lib/interrupts.c to test for the > supervisor instead of the machine timer and external interrupts when > CONFIG_RISCV_SMODE is set? > Sure, will do. Regards, Anup
Hi Palmer, On Thu, Nov 22, 2018 at 12:28 AM Palmer Dabbelt <palmer@sifive.com> wrote: > > On Tue, 20 Nov 2018 19:41:10 PST (-0800), anup@brainfault.org wrote: > > 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. > > Ya, I don't like this. Our current boot protocol puts mhartid in a1 upon > entering the supervisor, and allows the detection of misa via a device tree > pointer provided in a0. > As Anup mentioned in the commit message, the read of mhartid in *S-mode* should be fixed in future patches. We still need it for M-mode. > As long as everyone agrees this isn't what we're actually looking for then I'm > fine with the patch, I just don't want to end up requiring this "M mode > emulates some CSRs for S mode" interface. Since we don't have an actual > platform spec we've got to be careful to avoid a bunch of defacto interfaces > that we'll need to support later. > Agreed. Regards, Bin
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 3e0af55e71..8f2139ff60 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 an 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 5af189b338..e4276e8e19 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -39,10 +39,18 @@ _start: mv s1, a1 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 @@ -164,7 +172,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 */ @@ -236,17 +248,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) @@ -279,4 +308,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