[U-Boot] [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot in S-mode
Anup Patel
anup at brainfault.org
Fri Nov 30 08:53:25 UTC 2018
On Fri, Nov 30, 2018 at 12:34 PM Rick Chen <rickchen36 at gmail.com> wrote:
>
> Anup Patel <anup at brainfault.org> 於 2018年11月27日 週二 下午8:38寫道：
> >
> > On Tue, Nov 27, 2018 at 4:17 PM Alexander Graf <agraf at suse.de> wrote:
> > >
> > >
> > >
> > > On 27.11.18 07:52, Anup Patel wrote:
> > > > On Tue, Nov 27, 2018 at 12:09 PM Rick Chen <rickchen36 at 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 at brainfault.org>
> > > >>>> Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> > > >>>> Tested-by: Bin Meng <bmeng.cn at gmail.com>
> > > >>>> Reviewed-by: Lukas Auer <lukas.auer at 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
More information about the U-Boot
mailing list