From mboxrd@z Thu Jan  1 00:00:00 1970
From: Rick Chen <rickchen36@gmail.com>
Date: Fri, 30 Nov 2018 15:05:46 +0800
Subject: [U-Boot] [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot
	in S-mode
In-Reply-To: <CAAhSdy1qmsfjsuENJdwhCe_OoE+E7pYrVvjSMCtViisNom1fJg@mail.gmail.com>
References: <20181126103910.14457-1-anup@brainfault.org>
 <20181126103910.14457-2-anup@brainfault.org>
 <752D002CFF5D0F4FA35C0100F1D73F3FA3A50B90@ATCPCS16.andestech.com>
 <CAN5B=eLTSGa1E_ZPTUzZtBU20pPtb_tmpB906YFqBmMX8+mdQg@mail.gmail.com>
 <CAAhSdy1AkKsvLC=EyM7F0vDw76+SottUbxvv6KDSBYOh8=sPgQ@mail.gmail.com>
 <abe184fc-23b7-bf1d-dcaf-91de01be30df@suse.de>
 <CAAhSdy1qmsfjsuENJdwhCe_OoE+E7pYrVvjSMCtViisNom1fJg@mail.gmail.com>
Message-ID: <CAN5B=eJpUBrC6dg61fRSguX8tF9EU19-BuNhn1KCTWuXS6DwTw@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
To: u-boot@lists.denx.de

Anup Patel <anup@brainfault.org> 於 2018年11月27日 週二 下午8:38寫道：
>
> On Tue, Nov 27, 2018 at 4:17 PM Alexander Graf <agraf@suse.de> wrote:
> >
> >
> >
> > On 27.11.18 07:52, Anup Patel wrote:
> > > On Tue, Nov 27, 2018 at 12:09 PM Rick Chen <rickchen36@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@brainfault.org>
> > >>>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > >>>> Tested-by: Bin Meng <bmeng.cn@gmail.com>
> > >>>> Reviewed-by: Lukas Auer <lukas.auer@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.

B.R
Rick




> > On AArch64, we determine the current EL during runtime and then branch
> > to respective labels for different ELs (see the switch_el macro for asm
> > as well as current_el() for C).
> >
> > Wouldn't it make sense to attempt something similar with S-mode, so that
> > the same binary can run either in M or in S depending on how it was started?
>
> As-per my understand, there is no direct way of knowing current
> execution mode in RISC-V at runtime.
>
> Of course, software can step from M to S/U using mret OR from
> S to U using sret. Further software running in U-mode can do
> ecall to S-mode and S-mode can do ecall to M-mode.
>
> Software is generally written for a particular execution mode (M, S, or U).
>
> Regards,
> Anup