On Thu, Sep 15, 2022 at 6:15 PM Kautuk Consul kconsul@ventanamicro.com 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-...

Signed-off-by: Anup Patel apatel@ventanamicro.com Signed-off-by: Kautuk Consul kconsul@ventanamicro.com

---

arch/riscv/Kconfig | 45 ++++++++ arch/riscv/include/asm/semihosting.h | 11 ++ arch/riscv/include/asm/spl.h | 1 + arch/riscv/lib/Makefile | 7 ++ arch/riscv/lib/semihosting.c | 166 +++++++++++++++++++++++++++ arch/riscv/lib/semihosting_mmode.c | 77 +++++++++++++ arch/riscv/lib/semihosting_smode.c | 77 +++++++++++++ 7 files changed, 384 insertions(+) create mode 100644 arch/riscv/include/asm/semihosting.h create mode 100644 arch/riscv/lib/semihosting.c create mode 100644 arch/riscv/lib/semihosting_mmode.c create mode 100644 arch/riscv/lib/semihosting_smode.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 78e964db12..1b23d1c6c1 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -371,4 +371,49 @@ config TPL_USE_ARCH_MEMSET

endmenu

+config SEMIHOSTING

•   bool "Support RISCV semihosting"
  

•   help
  

•     Semihosting is a method for a target to communicate with a host
  

•     debugger. It uses special instructions which the debugger will trap
  

•     on and interpret. This allows U-Boot to read/write files, print to
  

•     the console, and execute arbitrary commands on the host system.
  

•     Enabling this option will add support for reading and writing files
  

•     on the host system. If you don't have a debugger attached then trying
  

•     to do this will likely cause U-Boot to hang. Say 'n' if you are unsure.
  

+config SEMIHOSTING_FALLBACK

•   bool "Recover gracefully when semihosting fails"
  

•   depends on SEMIHOSTING && RISCV
  

•   default y
  

•   help
  

•     Normally, if U-Boot makes a semihosting call and no debugger is
  

•     attached, then it will panic due to a synchronous abort
  

•     exception. This config adds an exception handler which will allow
  

•     U-Boot to recover. Say 'y' if unsure.
  

+config SPL_SEMIHOSTING

•   bool "Support RISCV semihosting in SPL"
  

•   depends on SPL
  

•   help
  

•     Semihosting is a method for a target to communicate with a host
  

•     debugger. It uses special instructions which the debugger will trap
  

•     on and interpret. This allows U-Boot to read/write files, print to
  

•     the console, and execute arbitrary commands on the host system.
  

•     Enabling this option will add support for reading and writing files
  

•     on the host system. If you don't have a debugger attached then trying
  

•     to do this will likely cause U-Boot to hang. Say 'n' if you are unsure.
  

+config SPL_SEMIHOSTING_FALLBACK

•   bool "Recover gracefully when semihosting fails in SPL"
  

•   depends on SPL_SEMIHOSTING && RISCV
  

•   default y
  

•   help
  

•     Normally, if U-Boot makes a semihosting call and no debugger is
  

•     attached, then it will panic due to a synchronous abort
  

•     exception. This config adds an exception handler which will allow
  

•     U-Boot to recover. Say 'y' if unsure.
  

endmenu diff --git a/arch/riscv/include/asm/semihosting.h b/arch/riscv/include/asm/semihosting.h new file mode 100644 index 0000000000..7042821e00 --- /dev/null +++ b/arch/riscv/include/asm/semihosting.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Copyright (C) 2022 Ventana Micro Systems Inc.
• */

+#ifndef __ASM_RISCV_SEMIHOSTING_H +#define __ASM_RISCV_SEMIHOSTING_H

+long smh_trap(int sysnum, void *addr);

+#endif /* __ASM_RISCV_SEMIHOSTING_H */ 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..2c89c3a2fa 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -42,3 +42,10 @@ 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 +ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),y) +obj-$(CONFIG_$(SPL_TPL_)SEMIHOSTING) += semihosting_mmode.o +else +obj-$(CONFIG_$(SPL_TPL_)SEMIHOSTING) += semihosting_smode.o +endif diff --git a/arch/riscv/lib/semihosting.c b/arch/riscv/lib/semihosting.c new file mode 100644 index 0000000000..504c9a1ddc --- /dev/null +++ b/arch/riscv/lib/semihosting.c @@ -0,0 +1,166 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2022 Ventana Micro Systems Inc.
• */

+#include <common.h> +#include <log.h> +#include <semihosting.h> +#include <asm/semihosting.h>

+#define SYSOPEN 0x01 +#define SYSCLOSE 0x02 +#define SYSWRITEC 0x03 +#define SYSWRITE0 0x04 +#define SYSWRITE 0x05 +#define SYSREAD 0x06 +#define SYSREADC 0x07 +#define SYSISERROR 0x08 +#define SYSSEEK 0x0A +#define SYSFLEN 0x0C +#define SYSERRNO 0x13

