[PATCH v2 0/2] hw/riscv: Add fw_cfg support, allow ramfb

[PATCH v2 0/2] hw/riscv: Add fw_cfg support, allow ramfb

[Asherah Connor]

Here's version two of the series to bring fw_cfg support to riscv's virt
machine.  We add support for the DMA interface, as this is needed for
writes.

The ultimate goal is to add ramfb support, in the second patch.  It
works well!

Changes in v2:
* Add DMA interface support.
* Add ramfb as allowed on riscv virt machine class.

Asherah Connor (2):
  hw/riscv: Add fw_cfg support to virt
  hw/riscv: allow ramfb on virt

 hw/riscv/Kconfig        |  1 +
 hw/riscv/virt.c         | 30 ++++++++++++++++++++++++++++++
 include/hw/riscv/virt.h |  4 +++-
 3 files changed, 34 insertions(+), 1 deletion(-)

-- 
2.24.3 (Apple Git-128)

[PATCH v2 1/2] hw/riscv: Add fw_cfg support to virt

[Asherah Connor]

Provides fw_cfg for the virt machine on riscv.  This enables
using e.g.  ramfb later.

Signed-off-by: Asherah Connor <ashe@kivikakk.ee>
---

Changes in v2:
* Add DMA support (needed for writes).

 hw/riscv/Kconfig        |  1 +
 hw/riscv/virt.c         | 27 +++++++++++++++++++++++++++
 include/hw/riscv/virt.h |  4 +++-
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index facb0cbacc..afaa5e58bb 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -33,6 +33,7 @@ config RISCV_VIRT
     select SIFIVE_PLIC
     select SIFIVE_TEST
     select VIRTIO_MMIO
+    select FW_CFG_DMA
 
 config SIFIVE_E
     bool
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 2299b3a6be..a10f218c43 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -56,6 +56,7 @@ static const struct MemmapEntry {
     [VIRT_PLIC] =        {  0xc000000, VIRT_PLIC_SIZE(VIRT_CPUS_MAX * 2) },
     [VIRT_UART0] =       { 0x10000000,         0x100 },
     [VIRT_VIRTIO] =      { 0x10001000,        0x1000 },
+    [VIRT_FW_CFG] =      { 0x10100000,          0x18 },
     [VIRT_FLASH] =       { 0x20000000,     0x4000000 },
     [VIRT_PCIE_ECAM] =   { 0x30000000,    0x10000000 },
     [VIRT_PCIE_MMIO] =   { 0x40000000,    0x40000000 },
@@ -488,6 +489,28 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
     return dev;
 }
 
+static FWCfgState *create_fw_cfg(const RISCVVirtState *s)
+{
+    hwaddr base = virt_memmap[VIRT_FW_CFG].base;
+    hwaddr size = virt_memmap[VIRT_FW_CFG].size;
+    FWCfgState *fw_cfg;
+    char *nodename;
+
+    fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16,
+                                  &address_space_memory);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)MACHINE(s)->smp.cpus);
+
+    nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
+    qemu_fdt_add_subnode(s->fdt, nodename);
+    qemu_fdt_setprop_string(s->fdt, nodename,
+                            "compatible", "qemu,fw-cfg-mmio");
+    qemu_fdt_setprop_sized_cells(s->fdt, nodename, "reg",
+                                 2, base, 2, size);
+    qemu_fdt_setprop(s->fdt, nodename, "dma-coherent", NULL, 0);
+    g_free(nodename);
+    return fw_cfg;
+}
+
 static void virt_machine_init(MachineState *machine)
 {
     const struct MemmapEntry *memmap = virt_memmap;
@@ -652,6 +675,10 @@ static void virt_machine_init(MachineState *machine)
         start_addr = virt_memmap[VIRT_FLASH].base;
     }
 
+    /* init fw_cfg */
+    s->fw_cfg = create_fw_cfg(s);
+    rom_set_fw(s->fw_cfg);
+
     /* Compute the fdt load address in dram */
     fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
                                    machine->ram_size, s->fdt);
diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 84b7a3848f..3b81a2e3f6 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -40,6 +40,7 @@ struct RISCVVirtState {
     RISCVHartArrayState soc[VIRT_SOCKETS_MAX];
     DeviceState *plic[VIRT_SOCKETS_MAX];
     PFlashCFI01 *flash[2];
+    FWCfgState *fw_cfg;
 
     void *fdt;
     int fdt_size;
@@ -58,7 +59,8 @@ enum {
     VIRT_DRAM,
     VIRT_PCIE_MMIO,
     VIRT_PCIE_PIO,
-    VIRT_PCIE_ECAM
+    VIRT_PCIE_ECAM,
+    VIRT_FW_CFG
 };
 
 enum {
-- 
2.24.3 (Apple Git-128)

[PATCH v2 2/2] hw/riscv: allow ramfb on virt

[Asherah Connor]

Allow ramfb on virt.  This lets `-device ramfb' work.

Signed-off-by: Asherah Connor <ashe@kivikakk.ee>
---

Changes in v2:
* Add DMA interface support.
* Add ramfb as allowed on riscv virt machine class.

 hw/riscv/virt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index a10f218c43..dbd40c41da 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -42,6 +42,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/pci/pci.h"
 #include "hw/pci-host/gpex.h"
+#include "hw/display/ramfb.h"
 
 static const struct MemmapEntry {
     hwaddr base;
@@ -740,6 +741,8 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props;
     mc->get_default_cpu_node_id = riscv_numa_get_default_cpu_node_id;
     mc->numa_mem_supported = true;
+
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
 }
 
 static const TypeInfo virt_machine_typeinfo = {
-- 
2.24.3 (Apple Git-128)

Re: [PATCH v2 1/2] hw/riscv: Add fw_cfg support to virt

[Bin Meng]

On Fri, Feb 26, 2021 at 12:31 PM Asherah Connor <ashe@kivikakk.ee> wrote:
>
> Provides fw_cfg for the virt machine on riscv.  This enables
> using e.g.  ramfb later.
>
> Signed-off-by: Asherah Connor <ashe@kivikakk.ee>
> ---
>
> Changes in v2:
> * Add DMA support (needed for writes).
>
>  hw/riscv/Kconfig        |  1 +
>  hw/riscv/virt.c         | 27 +++++++++++++++++++++++++++
>  include/hw/riscv/virt.h |  4 +++-
>  3 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index facb0cbacc..afaa5e58bb 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -33,6 +33,7 @@ config RISCV_VIRT
>      select SIFIVE_PLIC
>      select SIFIVE_TEST
>      select VIRTIO_MMIO
> +    select FW_CFG_DMA
>
>  config SIFIVE_E
>      bool
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 2299b3a6be..a10f218c43 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -56,6 +56,7 @@ static const struct MemmapEntry {
>      [VIRT_PLIC] =        {  0xc000000, VIRT_PLIC_SIZE(VIRT_CPUS_MAX * 2) },
>      [VIRT_UART0] =       { 0x10000000,         0x100 },
>      [VIRT_VIRTIO] =      { 0x10001000,        0x1000 },
> +    [VIRT_FW_CFG] =      { 0x10100000,          0x18 },
>      [VIRT_FLASH] =       { 0x20000000,     0x4000000 },
>      [VIRT_PCIE_ECAM] =   { 0x30000000,    0x10000000 },
>      [VIRT_PCIE_MMIO] =   { 0x40000000,    0x40000000 },
> @@ -488,6 +489,28 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
>      return dev;
>  }
>
> +static FWCfgState *create_fw_cfg(const RISCVVirtState *s)
> +{
> +    hwaddr base = virt_memmap[VIRT_FW_CFG].base;
> +    hwaddr size = virt_memmap[VIRT_FW_CFG].size;
> +    FWCfgState *fw_cfg;
> +    char *nodename;
> +
> +    fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16,
> +                                  &address_space_memory);
> +    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)MACHINE(s)->smp.cpus);
> +
> +    nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
> +    qemu_fdt_add_subnode(s->fdt, nodename);
> +    qemu_fdt_setprop_string(s->fdt, nodename,
> +                            "compatible", "qemu,fw-cfg-mmio");
> +    qemu_fdt_setprop_sized_cells(s->fdt, nodename, "reg",
> +                                 2, base, 2, size);
> +    qemu_fdt_setprop(s->fdt, nodename, "dma-coherent", NULL, 0);
> +    g_free(nodename);
> +    return fw_cfg;
> +}
> +
>  static void virt_machine_init(MachineState *machine)
>  {
>      const struct MemmapEntry *memmap = virt_memmap;
> @@ -652,6 +675,10 @@ static void virt_machine_init(MachineState *machine)
>          start_addr = virt_memmap[VIRT_FLASH].base;
>      }
>
> +    /* init fw_cfg */

I guess this is put here because riscv_load_fdt() is trying to touch
the device tree, and creating fw_cfg has to be done before that?
Maybe a comment is needed to prevent whoever later wanted to move the
codes around?

> +    s->fw_cfg = create_fw_cfg(s);
> +    rom_set_fw(s->fw_cfg);
> +
>      /* Compute the fdt load address in dram */
>      fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
>                                     machine->ram_size, s->fdt);
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 84b7a3848f..3b81a2e3f6 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -40,6 +40,7 @@ struct RISCVVirtState {
>      RISCVHartArrayState soc[VIRT_SOCKETS_MAX];
>      DeviceState *plic[VIRT_SOCKETS_MAX];
>      PFlashCFI01 *flash[2];
> +    FWCfgState *fw_cfg;
>
>      void *fdt;
>      int fdt_size;
> @@ -58,7 +59,8 @@ enum {
>      VIRT_DRAM,
>      VIRT_PCIE_MMIO,
>      VIRT_PCIE_PIO,
> -    VIRT_PCIE_ECAM
> +    VIRT_PCIE_ECAM,
> +    VIRT_FW_CFG

nits: insert this before VIRT_FLASH

>  };
>
>  enum {
> --

Regards,
Bin

Re: [PATCH v2 2/2] hw/riscv: allow ramfb on virt

[Bin Meng]

On Fri, Feb 26, 2021 at 12:34 PM Asherah Connor <ashe@kivikakk.ee> wrote:
>
> Allow ramfb on virt.  This lets `-device ramfb' work.
>
> Signed-off-by: Asherah Connor <ashe@kivikakk.ee>
> ---
>
> Changes in v2:
> * Add DMA interface support.
> * Add ramfb as allowed on riscv virt machine class.
>
>  hw/riscv/virt.c | 3 +++
>  1 file changed, 3 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH v2 1/2] hw/riscv: Add fw_cfg support to virt

[Asherah Connor]

Hi Bin,

Thanks very much for your review.

On 21/02/28 02:02:p, Bin Meng wrote:
> I guess this is put here because riscv_load_fdt() is trying to touch
> the device tree, and creating fw_cfg has to be done before that?
> Maybe a comment is needed to prevent whoever later wanted to move the
> codes around?

Done!

> > -    VIRT_PCIE_ECAM
> > +    VIRT_PCIE_ECAM,
> > +    VIRT_FW_CFG
> 
> nits: insert this before VIRT_FLASH

Done.  I've included your Reviewed-by: on the unmodified commit in the
series; I hope this is the correct thing to do.

Best,

Asherah

Re: [PATCH v2 1/2] hw/riscv: Add fw_cfg support to virt

[Bin Meng]

Hi Asherah,

On Sun, Feb 28, 2021 at 7:18 PM Asherah Connor <ashe@kivikakk.ee> wrote:
>
> Hi Bin,
>
> Thanks very much for your review.
>
> On 21/02/28 02:02:p, Bin Meng wrote:
> > I guess this is put here because riscv_load_fdt() is trying to touch
> > the device tree, and creating fw_cfg has to be done before that?
> > Maybe a comment is needed to prevent whoever later wanted to move the
> > codes around?
>
> Done!
>
> > > -    VIRT_PCIE_ECAM
> > > +    VIRT_PCIE_ECAM,
> > > +    VIRT_FW_CFG
> >
> > nits: insert this before VIRT_FLASH
>
> Done.  I've included your Reviewed-by: on the unmodified commit in the
> series; I hope this is the correct thing to do.
>

Yep, that's correct. Thanks!

Regards,
Bin