[PATCH 0/2] riscv: Add SiFive test device to sifive targets

[PATCH 0/2] riscv: Add SiFive test device to sifive targets

[Keith Packard via]

The SiFive test device, which is already available in the qemu tree,
allows the system under emulation to shut down qemu. This is currently
used by OpenSBI to terminate QEMU at powerdown time. These two patches
add this device to the two sifive models.

[PATCH 1/2] riscv: Add sifive test device to sifive_e target

[Keith Packard via]

The SiFive test device provides a mechanism for terminating the qemu
instance from the emulated system. This patch adds that device to the
sifive_e target.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 hw/riscv/sifive_e.c         | 4 ++++
 include/hw/riscv/sifive_e.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index fcfac16816..417e3a5409 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -11,6 +11,7 @@
  * 3) PRCI (Power, Reset, Clock, Interrupt)
  * 4) Registers emulated as RAM: AON, GPIO, QSPI, PWM
  * 5) Flash memory emulated as RAM
+ * 6) TEST (Test device)
  *
  * The Mask ROM reset vector jumps to the flash payload at 0x2040_0000.
  * The OTP ROM and Flash boot code will be emulated in a future version.
@@ -45,6 +46,7 @@
 #include "hw/intc/sifive_clint.h"
 #include "hw/intc/sifive_plic.h"
 #include "hw/misc/sifive_e_prci.h"
+#include "hw/misc/sifive_test.h"
 #include "chardev/char.h"
 #include "sysemu/arch_init.h"
 #include "sysemu/sysemu.h"
@@ -57,6 +59,7 @@ static const struct MemmapEntry {
     [SIFIVE_E_DEV_DEBUG] =    {        0x0,     0x1000 },
     [SIFIVE_E_DEV_MROM] =     {     0x1000,     0x2000 },
     [SIFIVE_E_DEV_OTP] =      {    0x20000,     0x2000 },
+    [SIFIVE_E_DEV_TEST] =     {   0x100000,     0x1000 },
     [SIFIVE_E_DEV_CLINT] =    {  0x2000000,    0x10000 },
     [SIFIVE_E_DEV_PLIC] =     {  0xc000000,  0x4000000 },
     [SIFIVE_E_DEV_AON] =      { 0x10000000,     0x8000 },
@@ -216,6 +219,7 @@ static void sifive_e_soc_realize(DeviceState *dev, Error **errp)
         memmap[SIFIVE_E_DEV_CLINT].size, 0, ms->smp.cpus,
         SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE,
         SIFIVE_CLINT_TIMEBASE_FREQ, false);
+    sifive_test_create(memmap[SIFIVE_E_DEV_TEST].base);
     create_unimplemented_device("riscv.sifive.e.aon",
         memmap[SIFIVE_E_DEV_AON].base, memmap[SIFIVE_E_DEV_AON].size);
     sifive_e_prci_create(memmap[SIFIVE_E_DEV_PRCI].base);
diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
index 83604da805..92bab6d0d4 100644
--- a/include/hw/riscv/sifive_e.h
+++ b/include/hw/riscv/sifive_e.h
@@ -56,6 +56,7 @@ enum {
     SIFIVE_E_DEV_DEBUG,
     SIFIVE_E_DEV_MROM,
     SIFIVE_E_DEV_OTP,
+    SIFIVE_E_DEV_TEST,
     SIFIVE_E_DEV_CLINT,
     SIFIVE_E_DEV_PLIC,
     SIFIVE_E_DEV_AON,
-- 
2.28.0

[PATCH 2/2] riscv: Add sifive test device to sifive_u target

[Keith Packard via]

The SiFive test device provides a mechanism for terminating the qemu
instance from the emulated system. This patch adds that device to the
sifive_u target, including constructing a suitable FDT node.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 hw/riscv/sifive_u.c         | 15 +++++++++++++++
 include/hw/riscv/sifive_u.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 6ad975d692..8d803fe7c0 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -15,6 +15,7 @@
  * 5) OTP (One-Time Programmable) memory with stored serial number
  * 6) GEM (Gigabit Ethernet Controller) and management block
  * 7) DMA (Direct Memory Access Controller)
+ * 8) TEST (Test device)
  *
  * This board currently generates devicetree dynamically that indicates at least
  * two harts and up to five harts.
