[PATCH v2 1/3] serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h

[PATCH v2 1/3] serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h

[Bin Meng]

Move smh_putc() variants in respective arch/*/include/asm/semihost.h,
in preparation to add RISC-V support.

Signed-off-by: Bin Meng <bmeng@tinylab.org>

---

Changes in v2:
- new patch: "serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h"

 arch/arm/include/asm/semihost.h            | 23 ++++++++++++++++++++
 arch/arm64/include/asm/semihost.h          | 17 +++++++++++++++
 drivers/tty/serial/earlycon-arm-semihost.c | 25 +---------------------
 3 files changed, 41 insertions(+), 24 deletions(-)
 create mode 100644 arch/arm/include/asm/semihost.h
 create mode 100644 arch/arm64/include/asm/semihost.h

diff --git a/arch/arm/include/asm/semihost.h b/arch/arm/include/asm/semihost.h
new file mode 100644
index 000000000000..c33cb5124376
--- /dev/null
+++ b/arch/arm/include/asm/semihost.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2012 ARM Ltd.
+ * Author: Marc Zyngier <marc.zyngier@arm.com>
+ *
+ * Adapted for ARM and earlycon:
+ * Copyright (C) 2014 Linaro Ltd.
+ * Author: Rob Herring <robh@kernel.org>
+ */
+
+#ifdef CONFIG_THUMB2_KERNEL
+#define SEMIHOST_SWI	"0xab"
+#else
+#define SEMIHOST_SWI	"0x123456"
+#endif
+
+static inline void smh_putc(struct uart_port *port, unsigned char c)
+{
+	asm volatile("mov  r1, %0\n"
+		     "mov  r0, #3\n"
+		     "svc  " SEMIHOST_SWI "\n"
+		     : : "r" (&c) : "r0", "r1", "memory");
+}
diff --git a/arch/arm64/include/asm/semihost.h b/arch/arm64/include/asm/semihost.h
new file mode 100644
index 000000000000..9e56d38fe5fd
--- /dev/null
+++ b/arch/arm64/include/asm/semihost.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2012 ARM Ltd.
+ * Author: Marc Zyngier <marc.zyngier@arm.com>
+ *
+ * Adapted for ARM and earlycon:
+ * Copyright (C) 2014 Linaro Ltd.
+ * Author: Rob Herring <robh@kernel.org>
+ */
+
+static inline void smh_putc(struct uart_port *port, unsigned char c)
+{
+	asm volatile("mov  x1, %0\n"
+		     "mov  x0, #3\n"
+		     "hlt  0xf000\n"
+		     : : "r" (&c) : "x0", "x1", "memory");
+}
diff --git a/drivers/tty/serial/earlycon-arm-semihost.c b/drivers/tty/serial/earlycon-arm-semihost.c
index fcdec5f42376..e4692a8433f9 100644
--- a/drivers/tty/serial/earlycon-arm-semihost.c
+++ b/drivers/tty/serial/earlycon-arm-semihost.c
@@ -11,30 +11,7 @@
 #include <linux/console.h>
 #include <linux/init.h>
 #include <linux/serial_core.h>
