[PATCH v2 7/9] hw/riscv/virt.c: use s->memmap in create_fdt_virtio()

Daniel Henrique Barboza posted 9 patches 6 months, 2 weeks ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
[PATCH v2 7/9] hw/riscv/virt.c: use s->memmap in create_fdt_virtio()
Posted by Daniel Henrique Barboza 6 months, 2 weeks ago
create_fdt_virtio() can use s->memmap instead of having an extra
argument for it.

While we're at it rewrite it a little bit to avoid the clunky line in
'name' and code repetition:

- declare 'virtio_base' out of the loop since it never changes;
- declare a 'size' variable. Use it to calculate the address of the
  virtio device in an 'addr' variable;
- use 'addr' in the 'name' g_strdup_printf();
- use 'addr' and 'size' when creating the 'reg' property.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 hw/riscv/virt.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 2383a557bd..46ac42058e 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -841,21 +841,24 @@ static void create_fdt_sockets(RISCVVirtState *s,
     riscv_socket_fdt_write_distance_matrix(ms);
 }
 
-static void create_fdt_virtio(RISCVVirtState *s, const MemMapEntry *memmap,
-                              uint32_t irq_virtio_phandle)
+static void create_fdt_virtio(RISCVVirtState *s, uint32_t irq_virtio_phandle)
 {
     int i;
     MachineState *ms = MACHINE(s);
+    hwaddr virtio_base = s->memmap[VIRT_VIRTIO].base;
 
     for (i = 0; i < VIRTIO_COUNT; i++) {
-        g_autofree char *name =  g_strdup_printf("/soc/virtio_mmio@%lx",
-            (long)(memmap[VIRT_VIRTIO].base + i * memmap[VIRT_VIRTIO].size));
+        g_autofree char *name = NULL;
+        uint64_t size = s->memmap[VIRT_VIRTIO].size;
+        hwaddr addr = virtio_base + i * size;
+
+        name = g_strdup_printf("/soc/virtio_mmio@%"HWADDR_PRIx, addr);
 
         qemu_fdt_add_subnode(ms->fdt, name);
         qemu_fdt_setprop_string(ms->fdt, name, "compatible", "virtio,mmio");
         qemu_fdt_setprop_cells(ms->fdt, name, "reg",
-            0x0, memmap[VIRT_VIRTIO].base + i * memmap[VIRT_VIRTIO].size,
-            0x0, memmap[VIRT_VIRTIO].size);
+                               0x0, addr,
+                               0x0, size);
         qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent",
             irq_virtio_phandle);
         if (s->aia_type == VIRT_AIA_TYPE_NONE) {
@@ -1134,7 +1137,7 @@ static void finalize_fdt(RISCVVirtState *s)
                        &irq_pcie_phandle, &irq_virtio_phandle,
                        &msi_pcie_phandle);
 
-    create_fdt_virtio(s, s->memmap, irq_virtio_phandle);
+    create_fdt_virtio(s, irq_virtio_phandle);
 
     if (virt_is_iommu_sys_enabled(s)) {
         create_fdt_iommu_sys(s, irq_mmio_phandle, msi_pcie_phandle,
-- 
2.49.0
Re: [PATCH v2 7/9] hw/riscv/virt.c: use s->memmap in create_fdt_virtio()
Posted by Alistair Francis 6 months, 2 weeks ago
On Tue, Apr 29, 2025 at 11:02 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> create_fdt_virtio() can use s->memmap instead of having an extra
> argument for it.
>
> While we're at it rewrite it a little bit to avoid the clunky line in
> 'name' and code repetition:
>
> - declare 'virtio_base' out of the loop since it never changes;
> - declare a 'size' variable. Use it to calculate the address of the
>   virtio device in an 'addr' variable;
> - use 'addr' in the 'name' g_strdup_printf();
> - use 'addr' and 'size' when creating the 'reg' property.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/virt.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 2383a557bd..46ac42058e 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -841,21 +841,24 @@ static void create_fdt_sockets(RISCVVirtState *s,
>      riscv_socket_fdt_write_distance_matrix(ms);
>  }
>
> -static void create_fdt_virtio(RISCVVirtState *s, const MemMapEntry *memmap,
> -                              uint32_t irq_virtio_phandle)
> +static void create_fdt_virtio(RISCVVirtState *s, uint32_t irq_virtio_phandle)
>  {
>      int i;
>      MachineState *ms = MACHINE(s);
> +    hwaddr virtio_base = s->memmap[VIRT_VIRTIO].base;
>
>      for (i = 0; i < VIRTIO_COUNT; i++) {
> -        g_autofree char *name =  g_strdup_printf("/soc/virtio_mmio@%lx",
> -            (long)(memmap[VIRT_VIRTIO].base + i * memmap[VIRT_VIRTIO].size));
> +        g_autofree char *name = NULL;
> +        uint64_t size = s->memmap[VIRT_VIRTIO].size;
> +        hwaddr addr = virtio_base + i * size;
> +
> +        name = g_strdup_printf("/soc/virtio_mmio@%"HWADDR_PRIx, addr);
>
>          qemu_fdt_add_subnode(ms->fdt, name);
>          qemu_fdt_setprop_string(ms->fdt, name, "compatible", "virtio,mmio");
>          qemu_fdt_setprop_cells(ms->fdt, name, "reg",
> -            0x0, memmap[VIRT_VIRTIO].base + i * memmap[VIRT_VIRTIO].size,
> -            0x0, memmap[VIRT_VIRTIO].size);
> +                               0x0, addr,
> +                               0x0, size);
>          qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent",
>              irq_virtio_phandle);
>          if (s->aia_type == VIRT_AIA_TYPE_NONE) {
> @@ -1134,7 +1137,7 @@ static void finalize_fdt(RISCVVirtState *s)
>                         &irq_pcie_phandle, &irq_virtio_phandle,
>                         &msi_pcie_phandle);
>
> -    create_fdt_virtio(s, s->memmap, irq_virtio_phandle);
> +    create_fdt_virtio(s, irq_virtio_phandle);
>
>      if (virt_is_iommu_sys_enabled(s)) {
>          create_fdt_iommu_sys(s, irq_mmio_phandle, msi_pcie_phandle,
> --
> 2.49.0
>
>