@@ -44,6 +45,7 @@
 #include "hw/char/serial.h"
 #include "hw/cpu/cluster.h"
 #include "hw/misc/unimp.h"
+#include "hw/misc/sifive_test.h"
 #include "target/riscv/cpu.h"
 #include "hw/riscv/riscv_hart.h"
 #include "hw/riscv/sifive_u.h"
@@ -72,6 +74,7 @@ static const struct MemmapEntry {
 } sifive_u_memmap[] = {
     [SIFIVE_U_DEV_DEBUG] =    {        0x0,      0x100 },
     [SIFIVE_U_DEV_MROM] =     {     0x1000,     0xf000 },
+    [SIFIVE_U_DEV_TEST] =     {   0x100000,     0x1000 },
     [SIFIVE_U_DEV_CLINT] =    {  0x2000000,    0x10000 },
     [SIFIVE_U_DEV_L2CC] =     {  0x2010000,     0x1000 },
     [SIFIVE_U_DEV_PDMA] =     {  0x3000000,   0x100000 },
@@ -397,6 +400,16 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
     qemu_fdt_setprop_string(fdt, "/aliases", "serial0", nodename);
 
     g_free(nodename);
+
+    nodename = g_strdup_printf("/soc/test@%lx",
+                               (long)memmap[SIFIVE_U_DEV_TEST].base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "sifive,test0");
+    qemu_fdt_setprop_cells(fdt, nodename, "reg",
+                           0x0, memmap[SIFIVE_U_DEV_TEST].base,
+                           0x0, memmap[SIFIVE_U_DEV_TEST].size);
+
+    g_free(nodename);
 }
 
 static void sifive_u_machine_reset(void *opaque, int n, int level)
@@ -780,6 +793,8 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem), 0,
                        qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_U_GEM_IRQ));
 