-
-#ifdef CONFIG_THUMB2_KERNEL
-#define SEMIHOST_SWI	"0xab"
-#else
-#define SEMIHOST_SWI	"0x123456"
-#endif
-
-/*
- * Semihosting-based debug console
- */
-static void smh_putc(struct uart_port *port, unsigned char c)
-{
-#ifdef CONFIG_ARM64
-	asm volatile("mov  x1, %0\n"
-		     "mov  x0, #3\n"
-		     "hlt  0xf000\n"
-		     : : "r" (&c) : "x0", "x1", "memory");
-#else
-	asm volatile("mov  r1, %0\n"
-		     "mov  r0, #3\n"
-		     "svc  " SEMIHOST_SWI "\n"
-		     : : "r" (&c) : "r0", "r1", "memory");
-#endif
-}
+#include <asm/semihost.h>
 
 static void smh_write(struct console *con, const char *s, unsigned n)
 {
-- 
2.34.1

[PATCH v2 2/3] riscv: Implement semihost.h for earlycon semihost driver

[Bin Meng]

Per RISC-V semihosting spec [1], implement semihost.h for the existing
Arm semihosting earlycon driver to work on RISC-V.

[1] https://github.com/riscv/riscv-semihosting-spec/blob/main/riscv-semihosting-spec.adoc

Signed-off-by: Bin Meng <bmeng@tinylab.org>

---

Changes in v2:
- Move the RISC-V implementation to semihost.h

 arch/riscv/include/asm/semihost.h | 19 +++++++++++++++++++
 drivers/tty/serial/Kconfig        |  2 +-
 2 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 arch/riscv/include/asm/semihost.h

diff --git a/arch/riscv/include/asm/semihost.h b/arch/riscv/include/asm/semihost.h
new file mode 100644
index 000000000000..886f21d7a476
--- /dev/null
+++ b/arch/riscv/include/asm/semihost.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 tinylab.org
+ * Author: Bin Meng <bmeng@tinylab.org>
+ */
+
+static inline void smh_putc(struct uart_port *port, unsigned char c)
+{
+	asm volatile("addi    a1, %0, 0\n"
+		     "addi    a0, zero, 3\n"
+		     ".balign 16\n"
+		     ".option push\n"
+		     ".option norvc\n"
+		     "slli    zero, zero, 0x1f\n"
+		     "ebreak\n"
+		     "srai    zero, zero, 0x7\n"
+		     ".option pop\n"
+		     : : "r" (&c) : "a0", "a1", "memory");
+}
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 434f83168546..e94d1265151c 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -75,7 +75,7 @@ config SERIAL_AMBA_PL011_CONSOLE
 
 config SERIAL_EARLYCON_ARM_SEMIHOST
 	bool "Early console using ARM semihosting"
-	depends on ARM64 || ARM
+	depends on ARM64 || ARM || RISCV
 	select SERIAL_CORE
 	select SERIAL_CORE_CONSOLE
 	select SERIAL_EARLYCON
-- 
2.34.1

Re: [PATCH v2 2/3] riscv: Implement semihost.h for earlycon semihost driver

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 480 bytes --]

Hello,

Two tiny nits for whenever other comments require a v3.

On Wed, Dec 07, 2022 at 09:53:51PM +0800, Bin Meng wrote:
> Per RISC-V semihosting spec [1], implement semihost.h for the existing
> Arm semihosting earlycon driver to work on RISC-V.
> 
> [1] https://github.com/riscv/riscv-semihosting-spec/blob/main/riscv-semihosting-spec.adoc

Could you please use regular Link: tags? Also, for a multi-patch series
please send a cover letter :)

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 3/3] serial: Rename earlycon semihost driver

[Bin Meng]

Now that earlycon semihost driver works on RISC-V too, let's use a
much more generic name for the driver.

Signed-off-by: Bin Meng <bmeng@tinylab.org>

---

(no changes since v1)

 drivers/tty/serial/Kconfig                           | 12 ++++++------
 drivers/tty/serial/Makefile                          |  2 +-
 .../{earlycon-arm-semihost.c => earlycon-semihost.c} |  0
 3 files changed, 7 insertions(+), 7 deletions(-)
 rename drivers/tty/serial/{earlycon-arm-semihost.c => earlycon-semihost.c} (100%)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index e94d1265151c..a3779472edf6 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -73,17 +73,17 @@ config SERIAL_AMBA_PL011_CONSOLE
 	  your boot loader (lilo or loadlin) about how to pass options to the
 	  kernel at boot time.)
 
-config SERIAL_EARLYCON_ARM_SEMIHOST
-	bool "Early console using ARM semihosting"
+config SERIAL_EARLYCON_SEMIHOST
+	bool "Early console using Arm compatible semihosting"
 	depends on ARM64 || ARM || RISCV
 	select SERIAL_CORE
 	select SERIAL_CORE_CONSOLE
 	select SERIAL_EARLYCON
 	help
-	  Support for early debug console using ARM semihosting. This enables
-	  the console before standard serial driver is probed. This is enabled
-	  with "earlycon=smh" on the kernel command line. The console is
-	  enabled when early_param is processed.
+	  Support for early debug console using Arm compatible semihosting.
+	  This enables the console before standard serial driver is probed.
+	  This is enabled with "earlycon=smh" on the kernel command line.
+	  The console is enabled when early_param is processed.
 
 config SERIAL_EARLYCON_RISCV_SBI
 	bool "Early console using RISC-V SBI"
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 238a9557b487..cd9afd9e3018 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -6,7 +6,7 @@
 obj-$(CONFIG_SERIAL_CORE) += serial_core.o
 
 obj-$(CONFIG_SERIAL_EARLYCON) += earlycon.o
-obj-$(CONFIG_SERIAL_EARLYCON_ARM_SEMIHOST) += earlycon-arm-semihost.o
+obj-$(CONFIG_SERIAL_EARLYCON_SEMIHOST) += earlycon-semihost.o
 obj-$(CONFIG_SERIAL_EARLYCON_RISCV_SBI) += earlycon-riscv-sbi.o
 
 # These Sparc drivers have to appear before others such as 8250
diff --git a/drivers/tty/serial/earlycon-arm-semihost.c b/drivers/tty/serial/earlycon-semihost.c
similarity index 100%
rename from drivers/tty/serial/earlycon-arm-semihost.c
rename to drivers/tty/serial/earlycon-semihost.c
-- 
2.34.1

Re: [PATCH v2 1/3] serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h

[Jiri Slaby]

On 07. 12. 22, 14:53, Bin Meng wrote:
> Move smh_putc() variants in respective arch/*/include/asm/semihost.h,
> in preparation to add RISC-V support.
> 
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
...
> --- /dev/null
> +++ b/arch/arm/include/asm/semihost.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2012 ARM Ltd.
> + * Author: Marc Zyngier <marc.zyngier@arm.com>
> + *
> + * Adapted for ARM and earlycon:
> + * Copyright (C) 2014 Linaro Ltd.
> + * Author: Rob Herring <robh@kernel.org>
> + */

Much better. There are three minor issues:
1) protection against multiple #include-s is missing here.

> +#ifdef CONFIG_THUMB2_KERNEL
> +#define SEMIHOST_SWI	"0xab"
> +#else
> +#define SEMIHOST_SWI	"0x123456"
> +#endif
> +
> +static inline void smh_putc(struct uart_port *port, unsigned char c)

2) port is unused in all implementations. So it should be dropped.
3) can you make "c" an explicit u8?

> +{
> +	asm volatile("mov  r1, %0\n"
> +		     "mov  r0, #3\n"
> +		     "svc  " SEMIHOST_SWI "\n"
> +		     : : "r" (&c) : "r0", "r1", "memory");
> +}

thanks,
-- 
js
suse labs

Re: [PATCH v2 1/3] serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h

[Bin Meng]

On 2022/12/8 14:08:33, "Jiri Slaby" <jirislaby@kernel.org> wrote:

>On 07. 12. 22, 14:53, Bin Meng wrote:
>>Move smh_putc() variants in respective arch/*/include/asm/semihost.h,
>>in preparation to add RISC-V support.
>>
>>Signed-off-by: Bin Meng <bmeng@tinylab.org>
>...
>>--- /dev/null
>>+++ b/arch/arm/include/asm/semihost.h
>>@@ -0,0 +1,23 @@
>>+/* SPDX-License-Identifier: GPL-2.0 */
>>+/*
>>+ * Copyright (C) 2012 ARM Ltd.
>>+ * Author: Marc Zyngier <marc.zyngier@arm.com>
>>+ *
>>+ * Adapted for ARM and earlycon:
>>+ * Copyright (C) 2014 Linaro Ltd.
>>+ * Author: Rob Herring <robh@kernel.org>
>>+ */
>
>Much better. There are three minor issues:
>1) protection against multiple #include-s is missing here.

Oops, will add in v3.

>
>
>>+#ifdef CONFIG_THUMB2_KERNEL
>>+#define SEMIHOST_SWI	"0xab"
>>+#else
>>+#define SEMIHOST_SWI	"0x123456"
>>+#endif
>>+
>>+static inline void smh_putc(struct uart_port *port, unsigned char c)
>
>2) port is unused in all implementations. So it should be dropped.
>3) can you make "c" an explicit u8?

The smh_putc function signature is defined by the uart_console_write 
helper. I don't think we can change it.

>
>>+{
>>+	asm volatile("mov  r1, %0\n"
>>+		     "mov  r0, #3\n"
>>+		     "svc  " SEMIHOST_SWI "\n"
>>+		     : : "r" (&c) : "r0", "r1", "memory");
>>+}

Regards,
Bin

Re: [PATCH v2 1/3] serial: earlycon-arm-semihost: Move smh_putc() variants in respective arch's semihost.h

[Jiri Slaby]

On 08. 12. 22, 10:32, Bin Meng wrote:
>>> +#ifdef CONFIG_THUMB2_KERNEL
>>> +#define SEMIHOST_SWI    "0xab"
>>> +#else
>>> +#define SEMIHOST_SWI    "0x123456"
>>> +#endif
>>> +
>>> +static inline void smh_putc(struct uart_port *port, unsigned char c)
>>
>> 2) port is unused in all implementations. So it should be dropped.
>> 3) can you make "c" an explicit u8?
> 
> The smh_putc function signature is defined by the uart_console_write 
> helper. I don't think we can change it.

Ah. Of course. Then at least forward-declare struct uart_port here. So 
that it works also when someone decides to include the header outside 
serial.

thanks,
-- 
js
suse labs