Re: [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.a...@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), a...@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 <a...@brainfault.org>

> Reviewed-by: Bin Meng <bmeng...@gmail.com>
> Tested-by: Bin Meng <bmeng...@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

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot