From: Anup Patel <anup@brainfault.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot in S-mode
Date: Tue, 27 Nov 2018 12:22:07 +0530 [thread overview]
Message-ID: <CAAhSdy1AkKsvLC=EyM7F0vDw76+SottUbxvv6KDSBYOh8=sPgQ@mail.gmail.com> (raw)
In-Reply-To: <CAN5B=eLTSGa1E_ZPTUzZtBU20pPtb_tmpB906YFqBmMX8+mdQg@mail.gmail.com>
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.
Regards,
Anup
next prev parent reply other threads:[~2018-11-27 6:52 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-26 10:39 [U-Boot] [PATCH v5 0/4] RISC-V S-mode support Anup Patel
2018-11-26 10:39 ` [U-Boot] [PATCH v5 1/4] riscv: Add kconfig option to run U-Boot in S-mode Anup Patel
[not found] ` <752D002CFF5D0F4FA35C0100F1D73F3FA3A50B90@ATCPCS16.andestech.com>
2018-11-27 6:40 ` Rick Chen
2018-11-27 6:52 ` Anup Patel [this message]
2018-11-27 10:47 ` Alexander Graf
2018-11-27 12:38 ` Anup Patel
2018-11-30 7:05 ` Rick Chen
2018-11-30 8:53 ` Anup Patel
2018-11-26 10:39 ` [U-Boot] [PATCH v5 2/4] riscv: qemu: Use different SYS_TEXT_BASE for S-mode Anup Patel
2018-11-26 10:39 ` [U-Boot] [PATCH v5 3/4] riscv: Add S-mode defconfigs for QEMU virt machine Anup Patel
2018-11-26 10:39 ` [U-Boot] [PATCH v5 4/4] riscv: Remove redundant a2 store on DRAM base in start.S Anup Patel
2018-11-26 15:10 ` Bin Meng
2018-11-26 22:10 ` Auer, Lukas
[not found] ` <752D002CFF5D0F4FA35C0100F1D73F3FA3A50A96@ATCPCS16.andestech.com>
2018-11-27 3:21 ` Rick Chen
2018-11-27 3:28 ` Anup Patel
2018-11-27 5:20 ` Rick Chen
2018-11-27 5:44 ` Anup Patel
2018-11-27 5:47 ` Anup Patel
2018-11-27 6:09 ` Rick Chen
2018-11-27 6:13 ` Anup Patel
2018-11-27 6:31 ` Rick Chen
2018-11-27 6:40 ` Anup Patel
2018-11-27 7:01 ` Rick Chen
2018-11-27 7:56 ` Anup Patel
2018-11-27 8:42 ` Anup Patel
2018-11-27 10:41 ` Alexander Graf
2018-11-29 10:44 ` Rick Chen
2018-11-27 8:43 ` Rick Chen
2018-11-27 10:07 ` Bin Meng
2018-11-29 10:42 ` Rick Chen
2018-11-29 11:29 ` Rick Chen
2018-11-30 1:47 ` Bin Meng
2018-11-30 6:06 ` Rick Chen
2018-11-30 6:21 ` Bin Meng
2018-11-30 6:41 ` Rick Chen
2018-11-30 6:57 ` Bin Meng
2018-11-30 7:16 ` Rick Chen
2018-11-30 7:26 ` Bin Meng
2018-11-30 7:31 ` Rick Chen
2018-11-27 6:45 ` Rick Chen
2018-11-27 6:56 ` Anup Patel
2018-11-27 10:00 ` Bin Meng
2018-11-27 9:56 ` Bin Meng
2018-11-28 12:22 ` [U-Boot] [PATCH v5 0/4] RISC-V S-mode support Anup Patel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAhSdy1AkKsvLC=EyM7F0vDw76+SottUbxvv6KDSBYOh8=sPgQ@mail.gmail.com' \
--to=anup@brainfault.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.