+/**

• • smh_errno() - Read the host's errno
• • This gets the value of the host's errno and negates it. The host's errno may
• • or may not be set, so only call this function if a previous semihosting call
• • has failed.
• • Return: a negative error value
• */

+static int smh_errno(void) +{

•   long ret = smh_trap(SYSERRNO, NULL);
  

•   if (ret > 0 && ret < INT_MAX)
  

•           return -ret;
  

•   return -EIO;
  

+}

+long smh_open(const char *fname, enum smh_open_mode mode) +{

•   long fd;
  

•   struct smh_open_s {
  

•           const char *fname;
  

•           unsigned long mode;
  

•           size_t len;
  

•   } open;
  

•   debug("%s: file \'%s\', mode \'%u\'\n", __func__, fname, mode);
  

•   open.fname = fname;
  

•   open.len = strlen(fname);
  

•   open.mode = mode;
  

•   /* Open the file on the host */
  

•   fd = smh_trap(SYSOPEN, &open);
  

•   if (fd == -1)
  

•           return smh_errno();
  

•   return fd;
  

+}

+/**

• • struct smg_rdwr_s - Arguments for read and write
• • @fd: A file descriptor returned from smh_open()
• • @memp: Pointer to a buffer of memory of at least @len bytes
• • @len: The number of bytes to read or write
• */

+struct smh_rdwr_s {

•   long fd;
  

•   void *memp;
  

•   size_t len;
  

+};

+long smh_read(long fd, void *memp, size_t len) +{

•   long ret;
  

•   struct smh_rdwr_s read;
  

•   debug("%s: fd %ld, memp %p, len %zu\n", __func__, fd, memp, len);
  

•   read.fd = fd;
  

•   read.memp = memp;
  

•   read.len = len;
  

•   ret = smh_trap(SYSREAD, &read);
  

•   if (ret < 0)
  

•           return smh_errno();
  

•   return len - ret;
  

+}

+long smh_write(long fd, const void *memp, size_t len, ulong *written) +{

•   long ret;
  

•   struct smh_rdwr_s write;
  

•   debug("%s: fd %ld, memp %p, len %zu\n", __func__, fd, memp, len);
  

•   write.fd = fd;
  

•   write.memp = (void *)memp;
  

•   write.len = len;
  

•   ret = smh_trap(SYSWRITE, &write);
  

•   *written = len - ret;
  

•   if (ret)
  

•           return smh_errno();
  

•   return 0;
  

+}

+long smh_close(long fd) +{

•   long ret;
  

•   debug("%s: fd %ld\n", __func__, fd);
  

•   ret = smh_trap(SYSCLOSE, &fd);
  

•   if (ret == -1)
  

•           return smh_errno();
  

•   return 0;
  

+}

+long smh_flen(long fd) +{

•   long ret;
  

•   debug("%s: fd %ld\n", __func__, fd);
  

•   ret = smh_trap(SYSFLEN, &fd);
  

•   if (ret == -1)
  

•           return smh_errno();
  

•   return ret;
  

+}

+long smh_seek(long fd, long pos) +{

•   long ret;
  

•   struct smh_seek_s {
  

•           long fd;
  

•           long pos;
  

•   } seek;
  

•   debug("%s: fd %ld pos %ld\n", __func__, fd, pos);
  

•   seek.fd = fd;
  

•   seek.pos = pos;
  

•   ret = smh_trap(SYSSEEK, &seek);
  

•   if (ret)
  

•           return smh_errno();
  

•   return 0;
  

+}

+int smh_getc(void) +{

•   return smh_trap(SYSREADC, NULL);
  

+}

+void smh_putc(char ch) +{

•   smh_trap(SYSWRITEC, &ch);
  

+}

+void smh_puts(const char *s) +{

•   smh_trap(SYSWRITE0, (char *)s);
  

+} diff --git a/arch/riscv/lib/semihosting_mmode.c b/arch/riscv/lib/semihosting_mmode.c new file mode 100644 index 0000000000..7bc97a1a66 --- /dev/null +++ b/arch/riscv/lib/semihosting_mmode.c @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2022 Ventana Micro Systems Inc.
• */

+#include <common.h>

+#define SYSERRNO 0x13

+long smh_trap(int sysnum, void *addr) +{

•   register int ret asm ("a0") = sysnum;
  

•   register void *param0 asm ("a1") = addr;
  

•   asm volatile ("\t.option push\n"
  

•           "\t.option norvc\n"
  

•           "\tj 1f\n"
  

•           "\t.align 4\n"
  

•           "\t1: slli zero, zero, 0x1f\n"
  

•           "\tebreak\n"
  

•           "\tsrai zero, zero, 7\n"
  

•           "\t.option pop\n"
  

•           : "+r" (ret) : "r" (param0) : "memory");
  

•   return ret;
  

+}

The smh_trap() is exactly same for both M-mode and S-mode so it should be moved to semihosting.c

Also, replacing "\t" in the inline assembly with actual tabs looks cleaner.

+#if CONFIG_IS_ENABLED(SEMIHOSTING_FALLBACK) +static bool _semihosting_enabled = true; +static bool try_semihosting = true;

+bool semihosting_enabled(void) +{

•   unsigned long tmp = 0;
  

•   register int ret asm ("a0") = SYSERRNO;
  

•   register void *param0 asm ("a1") = NULL;
  

•   if (!try_semihosting)
  

•           return _semihosting_enabled;
  

•   asm volatile ("\t.option push\n"
  

•           "\t.option norvc\n"
  

•           "\tj _semihost_test_vector_next\n"
  

•           "\t.align 4\n"
  

•           "\t_semihost_test_vector:\n"
  

•           "\t\tcsrr %[en], mepc\n"
  

•           "\t\taddi %[en], %[en], 4\n"
  

•           "\t\tcsrw mepc, %[en]\n"
  

•           "\t\tadd %[en], zero, zero\n"
  

•           "\t\tmret\n"
  

•           "\t_semihost_test_vector_next:\n"
  

•           "\tla %[tmp], _semihost_test_vector\n"
  

•           "\tcsrrw %[tmp], mtvec, %[tmp]\n"
  

•           "\tj 1f\n"
  

•           "\t.align 4\n"
  

•           "\t1: slli zero, zero, 0x1f\n"
  

•           "\tebreak\n"
  

•           "\tsrai zero, zero, 7\n"
  

•           "\tcsrw mtvec, %[tmp]\n"
  

•           "\t.option pop\n"
  

•           : [tmp] "+r" (tmp), [en] "+r" (_semihosting_enabled),
  

•             [ret] "+r" (ret)
  

•           : "r" (param0) : "memory");
  

Replace "\t" in the inline assembly with actual tabs.

•   try_semihosting = false;
  

•   return _semihosting_enabled;
  

+}

+void disable_semihosting(void) +{

•   _semihosting_enabled = false;
  

+} +#endif diff --git a/arch/riscv/lib/semihosting_smode.c b/arch/riscv/lib/semihosting_smode.c new file mode 100644 index 0000000000..d7ce07985b --- /dev/null +++ b/arch/riscv/lib/semihosting_smode.c @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2022 Ventana Micro Systems Inc.
• */

+#include <common.h>

+#define SYSERRNO 0x13

+long smh_trap(int sysnum, void *addr) +{

•   register int ret asm ("a0") = sysnum;
  

•   register void *param0 asm ("a1") = addr;
  

•   asm volatile ("\t.option push\n"
  

•           "\t.option norvc\n"
  

•           "\tj 1f\n"
  

•           "\t.align 4\n"
  

•           "\t1: slli zero, zero, 0x1f\n"
  

•           "\tebreak\n"
  

•           "\tsrai zero, zero, 7\n"
  

•           "\t.option pop\n"
  

•           : "+r" (ret) : "r" (param0) : "memory");
  

•   return ret;
  

+}

Same comment as above.

+#if CONFIG_IS_ENABLED(SEMIHOSTING_FALLBACK) +static bool _semihosting_enabled = true; +static bool try_semihosting = true;

+bool semihosting_enabled(void) +{

•   unsigned long tmp = 0;
  

•   register int ret asm ("a0") = SYSERRNO;
  

•   register void *param0 asm ("a1") = NULL;
  

•   if (!try_semihosting)
  

•           return _semihosting_enabled;
  

•   asm volatile ("\t.option push\n"
  

•           "\t.option norvc\n"
  

•           "\tj _semihost_test_vector_next\n"
  

•           "\t.align 4\n"
  

•           "\t_semihost_test_vector:\n"
  

•           "\t\tcsrr %[en], sepc\n"
  

•           "\t\taddi %[en], %[en], 4\n"
  

•           "\t\tcsrw sepc, %[en]\n"
  

•           "\t\tadd %[en], zero, zero\n"
  

•           "\t\tsret\n"
  

•           "\t_semihost_test_vector_next:\n"
  

•           "\tla %[tmp], _semihost_test_vector\n"
  

•           "\tcsrrw %[tmp], stvec, %[tmp]\n"
  

•           "\tj 1f\n"
  

•           "\t.align 4\n"
  

•           "\t1: slli zero, zero, 0x1f\n"
  

•           "\tebreak\n"
  

•           "\tsrai zero, zero, 7\n"
  

•           "\tcsrw stvec, %[tmp]\n"
  

•           "\t.option pop\n"
  

•           : [tmp] "+r" (tmp), [en] "+r" (_semihosting_enabled),
  

•             [ret] "+r" (ret)
  

•           : "r" (param0) : "memory");
  

•   try_semihosting = false;
  

•   return _semihosting_enabled;
  

+}

Replace "\t" in the inline assembly with actual tabs.

+void disable_semihosting(void) +{

•   _semihosting_enabled = false;
  

+}

+#endif

2.34.1

Regards, Anup

Hi Sean,

Don't know about the DCSR.EBREAK option but it will be better for us to extend the existing trap vector functionality as you mentioned. Will handle this in v2. This also removes the need for us to implement our semihosting_enabled() in inline assembly as it will become a generic lib/semihosting.c function.

On Thu, Sep 15, 2022 at 9:32 PM Sean Anderson sean.anderson@seco.com wrote:

Hi Kautuk,

I've already noted my general remarks on this approach in response to your cover letter. This just has my comments on the RISC-V-specific parts.

On 9/15/22 8:45 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-...

Signed-off-by: Anup Patel apatel@ventanamicro.com Signed-off-by: Kautuk Consul kconsul@ventanamicro.com

---

arch/riscv/Kconfig | 45 ++++++++ arch/riscv/include/asm/semihosting.h | 11 ++ arch/riscv/include/asm/spl.h | 1 + arch/riscv/lib/Makefile | 7 ++ arch/riscv/lib/semihosting.c | 166 +++++++++++++++++++++++++++ arch/riscv/lib/semihosting_mmode.c | 77 +++++++++++++ arch/riscv/lib/semihosting_smode.c | 77 +++++++++++++ 7 files changed, 384 insertions(+) create mode 100644 arch/riscv/include/asm/semihosting.h create mode 100644 arch/riscv/lib/semihosting.c create mode 100644 arch/riscv/lib/semihosting_mmode.c create mode 100644 arch/riscv/lib/semihosting_smode.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 78e964db12..1b23d1c6c1 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -371,4 +371,49 @@ config TPL_USE_ARCH_MEMSET

endmenu

+config SEMIHOSTING

•   bool "Support RISCV semihosting"
  

•   help
  

•     Semihosting is a method for a target to communicate with a host
  

•     debugger. It uses special instructions which the debugger will trap
  

•     on and interpret. This allows U-Boot to read/write files, print to
  

•     the console, and execute arbitrary commands on the host system.
  

•     Enabling this option will add support for reading and writing files
  

•     on the host system. If you don't have a debugger attached then trying
  

•     to do this will likely cause U-Boot to hang. Say 'n' if you are unsure.
  

+config SEMIHOSTING_FALLBACK

•   bool "Recover gracefully when semihosting fails"
  

•   depends on SEMIHOSTING && RISCV
  

•   default y
  

•   help
  

•     Normally, if U-Boot makes a semihosting call and no debugger is
  

•     attached, then it will panic due to a synchronous abort
  

•     exception. This config adds an exception handler which will allow
  

•     U-Boot to recover. Say 'y' if unsure.
  

+config SPL_SEMIHOSTING

•   bool "Support RISCV semihosting in SPL"
  

•   depends on SPL
  

•   help
  

•     Semihosting is a method for a target to communicate with a host
  

•     debugger. It uses special instructions which the debugger will trap
  

•     on and interpret. This allows U-Boot to read/write files, print to
  

•     the console, and execute arbitrary commands on the host system.
  

•     Enabling this option will add support for reading and writing files
  

•     on the host system. If you don't have a debugger attached then trying
  

•     to do this will likely cause U-Boot to hang. Say 'n' if you are unsure.
  

+config SPL_SEMIHOSTING_FALLBACK

•   bool "Recover gracefully when semihosting fails in SPL"
  

•   depends on SPL_SEMIHOSTING && RISCV
  

•   default y
  

•   help
  

•     Normally, if U-Boot makes a semihosting call and no debugger is
  

•     attached, then it will panic due to a synchronous abort
  

•     exception. This config adds an exception handler which will allow
  

•     U-Boot to recover. Say 'y' if unsure.
  

endmenu diff --git a/arch/riscv/include/asm/semihosting.h b/arch/riscv/include/asm/semihosting.h new file mode 100644 index 0000000000..7042821e00 --- /dev/null +++ b/arch/riscv/include/asm/semihosting.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Copyright (C) 2022 Ventana Micro Systems Inc.
• */

+#ifndef __ASM_RISCV_SEMIHOSTING_H +#define __ASM_RISCV_SEMIHOSTING_H

+long smh_trap(int sysnum, void *addr);

+#endif /* __ASM_RISCV_SEMIHOSTING_H */ 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..2c89c3a2fa 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -42,3 +42,10 @@ 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 +ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),y) +obj-$(CONFIG_$(SPL_TPL_)SEMIHOSTING) += semihosting_mmode.o +else +obj-$(CONFIG_$(SPL_TPL_)SEMIHOSTING) += semihosting_smode.o +endif diff --git a/arch/riscv/lib/semihosting.c b/arch/riscv/lib/semihosting.c new file mode 100644 index 0000000000..504c9a1ddc --- /dev/null +++ b/arch/riscv/lib/semihosting.c @@ -0,0 +1,166 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2022 Ventana Micro Systems Inc.

Please retain the original copyright notices when copying files.

...

• */

+#include <common.h> +#include <log.h> +#include <semihosting.h> +#include <asm/semihosting.h>

+#define SYSOPEN 0x01 +#define SYSCLOSE 0x02 +#define SYSWRITEC 0x03 +#define SYSWRITE0 0x04 +#define SYSWRITE 0x05 +#define SYSREAD 0x06 +#define SYSREADC 0x07 +#define SYSISERROR 0x08 +#define SYSSEEK 0x0A +#define SYSFLEN 0x0C +#define SYSERRNO 0x13

+/**

• • smh_errno() - Read the host's errno
• • This gets the value of the host's errno and negates it. The host's errno may
• • or may not be set, so only call this function if a previous semihosting call
• • has failed.
• • Return: a negative error value
• */

+static int smh_errno(void) +{

•   long ret = smh_trap(SYSERRNO, NULL);
  

•   if (ret > 0 && ret < INT_MAX)
  

•           return -ret;
  

•   return -EIO;
  

+}

+long smh_open(const char *fname, enum smh_open_mode mode) +{

•   long fd;
  

•   struct smh_open_s {
  

•           const char *fname;
  

•           unsigned long mode;
  

•           size_t len;
  

•   } open;
  

•   debug("%s: file \'%s\', mode \'%u\'\n", __func__, fname, mode);
  

•   open.fname = fname;
  

•   open.len = strlen(fname);
  

•   open.mode = mode;
  

•   /* Open the file on the host */
  

•   fd = smh_trap(SYSOPEN, &open);
  

•   if (fd == -1)
  

•           return smh_errno();
  

•   return fd;
  

+}

+/**

• • struct smg_rdwr_s - Arguments for read and write
• • @fd: A file descriptor returned from smh_open()
• • @memp: Pointer to a buffer of memory of at least @len bytes
• • @len: The number of bytes to read or write
• */

+struct smh_rdwr_s {

•   long fd;
  

•   void *memp;
  

•   size_t len;
  

+};

+long smh_read(long fd, void *memp, size_t len) +{

•   long ret;
  

•   struct smh_rdwr_s read;
  

•   debug("%s: fd %ld, memp %p, len %zu\n", __func__, fd, memp, len);
  

•   read.fd = fd;
  

•   read.memp = memp;
  

•   read.len = len;
  

•   ret = smh_trap(SYSREAD, &read);
  

•   if (ret < 0)
  

•           return smh_errno();
  

•   return len - ret;
  

+}

+long smh_write(long fd, const void *memp, size_t len, ulong *written) +{

•   long ret;
  

•   struct smh_rdwr_s write;
  

•   debug("%s: fd %ld, memp %p, len %zu\n", __func__, fd, memp, len);
  

•   write.fd = fd;
  

•   write.memp = (void *)memp;
  

•   write.len = len;
  

•   ret = smh_trap(SYSWRITE, &write);
  

•   *written = len - ret;
  

•   if (ret)
  

•           return smh_errno();
  

•   return 0;
  

+}

+long smh_close(long fd) +{

•   long ret;
  

•   debug("%s: fd %ld\n", __func__, fd);
  

•   ret = smh_trap(SYSCLOSE, &fd);
  

•   if (ret == -1)
  

•           return smh_errno();
  

•   return 0;
  

+}

+long smh_flen(long fd) +{

•   long ret;
  

•   debug("%s: fd %ld\n", __func__, fd);
  

•   ret = smh_trap(SYSFLEN, &fd);
  

•   if (ret == -1)
  

•           return smh_errno();
  

•   return ret;
  

+}

+long smh_seek(long fd, long pos) +{

•   long ret;
  

•   struct smh_seek_s {
  

•           long fd;
  

•           long pos;
  

•   } seek;
  

•   debug("%s: fd %ld pos %ld\n", __func__, fd, pos);
  

•   seek.fd = fd;
  

•   seek.pos = pos;
  

•   ret = smh_trap(SYSSEEK, &seek);
  

•   if (ret)
  

•           return smh_errno();
  

•   return 0;
  

+}

+int smh_getc(void) +{

•   return smh_trap(SYSREADC, NULL);
  

+}

+void smh_putc(char ch) +{

•   smh_trap(SYSWRITEC, &ch);
  

+}

+void smh_puts(const char *s) +{

•   smh_trap(SYSWRITE0, (char *)s);
  

+} diff --git a/arch/riscv/lib/semihosting_mmode.c b/arch/riscv/lib/semihosting_mmode.c new file mode 100644 index 0000000000..7bc97a1a66 --- /dev/null +++ b/arch/riscv/lib/semihosting_mmode.c @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2022 Ventana Micro Systems Inc.
• */

+#include <common.h>

+#define SYSERRNO 0x13

Maybe we should move these defines to semihosting.h?

...

+long smh_trap(int sysnum, void *addr) +{

•   register int ret asm ("a0") = sysnum;
  

•   register void *param0 asm ("a1") = addr;
  

•   asm volatile ("       .option push\n"
  

•           "     .option norvc\n"
  

•           "     j 1f\n"
  

•           "     .align 4\n"
  

•           "     1: slli zero, zero, 0x1f\n"
  

•           "     ebreak\n"
  

•           "     srai zero, zero, 7\n"
  

•           "     .option pop\n"
  

•           : "+r" (ret) : "r" (param0) : "memory");
  

•   return ret;
  

+}

I wonder if we should just do this in a .S file. I was thinking about this for arm; I think it will be cleaner than inline assembly.

...

+#if CONFIG_IS_ENABLED(SEMIHOSTING_FALLBACK) +static bool _semihosting_enabled = true; +static bool try_semihosting = true;

+bool semihosting_enabled(void) +{

•   unsigned long tmp = 0;
  

•   register int ret asm ("a0") = SYSERRNO;
  

•   register void *param0 asm ("a1") = NULL;
  

•   if (!try_semihosting)
  

•           return _semihosting_enabled;
  

•   asm volatile ("       .option push\n"
  

•           "     .option norvc\n"
  

•           "     j _semihost_test_vector_next\n"
  

•           "     .align 4\n"
  

•           "     _semihost_test_vector:\n"
  

•           "             csrr %[en], mepc\n"
  

•           "             addi %[en], %[en], 4\n"
  

•           "             csrw mepc, %[en]\n"
  

•           "             add %[en], zero, zero\n"
  

•           "             mret\n"
  

•           "     _semihost_test_vector_next:\n"
  

•           "     la %[tmp], _semihost_test_vector\n"
  

•           "     csrrw %[tmp], mtvec, %[tmp]\n"
  

•           "     j 1f\n"
  

•           "     .align 4\n"
  

•           "     1: slli zero, zero, 0x1f\n"
  

•           "     ebreak\n"
  

•           "     srai zero, zero, 7\n"
  

•           "     csrw mtvec, %[tmp]\n"
  

•           "     .option pop\n"
  

•           : [tmp] "+r" (tmp), [en] "+r" (_semihosting_enabled),
  

•             [ret] "+r" (ret)
  

•           : "r" (param0) : "memory");
  

Can you comment on why you chose this approach (setting a new trap vector when testing for semihosting) as opposed to the approach used on arm (extending the existing trap vector) or reading DCSR.EBREAKx? The advantage of extending the existing trap vector is that the user can disconnect the debugger and U-Boot will continue running (albeit perhaps in a state of reduced functionality). There is also never a window where we can crash with no output (since the original trap handler remains in place). The disadvantage is that we have to add some extra code so we are very sure that we have a semihosting halt (instead of some other illegal instruction).

The other option is to just read DCSR.EBREAKx directly [1]. I think this could be the cleanest approach, since we can add a check in smh_trap, avoiding the ebreak completely if there's no debugger attached. I'm not sure if this csr is readable by regular code (maybe you have to be in D-mode). There's also a small race condition if the debugger is detached in between when we check DCSR.EBREAKx and when we execute ebreak.

[1] https://tomverbeure.github.io/2021/12/30/Semihosting-on-RISCV.html#when-does...

...

•   try_semihosting = false;
  

•   return _semihosting_enabled;
  

+}

+void disable_semihosting(void) +{

•   _semihosting_enabled = false;
  

+} +#endif diff --git a/arch/riscv/lib/semihosting_smode.c b/arch/riscv/lib/semihosting_smode.c new file mode 100644 index 0000000000..d7ce07985b --- /dev/null +++ b/arch/riscv/lib/semihosting_smode.c @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2022 Ventana Micro Systems Inc.
• */

+#include <common.h>

+#define SYSERRNO 0x13

+long smh_trap(int sysnum, void *addr) +{

•   register int ret asm ("a0") = sysnum;
  

•   register void *param0 asm ("a1") = addr;
  

•   asm volatile ("       .option push\n"
  

•           "     .option norvc\n"
  

•           "     j 1f\n"
  

•           "     .align 4\n"
  

•           "     1: slli zero, zero, 0x1f\n"
  

•           "     ebreak\n"
  

•           "     srai zero, zero, 7\n"
  

•           "     .option pop\n"
  

•           : "+r" (ret) : "r" (param0) : "memory");
  

•   return ret;
  

+}

This looks identical to the M-mode version.

...

+#if CONFIG_IS_ENABLED(SEMIHOSTING_FALLBACK) +static bool _semihosting_enabled = true; +static bool try_semihosting = true;

+bool semihosting_enabled(void) +{

•   unsigned long tmp = 0;
  

•   register int ret asm ("a0") = SYSERRNO;
  

•   register void *param0 asm ("a1") = NULL;
  

•   if (!try_semihosting)
  

•           return _semihosting_enabled;
  

•   asm volatile ("       .option push\n"
  

•           "     .option norvc\n"
  

•           "     j _semihost_test_vector_next\n"
  

•           "     .align 4\n"
  

•           "     _semihost_test_vector:\n"
  

•           "             csrr %[en], sepc\n"
  

•           "             addi %[en], %[en], 4\n"
  

•           "             csrw sepc, %[en]\n"
  

•           "             add %[en], zero, zero\n"
  

•           "             sret\n"
  

As does this, with the exception of the csrs and rets. I think we have some macros to abstract this sort of thing away.

...

•           "     _semihost_test_vector_next:\n"
  

•           "     la %[tmp], _semihost_test_vector\n"
  

•           "     csrrw %[tmp], stvec, %[tmp]\n"
  

•           "     j 1f\n"
  

•           "     .align 4\n"
  

•           "     1: slli zero, zero, 0x1f\n"
  

•           "     ebreak\n"
  

•           "     srai zero, zero, 7\n"
  

•           "     csrw stvec, %[tmp]\n"
  

•           "     .option pop\n"
  

•           : [tmp] "+r" (tmp), [en] "+r" (_semihosting_enabled),
  

•             [ret] "+r" (ret)
  

•           : "r" (param0) : "memory");
  

•   try_semihosting = false;
  

•   return _semihosting_enabled;
  

+}

+void disable_semihosting(void) +{

•   _semihosting_enabled = false;
  

+}

+#endif

2.34.1

--Sean

On 9/15/22 8:45 AM, Kautuk Consul wrote:

[You don't often get email from kconsul@ventanamicro.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

To use semihosting on qemu RISCV virt machine, we need the CONFIG_SPL_FS_LOAD_PAYLOAD_NAME define in qemu-riscv.h.

We also need to enable the following configs in defconfigs: CONFIG_SEMIHOSTING CONFIG_SPL_SEMIHOSTING CONFIG_SEMIHOSTING_SERIAL CONFIG_SERIAL_PROBE_ALL

Signed-off-by: Kautuk Consul kconsul@ventanamicro.com

configs/qemu-riscv32_defconfig | 4 ++++ configs/qemu-riscv32_smode_defconfig | 4 ++++ configs/qemu-riscv32_spl_defconfig | 5 +++++ configs/qemu-riscv64_defconfig | 4 ++++ configs/qemu-riscv64_smode_defconfig | 4 ++++ configs/qemu-riscv64_spl_defconfig | 5 +++++ include/configs/qemu-riscv.h | 2 ++ 7 files changed, 28 insertions(+)

diff --git a/configs/qemu-riscv32_defconfig b/configs/qemu-riscv32_defconfig index 9634d7f77f..4961652548 100644 --- a/configs/qemu-riscv32_defconfig +++ b/configs/qemu-riscv32_defconfig @@ -1,4 +1,5 @@ CONFIG_RISCV=y +CONFIG_SEMIHOSTING=y CONFIG_SYS_MALLOC_LEN=0x800000 CONFIG_NR_DRAM_BANKS=1 CONFIG_ENV_SIZE=0x20000 @@ -20,3 +21,6 @@ CONFIG_CMD_NVEDIT_EFI=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_DM_MTD=y CONFIG_SYS_MAX_FLASH_BANKS=2 +# CONFIG_SERIAL_PUTS is not set

Does this have any affect on performance? It does for hardware, but for software the overhead should be much smaller.

+CONFIG_SERIAL_PROBE_ALL=y +CONFIG_SEMIHOSTING_SERIAL=y diff --git a/configs/qemu-riscv32_smode_defconfig b/configs/qemu-riscv32_smode_defconfig index 1c5a0617aa..91e4ffebc2 100644 --- a/configs/qemu-riscv32_smode_defconfig +++ b/configs/qemu-riscv32_smode_defconfig @@ -1,4 +1,5 @@ CONFIG_RISCV=y +CONFIG_SEMIHOSTING=y CONFIG_SYS_MALLOC_LEN=0x800000 CONFIG_NR_DRAM_BANKS=1 CONFIG_ENV_SIZE=0x20000 @@ -21,4 +22,7 @@ CONFIG_CMD_NVEDIT_EFI=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_DM_MTD=y CONFIG_SYS_MAX_FLASH_BANKS=2 +# CONFIG_SERIAL_PUTS is not set +CONFIG_SERIAL_PROBE_ALL=y +CONFIG_SEMIHOSTING_SERIAL=y CONFIG_SYSRESET_SBI=y diff --git a/configs/qemu-riscv32_spl_defconfig b/configs/qemu-riscv32_spl_defconfig index 2421c9a371..90dbc329e0 100644 --- a/configs/qemu-riscv32_spl_defconfig +++ b/configs/qemu-riscv32_spl_defconfig @@ -1,4 +1,6 @@ CONFIG_RISCV=y +CONFIG_SEMIHOSTING=y +CONFIG_SPL_SEMIHOSTING=y CONFIG_SYS_MALLOC_LEN=0x800000 CONFIG_NR_DRAM_BANKS=1 CONFIG_ENV_SIZE=0x20000 @@ -25,5 +27,8 @@ CONFIG_SYS_BOOTM_LEN=0x4000000 CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_DM_MTD=y CONFIG_SYS_MAX_FLASH_BANKS=2 +# CONFIG_SERIAL_PUTS is not set +CONFIG_SERIAL_PROBE_ALL=y +CONFIG_SEMIHOSTING_SERIAL=y CONFIG_SYSRESET_SBI=y # CONFIG_BINMAN_FDT is not set diff --git a/configs/qemu-riscv64_defconfig b/configs/qemu-riscv64_defconfig index d5eae95c80..87478f4481 100644 --- a/configs/qemu-riscv64_defconfig +++ b/configs/qemu-riscv64_defconfig @@ -1,4 +1,5 @@ CONFIG_RISCV=y +CONFIG_SEMIHOSTING=y CONFIG_SYS_MALLOC_LEN=0x800000 CONFIG_NR_DRAM_BANKS=1 CONFIG_ENV_SIZE=0x20000 @@ -21,3 +22,6 @@ CONFIG_CMD_NVEDIT_EFI=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_DM_MTD=y CONFIG_SYS_MAX_FLASH_BANKS=2 +# CONFIG_SERIAL_PUTS is not set +CONFIG_SERIAL_PROBE_ALL=y +CONFIG_SEMIHOSTING_SERIAL=y diff --git a/configs/qemu-riscv64_smode_defconfig b/configs/qemu-riscv64_smode_defconfig index 2861d07f97..5e9d6af3be 100644 --- a/configs/qemu-riscv64_smode_defconfig +++ b/configs/qemu-riscv64_smode_defconfig @@ -1,4 +1,5 @@ CONFIG_RISCV=y +CONFIG_SEMIHOSTING=y CONFIG_SYS_MALLOC_LEN=0x800000 CONFIG_NR_DRAM_BANKS=1 CONFIG_ENV_SIZE=0x20000 @@ -24,4 +25,7 @@ CONFIG_CMD_NVEDIT_EFI=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_DM_MTD=y CONFIG_SYS_MAX_FLASH_BANKS=2 +# CONFIG_SERIAL_PUTS is not set +CONFIG_SERIAL_PROBE_ALL=y +CONFIG_SEMIHOSTING_SERIAL=y CONFIG_SYSRESET_SBI=y diff --git a/configs/qemu-riscv64_spl_defconfig b/configs/qemu-riscv64_spl_defconfig index 1ecfa27ce2..fb951293dd 100644 --- a/configs/qemu-riscv64_spl_defconfig +++ b/configs/qemu-riscv64_spl_defconfig @@ -1,4 +1,6 @@ CONFIG_RISCV=y +CONFIG_SEMIHOSTING=y +CONFIG_SPL_SEMIHOSTING=y CONFIG_SYS_MALLOC_LEN=0x800000 CONFIG_NR_DRAM_BANKS=1 CONFIG_ENV_SIZE=0x20000 @@ -25,5 +27,8 @@ CONFIG_SYS_BOOTM_LEN=0x4000000 CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_DM_MTD=y CONFIG_SYS_MAX_FLASH_BANKS=2 +# CONFIG_SERIAL_PUTS is not set +CONFIG_SERIAL_PROBE_ALL=y +CONFIG_SEMIHOSTING_SERIAL=y CONFIG_SYSRESET_SBI=y # CONFIG_BINMAN_FDT is not set diff --git a/include/configs/qemu-riscv.h b/include/configs/qemu-riscv.h index d81e5d6c86..257d9c5d93 100644 --- a/include/configs/qemu-riscv.h +++ b/include/configs/qemu-riscv.h @@ -48,4 +48,6 @@ "ramdisk_addr_r=0x8c300000\0" \ BOOTENV

+# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot.itb"

This is a Kconfig since 4a11e34bc9c ("Convert CONFIG_SPL_FS_LOAD_PAYLOAD_NAME et al to Kconfig")

#endif /* __CONFIG_H */

2.34.1