[PATCH 1/1] riscv: riscv_get_time() implementation for SMODE

Anup Patel anup at brainfault.org
Fri Aug 14 20:38:41 CEST 2020


On Fri, Aug 14, 2020 at 11:35 PM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 14.08.20 19:52, Anup Patel wrote:
> > On Fri, Aug 14, 2020 at 11:15 PM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>
> >> On the Kendryte K210 OpenBSI cannot emulate the rdtime instruction. So we
> >> have to use the Sifive CLINT driver to provide riscv_get_time() in SMODE.
> >
> > Can you elaborate why ?
> >
> > The rdtime instruction should generate an illegal instruction fault which
> > OpenSBI will emulate.
>
> The RISC-V Instruction Set Manual Volume II Privileged architectur 1.11
> has incompatible changes relative to version 1.9.1
>
> mtval on the Kendryte K210 delivers the bad virtual address and not the
> instruction:

Ahh, I see. The MTVAL CSR is actually legacy MBADADDR CSR and this
CSR.

The S-mode software always has working rdtime instruction for all
RISC-V systems. If HW does not implement TIME CSR then OpenSBI
emulates it. Please don't break this convention for U-Boot S-mode
because we do have RISC-V systems where TIME CSR is implemeted
in HW so this will patch will break U-Boot S-mode system because
on those system we are supposed to use TIME CSR from S-mode.

>
> lib/sbi/sbi_illegal_insn.c(123) sbi_illegal_insn_handler:
> insn 0x4114121602, epc 0x8058c168.
>
> The illegal instruction being
> c01027f3    rdtime a5
>
> In the long run it may make sense to provide backwards compatibility in
> OpenSBI.

Yes, let's try to explore possible fixes in OpenSBI.

Instead of this patch, try the following change in OpenSBI ...