+    sifive_test_create(memmap[SIFIVE_U_DEV_TEST].base);
+
     create_unimplemented_device("riscv.sifive.u.gem-mgmt",
         memmap[SIFIVE_U_DEV_GEM_MGMT].base, memmap[SIFIVE_U_DEV_GEM_MGMT].size);
 
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 22e7e6efa1..4b3ebc3fc6 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -72,6 +72,7 @@ typedef struct SiFiveUState {
 enum {
     SIFIVE_U_DEV_DEBUG,
     SIFIVE_U_DEV_MROM,
+    SIFIVE_U_DEV_TEST,
     SIFIVE_U_DEV_CLINT,
     SIFIVE_U_DEV_L2CC,
     SIFIVE_U_DEV_PDMA,
-- 
2.28.0

Re: [PATCH 1/2] riscv: Add sifive test device to sifive_e target

[Alistair Francis]

On Fri, Oct 23, 2020 at 2:45 PM Keith Packard via <qemu-devel@nongnu.org> wrote:
>
> The SiFive test device provides a mechanism for terminating the qemu
> instance from the emulated system. This patch adds that device to the
> sifive_e target.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  hw/riscv/sifive_e.c         | 4 ++++
>  include/hw/riscv/sifive_e.h | 1 +
>  2 files changed, 5 insertions(+)
>
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index fcfac16816..417e3a5409 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -11,6 +11,7 @@
>   * 3) PRCI (Power, Reset, Clock, Interrupt)
>   * 4) Registers emulated as RAM: AON, GPIO, QSPI, PWM
>   * 5) Flash memory emulated as RAM
> + * 6) TEST (Test device)
>   *
>   * The Mask ROM reset vector jumps to the flash payload at 0x2040_0000.
>   * The OTP ROM and Flash boot code will be emulated in a future version.
> @@ -45,6 +46,7 @@
>  #include "hw/intc/sifive_clint.h"
>  #include "hw/intc/sifive_plic.h"
>  #include "hw/misc/sifive_e_prci.h"
> +#include "hw/misc/sifive_test.h"
>  #include "chardev/char.h"
>  #include "sysemu/arch_init.h"
>  #include "sysemu/sysemu.h"
> @@ -57,6 +59,7 @@ static const struct MemmapEntry {
>      [SIFIVE_E_DEV_DEBUG] =    {        0x0,     0x1000 },
>      [SIFIVE_E_DEV_MROM] =     {     0x1000,     0x2000 },
>      [SIFIVE_E_DEV_OTP] =      {    0x20000,     0x2000 },
> +    [SIFIVE_E_DEV_TEST] =     {   0x100000,     0x1000 },

I don't see this mentioned in the FE310 data sheet. Is it included in
the hardware?

Alistair

>      [SIFIVE_E_DEV_CLINT] =    {  0x2000000,    0x10000 },
>      [SIFIVE_E_DEV_PLIC] =     {  0xc000000,  0x4000000 },
>      [SIFIVE_E_DEV_AON] =      { 0x10000000,     0x8000 },
> @@ -216,6 +219,7 @@ static void sifive_e_soc_realize(DeviceState *dev, Error **errp)
>          memmap[SIFIVE_E_DEV_CLINT].size, 0, ms->smp.cpus,
>          SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE,
>          SIFIVE_CLINT_TIMEBASE_FREQ, false);
> +    sifive_test_create(memmap[SIFIVE_E_DEV_TEST].base);
>      create_unimplemented_device("riscv.sifive.e.aon",
>          memmap[SIFIVE_E_DEV_AON].base, memmap[SIFIVE_E_DEV_AON].size);
>      sifive_e_prci_create(memmap[SIFIVE_E_DEV_PRCI].base);
> diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
> index 83604da805..92bab6d0d4 100644
> --- a/include/hw/riscv/sifive_e.h
> +++ b/include/hw/riscv/sifive_e.h
> @@ -56,6 +56,7 @@ enum {
>      SIFIVE_E_DEV_DEBUG,
>      SIFIVE_E_DEV_MROM,
>      SIFIVE_E_DEV_OTP,
> +    SIFIVE_E_DEV_TEST,
>      SIFIVE_E_DEV_CLINT,
>      SIFIVE_E_DEV_PLIC,
>      SIFIVE_E_DEV_AON,
> --
> 2.28.0
>
>

Re: [PATCH 2/2] riscv: Add sifive test device to sifive_u target

[Alistair Francis]

On Fri, Oct 23, 2020 at 2:48 PM Keith Packard via <qemu-devel@nongnu.org> wrote:
>
> The SiFive test device provides a mechanism for terminating the qemu
> instance from the emulated system. This patch adds that device to the
> sifive_u target, including constructing a suitable FDT node.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  hw/riscv/sifive_u.c         | 15 +++++++++++++++
>  include/hw/riscv/sifive_u.h |  1 +
>  2 files changed, 16 insertions(+)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 6ad975d692..8d803fe7c0 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -15,6 +15,7 @@
>   * 5) OTP (One-Time Programmable) memory with stored serial number
>   * 6) GEM (Gigabit Ethernet Controller) and management block
>   * 7) DMA (Direct Memory Access Controller)
> + * 8) TEST (Test device)
>   *
>   * This board currently generates devicetree dynamically that indicates at least
>   * two harts and up to five harts.
> @@ -44,6 +45,7 @@
>  #include "hw/char/serial.h"
>  #include "hw/cpu/cluster.h"
>  #include "hw/misc/unimp.h"
> +#include "hw/misc/sifive_test.h"
>  #include "target/riscv/cpu.h"
>  #include "hw/riscv/riscv_hart.h"
>  #include "hw/riscv/sifive_u.h"
> @@ -72,6 +74,7 @@ static const struct MemmapEntry {
>  } sifive_u_memmap[] = {
>      [SIFIVE_U_DEV_DEBUG] =    {        0x0,      0x100 },
>      [SIFIVE_U_DEV_MROM] =     {     0x1000,     0xf000 },
> +    [SIFIVE_U_DEV_TEST] =     {   0x100000,     0x1000 },

I also don't see this in the FU540 memory map.

Alistair

>      [SIFIVE_U_DEV_CLINT] =    {  0x2000000,    0x10000 },
>      [SIFIVE_U_DEV_L2CC] =     {  0x2010000,     0x1000 },
>      [SIFIVE_U_DEV_PDMA] =     {  0x3000000,   0x100000 },
> @@ -397,6 +400,16 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
>      qemu_fdt_setprop_string(fdt, "/aliases", "serial0", nodename);
>
>      g_free(nodename);
> +
> +    nodename = g_strdup_printf("/soc/test@%lx",
> +                               (long)memmap[SIFIVE_U_DEV_TEST].base);
> +    qemu_fdt_add_subnode(fdt, nodename);
> +    qemu_fdt_setprop_string(fdt, nodename, "compatible", "sifive,test0");
> +    qemu_fdt_setprop_cells(fdt, nodename, "reg",
> +                           0x0, memmap[SIFIVE_U_DEV_TEST].base,
> +                           0x0, memmap[SIFIVE_U_DEV_TEST].size);
> +
> +    g_free(nodename);
>  }
>
>  static void sifive_u_machine_reset(void *opaque, int n, int level)
> @@ -780,6 +793,8 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem), 0,
>                         qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_U_GEM_IRQ));
>
> +    sifive_test_create(memmap[SIFIVE_U_DEV_TEST].base);
> +
>      create_unimplemented_device("riscv.sifive.u.gem-mgmt",
>          memmap[SIFIVE_U_DEV_GEM_MGMT].base, memmap[SIFIVE_U_DEV_GEM_MGMT].size);
>
> diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
> index 22e7e6efa1..4b3ebc3fc6 100644
> --- a/include/hw/riscv/sifive_u.h
> +++ b/include/hw/riscv/sifive_u.h
> @@ -72,6 +72,7 @@ typedef struct SiFiveUState {
>  enum {
>      SIFIVE_U_DEV_DEBUG,
>      SIFIVE_U_DEV_MROM,
> +    SIFIVE_U_DEV_TEST,
>      SIFIVE_U_DEV_CLINT,
>      SIFIVE_U_DEV_L2CC,
>      SIFIVE_U_DEV_PDMA,
> --
> 2.28.0
>
>

Re: [PATCH 1/2] riscv: Add sifive test device to sifive_e target

[Alistair Francis]

On Fri, Oct 23, 2020 at 5:13 PM Keith Packard <keithp@keithp.com> wrote:
>
> Alistair Francis <alistair23@gmail.com> writes:
>
> > I don't see this mentioned in the FE310 data sheet. Is it included in
> > the hardware?
>
> This is strictly a virtual device used to support 'power off' when
> running under QEMU. We need either this or semihosting support for doing

The sifive_u and sifive_e model real hardware (the names are confusing
I agree) so I would rather not add a virtual device.

> automated testing. Our tests currently use this device, but I'd be happy
> to switch them over to using semihosting instead, if you'd be interested
> in getting that merged...

Looking at it now.

Alistair

>
> --
> -keith

Re: [PATCH 1/2] riscv: Add sifive test device to sifive_e target

[Keith Packard via]

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

Alistair Francis <alistair23@gmail.com> writes:

> I don't see this mentioned in the FE310 data sheet. Is it included in
> the hardware?

This is strictly a virtual device used to support 'power off' when
running under QEMU. We need either this or semihosting support for doing
automated testing. Our tests currently use this device, but I'd be happy
to switch them over to using semihosting instead, if you'd be interested
in getting that merged...

-- 
-keith

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

Re: [PATCH 2/2] riscv: Add sifive test device to sifive_u target

[Keith Packard via]

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

Alistair Francis <alistair23@gmail.com> writes:

> I also don't see this in the FU540 memory map.

Same as for the FE310 -- this is a QEMU-only device. In addition,
OpenSBI expects to use this when built for QEMU.

-- 
-keith

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

Re: [PATCH 1/2] riscv: Add sifive test device to sifive_e target

[Keith Packard via]

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

Alistair Francis <alistair23@gmail.com> writes:

> The sifive_u and sifive_e model real hardware (the names are confusing
> I agree) so I would rather not add a virtual device.

That seems reasonable; semihosting will be a much better solution for
the long term anyways. I posted this because we've been maintaining it
out-of-tree for several years now and I wondered if it could be merged
to reduce our ongoing qemu support costs.

-- 
-keith

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

Re: [PATCH 2/2] riscv: Add sifive test device to sifive_u target

[Bin Meng]

Hi Keith,

On Sat, Oct 24, 2020 at 8:17 AM Keith Packard via <qemu-devel@nongnu.org> wrote:
>
> Alistair Francis <alistair23@gmail.com> writes:
>
> > I also don't see this in the FU540 memory map.
>
> Same as for the FE310 -- this is a QEMU-only device. In addition,
> OpenSBI expects to use this when built for QEMU.

I agree with Alistair. This should not be put in the sifive_u machine.
For sifive_u, the hardware provides "gpio-restart" to reset the board.

Regards,
Bin