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

[Palmer Dabbelt]

On Wed, 21 Nov 2018 14:06:00 PST (-0800), lukas.auer at 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 at 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 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
>> > +
>> >  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