[PATCH 0/3] QEMU spike machine support for U-Boot

[PATCH 0/3] QEMU spike machine support for U-Boot

[Anup Patel]

We can use same U-Boot binary compiled using qemu-riscv64_smode_defconfig
on QEMU virt machine and QEMU spike machine. To achieve this, we need HTIF
console support for U-Boot QEMU RISC-V board hence this series.

To test this series with latest OpenSBI, we can use the following command:
qemu-system-riscv64 -M spike -m 256M -display none -serial stdio \
-bios opensbi/build/platform/generic/firmware/fw_jump.bin -kernel \
./u-boot/u-boot.bin

These patch can be found in qemu_riscv_htif_v1 branch at:
https://github.com/avpatel/u-boot.git

Anup Patel (3):
  serial: Add RISC-V HTIF console driver
  riscv: qemu: Enable HTIF console support
  riscv: qemu: Implement is_flash_available() for MTD NOR

 board/emulation/qemu-riscv/Kconfig      |   1 +
 board/emulation/qemu-riscv/qemu-riscv.c |  17 +++
 drivers/serial/Kconfig                  |   8 ++
 drivers/serial/Makefile                 |   1 +
 drivers/serial/serial_htif.c            | 178 ++++++++++++++++++++++++
 5 files changed, 205 insertions(+)
 create mode 100644 drivers/serial/serial_htif.c

-- 
2.25.1

[PATCH 1/3] serial: Add RISC-V HTIF console driver

[Anup Patel]

Quite a few RISC-V emulators and ISS (including Spike) have host
transfer interface (HTIF) based console. This patch adds HTIF
based console driver for RISC-V platforms which depends totally
on DT node for HTIF register base address.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---
 drivers/serial/Kconfig       |   8 ++
 drivers/serial/Makefile      |   1 +
 drivers/serial/serial_htif.c | 178 +++++++++++++++++++++++++++++++++++
 3 files changed, 187 insertions(+)
 create mode 100644 drivers/serial/serial_htif.c

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 6c8fdda9a0..345d1881f5 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -866,6 +866,14 @@ config PXA_SERIAL
 	  If you have a machine based on a Marvell XScale PXA2xx CPU you
 	  can enable its onboard serial ports by enabling this option.
 
+config HTIF_CONSOLE
+	bool "RISC-V HTIF console support"
+	depends on DM_SERIAL && 64BIT
+	help
+	  Select this to enable host transfer interface (HTIF) based serial
+	  console. The HTIF device is quite common in RISC-V emulators and
+	  RISC-V ISS so this driver allows using U-Boot on such platforms.
+
 config SIFIVE_SERIAL
 	bool "SiFive UART support"
 	depends on DM_SERIAL
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index 8168af640f..52e70aa191 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -73,6 +73,7 @@ obj-$(CONFIG_OWL_SERIAL) += serial_owl.o
 obj-$(CONFIG_OMAP_SERIAL) += serial_omap.o
 obj-$(CONFIG_MTK_SERIAL) += serial_mtk.o
 obj-$(CONFIG_MT7620_SERIAL) += serial_mt7620.o
+obj-$(CONFIG_HTIF_CONSOLE) += serial_htif.o
 obj-$(CONFIG_SIFIVE_SERIAL) += serial_sifive.o
 obj-$(CONFIG_XEN_SERIAL) += serial_xen.o
 
diff --git a/drivers/serial/serial_htif.c b/drivers/serial/serial_htif.c
new file mode 100644
index 0000000000..5d2bf0aaeb
--- /dev/null
+++ b/drivers/serial/serial_htif.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2022 Ventana Micro Systems Inc.
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdtdec.h>
+#include <log.h>
+#include <watchdog.h>
+#include <asm/global_data.h>
+#include <asm/io.h>
+#include <linux/compiler.h>
+#include <serial.h>
+#include <linux/err.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define HTIF_DATA_BITS		48
+#define HTIF_DATA_MASK		((1ULL << HTIF_DATA_BITS) - 1)
+#define HTIF_DATA_SHIFT		0
+#define HTIF_CMD_BITS		8
+#define HTIF_CMD_MASK		((1ULL << HTIF_CMD_BITS) - 1)
+#define HTIF_CMD_SHIFT		48
+#define HTIF_DEV_BITS		8
+#define HTIF_DEV_MASK		((1ULL << HTIF_DEV_BITS) - 1)
+#define HTIF_DEV_SHIFT		56
+
+#define HTIF_DEV_SYSTEM		0
+#define HTIF_DEV_CONSOLE	1
+
+#define HTIF_CONSOLE_CMD_GETC	0
+#define HTIF_CONSOLE_CMD_PUTC	1
+
+#if __riscv_xlen == 64
+# define TOHOST_CMD(dev, cmd, payload) \
+	(((u64)(dev) << HTIF_DEV_SHIFT) | \
+	 ((u64)(cmd) << HTIF_CMD_SHIFT) | \
+	 (u64)(payload))
+#else
+# define TOHOST_CMD(dev, cmd, payload) ({ \
+	if ((dev) || (cmd)) \
+		__builtin_trap(); \
+	(payload); })
+#endif
+#define FROMHOST_DEV(fromhost_value) \
+	((u64)((fromhost_value) >> HTIF_DEV_SHIFT) & HTIF_DEV_MASK)
+#define FROMHOST_CMD(fromhost_value) \
+	((u64)((fromhost_value) >> HTIF_CMD_SHIFT) & HTIF_CMD_MASK)
+#define FROMHOST_DATA(fromhost_value) \
+	((u64)((fromhost_value) >> HTIF_DATA_SHIFT) & HTIF_DATA_MASK)
+
+struct htif_plat {
+	void *fromhost;
+	void *tohost;
+	int console_char;
+};
+
+static void __check_fromhost(struct htif_plat *plat)
+{
+	u64 fh = readq(plat->fromhost);
+
+	if (!fh)
+		return;
+	writeq(0, plat->fromhost);
+
+	/* this should be from the console */
+	if (FROMHOST_DEV(fh) != HTIF_DEV_CONSOLE)
+		__builtin_trap();
+	switch (FROMHOST_CMD(fh)) {
+	case HTIF_CONSOLE_CMD_GETC:
+		plat->console_char = 1 + (u8)FROMHOST_DATA(fh);
+		break;
+	case HTIF_CONSOLE_CMD_PUTC:
+		break;
+	default:
+		__builtin_trap();
+	}
+}
+
+static void __set_tohost(struct htif_plat *plat,
+			 u64 dev, u64 cmd, u64 data)
+{
+	while (readq(plat->tohost))
+		__check_fromhost(plat);
+	writeq(TOHOST_CMD(dev, cmd, data), plat->tohost);
+}
+
+static int htif_serial_putc(struct udevice *dev, const char ch)
+{
+	struct htif_plat *plat = dev_get_plat(dev);
+
+	__set_tohost(plat, HTIF_DEV_CONSOLE, HTIF_CONSOLE_CMD_PUTC, ch);
+	return 0;
+}
+
+static int htif_serial_getc(struct udevice *dev)
+{
+	int ch;
+	struct htif_plat *plat = dev_get_plat(dev);
+
+	if (plat->console_char < 0)
+		__check_fromhost(plat);
+
+	if (plat->console_char >= 0) {
+		ch = plat->console_char;
+		plat->console_char = -1;
+		__set_tohost(plat, HTIF_DEV_CONSOLE, HTIF_CONSOLE_CMD_GETC, 0);
+		return (ch) ? ch - 1 : -EAGAIN;
+	}
+
+	return -EAGAIN;
+}
+
+static int htif_serial_pending(struct udevice *dev, bool input)
+{
+	struct htif_plat *plat = dev_get_plat(dev);
+
+	if (!input)
+		return 0;
+
+	if (plat->console_char < 0)
+		__check_fromhost(plat);
+
+	return (plat->console_char >= 0) ? 1 : 0;
+}
+
+static int htif_serial_probe(struct udevice *dev)
+{
+	struct htif_plat *plat = dev_get_plat(dev);
+
+	/* Queue first getc request */
+	__set_tohost(plat, HTIF_DEV_CONSOLE, HTIF_CONSOLE_CMD_GETC, 0);
+
+	return 0;
+}
+
+static int htif_serial_of_to_plat(struct udevice *dev)
+{
+	fdt_addr_t addr;
+	struct htif_plat *plat = dev_get_plat(dev);
+
+	addr = dev_read_addr_index(dev, 0);
+	if (addr == FDT_ADDR_T_NONE)
+		return -ENODEV;
+	plat->fromhost = (void *)(uintptr_t)addr;
+	plat->tohost = plat->fromhost + sizeof(u64);
+
+	addr = dev_read_addr_index(dev, 1);
+	if (addr != FDT_ADDR_T_NONE)
+		plat->tohost = (void *)(uintptr_t)addr;
+
+	plat->console_char = -1;
+
+	return 0;
+}
+
+static const struct dm_serial_ops htif_serial_ops = {
+	.putc = htif_serial_putc,
+	.getc = htif_serial_getc,
+	.pending = htif_serial_pending,
+};
+
+static const struct udevice_id htif_serial_ids[] = {
+	{ .compatible = "ucb,htif0" },
+	{ }
+};
+
+U_BOOT_DRIVER(serial_htif) = {
+	.name		= "serial_htif",
+	.id		= UCLASS_SERIAL,
+	.of_match	= htif_serial_ids,
+	.of_to_plat	= htif_serial_of_to_plat,
+	.plat_auto	= sizeof(struct htif_plat),
+	.probe		= htif_serial_probe,
+	.ops		= &htif_serial_ops,
+};
-- 
2.25.1

Re: [PATCH 1/3] serial: Add RISC-V HTIF console driver

[Rick Chen]

> From: Anup Patel <apatel@ventanamicro.com>
> Sent: Saturday, January 15, 2022 12:20 AM
> To: Rick Jian-Zhi Chen(陳建志) <rick@andestech.com>; Bin Meng <bmeng.cn@gmail.com>
> Cc: Atish Patra <atishp@atishpatra.org>; Alistair Francis <Alistair.Francis@wdc.com>; Anup Patel <anup@brainfault.org>; U-Boot Mailing List <u-boot@lists.denx.de>; Anup Patel <apatel@ventanamicro.com>; Philipp Tomsich <philipp.tomsich@vrull.eu>
> Subject: [PATCH 1/3] serial: Add RISC-V HTIF console driver
>
> Quite a few RISC-V emulators and ISS (including Spike) have host transfer interface (HTIF) based console. This patch adds HTIF based console driver for RISC-V platforms which depends totally on DT node for HTIF register base address.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> ---
>  drivers/serial/Kconfig       |   8 ++
>  drivers/serial/Makefile      |   1 +
>  drivers/serial/serial_htif.c | 178 +++++++++++++++++++++++++++++++++++
>  3 files changed, 187 insertions(+)

Reviewed-by: Rick Chen <rick@andestech.com>

Re: [PATCH 1/3] serial: Add RISC-V HTIF console driver

[Bin Meng]

On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> Quite a few RISC-V emulators and ISS (including Spike) have host
> transfer interface (HTIF) based console. This patch adds HTIF
> based console driver for RISC-V platforms which depends totally
> on DT node for HTIF register base address.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> ---
>  drivers/serial/Kconfig       |   8 ++
>  drivers/serial/Makefile      |   1 +
>  drivers/serial/serial_htif.c | 178 +++++++++++++++++++++++++++++++++++
>  3 files changed, 187 insertions(+)
>  create mode 100644 drivers/serial/serial_htif.c
>
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 6c8fdda9a0..345d1881f5 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -866,6 +866,14 @@ config PXA_SERIAL
>           If you have a machine based on a Marvell XScale PXA2xx CPU you
>           can enable its onboard serial ports by enabling this option.
>
> +config HTIF_CONSOLE
> +       bool "RISC-V HTIF console support"
> +       depends on DM_SERIAL && 64BIT

Does this driver not work on 32-bit?

> +       help
> +         Select this to enable host transfer interface (HTIF) based serial
> +         console. The HTIF device is quite common in RISC-V emulators and
> +         RISC-V ISS so this driver allows using U-Boot on such platforms.
> +
>  config SIFIVE_SERIAL
>         bool "SiFive UART support"
>         depends on DM_SERIAL

Regards,
Bin

Re: [PATCH 1/3] serial: Add RISC-V HTIF console driver

[Anup Patel]

On Tue, Jan 25, 2022 at 10:22 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > Quite a few RISC-V emulators and ISS (including Spike) have host
> > transfer interface (HTIF) based console. This patch adds HTIF
> > based console driver for RISC-V platforms which depends totally
> > on DT node for HTIF register base address.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > Reviewed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > ---
> >  drivers/serial/Kconfig       |   8 ++
> >  drivers/serial/Makefile      |   1 +
> >  drivers/serial/serial_htif.c | 178 +++++++++++++++++++++++++++++++++++
> >  3 files changed, 187 insertions(+)
> >  create mode 100644 drivers/serial/serial_htif.c
> >
> > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> > index 6c8fdda9a0..345d1881f5 100644
> > --- a/drivers/serial/Kconfig
> > +++ b/drivers/serial/Kconfig
> > @@ -866,6 +866,14 @@ config PXA_SERIAL
> >           If you have a machine based on a Marvell XScale PXA2xx CPU you
> >           can enable its onboard serial ports by enabling this option.
> >
> > +config HTIF_CONSOLE
> > +       bool "RISC-V HTIF console support"
> > +       depends on DM_SERIAL && 64BIT
>
> Does this driver not work on 32-bit?

Only putc() works but getc() does not work on 32-bit. Same issue
is there with OpenSBI and BBL as well. That's why I have restricted
this driver to 64-bit only.

>
> > +       help
> > +         Select this to enable host transfer interface (HTIF) based serial
> > +         console. The HTIF device is quite common in RISC-V emulators and
> > +         RISC-V ISS so this driver allows using U-Boot on such platforms.
> > +
> >  config SIFIVE_SERIAL
> >         bool "SiFive UART support"
> >         depends on DM_SERIAL
>
> Regards,
> Bin

Regards,
Anup

Re: [PATCH 1/3] serial: Add RISC-V HTIF console driver

[Bin Meng]

On Tue, Jan 25, 2022 at 2:00 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> On Tue, Jan 25, 2022 at 10:22 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote:
> > >
> > > Quite a few RISC-V emulators and ISS (including Spike) have host
> > > transfer interface (HTIF) based console. This patch adds HTIF
> > > based console driver for RISC-V platforms which depends totally
> > > on DT node for HTIF register base address.
> > >
> > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > Reviewed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > > ---
> > >  drivers/serial/Kconfig       |   8 ++
> > >  drivers/serial/Makefile      |   1 +
> > >  drivers/serial/serial_htif.c | 178 +++++++++++++++++++++++++++++++++++
> > >  3 files changed, 187 insertions(+)
> > >  create mode 100644 drivers/serial/serial_htif.c
> > >
> > > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> > > index 6c8fdda9a0..345d1881f5 100644
> > > --- a/drivers/serial/Kconfig
> > > +++ b/drivers/serial/Kconfig
> > > @@ -866,6 +866,14 @@ config PXA_SERIAL
> > >           If you have a machine based on a Marvell XScale PXA2xx CPU you
> > >           can enable its onboard serial ports by enabling this option.
> > >
> > > +config HTIF_CONSOLE
> > > +       bool "RISC-V HTIF console support"
> > > +       depends on DM_SERIAL && 64BIT
> >
> > Does this driver not work on 32-bit?
>
> Only putc() works but getc() does not work on 32-bit. Same issue
> is there with OpenSBI and BBL as well. That's why I have restricted
> this driver to 64-bit only.
>

I don't get it. Is this a QEMU riscv32 bug?

Regards,
Bin

Re: [PATCH 1/3] serial: Add RISC-V HTIF console driver

[Anup Patel]

On Tue, Jan 25, 2022 at 2:46 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Tue, Jan 25, 2022 at 2:00 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > On Tue, Jan 25, 2022 at 10:22 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote:
> > > >
> > > > Quite a few RISC-V emulators and ISS (including Spike) have host
> > > > transfer interface (HTIF) based console. This patch adds HTIF
> > > > based console driver for RISC-V platforms which depends totally
> > > > on DT node for HTIF register base address.
> > > >
> > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > > Reviewed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > > > ---
> > > >  drivers/serial/Kconfig       |   8 ++
> > > >  drivers/serial/Makefile      |   1 +
> > > >  drivers/serial/serial_htif.c | 178 +++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 187 insertions(+)
> > > >  create mode 100644 drivers/serial/serial_htif.c
> > > >
> > > > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> > > > index 6c8fdda9a0..345d1881f5 100644
> > > > --- a/drivers/serial/Kconfig
> > > > +++ b/drivers/serial/Kconfig
> > > > @@ -866,6 +866,14 @@ config PXA_SERIAL
> > > >           If you have a machine based on a Marvell XScale PXA2xx CPU you
> > > >           can enable its onboard serial ports by enabling this option.
> > > >
> > > > +config HTIF_CONSOLE
> > > > +       bool "RISC-V HTIF console support"
> > > > +       depends on DM_SERIAL && 64BIT
> > >
> > > Does this driver not work on 32-bit?
> >
> > Only putc() works but getc() does not work on 32-bit. Same issue
> > is there with OpenSBI and BBL as well. That's why I have restricted
> > this driver to 64-bit only.
> >
>
> I don't get it. Is this a QEMU riscv32 bug?

It's a bug in the HTIF device itself. There is no documentation of
HTIF as well. All implementations (including QEMU spike) simply
follow how Spike ISS implements HTIF.

In other words, HTIF is yet another undocumented device in
RISC-V world.

Regards,
Anup

[PATCH 2/3] riscv: qemu: Enable HTIF console support

[Anup Patel]

Enable support for HTIF console so that we can use QEMU RISC-V U-Boot
on RISC-V emulators and ISS having it.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---
 board/emulation/qemu-riscv/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/board/emulation/qemu-riscv/Kconfig b/board/emulation/qemu-riscv/Kconfig
index 1bbf1bc84a..f7538bc496 100644
--- a/board/emulation/qemu-riscv/Kconfig
+++ b/board/emulation/qemu-riscv/Kconfig
@@ -56,6 +56,7 @@ config BOARD_SPECIFIC_OPTIONS # dummy
 	imply DM_SCSI
 	imply SYS_NS16550
 	imply SIFIVE_SERIAL
+	imply HTIF_CONSOLE if 64BIT
 	imply SYSRESET
 	imply SYSRESET_CMD_POWEROFF
 	imply SYSRESET_SYSCON
-- 
2.25.1

Re: [PATCH 2/3] riscv: qemu: Enable HTIF console support

[Rick Chen]

> From: Anup Patel <apatel@ventanamicro.com>
> Sent: Saturday, January 15, 2022 12:20 AM
> To: Rick Jian-Zhi Chen(陳建志) <rick@andestech.com>; Bin Meng <bmeng.cn@gmail.com>
> Cc: Atish Patra <atishp@atishpatra.org>; Alistair Francis <Alistair.Francis@wdc.com>; Anup Patel <anup@brainfault.org>; U-Boot Mailing List <u-boot@lists.denx.de>; Anup Patel <apatel@ventanamicro.com>; Philipp Tomsich <philipp.tomsich@vrull.eu>
> Subject: [PATCH 2/3] riscv: qemu: Enable HTIF console support
>
> Enable support for HTIF console so that we can use QEMU RISC-V U-Boot on RISC-V emulators and ISS having it.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> ---
>  board/emulation/qemu-riscv/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Rick Chen <rick@andestech.com>

[PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR

[Anup Patel]

Currently, if MTD NOR is enabled then U-Boot tries to issue flash
commands even when CFI flash DT node is not present. This causes
access fault on RISC-V emulators or ISS which do not emulate CFI
flash. To handle this issue, we implement is_flash_available() for
qemu-riscv board which will return 1 only if CFI flash DT node is
present.

Fixes: d248627f9d42 ("riscv: qemu: Enable MTD NOR flash support")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 board/emulation/qemu-riscv/qemu-riscv.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c
index b0d9dd59b1..cd02dae1ab 100644
--- a/board/emulation/qemu-riscv/qemu-riscv.c
+++ b/board/emulation/qemu-riscv/qemu-riscv.c
@@ -8,6 +8,7 @@
 #include <env.h>
 #include <fdtdec.h>
 #include <image.h>
+#include <linux/libfdt.h>
 #include <log.h>
 #include <spl.h>
 #include <init.h>
@@ -16,6 +17,22 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#if IS_ENABLED(CONFIG_MTD_NOR_FLASH)
+int is_flash_available(void)
+{
+	if (IS_ENABLED(CONFIG_OF_SEPARATE) || IS_ENABLED(CONFIG_OF_BOARD)) {
+		const void *fdt =
+			(const void *)(uintptr_t)gd->arch.firmware_fdt_addr;
+		int rc = fdt_node_offset_by_compatible(fdt, -1, "cfi-flash");
+
+		if (rc >= 0)
+			return 1;
+	}
+
+	return 0;
+}
+#endif
+
 int board_init(void)
 {
 	/*
-- 
2.25.1

Re: [PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR

[Rick Chen]

> From: Anup Patel <apatel@ventanamicro.com>
> Sent: Saturday, January 15, 2022 12:20 AM
> To: Rick Jian-Zhi Chen(陳建志) <rick@andestech.com>; Bin Meng <bmeng.cn@gmail.com>
> Cc: Atish Patra <atishp@atishpatra.org>; Alistair Francis <Alistair.Francis@wdc.com>; Anup Patel <anup@brainfault.org>; U-Boot Mailing List <u-boot@lists.denx.de>; Anup Patel <apatel@ventanamicro.com>
> Subject: [PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR
>
> Currently, if MTD NOR is enabled then U-Boot tries to issue flash commands even when CFI flash DT node is not present. This causes access fault on RISC-V emulators or ISS which do not emulate CFI flash. To handle this issue, we implement is_flash_available() for qemu-riscv board which will return 1 only if CFI flash DT node is present.
>
> Fixes: d248627f9d42 ("riscv: qemu: Enable MTD NOR flash support")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  board/emulation/qemu-riscv/qemu-riscv.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

Reviewed-by: Rick Chen <rick@andestech.com>

Re: [PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR

[Bin Meng]

On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> Currently, if MTD NOR is enabled then U-Boot tries to issue flash
> commands even when CFI flash DT node is not present. This causes
> access fault on RISC-V emulators or ISS which do not emulate CFI
> flash. To handle this issue, we implement is_flash_available() for
> qemu-riscv board which will return 1 only if CFI flash DT node is
> present.
>
> Fixes: d248627f9d42 ("riscv: qemu: Enable MTD NOR flash support")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  board/emulation/qemu-riscv/qemu-riscv.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c
> index b0d9dd59b1..cd02dae1ab 100644
> --- a/board/emulation/qemu-riscv/qemu-riscv.c
> +++ b/board/emulation/qemu-riscv/qemu-riscv.c
> @@ -8,6 +8,7 @@
>  #include <env.h>
>  #include <fdtdec.h>
>  #include <image.h>
> +#include <linux/libfdt.h>
>  #include <log.h>
>  #include <spl.h>
>  #include <init.h>
> @@ -16,6 +17,22 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +#if IS_ENABLED(CONFIG_MTD_NOR_FLASH)
> +int is_flash_available(void)
> +{
> +       if (IS_ENABLED(CONFIG_OF_SEPARATE) || IS_ENABLED(CONFIG_OF_BOARD)) {

Why is this if statement needed?

All QEMU riscv* defconfigs are using CONFIG_OF_BOARD, and there is no
OF_SEPARATE use case since QEMU DTBs are always consumed by U-Boot.

> +               const void *fdt =
> +                       (const void *)(uintptr_t)gd->arch.firmware_fdt_addr;
> +               int rc = fdt_node_offset_by_compatible(fdt, -1, "cfi-flash");
> +
> +               if (rc >= 0)
> +                       return 1;
> +       }
> +
> +       return 0;
> +}
> +#endif
> +
>  int board_init(void)
>  {
>         /*

Regards,
Bin

Re: [PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR

[Anup Patel]

On Tue, Jan 25, 2022 at 10:33 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > Currently, if MTD NOR is enabled then U-Boot tries to issue flash
> > commands even when CFI flash DT node is not present. This causes
> > access fault on RISC-V emulators or ISS which do not emulate CFI
> > flash. To handle this issue, we implement is_flash_available() for
> > qemu-riscv board which will return 1 only if CFI flash DT node is
> > present.
> >
> > Fixes: d248627f9d42 ("riscv: qemu: Enable MTD NOR flash support")
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  board/emulation/qemu-riscv/qemu-riscv.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c
> > index b0d9dd59b1..cd02dae1ab 100644
> > --- a/board/emulation/qemu-riscv/qemu-riscv.c
> > +++ b/board/emulation/qemu-riscv/qemu-riscv.c
> > @@ -8,6 +8,7 @@
> >  #include <env.h>
> >  #include <fdtdec.h>
> >  #include <image.h>
> > +#include <linux/libfdt.h>
> >  #include <log.h>
> >  #include <spl.h>
> >  #include <init.h>
> > @@ -16,6 +17,22 @@
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > +#if IS_ENABLED(CONFIG_MTD_NOR_FLASH)
> > +int is_flash_available(void)
> > +{
> > +       if (IS_ENABLED(CONFIG_OF_SEPARATE) || IS_ENABLED(CONFIG_OF_BOARD)) {
>
> Why is this if statement needed?
>
> All QEMU riscv* defconfigs are using CONFIG_OF_BOARD, and there is no
> OF_SEPARATE use case since QEMU DTBs are always consumed by U-Boot.

I added the if statement for the case if someone disables CONFIG_OF_BOARD
but I can remove it if you insist.

>
> > +               const void *fdt =
> > +                       (const void *)(uintptr_t)gd->arch.firmware_fdt_addr;
> > +               int rc = fdt_node_offset_by_compatible(fdt, -1, "cfi-flash");
> > +
> > +               if (rc >= 0)
> > +                       return 1;
> > +       }
> > +
> > +       return 0;
> > +}
> > +#endif
> > +
> >  int board_init(void)
> >  {
> >         /*
>
> Regards,
> Bin

Regards,
Anup

Re: [PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR

[Bin Meng]

On Tue, Jan 25, 2022 at 2:02 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> On Tue, Jan 25, 2022 at 10:33 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote:
> > >
> > > Currently, if MTD NOR is enabled then U-Boot tries to issue flash
> > > commands even when CFI flash DT node is not present. This causes
> > > access fault on RISC-V emulators or ISS which do not emulate CFI
> > > flash. To handle this issue, we implement is_flash_available() for
> > > qemu-riscv board which will return 1 only if CFI flash DT node is
> > > present.
> > >
> > > Fixes: d248627f9d42 ("riscv: qemu: Enable MTD NOR flash support")
> > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > ---
> > >  board/emulation/qemu-riscv/qemu-riscv.c | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > >
> > > diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c
> > > index b0d9dd59b1..cd02dae1ab 100644
> > > --- a/board/emulation/qemu-riscv/qemu-riscv.c
> > > +++ b/board/emulation/qemu-riscv/qemu-riscv.c
> > > @@ -8,6 +8,7 @@
> > >  #include <env.h>
> > >  #include <fdtdec.h>
> > >  #include <image.h>
> > > +#include <linux/libfdt.h>
> > >  #include <log.h>
> > >  #include <spl.h>
> > >  #include <init.h>
> > > @@ -16,6 +17,22 @@
> > >
> > >  DECLARE_GLOBAL_DATA_PTR;
> > >
> > > +#if IS_ENABLED(CONFIG_MTD_NOR_FLASH)
> > > +int is_flash_available(void)
> > > +{
> > > +       if (IS_ENABLED(CONFIG_OF_SEPARATE) || IS_ENABLED(CONFIG_OF_BOARD)) {
> >
> > Why is this if statement needed?
> >
> > All QEMU riscv* defconfigs are using CONFIG_OF_BOARD, and there is no
> > OF_SEPARATE use case since QEMU DTBs are always consumed by U-Boot.
>
> I added the if statement for the case if someone disables CONFIG_OF_BOARD
> but I can remove it if you insist.
>

I see. But I if we give a dtb to QEMU without using CONFIG_OF_BOARD,
the dtb may still contain a node for flash so the logic you added is
still needed.

So I think you can just remove the if statement.

Also instead of using fdt_node_offset_by_compatible API please use
ofnode_device_is_compatible.

Regards,
Bin

Re: [PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR

[Anup Patel]

On Tue, Jan 25, 2022 at 3:16 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Tue, Jan 25, 2022 at 2:02 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > On Tue, Jan 25, 2022 at 10:33 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote:
> > > >
> > > > Currently, if MTD NOR is enabled then U-Boot tries to issue flash
> > > > commands even when CFI flash DT node is not present. This causes
> > > > access fault on RISC-V emulators or ISS which do not emulate CFI
> > > > flash. To handle this issue, we implement is_flash_available() for
> > > > qemu-riscv board which will return 1 only if CFI flash DT node is
> > > > present.
> > > >
> > > > Fixes: d248627f9d42 ("riscv: qemu: Enable MTD NOR flash support")
> > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > > ---
> > > >  board/emulation/qemu-riscv/qemu-riscv.c | 17 +++++++++++++++++
> > > >  1 file changed, 17 insertions(+)
> > > >
> > > > diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c
> > > > index b0d9dd59b1..cd02dae1ab 100644
> > > > --- a/board/emulation/qemu-riscv/qemu-riscv.c
> > > > +++ b/board/emulation/qemu-riscv/qemu-riscv.c
> > > > @@ -8,6 +8,7 @@
> > > >  #include <env.h>
> > > >  #include <fdtdec.h>
> > > >  #include <image.h>
> > > > +#include <linux/libfdt.h>
> > > >  #include <log.h>
> > > >  #include <spl.h>
> > > >  #include <init.h>
> > > > @@ -16,6 +17,22 @@
> > > >
> > > >  DECLARE_GLOBAL_DATA_PTR;
> > > >
> > > > +#if IS_ENABLED(CONFIG_MTD_NOR_FLASH)
> > > > +int is_flash_available(void)
> > > > +{
> > > > +       if (IS_ENABLED(CONFIG_OF_SEPARATE) || IS_ENABLED(CONFIG_OF_BOARD)) {
> > >
> > > Why is this if statement needed?
> > >
> > > All QEMU riscv* defconfigs are using CONFIG_OF_BOARD, and there is no
> > > OF_SEPARATE use case since QEMU DTBs are always consumed by U-Boot.
> >
> > I added the if statement for the case if someone disables CONFIG_OF_BOARD
> > but I can remove it if you insist.
> >
>
> I see. But I if we give a dtb to QEMU without using CONFIG_OF_BOARD,
> the dtb may still contain a node for flash so the logic you added is
> still needed.
>
> So I think you can just remove the if statement.

Okay, I will remove the "if ()" check.

>
> Also instead of using fdt_node_offset_by_compatible API please use
> ofnode_device_is_compatible.

Sure, I will try ofnode_device_is_compatible().

>
> Regards,
> Bin

Regards,
Anup

Re: [PATCH 0/3] QEMU spike machine support for U-Boot

[Bin Meng]

On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> We can use same U-Boot binary compiled using qemu-riscv64_smode_defconfig
> on QEMU virt machine and QEMU spike machine. To achieve this, we need HTIF
> console support for U-Boot QEMU RISC-V board hence this series.
>
> To test this series with latest OpenSBI, we can use the following command:
> qemu-system-riscv64 -M spike -m 256M -display none -serial stdio \
> -bios opensbi/build/platform/generic/firmware/fw_jump.bin -kernel \

I think you forgot to mention this needs an updated QEMU too, due to
plain binary is now used for spike?

> ./u-boot/u-boot.bin
>
> These patch can be found in qemu_riscv_htif_v1 branch at:
> https://github.com/avpatel/u-boot.git
>
> Anup Patel (3):
>   serial: Add RISC-V HTIF console driver
>   riscv: qemu: Enable HTIF console support
>   riscv: qemu: Implement is_flash_available() for MTD NOR
>
>  board/emulation/qemu-riscv/Kconfig      |   1 +
>  board/emulation/qemu-riscv/qemu-riscv.c |  17 +++
>  drivers/serial/Kconfig                  |   8 ++
>  drivers/serial/Makefile                 |   1 +
>  drivers/serial/serial_htif.c            | 178 ++++++++++++++++++++++++
>  5 files changed, 205 insertions(+)
>  create mode 100644 drivers/serial/serial_htif.c
>

Regards,
Bin

Re: [PATCH 0/3] QEMU spike machine support for U-Boot

[Anup Patel]

On Tue, Jan 18, 2022 at 3:41 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > We can use same U-Boot binary compiled using qemu-riscv64_smode_defconfig
> > on QEMU virt machine and QEMU spike machine. To achieve this, we need HTIF
> > console support for U-Boot QEMU RISC-V board hence this series.
> >
> > To test this series with latest OpenSBI, we can use the following command:
> > qemu-system-riscv64 -M spike -m 256M -display none -serial stdio \
> > -bios opensbi/build/platform/generic/firmware/fw_jump.bin -kernel \
>
> I think you forgot to mention this needs an updated QEMU too, due to
> plain binary is now used for spike?

Ahh, yes. I should have provided the link to the QEMU spike machine patch.

QEMU patch link:
https://lore.kernel.org/all/20220114160457.70134-1-apatel@ventanamicro.com/

Regards,
Anup


>
> > ./u-boot/u-boot.bin
> >
> > These patch can be found in qemu_riscv_htif_v1 branch at:
> > https://github.com/avpatel/u-boot.git
> >
> > Anup Patel (3):
> >   serial: Add RISC-V HTIF console driver
> >   riscv: qemu: Enable HTIF console support
> >   riscv: qemu: Implement is_flash_available() for MTD NOR
> >
> >  board/emulation/qemu-riscv/Kconfig      |   1 +
> >  board/emulation/qemu-riscv/qemu-riscv.c |  17 +++
> >  drivers/serial/Kconfig                  |   8 ++
> >  drivers/serial/Makefile                 |   1 +
> >  drivers/serial/serial_htif.c            | 178 ++++++++++++++++++++++++
> >  5 files changed, 205 insertions(+)
> >  create mode 100644 drivers/serial/serial_htif.c
> >
>
> Regards,
> Bin

Re: [PATCH 0/3] QEMU spike machine support for U-Boot

[Bin Meng]

On Tue, Jan 18, 2022 at 6:56 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Jan 18, 2022 at 3:41 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote:
> > >
> > > We can use same U-Boot binary compiled using qemu-riscv64_smode_defconfig
> > > on QEMU virt machine and QEMU spike machine. To achieve this, we need HTIF
> > > console support for U-Boot QEMU RISC-V board hence this series.
> > >
> > > To test this series with latest OpenSBI, we can use the following command:
> > > qemu-system-riscv64 -M spike -m 256M -display none -serial stdio \
> > > -bios opensbi/build/platform/generic/firmware/fw_jump.bin -kernel \
> >
> > I think you forgot to mention this needs an updated QEMU too, due to
> > plain binary is now used for spike?
>
> Ahh, yes. I should have provided the link to the QEMU spike machine patch.
>
> QEMU patch link:
> https://lore.kernel.org/all/20220114160457.70134-1-apatel@ventanamicro.com/
>

Please also include a patch to update the QEMU riscv board doc, with
the updated instructions on QEMU for Spike machine.

Regards,
Bin

Re: [PATCH 0/3] QEMU spike machine support for U-Boot

[Anup Patel]

On Tue, Jan 25, 2022 at 10:40 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Tue, Jan 18, 2022 at 6:56 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Tue, Jan 18, 2022 at 3:41 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote:
> > > >
> > > > We can use same U-Boot binary compiled using qemu-riscv64_smode_defconfig
> > > > on QEMU virt machine and QEMU spike machine. To achieve this, we need HTIF
> > > > console support for U-Boot QEMU RISC-V board hence this series.
> > > >
> > > > To test this series with latest OpenSBI, we can use the following command:
> > > > qemu-system-riscv64 -M spike -m 256M -display none -serial stdio \
> > > > -bios opensbi/build/platform/generic/firmware/fw_jump.bin -kernel \
> > >
> > > I think you forgot to mention this needs an updated QEMU too, due to
> > > plain binary is now used for spike?
> >
> > Ahh, yes. I should have provided the link to the QEMU spike machine patch.
> >
> > QEMU patch link:
> > https://lore.kernel.org/all/20220114160457.70134-1-apatel@ventanamicro.com/
> >
>
> Please also include a patch to update the QEMU riscv board doc, with
> the updated instructions on QEMU for Spike machine.

Okay, I will add a separate patch in the next revision.

Regards,
Anup

>
> Regards,
> Bin