[PATCH v4 2/3] arch/riscv: add semihosting support for RISC-V

[Kautuk Consul]

Hi Sean,

On Thu, Sep 22, 2022 at 10:35 PM Sean Anderson <sean.anderson at seco.com> wrote:
>
>
>
> On 9/19/22 7:49 AM, Kautuk Consul wrote:
> > We add RISC-V semihosting based serial console for JTAG based early
> > debugging.
> >
> > The RISC-V semihosting specification is available at:
> > https://github.com/riscv/riscv-semihosting-spec/blob/main/riscv-semihosting-spec.adoc
> >
> > Signed-off-by: Anup Patel <apatel at ventanamicro.com>
> > Signed-off-by: Kautuk Consul <kconsul at ventanamicro.com>
> > ---
> >  arch/riscv/include/asm/spl.h |  1 +
> >  arch/riscv/lib/Makefile      |  2 ++
> >  arch/riscv/lib/interrupts.c  | 11 +++++++++++
> >  arch/riscv/lib/semihosting.c | 24 ++++++++++++++++++++++++
> >  lib/Kconfig                  |  6 +++---
> >  5 files changed, 41 insertions(+), 3 deletions(-)
> >  create mode 100644 arch/riscv/lib/semihosting.c
> >
> > diff --git a/arch/riscv/include/asm/spl.h b/arch/riscv/include/asm/spl.h
> > index e8a94fcb1f..2898a770ee 100644
> > --- a/arch/riscv/include/asm/spl.h
> > +++ b/arch/riscv/include/asm/spl.h
> > @@ -25,6 +25,7 @@ enum {
> >       BOOT_DEVICE_DFU,
> >       BOOT_DEVICE_XIP,
> >       BOOT_DEVICE_BOOTROM,
> > +     BOOT_DEVICE_SMH,
> >       BOOT_DEVICE_NONE
> >  };
> >
> > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> > index 06020fcc2a..64e29804c1 100644
> > --- a/arch/riscv/lib/Makefile
> > +++ b/arch/riscv/lib/Makefile
> > @@ -42,3 +42,5 @@ extra-$(CONFIG_EFI) += $(EFI_CRT0) $(EFI_RELOC)
> >  obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMSET) += memset.o
> >  obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMMOVE) += memmove.o
> >  obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMCPY) += memcpy.o
> > +
> > +obj-$(CONFIG_$(SPL_TPL_)SEMIHOSTING) += semihosting.o
> > diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c
> > index 100be2e966..bd7cd772b8 100644
> > --- a/arch/riscv/lib/interrupts.c
> > +++ b/arch/riscv/lib/interrupts.c
> > @@ -17,6 +17,7 @@
> >  #include <asm/ptrace.h>
> >  #include <asm/system.h>
> >  #include <asm/encoding.h>
> > +#include <semihosting.h>
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -149,6 +150,16 @@ ulong handle_trap(ulong cause, ulong epc, ulong tval, struct pt_regs *regs)
> >       /* An UEFI application may have changed gd. Restore U-Boot's gd. */
> >       efi_restore_gd();
> >
> > +     if (cause == CAUSE_BREAKPOINT &&
> > +         CONFIG_IS_ENABLED(SEMIHOSTING_FALLBACK)) {
>
> Hm, shouldn't we check to see if this is actually a semihosting
> breakpoint? This involves reading the prior and posterior
> instructions, which might be a bit too much. Alternatively, we
> could only take this trap if semihosting_enabled(), which would
> limit any suppression of spurious breakpoints to one instruction.
>

Agreed. Will read the prior and posterior instructions in v5 and send this out.
Thanks.

> > +             /* For semihosting fallback we simply skip the ebreak
> > +              * instruction.
> > +              */
> > +             disable_semihosting();
> > +             epc += 4;
> > +             return epc;
> > +     }
> > +
> >       is_irq = (cause & MCAUSE_INT);
> >       irq = (cause & ~MCAUSE_INT);
> >
> > diff --git a/arch/riscv/lib/semihosting.c b/arch/riscv/lib/semihosting.c
> > new file mode 100644
> > index 0000000000..d6593b02a6
> > --- /dev/null
> > +++ b/arch/riscv/lib/semihosting.c
> > @@ -0,0 +1,24 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2022 Ventana Micro Systems Inc.
> > + */
> > +
> > +#include <common.h>
> > +
> > +long smh_trap(int sysnum, void *addr)
> > +{
> > +     register int ret asm ("a0") = sysnum;
> > +     register void *param0 asm ("a1") = addr;
> > +
> > +     asm volatile (".align 4\n"
> > +             ".option push\n"
> > +             ".option norvc\n"
> > +
> > +             "slli zero, zero, 0x1f\n"
> > +             "ebreak\n"
> > +             "srai zero, zero, 7\n"
> > +             ".option pop\n"
> > +             : "+r" (ret) : "r" (param0) : "memory");
> > +
> > +     return ret;
> > +}
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index 97920e7552..eed3a231d9 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -85,7 +85,7 @@ config SEMIHOSTING
> >
> >  config SEMIHOSTING_FALLBACK
> >       bool "Recover gracefully when semihosting fails"
> > -     depends on SEMIHOSTING && ARM64
> > +     depends on SEMIHOSTING && (ARM64 || RISCV)
> >       default y
> >       help
> >         Normally, if U-Boot makes a semihosting call and no debugger is
> > @@ -108,8 +108,8 @@ config SPL_SEMIHOSTING
> >
> >  config SPL_SEMIHOSTING_FALLBACK
> >       bool "Recover gracefully when semihosting fails in SPL"
> > -     depends on SPL_SEMIHOSTING && ARM64
> > -     select ARMV8_SPL_EXCEPTION_VECTORS
> > +     depends on SPL_SEMIHOSTING && (ARM64 || RISCV)
> > +     select ARMV8_SPL_EXCEPTION_VECTORS if ARM64
> >       default y
> >       help
> >         Normally, if U-Boot makes a semihosting call and no debugger is
> >
>
> The rest looks fine.
>
> --Sean