[U-Boot] [PATCH v3 1/3] riscv: Add kconfig option to run u-boot in S-mode

[Auer, Lukas]

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 at brainfault.org>
> > Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> > Tested-by: Bin Meng <bmeng.cn at 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 at 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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot