Re: [PATCH 1/2] arch/riscv: add semihosting support for RISC-V

2022-09-16 Thread Kautuk Consul
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  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-spec.adoc
> >
> > Signed-off-by: Anup Patel 
> > Signed-off-by: Kautuk Consul 
> > ---
> >  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 00..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_DE

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

2022-09-16 Thread Kautuk Consul
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 
Signed-off-by: Kautuk Consul 
---
 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 00..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 00..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 
+#inc

Re: [PATCH 1/2] arch/riscv: add semihosting support for RISC-V

2022-09-15 Thread Sean Anderson
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-spec.adoc
> 
> Signed-off-by: Anup Patel 
> Signed-off-by: Kautuk Consul 
> ---
>  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 00..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)

Re: [PATCH 1/2] arch/riscv: add semihosting support for RISC-V

2022-09-15 Thread Anup Patel
On Thu, Sep 15, 2022 at 6:15 PM 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 
> Signed-off-by: Kautuk Consul 
> ---
>  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 00..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