diff --git a/lib/sbi/sbi_illegal_insn.c b/lib/sbi/sbi_illegal_insn.c
index 0e5523f..c8f2e4a 100644
--- a/lib/sbi/sbi_illegal_insn.c
+++ b/lib/sbi/sbi_illegal_insn.c
@@ -119,12 +119,10 @@ int sbi_illegal_insn_handler(ulong insn, struct
sbi_trap_regs *regs)
        struct sbi_trap_info uptrap;

        if (unlikely((insn & 3) != 3)) {
-               if (insn == 0) {
-                       insn = sbi_get_insn(regs->mepc, &uptrap);
-                       if (uptrap.cause) {
-                               uptrap.epc = regs->mepc;
-                               return sbi_trap_redirect(regs, &uptrap);
-                       }
+               insn = sbi_get_insn(regs->mepc, &uptrap);
+               if (uptrap.cause) {
+                       uptrap.epc = regs->mepc;
+                       return sbi_trap_redirect(regs, &uptrap);
                }
                if ((insn & 3) != 3)
                        return truly_illegal_insn(insn, regs);

Regards,
Anup

>
> Without this patch I observe a crash in the loady command. So I cannot
> load a file over the UART.
>
> Best regards
>
> Heinrich
>
> >
> > Regards,
> > Anup
> >
> >>
> >> Adjust Kconfig and Makefile to allow compiling the Sifive CLINT driver in
> >> SMODE.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> >> ---
> >>  arch/riscv/Kconfig                   | 16 +++++++++++-----
> >>  arch/riscv/cpu/fu540/Kconfig         |  3 ++-
> >>  arch/riscv/cpu/generic/Kconfig       |  3 ++-
> >>  arch/riscv/include/asm/global_data.h |  2 +-
> >>  arch/riscv/lib/Makefile              |  2 +-
> >>  5 files changed, 17 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> >> index 009a545fcf..96c386225b 100644
> >> --- a/arch/riscv/Kconfig
> >> +++ b/arch/riscv/Kconfig
> >> @@ -153,12 +153,18 @@ config 64BIT
> >>         bool
> >>
> >>  config SIFIVE_CLINT
> >> -       bool
> >> -       depends on RISCV_MMODE || SPL_RISCV_MMODE
> >> +       bool "SiFive's Core Local Interruptor (CLINT) driver"
> >>         select REGMAP
> >>         select SYSCON
> >> -       select SPL_REGMAP if SPL
> >> -       select SPL_SYSCON if SPL
> >> +       help
> >> +         The SiFive CLINT block holds memory-mapped control and status registers
> >> +         associated with software and timer interrupts.
> >> +
> >> +config SPL_SIFIVE_CLINT
> >> +       bool "SiFive's Core Local Interruptor (CLINT) driver in SPL"
> >> +       depends on SPL
> >> +       select SPL_REGMAP
> >> +       select SPL_SYSCON
> >>         help
> >>           The SiFive CLINT block holds memory-mapped control and status registers
> >>           associated with software and timer interrupts.
> >> @@ -186,7 +192,7 @@ config ANDES_PLMT
> >>           associated with timer tick.
> >>
> >>  config RISCV_RDTIME
> >> -       bool
> >> +       bool "Timer API via rdtime instruction"
> >>         default y if RISCV_SMODE || SPL_RISCV_SMODE
> >>         help
> >>           The provides the riscv_get_time() API that is implemented using the
> >> diff --git a/arch/riscv/cpu/fu540/Kconfig b/arch/riscv/cpu/fu540/Kconfig
> >> index 2dcad8e27f..8b2c83039f 100644
> >> --- a/arch/riscv/cpu/fu540/Kconfig
> >> +++ b/arch/riscv/cpu/fu540/Kconfig
> >> @@ -8,7 +8,8 @@ config SIFIVE_FU540
> >>         imply CPU
> >>         imply CPU_RISCV
> >>         imply RISCV_TIMER
> >> -       imply SIFIVE_CLINT if (RISCV_MMODE || SPL_RISCV_MMODE)
> >> +       imply SIFIVE_CLINT if RISCV_MMODE
> >> +       imply SPL_SIFIVE_CLINT if SPL_RISCV_MMODE
> >>         imply CMD_CPU
> >>         imply SPL_CPU_SUPPORT
> >>         imply SPL_OPENSBI
> >> diff --git a/arch/riscv/cpu/generic/Kconfig b/arch/riscv/cpu/generic/Kconfig
> >> index b2cb155d6d..62fcadd710 100644
> >> --- a/arch/riscv/cpu/generic/Kconfig
> >> +++ b/arch/riscv/cpu/generic/Kconfig
> >> @@ -8,7 +8,8 @@ config GENERIC_RISCV
> >>         imply CPU
> >>         imply CPU_RISCV
> >>         imply RISCV_TIMER
> >> -       imply SIFIVE_CLINT if (RISCV_MMODE || SPL_RISCV_MMODE)
> >> +       imply SIFIVE_CLINT if RISCV_MMODE
> >> +       imply SPL_SIFIVE_CLINT if SPL_RISCV_MMODE
> >>         imply CMD_CPU
> >>         imply SPL_CPU_SUPPORT
> >>         imply SPL_OPENSBI
> >> diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
> >> index 2eb14815bc..b89b469d41 100644
> >> --- a/arch/riscv/include/asm/global_data.h
> >> +++ b/arch/riscv/include/asm/global_data.h
> >> @@ -18,7 +18,7 @@
> >>  struct arch_global_data {
> >>         long boot_hart;         /* boot hart id */
> >>         phys_addr_t firmware_fdt_addr;
> >> -#ifdef CONFIG_SIFIVE_CLINT
> >> +#if CONFIG_IS_ENABLED(SIFIVE_CLINT)
> >>         void __iomem *clint;    /* clint base address */
> >>  #endif
> >>  #ifdef CONFIG_ANDES_PLIC
> >> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> >> index 6c503ff2b2..861d7b489f 100644
> >> --- a/arch/riscv/lib/Makefile
> >> +++ b/arch/riscv/lib/Makefile
> >> @@ -10,8 +10,8 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o
> >>  obj-$(CONFIG_CMD_BOOTI) += bootm.o image.o
> >>  obj-$(CONFIG_CMD_GO) += boot.o
> >>  obj-y  += cache.o
> >> +obj-$(CONFIG_$(SPL_)SIFIVE_CLINT) += sifive_clint.o
> >>  ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),y)
> >> -obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
> >>  obj-$(CONFIG_ANDES_PLIC) += andes_plic.o
> >>  obj-$(CONFIG_ANDES_PLMT) += andes_plmt.o
> >>  else
> >> --
> >> 2.28.0
> >>
>


More information about the U-Boot mailing list