qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup
@ 2025-04-29 12:58 Daniel Henrique Barboza
  2025-04-29 12:58 ` [PATCH v2 1/9] hw/riscv/virt.c: enforce s->memmap use in machine_init() Daniel Henrique Barboza
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-29 12:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer, joel,
	Daniel Henrique Barboza

Hi,

In this new version a small change was made in patch 9 after
discussions with Joel during v1 [1]. The idea is that we want to be
consistent (as close as we can) with the idea that a
memory address is a hwaddr type.

This new version does not conflict with "hw/riscv/virt: device tree reg
cleanups" from Joel [2].

No other changes made. Patches based on alistair/riscv-to-apply.next.

Changes from v2:
- patch 9:
  - in create_fdt_socket_memory(), change 'addr' to hwaddr and use the
    HWADDR_PRIx fmt type
- v1 link: https://lore.kernel.org/qemu-riscv/20250423110630.2249904-1-dbarboza@ventanamicro.com/

[1] https://lore.kernel.org/qemu-riscv/d404d535-fc04-43ac-a7a7-2f216cad993c@ventanamicro.com/
[2] https://lore.kernel.org/qemu-riscv/20250429061223.1457166-1-joel@jms.id.au/

Daniel Henrique Barboza (9):
  hw/riscv/virt.c: enforce s->memmap use in machine_init()
  hw/riscv/virt.c: remove trivial virt_memmap references
  hw/riscv/virt.c: use s->memmap in virt_machine_done()
  hw/riscv/virt.c: add 'base' arg in create_fw_cfg()
  hw/riscv/virt.c: use s->memmap in create_fdt() path
  hw/riscv/virt.c: use s->memmap in create_fdt_sockets() path
  hw/riscv/virt.c: use s->memmap in create_fdt_virtio()
  hw/riscv/virt.c: use s->memmap in finalize_fdt() functions
  hw/riscv/virt.c: remove 'long' casts in fmt strings

 hw/riscv/virt.c | 272 +++++++++++++++++++++++++-----------------------
 1 file changed, 140 insertions(+), 132 deletions(-)

-- 
2.49.0



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/9] hw/riscv/virt.c: enforce s->memmap use in machine_init()
  2025-04-29 12:58 [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Daniel Henrique Barboza
@ 2025-04-29 12:58 ` Daniel Henrique Barboza
  2025-04-29 12:58 ` [PATCH v2 2/9] hw/riscv/virt.c: remove trivial virt_memmap references Daniel Henrique Barboza
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-29 12:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer, joel,
	Daniel Henrique Barboza

Throughout the code we're accessing the board memmap, most of the time,
by accessing it statically via 'virt_memmap'. This static map is also
assigned in the machine state in s->memmap.

We're also passing it as a variable to some fdt functions, which is
unorthodox since we can spare a function argument by accessing it
statically or via the machine state.

All the current forms are valid but not all of the are scalable. In the
future we will version this board, and then all this code will need
rework because it should point to the updated memmap. In this case,
we'll want to assign the adequate versioned memmap once during init,
in s->memmap like it is being done today, and the rest of the code
will access the updated map via s->memmap.

We're also enforcing the pattern of using s->memmap instead of assigning
it to a temp variable 'memmap'. Code is copy/pasted around all the time
and being consistent is important.

We'll start these rather mechanical changes with virt_machine_init().

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 hw/riscv/virt.c | 54 ++++++++++++++++++++++++-------------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 0ac9d63888..bcf0ddd6c6 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1526,7 +1526,6 @@ static void virt_machine_done(Notifier *notifier, void *data)
 
 static void virt_machine_init(MachineState *machine)
 {
-    const MemMapEntry *memmap = virt_memmap;
     RISCVVirtState *s = RISCV_VIRT_MACHINE(machine);
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
@@ -1534,6 +1533,8 @@ static void virt_machine_init(MachineState *machine)
     int i, base_hartid, hart_count;
     int socket_count = riscv_socket_count(machine);
 
+    s->memmap = virt_memmap;
+
     /* Check socket count limit */
     if (VIRT_SOCKETS_MAX < socket_count) {
         error_report("number of sockets/nodes should be less than %d",
@@ -1581,7 +1582,7 @@ static void virt_machine_init(MachineState *machine)
         if (virt_aclint_allowed() && s->have_aclint) {
             if (s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
                 /* Per-socket ACLINT MTIMER */
-                riscv_aclint_mtimer_create(memmap[VIRT_CLINT].base +
+                riscv_aclint_mtimer_create(s->memmap[VIRT_CLINT].base +
                             i * RISCV_ACLINT_DEFAULT_MTIMER_SIZE,
                         RISCV_ACLINT_DEFAULT_MTIMER_SIZE,
                         base_hartid, hart_count,
@@ -1590,28 +1591,28 @@ static void virt_machine_init(MachineState *machine)
                         RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ, true);
             } else {
                 /* Per-socket ACLINT MSWI, MTIMER, and SSWI */
-                riscv_aclint_swi_create(memmap[VIRT_CLINT].base +
-                            i * memmap[VIRT_CLINT].size,
+                riscv_aclint_swi_create(s->memmap[VIRT_CLINT].base +
+                            i * s->memmap[VIRT_CLINT].size,
                         base_hartid, hart_count, false);
-                riscv_aclint_mtimer_create(memmap[VIRT_CLINT].base +
-                            i * memmap[VIRT_CLINT].size +
+                riscv_aclint_mtimer_create(s->memmap[VIRT_CLINT].base +
+                            i * s->memmap[VIRT_CLINT].size +
                             RISCV_ACLINT_SWI_SIZE,
                         RISCV_ACLINT_DEFAULT_MTIMER_SIZE,
                         base_hartid, hart_count,
                         RISCV_ACLINT_DEFAULT_MTIMECMP,
                         RISCV_ACLINT_DEFAULT_MTIME,
                         RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ, true);
-                riscv_aclint_swi_create(memmap[VIRT_ACLINT_SSWI].base +
-                            i * memmap[VIRT_ACLINT_SSWI].size,
+                riscv_aclint_swi_create(s->memmap[VIRT_ACLINT_SSWI].base +
+                            i * s->memmap[VIRT_ACLINT_SSWI].size,
                         base_hartid, hart_count, true);
             }
         } else if (tcg_enabled()) {
             /* Per-socket SiFive CLINT */
             riscv_aclint_swi_create(
-                    memmap[VIRT_CLINT].base + i * memmap[VIRT_CLINT].size,
+                    s->memmap[VIRT_CLINT].base + i * s->memmap[VIRT_CLINT].size,
                     base_hartid, hart_count, false);
-            riscv_aclint_mtimer_create(memmap[VIRT_CLINT].base +
-                        i * memmap[VIRT_CLINT].size + RISCV_ACLINT_SWI_SIZE,
+            riscv_aclint_mtimer_create(s->memmap[VIRT_CLINT].base +
+                    i * s->memmap[VIRT_CLINT].size + RISCV_ACLINT_SWI_SIZE,
                     RISCV_ACLINT_DEFAULT_MTIMER_SIZE, base_hartid, hart_count,
                     RISCV_ACLINT_DEFAULT_MTIMECMP, RISCV_ACLINT_DEFAULT_MTIME,
                     RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ, true);
@@ -1619,11 +1620,11 @@ static void virt_machine_init(MachineState *machine)
 
         /* Per-socket interrupt controller */
         if (s->aia_type == VIRT_AIA_TYPE_NONE) {
-            s->irqchip[i] = virt_create_plic(memmap, i,
+            s->irqchip[i] = virt_create_plic(s->memmap, i,
                                              base_hartid, hart_count);
         } else {
             s->irqchip[i] = virt_create_aia(s->aia_type, s->aia_guests,
-                                            memmap, i, base_hartid,
+                                            s->memmap, i, base_hartid,
                                             hart_count);
         }
 
@@ -1645,8 +1646,8 @@ static void virt_machine_init(MachineState *machine)
     if (kvm_enabled() && virt_use_kvm_aia_aplic_imsic(s->aia_type)) {
         kvm_riscv_aia_create(machine, IMSIC_MMIO_GROUP_MIN_SHIFT,
                              VIRT_IRQCHIP_NUM_SOURCES, VIRT_IRQCHIP_NUM_MSIS,
-                             memmap[VIRT_APLIC_S].base,
-                             memmap[VIRT_IMSIC_S].base,
+                             s->memmap[VIRT_APLIC_S].base,
+                             s->memmap[VIRT_IMSIC_S].base,
                              s->aia_guests);
     }
 
@@ -1662,21 +1663,20 @@ static void virt_machine_init(MachineState *machine)
         virt_high_pcie_memmap.size = VIRT32_HIGH_PCIE_MMIO_SIZE;
     } else {
         virt_high_pcie_memmap.size = VIRT64_HIGH_PCIE_MMIO_SIZE;
-        virt_high_pcie_memmap.base = memmap[VIRT_DRAM].base + machine->ram_size;
+        virt_high_pcie_memmap.base = s->memmap[VIRT_DRAM].base +
+                                     machine->ram_size;
         virt_high_pcie_memmap.base =
             ROUND_UP(virt_high_pcie_memmap.base, virt_high_pcie_memmap.size);
     }
 
-    s->memmap = virt_memmap;
-
     /* register system main memory (actual RAM) */
-    memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
-        machine->ram);
+    memory_region_add_subregion(system_memory, s->memmap[VIRT_DRAM].base,
+                                machine->ram);
 
     /* boot rom */
     memory_region_init_rom(mask_rom, NULL, "riscv_virt_board.mrom",
-                           memmap[VIRT_MROM].size, &error_fatal);
-    memory_region_add_subregion(system_memory, memmap[VIRT_MROM].base,
+                           s->memmap[VIRT_MROM].size, &error_fatal);
+    memory_region_add_subregion(system_memory, s->memmap[VIRT_MROM].base,
                                 mask_rom);
 
     /*
@@ -1687,12 +1687,12 @@ static void virt_machine_init(MachineState *machine)
     rom_set_fw(s->fw_cfg);
 
     /* SiFive Test MMIO device */
-    sifive_test_create(memmap[VIRT_TEST].base);
+    sifive_test_create(s->memmap[VIRT_TEST].base);
 
     /* VirtIO MMIO devices */
     for (i = 0; i < VIRTIO_COUNT; i++) {
         sysbus_create_simple("virtio-mmio",
-            memmap[VIRT_VIRTIO].base + i * memmap[VIRT_VIRTIO].size,
+            s->memmap[VIRT_VIRTIO].base + i * s->memmap[VIRT_VIRTIO].size,
             qdev_get_gpio_in(virtio_irqchip, VIRTIO_IRQ + i));
     }
 
@@ -1700,11 +1700,11 @@ static void virt_machine_init(MachineState *machine)
 
     create_platform_bus(s, mmio_irqchip);
 
-    serial_mm_init(system_memory, memmap[VIRT_UART0].base,
+    serial_mm_init(system_memory, s->memmap[VIRT_UART0].base,
         0, qdev_get_gpio_in(mmio_irqchip, UART0_IRQ), 399193,
         serial_hd(0), DEVICE_LITTLE_ENDIAN);
 
-    sysbus_create_simple("goldfish_rtc", memmap[VIRT_RTC].base,
+    sysbus_create_simple("goldfish_rtc", s->memmap[VIRT_RTC].base,
         qdev_get_gpio_in(mmio_irqchip, RTC_IRQ));
 
     for (i = 0; i < ARRAY_SIZE(s->flash); i++) {
@@ -1722,7 +1722,7 @@ static void virt_machine_init(MachineState *machine)
             exit(1);
         }
     } else {
-        create_fdt(s, memmap);
+        create_fdt(s, s->memmap);
     }
 
     if (virt_is_iommu_sys_enabled(s)) {
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/9] hw/riscv/virt.c: remove trivial virt_memmap references
  2025-04-29 12:58 [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Daniel Henrique Barboza
  2025-04-29 12:58 ` [PATCH v2 1/9] hw/riscv/virt.c: enforce s->memmap use in machine_init() Daniel Henrique Barboza
@ 2025-04-29 12:58 ` Daniel Henrique Barboza
  2025-04-30 23:59   ` Alistair Francis
  2025-04-29 12:58 ` [PATCH v2 3/9] hw/riscv/virt.c: use s->memmap in virt_machine_done() Daniel Henrique Barboza
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-29 12:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer, joel,
	Daniel Henrique Barboza

We should use s->memmap instead of virt_memmap to be able to use an
updated memmap when we start versioning the board.

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

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index bcf0ddd6c6..b4a6916abb 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -166,8 +166,8 @@ static void virt_flash_map1(PFlashCFI01 *flash,
 static void virt_flash_map(RISCVVirtState *s,
                            MemoryRegion *sysmem)
 {
-    hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
-    hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
+    hwaddr flashsize = s->memmap[VIRT_FLASH].size / 2;
+    hwaddr flashbase = s->memmap[VIRT_FLASH].base;
 
     virt_flash_map1(s->flash[0], flashbase, flashsize,
                     sysmem);
@@ -998,8 +998,8 @@ static void create_fdt_rtc(RISCVVirtState *s, const MemMapEntry *memmap,
 static void create_fdt_flash(RISCVVirtState *s, const MemMapEntry *memmap)
 {
     MachineState *ms = MACHINE(s);
-    hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
-    hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
+    hwaddr flashsize = s->memmap[VIRT_FLASH].size / 2;
+    hwaddr flashbase = s->memmap[VIRT_FLASH].base;
     g_autofree char *name = g_strdup_printf("/flash@%" PRIx64, flashbase);
 
     qemu_fdt_add_subnode(ms->fdt, name);
@@ -1034,7 +1034,7 @@ static void create_fdt_virtio_iommu(RISCVVirtState *s, uint16_t bdf)
     g_autofree char *pci_node = NULL;
 
     pci_node = g_strdup_printf("/soc/pci@%lx",
-                               (long) virt_memmap[VIRT_PCIE_ECAM].base);
+                               (long) s->memmap[VIRT_PCIE_ECAM].base);
     iommu_node = g_strdup_printf("%s/virtio_iommu@%x,%x", pci_node,
                                  PCI_SLOT(bdf), PCI_FUNC(bdf));
     iommu_phandle = qemu_fdt_alloc_phandle(fdt);
@@ -1103,7 +1103,7 @@ static void create_fdt_iommu(RISCVVirtState *s, uint16_t bdf)
     g_autofree char *pci_node = NULL;
 
     pci_node = g_strdup_printf("/soc/pci@%lx",
-                               (long) virt_memmap[VIRT_PCIE_ECAM].base);
+                               (long) s->memmap[VIRT_PCIE_ECAM].base);
     iommu_node = g_strdup_printf("%s/iommu@%x", pci_node, bdf);
     iommu_phandle = qemu_fdt_alloc_phandle(fdt);
     qemu_fdt_add_subnode(fdt, iommu_node);
@@ -1125,24 +1125,24 @@ static void finalize_fdt(RISCVVirtState *s)
     uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
     uint32_t iommu_sys_phandle = 1;
 
-    create_fdt_sockets(s, virt_memmap, &phandle, &irq_mmio_phandle,
+    create_fdt_sockets(s, s->memmap, &phandle, &irq_mmio_phandle,
                        &irq_pcie_phandle, &irq_virtio_phandle,
                        &msi_pcie_phandle);
 
-    create_fdt_virtio(s, virt_memmap, irq_virtio_phandle);
+    create_fdt_virtio(s, s->memmap, irq_virtio_phandle);
 
     if (virt_is_iommu_sys_enabled(s)) {
         create_fdt_iommu_sys(s, irq_mmio_phandle, msi_pcie_phandle,
                              &iommu_sys_phandle);
     }
-    create_fdt_pcie(s, virt_memmap, irq_pcie_phandle, msi_pcie_phandle,
+    create_fdt_pcie(s, s->memmap, irq_pcie_phandle, msi_pcie_phandle,
                     iommu_sys_phandle);
 
-    create_fdt_reset(s, virt_memmap, &phandle);
+    create_fdt_reset(s, s->memmap, &phandle);
 
-    create_fdt_uart(s, virt_memmap, irq_mmio_phandle);
+    create_fdt_uart(s, s->memmap, irq_mmio_phandle);
 
-    create_fdt_rtc(s, virt_memmap, irq_mmio_phandle);
+    create_fdt_rtc(s, s->memmap, irq_mmio_phandle);
 }
 
 static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
@@ -1365,14 +1365,13 @@ static void create_platform_bus(RISCVVirtState *s, DeviceState *irqchip)
 {
     DeviceState *dev;
     SysBusDevice *sysbus;
-    const MemMapEntry *memmap = virt_memmap;
     int i;
     MemoryRegion *sysmem = get_system_memory();
 
     dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE);
     dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE);
     qdev_prop_set_uint32(dev, "num_irqs", VIRT_PLATFORM_BUS_NUM_IRQS);
-    qdev_prop_set_uint32(dev, "mmio_size", memmap[VIRT_PLATFORM_BUS].size);
+    qdev_prop_set_uint32(dev, "mmio_size", s->memmap[VIRT_PLATFORM_BUS].size);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
     s->platform_bus_dev = dev;
 
@@ -1383,7 +1382,7 @@ static void create_platform_bus(RISCVVirtState *s, DeviceState *irqchip)
     }
 
     memory_region_add_subregion(sysmem,
-                                memmap[VIRT_PLATFORM_BUS].base,
+                                s->memmap[VIRT_PLATFORM_BUS].base,
                                 sysbus_mmio_get_region(sysbus, 0));
 }
 
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 3/9] hw/riscv/virt.c: use s->memmap in virt_machine_done()
  2025-04-29 12:58 [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Daniel Henrique Barboza
  2025-04-29 12:58 ` [PATCH v2 1/9] hw/riscv/virt.c: enforce s->memmap use in machine_init() Daniel Henrique Barboza
  2025-04-29 12:58 ` [PATCH v2 2/9] hw/riscv/virt.c: remove trivial virt_memmap references Daniel Henrique Barboza
@ 2025-04-29 12:58 ` Daniel Henrique Barboza
  2025-05-01  0:02   ` Alistair Francis
  2025-04-29 12:58 ` [PATCH v2 4/9] hw/riscv/virt.c: add 'base' arg in create_fw_cfg() Daniel Henrique Barboza
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-29 12:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer, joel,
	Daniel Henrique Barboza

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

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index b4a6916abb..f324777161 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1429,9 +1429,8 @@ static void virt_machine_done(Notifier *notifier, void *data)
 {
     RISCVVirtState *s = container_of(notifier, RISCVVirtState,
                                      machine_done);
-    const MemMapEntry *memmap = virt_memmap;
     MachineState *machine = MACHINE(s);
-    hwaddr start_addr = memmap[VIRT_DRAM].base;
+    hwaddr start_addr = s->memmap[VIRT_DRAM].base;
     target_ulong firmware_end_addr, kernel_start_addr;
     const char *firmware_name = riscv_default_firmware_name(&s->soc[0]);
     uint64_t fdt_load_addr;
@@ -1475,14 +1474,14 @@ static void virt_machine_done(Notifier *notifier, void *data)
              * let's overwrite the address we jump to after reset to
              * the base of the flash.
              */
-            start_addr = virt_memmap[VIRT_FLASH].base;
+            start_addr = s->memmap[VIRT_FLASH].base;
         } else {
             /*
              * Pflash was supplied but either KVM guest or bios is not none.
              * In this case, base of the flash would contain S-mode payload.
              */
             riscv_setup_firmware_boot(machine);
-            kernel_entry = virt_memmap[VIRT_FLASH].base;
+            kernel_entry = s->memmap[VIRT_FLASH].base;
         }
     }
 
@@ -1496,15 +1495,15 @@ static void virt_machine_done(Notifier *notifier, void *data)
         kernel_entry = boot_info.image_low_addr;
     }
 
-    fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base,
-                                           memmap[VIRT_DRAM].size,
+    fdt_load_addr = riscv_compute_fdt_addr(s->memmap[VIRT_DRAM].base,
+                                           s->memmap[VIRT_DRAM].size,
                                            machine, &boot_info);
     riscv_load_fdt(fdt_load_addr, machine->fdt);
 
     /* load the reset vector */
     riscv_setup_rom_reset_vec(machine, &s->soc[0], start_addr,
-                              virt_memmap[VIRT_MROM].base,
-                              virt_memmap[VIRT_MROM].size, kernel_entry,
+                              s->memmap[VIRT_MROM].base,
+                              s->memmap[VIRT_MROM].size, kernel_entry,
                               fdt_load_addr);
 
     /*
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 4/9] hw/riscv/virt.c: add 'base' arg in create_fw_cfg()
  2025-04-29 12:58 [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Daniel Henrique Barboza
                   ` (2 preceding siblings ...)
  2025-04-29 12:58 ` [PATCH v2 3/9] hw/riscv/virt.c: use s->memmap in virt_machine_done() Daniel Henrique Barboza
@ 2025-04-29 12:58 ` Daniel Henrique Barboza
  2025-05-01  0:03   ` Alistair Francis
  2025-04-29 12:58 ` [PATCH v2 5/9] hw/riscv/virt.c: use s->memmap in create_fdt() path Daniel Henrique Barboza
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-29 12:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer, joel,
	Daniel Henrique Barboza, Philippe Mathieu-Daudé

The function can receive the value via s->memmap[VIRT_FW_CFG].base from
the caller, avoiding the use of virt_memmap.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/riscv/virt.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index f324777161..37f8abdd1c 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1261,9 +1261,8 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
     return dev;
 }
 
-static FWCfgState *create_fw_cfg(const MachineState *ms)
+static FWCfgState *create_fw_cfg(const MachineState *ms, hwaddr base)
 {
-    hwaddr base = virt_memmap[VIRT_FW_CFG].base;
     FWCfgState *fw_cfg;
 
     fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16,
@@ -1681,7 +1680,7 @@ static void virt_machine_init(MachineState *machine)
      * Init fw_cfg. Must be done before riscv_load_fdt, otherwise the
      * device tree cannot be altered and we get FDT_ERR_NOSPACE.
      */
-    s->fw_cfg = create_fw_cfg(machine);
+    s->fw_cfg = create_fw_cfg(machine, s->memmap[VIRT_FW_CFG].base);
     rom_set_fw(s->fw_cfg);
 
     /* SiFive Test MMIO device */
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 5/9] hw/riscv/virt.c: use s->memmap in create_fdt() path
  2025-04-29 12:58 [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Daniel Henrique Barboza
                   ` (3 preceding siblings ...)
  2025-04-29 12:58 ` [PATCH v2 4/9] hw/riscv/virt.c: add 'base' arg in create_fw_cfg() Daniel Henrique Barboza
@ 2025-04-29 12:58 ` Daniel Henrique Barboza
  2025-05-01  0:08   ` Alistair Francis
  2025-04-29 12:58 ` [PATCH v2 6/9] hw/riscv/virt.c: use s->memmap in create_fdt_sockets() path Daniel Henrique Barboza
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-29 12:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer, joel,
	Daniel Henrique Barboza

create_fdt(), create_fdt_flash() and create_fdt_fw_cfg() can access the
memmap via their RISCVVirtState pointers.

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

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 37f8abdd1c..5f31c95955 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -995,7 +995,7 @@ static void create_fdt_rtc(RISCVVirtState *s, const MemMapEntry *memmap,
     }
 }
 
-static void create_fdt_flash(RISCVVirtState *s, const MemMapEntry *memmap)
+static void create_fdt_flash(RISCVVirtState *s)
 {
     MachineState *ms = MACHINE(s);
     hwaddr flashsize = s->memmap[VIRT_FLASH].size / 2;
@@ -1010,11 +1010,11 @@ static void create_fdt_flash(RISCVVirtState *s, const MemMapEntry *memmap)
     qemu_fdt_setprop_cell(ms->fdt, name, "bank-width", 4);
 }
 
-static void create_fdt_fw_cfg(RISCVVirtState *s, const MemMapEntry *memmap)
+static void create_fdt_fw_cfg(RISCVVirtState *s)
 {
     MachineState *ms = MACHINE(s);
-    hwaddr base = memmap[VIRT_FW_CFG].base;
-    hwaddr size = memmap[VIRT_FW_CFG].size;
+    hwaddr base = s->memmap[VIRT_FW_CFG].base;
+    hwaddr size = s->memmap[VIRT_FW_CFG].size;
     g_autofree char *nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
 
     qemu_fdt_add_subnode(ms->fdt, nodename);
@@ -1145,7 +1145,7 @@ static void finalize_fdt(RISCVVirtState *s)
     create_fdt_rtc(s, s->memmap, irq_mmio_phandle);
 }
 
-static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
+static void create_fdt(RISCVVirtState *s)
 {
     MachineState *ms = MACHINE(s);
     uint8_t rng_seed[32];
@@ -1172,7 +1172,8 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
      * The "/soc/pci@..." node is needed for PCIE hotplugs
      * that might happen before finalize_fdt().
      */
-    name = g_strdup_printf("/soc/pci@%lx", (long) memmap[VIRT_PCIE_ECAM].base);
+    name = g_strdup_printf("/soc/pci@%lx",
+                           (long) s->memmap[VIRT_PCIE_ECAM].base);
     qemu_fdt_add_subnode(ms->fdt, name);
 
     qemu_fdt_add_subnode(ms->fdt, "/chosen");
@@ -1184,8 +1185,8 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
 
     qemu_fdt_add_subnode(ms->fdt, "/aliases");
 
-    create_fdt_flash(s, memmap);
-    create_fdt_fw_cfg(s, memmap);
+    create_fdt_flash(s);
+    create_fdt_fw_cfg(s);
     create_fdt_pmu(s);
 }
 
@@ -1719,7 +1720,7 @@ static void virt_machine_init(MachineState *machine)
             exit(1);
         }
     } else {
-        create_fdt(s, s->memmap);
+        create_fdt(s);
     }
 
     if (virt_is_iommu_sys_enabled(s)) {
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 6/9] hw/riscv/virt.c: use s->memmap in create_fdt_sockets() path
  2025-04-29 12:58 [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Daniel Henrique Barboza
                   ` (4 preceding siblings ...)
  2025-04-29 12:58 ` [PATCH v2 5/9] hw/riscv/virt.c: use s->memmap in create_fdt() path Daniel Henrique Barboza
@ 2025-04-29 12:58 ` Daniel Henrique Barboza
  2025-05-01  0:10   ` Alistair Francis
  2025-04-29 12:58 ` [PATCH v2 7/9] hw/riscv/virt.c: use s->memmap in create_fdt_virtio() Daniel Henrique Barboza
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-29 12:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer, joel,
	Daniel Henrique Barboza

create_fdt_sockets() and all its fdt helpers (create_fdt_socket_aplic(),
create_fdt_imsic(), create_fdt_socket_plic(), create_fdt_socket_aclint()
and create_fdt_socket_memory()) can use s->memmap from their
RISCVVirtState pointer instead of having an extra memmap argument.

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

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 5f31c95955..2383a557bd 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -300,14 +300,13 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
     }
 }
 
-static void create_fdt_socket_memory(RISCVVirtState *s,
-                                     const MemMapEntry *memmap, int socket)
+static void create_fdt_socket_memory(RISCVVirtState *s, int socket)
 {
     g_autofree char *mem_name = NULL;
     uint64_t addr, size;
     MachineState *ms = MACHINE(s);
 
-    addr = memmap[VIRT_DRAM].base + riscv_socket_mem_offset(ms, socket);
+    addr = s->memmap[VIRT_DRAM].base + riscv_socket_mem_offset(ms, socket);
     size = riscv_socket_mem_size(ms, socket);
     mem_name = g_strdup_printf("/memory@%lx", (long)addr);
     qemu_fdt_add_subnode(ms->fdt, mem_name);
@@ -318,7 +317,7 @@ static void create_fdt_socket_memory(RISCVVirtState *s,
 }
 
 static void create_fdt_socket_clint(RISCVVirtState *s,
-                                    const MemMapEntry *memmap, int socket,
+                                    int socket,
                                     uint32_t *intc_phandles)
 {
     int cpu;
@@ -339,21 +338,22 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
         clint_cells[cpu * 4 + 3] = cpu_to_be32(IRQ_M_TIMER);
     }
 
-    clint_addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket);
+    clint_addr = s->memmap[VIRT_CLINT].base +
+                 (s->memmap[VIRT_CLINT].size * socket);
     clint_name = g_strdup_printf("/soc/clint@%lx", clint_addr);
     qemu_fdt_add_subnode(ms->fdt, clint_name);
     qemu_fdt_setprop_string_array(ms->fdt, clint_name, "compatible",
                                   (char **)&clint_compat,
                                   ARRAY_SIZE(clint_compat));
     qemu_fdt_setprop_cells(ms->fdt, clint_name, "reg",
-        0x0, clint_addr, 0x0, memmap[VIRT_CLINT].size);
+        0x0, clint_addr, 0x0, s->memmap[VIRT_CLINT].size);
     qemu_fdt_setprop(ms->fdt, clint_name, "interrupts-extended",
         clint_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
     riscv_socket_fdt_write_id(ms, clint_name, socket);
 }
 
 static void create_fdt_socket_aclint(RISCVVirtState *s,
-                                     const MemMapEntry *memmap, int socket,
+                                     int socket,
                                      uint32_t *intc_phandles)
 {
     int cpu;
@@ -380,8 +380,10 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
     aclint_cells_size = s->soc[socket].num_harts * sizeof(uint32_t) * 2;
 
     if (s->aia_type != VIRT_AIA_TYPE_APLIC_IMSIC) {
-        addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket);
+        addr = s->memmap[VIRT_CLINT].base +
+               (s->memmap[VIRT_CLINT].size * socket);
         name = g_strdup_printf("/soc/mswi@%lx", addr);
+
         qemu_fdt_add_subnode(ms->fdt, name);
         qemu_fdt_setprop_string(ms->fdt, name, "compatible",
             "riscv,aclint-mswi");
@@ -396,13 +398,13 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
     }
 
     if (s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
-        addr = memmap[VIRT_CLINT].base +
+        addr = s->memmap[VIRT_CLINT].base +
                (RISCV_ACLINT_DEFAULT_MTIMER_SIZE * socket);
         size = RISCV_ACLINT_DEFAULT_MTIMER_SIZE;
     } else {
-        addr = memmap[VIRT_CLINT].base + RISCV_ACLINT_SWI_SIZE +
-            (memmap[VIRT_CLINT].size * socket);
-        size = memmap[VIRT_CLINT].size - RISCV_ACLINT_SWI_SIZE;
+        addr = s->memmap[VIRT_CLINT].base + RISCV_ACLINT_SWI_SIZE +
+               (s->memmap[VIRT_CLINT].size * socket);
+        size = s->memmap[VIRT_CLINT].size - RISCV_ACLINT_SWI_SIZE;
     }
     name = g_strdup_printf("/soc/mtimer@%lx", addr);
     qemu_fdt_add_subnode(ms->fdt, name);
@@ -419,14 +421,15 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
     g_free(name);
 
     if (s->aia_type != VIRT_AIA_TYPE_APLIC_IMSIC) {
-        addr = memmap[VIRT_ACLINT_SSWI].base +
-            (memmap[VIRT_ACLINT_SSWI].size * socket);
+        addr = s->memmap[VIRT_ACLINT_SSWI].base +
+               (s->memmap[VIRT_ACLINT_SSWI].size * socket);
+
         name = g_strdup_printf("/soc/sswi@%lx", addr);
         qemu_fdt_add_subnode(ms->fdt, name);
         qemu_fdt_setprop_string(ms->fdt, name, "compatible",
             "riscv,aclint-sswi");
         qemu_fdt_setprop_cells(ms->fdt, name, "reg",
-            0x0, addr, 0x0, memmap[VIRT_ACLINT_SSWI].size);
+            0x0, addr, 0x0, s->memmap[VIRT_ACLINT_SSWI].size);
         qemu_fdt_setprop(ms->fdt, name, "interrupts-extended",
             aclint_sswi_cells, aclint_cells_size);
         qemu_fdt_setprop(ms->fdt, name, "interrupt-controller", NULL, 0);
@@ -437,7 +440,7 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
 }
 
 static void create_fdt_socket_plic(RISCVVirtState *s,
-                                   const MemMapEntry *memmap, int socket,
+                                   int socket,
                                    uint32_t *phandle, uint32_t *intc_phandles,
                                    uint32_t *plic_phandles)
 {
@@ -451,7 +454,8 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
     };
 
     plic_phandles[socket] = (*phandle)++;
-    plic_addr = memmap[VIRT_PLIC].base + (memmap[VIRT_PLIC].size * socket);
+    plic_addr = s->memmap[VIRT_PLIC].base +
+                (s->memmap[VIRT_PLIC].size * socket);
     plic_name = g_strdup_printf("/soc/plic@%lx", plic_addr);
     qemu_fdt_add_subnode(ms->fdt, plic_name);
     qemu_fdt_setprop_cell(ms->fdt, plic_name,
@@ -490,7 +494,7 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
     }
 
     qemu_fdt_setprop_cells(ms->fdt, plic_name, "reg",
-        0x0, plic_addr, 0x0, memmap[VIRT_PLIC].size);
+        0x0, plic_addr, 0x0, s->memmap[VIRT_PLIC].size);
     qemu_fdt_setprop_cell(ms->fdt, plic_name, "riscv,ndev",
                           VIRT_IRQCHIP_NUM_SOURCES - 1);
     riscv_socket_fdt_write_id(ms, plic_name, socket);
@@ -499,8 +503,8 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
 
     if (!socket) {
         platform_bus_add_all_fdt_nodes(ms->fdt, plic_name,
-                                       memmap[VIRT_PLATFORM_BUS].base,
-                                       memmap[VIRT_PLATFORM_BUS].size,
+                                       s->memmap[VIRT_PLATFORM_BUS].base,
+                                       s->memmap[VIRT_PLATFORM_BUS].size,
                                        VIRT_PLATFORM_BUS_IRQ);
     }
 }
@@ -587,7 +591,7 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
     qemu_fdt_setprop_cell(ms->fdt, imsic_name, "phandle", msi_phandle);
 }
 
-static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
+static void create_fdt_imsic(RISCVVirtState *s,
                              uint32_t *phandle, uint32_t *intc_phandles,
                              uint32_t *msi_m_phandle, uint32_t *msi_s_phandle)
 {
@@ -596,12 +600,12 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
 
     if (!kvm_enabled()) {
         /* M-level IMSIC node */
-        create_fdt_one_imsic(s, memmap[VIRT_IMSIC_M].base, intc_phandles,
+        create_fdt_one_imsic(s, s->memmap[VIRT_IMSIC_M].base, intc_phandles,
                              *msi_m_phandle, true, 0);
     }
 
     /* S-level IMSIC node */
-    create_fdt_one_imsic(s, memmap[VIRT_IMSIC_S].base, intc_phandles,
+    create_fdt_one_imsic(s, s->memmap[VIRT_IMSIC_S].base, intc_phandles,
                          *msi_s_phandle, false,
                          imsic_num_bits(s->aia_guests + 1));
 
@@ -678,7 +682,7 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
 }
 
 static void create_fdt_socket_aplic(RISCVVirtState *s,
-                                    const MemMapEntry *memmap, int socket,
+                                    int socket,
                                     uint32_t msi_m_phandle,
                                     uint32_t msi_s_phandle,
                                     uint32_t *phandle,
@@ -695,18 +699,19 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
 
     if (!kvm_enabled()) {
         /* M-level APLIC node */
-        aplic_addr = memmap[VIRT_APLIC_M].base +
-                     (memmap[VIRT_APLIC_M].size * socket);
-        create_fdt_one_aplic(s, socket, aplic_addr, memmap[VIRT_APLIC_M].size,
+        aplic_addr = s->memmap[VIRT_APLIC_M].base +
+                     (s->memmap[VIRT_APLIC_M].size * socket);
+        create_fdt_one_aplic(s, socket, aplic_addr,
+                             s->memmap[VIRT_APLIC_M].size,
                              msi_m_phandle, intc_phandles,
                              aplic_m_phandle, aplic_s_phandle,
                              true, num_harts);
     }
 
     /* S-level APLIC node */
-    aplic_addr = memmap[VIRT_APLIC_S].base +
-                 (memmap[VIRT_APLIC_S].size * socket);
-    create_fdt_one_aplic(s, socket, aplic_addr, memmap[VIRT_APLIC_S].size,
+    aplic_addr = s->memmap[VIRT_APLIC_S].base +
+                 (s->memmap[VIRT_APLIC_S].size * socket);
+    create_fdt_one_aplic(s, socket, aplic_addr, s->memmap[VIRT_APLIC_S].size,
                          msi_s_phandle, intc_phandles,
                          aplic_s_phandle, 0,
                          false, num_harts);
@@ -714,8 +719,8 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
     if (!socket) {
         g_autofree char *aplic_name = fdt_get_aplic_nodename(aplic_addr);
         platform_bus_add_all_fdt_nodes(ms->fdt, aplic_name,
-                                       memmap[VIRT_PLATFORM_BUS].base,
-                                       memmap[VIRT_PLATFORM_BUS].size,
+                                       s->memmap[VIRT_PLATFORM_BUS].base,
+                                       s->memmap[VIRT_PLATFORM_BUS].size,
                                        VIRT_PLATFORM_BUS_IRQ);
     }
 
@@ -733,7 +738,7 @@ static void create_fdt_pmu(RISCVVirtState *s)
     riscv_pmu_generate_fdt_node(ms->fdt, hart.pmu_avail_ctrs, pmu_name);
 }
 
-static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
+static void create_fdt_sockets(RISCVVirtState *s,
                                uint32_t *phandle,
                                uint32_t *irq_mmio_phandle,
                                uint32_t *irq_pcie_phandle,
@@ -769,20 +774,20 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
         create_fdt_socket_cpus(s, socket, clust_name, phandle,
                                &intc_phandles[phandle_pos]);
 
-        create_fdt_socket_memory(s, memmap, socket);
+        create_fdt_socket_memory(s, socket);
 
         if (virt_aclint_allowed() && s->have_aclint) {
-            create_fdt_socket_aclint(s, memmap, socket,
+            create_fdt_socket_aclint(s, socket,
                                      &intc_phandles[phandle_pos]);
         } else if (tcg_enabled()) {
-            create_fdt_socket_clint(s, memmap, socket,
+            create_fdt_socket_clint(s, socket,
                                     &intc_phandles[phandle_pos]);
         }
     }
 
     if (s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
-        create_fdt_imsic(s, memmap, phandle, intc_phandles,
-            &msi_m_phandle, &msi_s_phandle);
+        create_fdt_imsic(s, phandle, intc_phandles,
+                         &msi_m_phandle, &msi_s_phandle);
         *msi_pcie_phandle = msi_s_phandle;
     }
 
@@ -791,7 +796,7 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
      * mode, we'll use only one APLIC instance.
      */
     if (!virt_use_emulated_aplic(s->aia_type)) {
-        create_fdt_socket_aplic(s, memmap, 0,
+        create_fdt_socket_aplic(s, 0,
                                 msi_m_phandle, msi_s_phandle, phandle,
                                 &intc_phandles[0], xplic_phandles,
                                 ms->smp.cpus);
@@ -805,11 +810,11 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
             phandle_pos -= s->soc[socket].num_harts;
 
             if (s->aia_type == VIRT_AIA_TYPE_NONE) {
-                create_fdt_socket_plic(s, memmap, socket, phandle,
+                create_fdt_socket_plic(s, socket, phandle,
                                        &intc_phandles[phandle_pos],
                                        xplic_phandles);
             } else {
-                create_fdt_socket_aplic(s, memmap, socket,
+                create_fdt_socket_aplic(s, socket,
                                         msi_m_phandle, msi_s_phandle, phandle,
                                         &intc_phandles[phandle_pos],
                                         xplic_phandles,
@@ -1125,7 +1130,7 @@ static void finalize_fdt(RISCVVirtState *s)
     uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
     uint32_t iommu_sys_phandle = 1;
 
-    create_fdt_sockets(s, s->memmap, &phandle, &irq_mmio_phandle,
+    create_fdt_sockets(s, &phandle, &irq_mmio_phandle,
                        &irq_pcie_phandle, &irq_virtio_phandle,
                        &msi_pcie_phandle);
 
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 7/9] hw/riscv/virt.c: use s->memmap in create_fdt_virtio()
  2025-04-29 12:58 [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Daniel Henrique Barboza
                   ` (5 preceding siblings ...)
  2025-04-29 12:58 ` [PATCH v2 6/9] hw/riscv/virt.c: use s->memmap in create_fdt_sockets() path Daniel Henrique Barboza
@ 2025-04-29 12:58 ` Daniel Henrique Barboza
  2025-05-01  0:11   ` Alistair Francis
  2025-04-29 12:58 ` [PATCH v2 8/9] hw/riscv/virt.c: use s->memmap in finalize_fdt() functions Daniel Henrique Barboza
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-29 12:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer, joel,
	Daniel Henrique Barboza

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



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 8/9] hw/riscv/virt.c: use s->memmap in finalize_fdt() functions
  2025-04-29 12:58 [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Daniel Henrique Barboza
                   ` (6 preceding siblings ...)
  2025-04-29 12:58 ` [PATCH v2 7/9] hw/riscv/virt.c: use s->memmap in create_fdt_virtio() Daniel Henrique Barboza
@ 2025-04-29 12:58 ` Daniel Henrique Barboza
  2025-05-01  0:12   ` Alistair Francis
  2025-04-29 12:58 ` [PATCH v2 9/9] hw/riscv/virt.c: remove 'long' casts in fmt strings Daniel Henrique Barboza
  2025-05-01  0:33 ` [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Alistair Francis
  9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-29 12:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer, joel,
	Daniel Henrique Barboza

Change create_fdt_pcie(), create_fdt_reset(), create_fdt_uart() and
create_fdt_rtc() to use s->memmap in their logic.

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

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 46ac42058e..f38b64d836 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -871,7 +871,7 @@ static void create_fdt_virtio(RISCVVirtState *s, uint32_t irq_virtio_phandle)
     }
 }
 
-static void create_fdt_pcie(RISCVVirtState *s, const MemMapEntry *memmap,
+static void create_fdt_pcie(RISCVVirtState *s,
                             uint32_t irq_pcie_phandle,
                             uint32_t msi_pcie_phandle,
                             uint32_t iommu_sys_phandle)
@@ -880,7 +880,7 @@ static void create_fdt_pcie(RISCVVirtState *s, const MemMapEntry *memmap,
     MachineState *ms = MACHINE(s);
 
     name = g_strdup_printf("/soc/pci@%lx",
-        (long) memmap[VIRT_PCIE_ECAM].base);
+        (long) s->memmap[VIRT_PCIE_ECAM].base);
     qemu_fdt_setprop_cell(ms->fdt, name, "#address-cells",
         FDT_PCI_ADDR_CELLS);
     qemu_fdt_setprop_cell(ms->fdt, name, "#interrupt-cells",
@@ -891,19 +891,19 @@ static void create_fdt_pcie(RISCVVirtState *s, const MemMapEntry *memmap,
     qemu_fdt_setprop_string(ms->fdt, name, "device_type", "pci");
     qemu_fdt_setprop_cell(ms->fdt, name, "linux,pci-domain", 0);
     qemu_fdt_setprop_cells(ms->fdt, name, "bus-range", 0,
-        memmap[VIRT_PCIE_ECAM].size / PCIE_MMCFG_SIZE_MIN - 1);
+        s->memmap[VIRT_PCIE_ECAM].size / PCIE_MMCFG_SIZE_MIN - 1);
     qemu_fdt_setprop(ms->fdt, name, "dma-coherent", NULL, 0);
     if (s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
         qemu_fdt_setprop_cell(ms->fdt, name, "msi-parent", msi_pcie_phandle);
     }
     qemu_fdt_setprop_cells(ms->fdt, name, "reg", 0,
-        memmap[VIRT_PCIE_ECAM].base, 0, memmap[VIRT_PCIE_ECAM].size);
+        s->memmap[VIRT_PCIE_ECAM].base, 0, s->memmap[VIRT_PCIE_ECAM].size);
     qemu_fdt_setprop_sized_cells(ms->fdt, name, "ranges",
         1, FDT_PCI_RANGE_IOPORT, 2, 0,
-        2, memmap[VIRT_PCIE_PIO].base, 2, memmap[VIRT_PCIE_PIO].size,
+        2, s->memmap[VIRT_PCIE_PIO].base, 2, s->memmap[VIRT_PCIE_PIO].size,
         1, FDT_PCI_RANGE_MMIO,
-        2, memmap[VIRT_PCIE_MMIO].base,
-        2, memmap[VIRT_PCIE_MMIO].base, 2, memmap[VIRT_PCIE_MMIO].size,
+        2, s->memmap[VIRT_PCIE_MMIO].base,
+        2, s->memmap[VIRT_PCIE_MMIO].base, 2, s->memmap[VIRT_PCIE_MMIO].size,
         1, FDT_PCI_RANGE_MMIO_64BIT,
         2, virt_high_pcie_memmap.base,
         2, virt_high_pcie_memmap.base, 2, virt_high_pcie_memmap.size);
@@ -917,8 +917,7 @@ static void create_fdt_pcie(RISCVVirtState *s, const MemMapEntry *memmap,
     create_pcie_irq_map(s, ms->fdt, name, irq_pcie_phandle);
 }
 
-static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
-                             uint32_t *phandle)
+static void create_fdt_reset(RISCVVirtState *s, uint32_t *phandle)
 {
     char *name;
     uint32_t test_phandle;
@@ -926,7 +925,7 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
 
     test_phandle = (*phandle)++;
     name = g_strdup_printf("/soc/test@%lx",
-        (long)memmap[VIRT_TEST].base);
+        (long)s->memmap[VIRT_TEST].base);
     qemu_fdt_add_subnode(ms->fdt, name);
     {
         static const char * const compat[3] = {
@@ -936,7 +935,7 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
                                       (char **)&compat, ARRAY_SIZE(compat));
     }
     qemu_fdt_setprop_cells(ms->fdt, name, "reg",
-        0x0, memmap[VIRT_TEST].base, 0x0, memmap[VIRT_TEST].size);
+        0x0, s->memmap[VIRT_TEST].base, 0x0, s->memmap[VIRT_TEST].size);
     qemu_fdt_setprop_cell(ms->fdt, name, "phandle", test_phandle);
     test_phandle = qemu_fdt_get_phandle(ms->fdt, name);
     g_free(name);
@@ -958,18 +957,19 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
     g_free(name);
 }
 
-static void create_fdt_uart(RISCVVirtState *s, const MemMapEntry *memmap,
+static void create_fdt_uart(RISCVVirtState *s,
                             uint32_t irq_mmio_phandle)
 {
     g_autofree char *name = NULL;
     MachineState *ms = MACHINE(s);
 
-    name = g_strdup_printf("/soc/serial@%lx", (long)memmap[VIRT_UART0].base);
+    name = g_strdup_printf("/soc/serial@%lx",
+                           (long)s->memmap[VIRT_UART0].base);
     qemu_fdt_add_subnode(ms->fdt, name);
     qemu_fdt_setprop_string(ms->fdt, name, "compatible", "ns16550a");
     qemu_fdt_setprop_cells(ms->fdt, name, "reg",
-        0x0, memmap[VIRT_UART0].base,
-        0x0, memmap[VIRT_UART0].size);
+        0x0, s->memmap[VIRT_UART0].base,
+        0x0, s->memmap[VIRT_UART0].size);
     qemu_fdt_setprop_cell(ms->fdt, name, "clock-frequency", 3686400);
     qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent", irq_mmio_phandle);
     if (s->aia_type == VIRT_AIA_TYPE_NONE) {
@@ -982,18 +982,18 @@ static void create_fdt_uart(RISCVVirtState *s, const MemMapEntry *memmap,
     qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial0", name);
 }
 
-static void create_fdt_rtc(RISCVVirtState *s, const MemMapEntry *memmap,
+static void create_fdt_rtc(RISCVVirtState *s,
                            uint32_t irq_mmio_phandle)
 {
     g_autofree char *name = NULL;
     MachineState *ms = MACHINE(s);
 
-    name = g_strdup_printf("/soc/rtc@%lx", (long)memmap[VIRT_RTC].base);
+    name = g_strdup_printf("/soc/rtc@%lx", (long)s->memmap[VIRT_RTC].base);
     qemu_fdt_add_subnode(ms->fdt, name);
     qemu_fdt_setprop_string(ms->fdt, name, "compatible",
         "google,goldfish-rtc");
     qemu_fdt_setprop_cells(ms->fdt, name, "reg",
-        0x0, memmap[VIRT_RTC].base, 0x0, memmap[VIRT_RTC].size);
+        0x0, s->memmap[VIRT_RTC].base, 0x0, s->memmap[VIRT_RTC].size);
     qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent",
         irq_mmio_phandle);
     if (s->aia_type == VIRT_AIA_TYPE_NONE) {
@@ -1143,14 +1143,14 @@ static void finalize_fdt(RISCVVirtState *s)
         create_fdt_iommu_sys(s, irq_mmio_phandle, msi_pcie_phandle,
                              &iommu_sys_phandle);
     }
-    create_fdt_pcie(s, s->memmap, irq_pcie_phandle, msi_pcie_phandle,
+    create_fdt_pcie(s, irq_pcie_phandle, msi_pcie_phandle,
                     iommu_sys_phandle);
 
-    create_fdt_reset(s, s->memmap, &phandle);
+    create_fdt_reset(s, &phandle);
 
-    create_fdt_uart(s, s->memmap, irq_mmio_phandle);
+    create_fdt_uart(s, irq_mmio_phandle);
 
-    create_fdt_rtc(s, s->memmap, irq_mmio_phandle);
+    create_fdt_rtc(s, irq_mmio_phandle);
 }
 
 static void create_fdt(RISCVVirtState *s)
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 9/9] hw/riscv/virt.c: remove 'long' casts in fmt strings
  2025-04-29 12:58 [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Daniel Henrique Barboza
                   ` (7 preceding siblings ...)
  2025-04-29 12:58 ` [PATCH v2 8/9] hw/riscv/virt.c: use s->memmap in finalize_fdt() functions Daniel Henrique Barboza
@ 2025-04-29 12:58 ` Daniel Henrique Barboza
  2025-05-01  0:13   ` Alistair Francis
  2025-05-01  0:33 ` [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Alistair Francis
  9 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2025-04-29 12:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, alistair.francis, liwei1518, zhiwei_liu, palmer, joel,
	Daniel Henrique Barboza, Philippe Mathieu-Daudé

We can avoid the 'long' casts by using PRIx64 and HWADDR_PRIx on the fmt
strings for uint64_t and hwaddr types.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/riscv/virt.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index f38b64d836..0020d8f404 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -303,12 +303,13 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
 static void create_fdt_socket_memory(RISCVVirtState *s, int socket)
 {
     g_autofree char *mem_name = NULL;
-    uint64_t addr, size;
+    hwaddr addr;
+    uint64_t size;
     MachineState *ms = MACHINE(s);
 
     addr = s->memmap[VIRT_DRAM].base + riscv_socket_mem_offset(ms, socket);
     size = riscv_socket_mem_size(ms, socket);
-    mem_name = g_strdup_printf("/memory@%lx", (long)addr);
+    mem_name = g_strdup_printf("/memory@%"HWADDR_PRIx, addr);
     qemu_fdt_add_subnode(ms->fdt, mem_name);
     qemu_fdt_setprop_cells(ms->fdt, mem_name, "reg",
         addr >> 32, addr, size >> 32, size);
@@ -879,8 +880,8 @@ static void create_fdt_pcie(RISCVVirtState *s,
     g_autofree char *name = NULL;
     MachineState *ms = MACHINE(s);
 
-    name = g_strdup_printf("/soc/pci@%lx",
-        (long) s->memmap[VIRT_PCIE_ECAM].base);
+    name = g_strdup_printf("/soc/pci@%"HWADDR_PRIx,
+                           s->memmap[VIRT_PCIE_ECAM].base);
     qemu_fdt_setprop_cell(ms->fdt, name, "#address-cells",
         FDT_PCI_ADDR_CELLS);
     qemu_fdt_setprop_cell(ms->fdt, name, "#interrupt-cells",
@@ -924,8 +925,8 @@ static void create_fdt_reset(RISCVVirtState *s, uint32_t *phandle)
     MachineState *ms = MACHINE(s);
 
     test_phandle = (*phandle)++;
-    name = g_strdup_printf("/soc/test@%lx",
-        (long)s->memmap[VIRT_TEST].base);
+    name = g_strdup_printf("/soc/test@%"HWADDR_PRIx,
+                           s->memmap[VIRT_TEST].base);
     qemu_fdt_add_subnode(ms->fdt, name);
     {
         static const char * const compat[3] = {
@@ -963,8 +964,8 @@ static void create_fdt_uart(RISCVVirtState *s,
     g_autofree char *name = NULL;
     MachineState *ms = MACHINE(s);
 
-    name = g_strdup_printf("/soc/serial@%lx",
-                           (long)s->memmap[VIRT_UART0].base);
+    name = g_strdup_printf("/soc/serial@%"HWADDR_PRIx,
+                           s->memmap[VIRT_UART0].base);
     qemu_fdt_add_subnode(ms->fdt, name);
     qemu_fdt_setprop_string(ms->fdt, name, "compatible", "ns16550a");
     qemu_fdt_setprop_cells(ms->fdt, name, "reg",
@@ -988,7 +989,8 @@ static void create_fdt_rtc(RISCVVirtState *s,
     g_autofree char *name = NULL;
     MachineState *ms = MACHINE(s);
 
-    name = g_strdup_printf("/soc/rtc@%lx", (long)s->memmap[VIRT_RTC].base);
+    name = g_strdup_printf("/soc/rtc@%"HWADDR_PRIx,
+                           s->memmap[VIRT_RTC].base);
     qemu_fdt_add_subnode(ms->fdt, name);
     qemu_fdt_setprop_string(ms->fdt, name, "compatible",
         "google,goldfish-rtc");
@@ -1041,8 +1043,8 @@ static void create_fdt_virtio_iommu(RISCVVirtState *s, uint16_t bdf)
     g_autofree char *iommu_node = NULL;
     g_autofree char *pci_node = NULL;
 
-    pci_node = g_strdup_printf("/soc/pci@%lx",
-                               (long) s->memmap[VIRT_PCIE_ECAM].base);
+    pci_node = g_strdup_printf("/soc/pci@%"HWADDR_PRIx,
+                               s->memmap[VIRT_PCIE_ECAM].base);
     iommu_node = g_strdup_printf("%s/virtio_iommu@%x,%x", pci_node,
                                  PCI_SLOT(bdf), PCI_FUNC(bdf));
     iommu_phandle = qemu_fdt_alloc_phandle(fdt);
@@ -1110,8 +1112,8 @@ static void create_fdt_iommu(RISCVVirtState *s, uint16_t bdf)
     g_autofree char *iommu_node = NULL;
     g_autofree char *pci_node = NULL;
 
-    pci_node = g_strdup_printf("/soc/pci@%lx",
-                               (long) s->memmap[VIRT_PCIE_ECAM].base);
+    pci_node = g_strdup_printf("/soc/pci@%"HWADDR_PRIx,
+                               s->memmap[VIRT_PCIE_ECAM].base);
     iommu_node = g_strdup_printf("%s/iommu@%x", pci_node, bdf);
     iommu_phandle = qemu_fdt_alloc_phandle(fdt);
     qemu_fdt_add_subnode(fdt, iommu_node);
@@ -1180,8 +1182,8 @@ static void create_fdt(RISCVVirtState *s)
      * The "/soc/pci@..." node is needed for PCIE hotplugs
      * that might happen before finalize_fdt().
      */
-    name = g_strdup_printf("/soc/pci@%lx",
-                           (long) s->memmap[VIRT_PCIE_ECAM].base);
+    name = g_strdup_printf("/soc/pci@%"HWADDR_PRIx,
+                           s->memmap[VIRT_PCIE_ECAM].base);
     qemu_fdt_add_subnode(ms->fdt, name);
 
     qemu_fdt_add_subnode(ms->fdt, "/chosen");
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/9] hw/riscv/virt.c: remove trivial virt_memmap references
  2025-04-29 12:58 ` [PATCH v2 2/9] hw/riscv/virt.c: remove trivial virt_memmap references Daniel Henrique Barboza
@ 2025-04-30 23:59   ` Alistair Francis
  0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2025-04-30 23:59 UTC (permalink / raw)
  To: Daniel Henrique Barboza
  Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
	palmer, joel

On Tue, Apr 29, 2025 at 10:59 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> We should use s->memmap instead of virt_memmap to be able to use an
> updated memmap when we start versioning the board.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  hw/riscv/virt.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index bcf0ddd6c6..b4a6916abb 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -166,8 +166,8 @@ static void virt_flash_map1(PFlashCFI01 *flash,
>  static void virt_flash_map(RISCVVirtState *s,
>                             MemoryRegion *sysmem)
>  {
> -    hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
> -    hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
> +    hwaddr flashsize = s->memmap[VIRT_FLASH].size / 2;
> +    hwaddr flashbase = s->memmap[VIRT_FLASH].base;
>
>      virt_flash_map1(s->flash[0], flashbase, flashsize,
>                      sysmem);
> @@ -998,8 +998,8 @@ static void create_fdt_rtc(RISCVVirtState *s, const MemMapEntry *memmap,
>  static void create_fdt_flash(RISCVVirtState *s, const MemMapEntry *memmap)
>  {
>      MachineState *ms = MACHINE(s);
> -    hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
> -    hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
> +    hwaddr flashsize = s->memmap[VIRT_FLASH].size / 2;
> +    hwaddr flashbase = s->memmap[VIRT_FLASH].base;
>      g_autofree char *name = g_strdup_printf("/flash@%" PRIx64, flashbase);
>
>      qemu_fdt_add_subnode(ms->fdt, name);
> @@ -1034,7 +1034,7 @@ static void create_fdt_virtio_iommu(RISCVVirtState *s, uint16_t bdf)
>      g_autofree char *pci_node = NULL;
>
>      pci_node = g_strdup_printf("/soc/pci@%lx",
> -                               (long) virt_memmap[VIRT_PCIE_ECAM].base);
> +                               (long) s->memmap[VIRT_PCIE_ECAM].base);
>      iommu_node = g_strdup_printf("%s/virtio_iommu@%x,%x", pci_node,
>                                   PCI_SLOT(bdf), PCI_FUNC(bdf));
>      iommu_phandle = qemu_fdt_alloc_phandle(fdt);
> @@ -1103,7 +1103,7 @@ static void create_fdt_iommu(RISCVVirtState *s, uint16_t bdf)
>      g_autofree char *pci_node = NULL;
>
>      pci_node = g_strdup_printf("/soc/pci@%lx",
> -                               (long) virt_memmap[VIRT_PCIE_ECAM].base);
> +                               (long) s->memmap[VIRT_PCIE_ECAM].base);
>      iommu_node = g_strdup_printf("%s/iommu@%x", pci_node, bdf);
>      iommu_phandle = qemu_fdt_alloc_phandle(fdt);
>      qemu_fdt_add_subnode(fdt, iommu_node);
> @@ -1125,24 +1125,24 @@ static void finalize_fdt(RISCVVirtState *s)
>      uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
>      uint32_t iommu_sys_phandle = 1;
>
> -    create_fdt_sockets(s, virt_memmap, &phandle, &irq_mmio_phandle,
> +    create_fdt_sockets(s, s->memmap, &phandle, &irq_mmio_phandle,
>                         &irq_pcie_phandle, &irq_virtio_phandle,
>                         &msi_pcie_phandle);
>
> -    create_fdt_virtio(s, virt_memmap, irq_virtio_phandle);
> +    create_fdt_virtio(s, s->memmap, irq_virtio_phandle);
>
>      if (virt_is_iommu_sys_enabled(s)) {
>          create_fdt_iommu_sys(s, irq_mmio_phandle, msi_pcie_phandle,
>                               &iommu_sys_phandle);
>      }
> -    create_fdt_pcie(s, virt_memmap, irq_pcie_phandle, msi_pcie_phandle,
> +    create_fdt_pcie(s, s->memmap, irq_pcie_phandle, msi_pcie_phandle,
>                      iommu_sys_phandle);
>
> -    create_fdt_reset(s, virt_memmap, &phandle);
> +    create_fdt_reset(s, s->memmap, &phandle);
>
> -    create_fdt_uart(s, virt_memmap, irq_mmio_phandle);
> +    create_fdt_uart(s, s->memmap, irq_mmio_phandle);
>
> -    create_fdt_rtc(s, virt_memmap, irq_mmio_phandle);
> +    create_fdt_rtc(s, s->memmap, irq_mmio_phandle);
>  }
>
>  static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
> @@ -1365,14 +1365,13 @@ static void create_platform_bus(RISCVVirtState *s, DeviceState *irqchip)
>  {
>      DeviceState *dev;
>      SysBusDevice *sysbus;
> -    const MemMapEntry *memmap = virt_memmap;
>      int i;
>      MemoryRegion *sysmem = get_system_memory();
>
>      dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE);
>      dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE);
>      qdev_prop_set_uint32(dev, "num_irqs", VIRT_PLATFORM_BUS_NUM_IRQS);
> -    qdev_prop_set_uint32(dev, "mmio_size", memmap[VIRT_PLATFORM_BUS].size);
> +    qdev_prop_set_uint32(dev, "mmio_size", s->memmap[VIRT_PLATFORM_BUS].size);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>      s->platform_bus_dev = dev;
>
> @@ -1383,7 +1382,7 @@ static void create_platform_bus(RISCVVirtState *s, DeviceState *irqchip)
>      }
>
>      memory_region_add_subregion(sysmem,
> -                                memmap[VIRT_PLATFORM_BUS].base,
> +                                s->memmap[VIRT_PLATFORM_BUS].base,
>                                  sysbus_mmio_get_region(sysbus, 0));
>  }
>
> --
> 2.49.0
>
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 3/9] hw/riscv/virt.c: use s->memmap in virt_machine_done()
  2025-04-29 12:58 ` [PATCH v2 3/9] hw/riscv/virt.c: use s->memmap in virt_machine_done() Daniel Henrique Barboza
@ 2025-05-01  0:02   ` Alistair Francis
  0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2025-05-01  0:02 UTC (permalink / raw)
  To: Daniel Henrique Barboza
  Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
	palmer, joel

On Tue, Apr 29, 2025 at 10:59 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  hw/riscv/virt.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index b4a6916abb..f324777161 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1429,9 +1429,8 @@ static void virt_machine_done(Notifier *notifier, void *data)
>  {
>      RISCVVirtState *s = container_of(notifier, RISCVVirtState,
>                                       machine_done);
> -    const MemMapEntry *memmap = virt_memmap;
>      MachineState *machine = MACHINE(s);
> -    hwaddr start_addr = memmap[VIRT_DRAM].base;
> +    hwaddr start_addr = s->memmap[VIRT_DRAM].base;
>      target_ulong firmware_end_addr, kernel_start_addr;
>      const char *firmware_name = riscv_default_firmware_name(&s->soc[0]);
>      uint64_t fdt_load_addr;
> @@ -1475,14 +1474,14 @@ static void virt_machine_done(Notifier *notifier, void *data)
>               * let's overwrite the address we jump to after reset to
>               * the base of the flash.
>               */
> -            start_addr = virt_memmap[VIRT_FLASH].base;
> +            start_addr = s->memmap[VIRT_FLASH].base;
>          } else {
>              /*
>               * Pflash was supplied but either KVM guest or bios is not none.
>               * In this case, base of the flash would contain S-mode payload.
>               */
>              riscv_setup_firmware_boot(machine);
> -            kernel_entry = virt_memmap[VIRT_FLASH].base;
> +            kernel_entry = s->memmap[VIRT_FLASH].base;
>          }
>      }
>
> @@ -1496,15 +1495,15 @@ static void virt_machine_done(Notifier *notifier, void *data)
>          kernel_entry = boot_info.image_low_addr;
>      }
>
> -    fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base,
> -                                           memmap[VIRT_DRAM].size,
> +    fdt_load_addr = riscv_compute_fdt_addr(s->memmap[VIRT_DRAM].base,
> +                                           s->memmap[VIRT_DRAM].size,
>                                             machine, &boot_info);
>      riscv_load_fdt(fdt_load_addr, machine->fdt);
>
>      /* load the reset vector */
>      riscv_setup_rom_reset_vec(machine, &s->soc[0], start_addr,
> -                              virt_memmap[VIRT_MROM].base,
> -                              virt_memmap[VIRT_MROM].size, kernel_entry,
> +                              s->memmap[VIRT_MROM].base,
> +                              s->memmap[VIRT_MROM].size, kernel_entry,
>                                fdt_load_addr);
>
>      /*
> --
> 2.49.0
>
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 4/9] hw/riscv/virt.c: add 'base' arg in create_fw_cfg()
  2025-04-29 12:58 ` [PATCH v2 4/9] hw/riscv/virt.c: add 'base' arg in create_fw_cfg() Daniel Henrique Barboza
@ 2025-05-01  0:03   ` Alistair Francis
  0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2025-05-01  0:03 UTC (permalink / raw)
  To: Daniel Henrique Barboza
  Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
	palmer, joel, Philippe Mathieu-Daudé

On Tue, Apr 29, 2025 at 11:00 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The function can receive the value via s->memmap[VIRT_FW_CFG].base from
> the caller, avoiding the use of virt_memmap.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

Alistair

> ---
>  hw/riscv/virt.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index f324777161..37f8abdd1c 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1261,9 +1261,8 @@ static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
>      return dev;
>  }
>
> -static FWCfgState *create_fw_cfg(const MachineState *ms)
> +static FWCfgState *create_fw_cfg(const MachineState *ms, hwaddr base)
>  {
> -    hwaddr base = virt_memmap[VIRT_FW_CFG].base;
>      FWCfgState *fw_cfg;
>
>      fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16,
> @@ -1681,7 +1680,7 @@ static void virt_machine_init(MachineState *machine)
>       * Init fw_cfg. Must be done before riscv_load_fdt, otherwise the
>       * device tree cannot be altered and we get FDT_ERR_NOSPACE.
>       */
> -    s->fw_cfg = create_fw_cfg(machine);
> +    s->fw_cfg = create_fw_cfg(machine, s->memmap[VIRT_FW_CFG].base);
>      rom_set_fw(s->fw_cfg);
>
>      /* SiFive Test MMIO device */
> --
> 2.49.0
>
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 5/9] hw/riscv/virt.c: use s->memmap in create_fdt() path
  2025-04-29 12:58 ` [PATCH v2 5/9] hw/riscv/virt.c: use s->memmap in create_fdt() path Daniel Henrique Barboza
@ 2025-05-01  0:08   ` Alistair Francis
  0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2025-05-01  0:08 UTC (permalink / raw)
  To: Daniel Henrique Barboza
  Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
	palmer, joel

On Tue, Apr 29, 2025 at 11:01 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> create_fdt(), create_fdt_flash() and create_fdt_fw_cfg() can access the
> memmap via their RISCVVirtState pointers.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  hw/riscv/virt.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 37f8abdd1c..5f31c95955 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -995,7 +995,7 @@ static void create_fdt_rtc(RISCVVirtState *s, const MemMapEntry *memmap,
>      }
>  }
>
> -static void create_fdt_flash(RISCVVirtState *s, const MemMapEntry *memmap)
> +static void create_fdt_flash(RISCVVirtState *s)
>  {
>      MachineState *ms = MACHINE(s);
>      hwaddr flashsize = s->memmap[VIRT_FLASH].size / 2;
> @@ -1010,11 +1010,11 @@ static void create_fdt_flash(RISCVVirtState *s, const MemMapEntry *memmap)
>      qemu_fdt_setprop_cell(ms->fdt, name, "bank-width", 4);
>  }
>
> -static void create_fdt_fw_cfg(RISCVVirtState *s, const MemMapEntry *memmap)
> +static void create_fdt_fw_cfg(RISCVVirtState *s)
>  {
>      MachineState *ms = MACHINE(s);
> -    hwaddr base = memmap[VIRT_FW_CFG].base;
> -    hwaddr size = memmap[VIRT_FW_CFG].size;
> +    hwaddr base = s->memmap[VIRT_FW_CFG].base;
> +    hwaddr size = s->memmap[VIRT_FW_CFG].size;
>      g_autofree char *nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
>
>      qemu_fdt_add_subnode(ms->fdt, nodename);
> @@ -1145,7 +1145,7 @@ static void finalize_fdt(RISCVVirtState *s)
>      create_fdt_rtc(s, s->memmap, irq_mmio_phandle);
>  }
>
> -static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
> +static void create_fdt(RISCVVirtState *s)
>  {
>      MachineState *ms = MACHINE(s);
>      uint8_t rng_seed[32];
> @@ -1172,7 +1172,8 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
>       * The "/soc/pci@..." node is needed for PCIE hotplugs
>       * that might happen before finalize_fdt().
>       */
> -    name = g_strdup_printf("/soc/pci@%lx", (long) memmap[VIRT_PCIE_ECAM].base);
> +    name = g_strdup_printf("/soc/pci@%lx",
> +                           (long) s->memmap[VIRT_PCIE_ECAM].base);
>      qemu_fdt_add_subnode(ms->fdt, name);
>
>      qemu_fdt_add_subnode(ms->fdt, "/chosen");
> @@ -1184,8 +1185,8 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
>
>      qemu_fdt_add_subnode(ms->fdt, "/aliases");
>
> -    create_fdt_flash(s, memmap);
> -    create_fdt_fw_cfg(s, memmap);
> +    create_fdt_flash(s);
> +    create_fdt_fw_cfg(s);
>      create_fdt_pmu(s);
>  }
>
> @@ -1719,7 +1720,7 @@ static void virt_machine_init(MachineState *machine)
>              exit(1);
>          }
>      } else {
> -        create_fdt(s, s->memmap);
> +        create_fdt(s);
>      }
>
>      if (virt_is_iommu_sys_enabled(s)) {
> --
> 2.49.0
>
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 6/9] hw/riscv/virt.c: use s->memmap in create_fdt_sockets() path
  2025-04-29 12:58 ` [PATCH v2 6/9] hw/riscv/virt.c: use s->memmap in create_fdt_sockets() path Daniel Henrique Barboza
@ 2025-05-01  0:10   ` Alistair Francis
  0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2025-05-01  0:10 UTC (permalink / raw)
  To: Daniel Henrique Barboza
  Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
	palmer, joel

On Tue, Apr 29, 2025 at 11:01 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> create_fdt_sockets() and all its fdt helpers (create_fdt_socket_aplic(),
> create_fdt_imsic(), create_fdt_socket_plic(), create_fdt_socket_aclint()
> and create_fdt_socket_memory()) can use s->memmap from their
> RISCVVirtState pointer instead of having an extra memmap argument.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  hw/riscv/virt.c | 89 ++++++++++++++++++++++++++-----------------------
>  1 file changed, 47 insertions(+), 42 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 5f31c95955..2383a557bd 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -300,14 +300,13 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
>      }
>  }
>
> -static void create_fdt_socket_memory(RISCVVirtState *s,
> -                                     const MemMapEntry *memmap, int socket)
> +static void create_fdt_socket_memory(RISCVVirtState *s, int socket)
>  {
>      g_autofree char *mem_name = NULL;
>      uint64_t addr, size;
>      MachineState *ms = MACHINE(s);
>
> -    addr = memmap[VIRT_DRAM].base + riscv_socket_mem_offset(ms, socket);
> +    addr = s->memmap[VIRT_DRAM].base + riscv_socket_mem_offset(ms, socket);
>      size = riscv_socket_mem_size(ms, socket);
>      mem_name = g_strdup_printf("/memory@%lx", (long)addr);
>      qemu_fdt_add_subnode(ms->fdt, mem_name);
> @@ -318,7 +317,7 @@ static void create_fdt_socket_memory(RISCVVirtState *s,
>  }
>
>  static void create_fdt_socket_clint(RISCVVirtState *s,
> -                                    const MemMapEntry *memmap, int socket,
> +                                    int socket,
>                                      uint32_t *intc_phandles)
>  {
>      int cpu;
> @@ -339,21 +338,22 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
>          clint_cells[cpu * 4 + 3] = cpu_to_be32(IRQ_M_TIMER);
>      }
>
> -    clint_addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket);
> +    clint_addr = s->memmap[VIRT_CLINT].base +
> +                 (s->memmap[VIRT_CLINT].size * socket);
>      clint_name = g_strdup_printf("/soc/clint@%lx", clint_addr);
>      qemu_fdt_add_subnode(ms->fdt, clint_name);
>      qemu_fdt_setprop_string_array(ms->fdt, clint_name, "compatible",
>                                    (char **)&clint_compat,
>                                    ARRAY_SIZE(clint_compat));
>      qemu_fdt_setprop_cells(ms->fdt, clint_name, "reg",
> -        0x0, clint_addr, 0x0, memmap[VIRT_CLINT].size);
> +        0x0, clint_addr, 0x0, s->memmap[VIRT_CLINT].size);
>      qemu_fdt_setprop(ms->fdt, clint_name, "interrupts-extended",
>          clint_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
>      riscv_socket_fdt_write_id(ms, clint_name, socket);
>  }
>
>  static void create_fdt_socket_aclint(RISCVVirtState *s,
> -                                     const MemMapEntry *memmap, int socket,
> +                                     int socket,
>                                       uint32_t *intc_phandles)
>  {
>      int cpu;
> @@ -380,8 +380,10 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
>      aclint_cells_size = s->soc[socket].num_harts * sizeof(uint32_t) * 2;
>
>      if (s->aia_type != VIRT_AIA_TYPE_APLIC_IMSIC) {
> -        addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket);
> +        addr = s->memmap[VIRT_CLINT].base +
> +               (s->memmap[VIRT_CLINT].size * socket);
>          name = g_strdup_printf("/soc/mswi@%lx", addr);
> +
>          qemu_fdt_add_subnode(ms->fdt, name);
>          qemu_fdt_setprop_string(ms->fdt, name, "compatible",
>              "riscv,aclint-mswi");
> @@ -396,13 +398,13 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
>      }
>
>      if (s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
> -        addr = memmap[VIRT_CLINT].base +
> +        addr = s->memmap[VIRT_CLINT].base +
>                 (RISCV_ACLINT_DEFAULT_MTIMER_SIZE * socket);
>          size = RISCV_ACLINT_DEFAULT_MTIMER_SIZE;
>      } else {
> -        addr = memmap[VIRT_CLINT].base + RISCV_ACLINT_SWI_SIZE +
> -            (memmap[VIRT_CLINT].size * socket);
> -        size = memmap[VIRT_CLINT].size - RISCV_ACLINT_SWI_SIZE;
> +        addr = s->memmap[VIRT_CLINT].base + RISCV_ACLINT_SWI_SIZE +
> +               (s->memmap[VIRT_CLINT].size * socket);
> +        size = s->memmap[VIRT_CLINT].size - RISCV_ACLINT_SWI_SIZE;
>      }
>      name = g_strdup_printf("/soc/mtimer@%lx", addr);
>      qemu_fdt_add_subnode(ms->fdt, name);
> @@ -419,14 +421,15 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
>      g_free(name);
>
>      if (s->aia_type != VIRT_AIA_TYPE_APLIC_IMSIC) {
> -        addr = memmap[VIRT_ACLINT_SSWI].base +
> -            (memmap[VIRT_ACLINT_SSWI].size * socket);
> +        addr = s->memmap[VIRT_ACLINT_SSWI].base +
> +               (s->memmap[VIRT_ACLINT_SSWI].size * socket);
> +
>          name = g_strdup_printf("/soc/sswi@%lx", addr);
>          qemu_fdt_add_subnode(ms->fdt, name);
>          qemu_fdt_setprop_string(ms->fdt, name, "compatible",
>              "riscv,aclint-sswi");
>          qemu_fdt_setprop_cells(ms->fdt, name, "reg",
> -            0x0, addr, 0x0, memmap[VIRT_ACLINT_SSWI].size);
> +            0x0, addr, 0x0, s->memmap[VIRT_ACLINT_SSWI].size);
>          qemu_fdt_setprop(ms->fdt, name, "interrupts-extended",
>              aclint_sswi_cells, aclint_cells_size);
>          qemu_fdt_setprop(ms->fdt, name, "interrupt-controller", NULL, 0);
> @@ -437,7 +440,7 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
>  }
>
>  static void create_fdt_socket_plic(RISCVVirtState *s,
> -                                   const MemMapEntry *memmap, int socket,
> +                                   int socket,
>                                     uint32_t *phandle, uint32_t *intc_phandles,
>                                     uint32_t *plic_phandles)
>  {
> @@ -451,7 +454,8 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
>      };
>
>      plic_phandles[socket] = (*phandle)++;
> -    plic_addr = memmap[VIRT_PLIC].base + (memmap[VIRT_PLIC].size * socket);
> +    plic_addr = s->memmap[VIRT_PLIC].base +
> +                (s->memmap[VIRT_PLIC].size * socket);
>      plic_name = g_strdup_printf("/soc/plic@%lx", plic_addr);
>      qemu_fdt_add_subnode(ms->fdt, plic_name);
>      qemu_fdt_setprop_cell(ms->fdt, plic_name,
> @@ -490,7 +494,7 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
>      }
>
>      qemu_fdt_setprop_cells(ms->fdt, plic_name, "reg",
> -        0x0, plic_addr, 0x0, memmap[VIRT_PLIC].size);
> +        0x0, plic_addr, 0x0, s->memmap[VIRT_PLIC].size);
>      qemu_fdt_setprop_cell(ms->fdt, plic_name, "riscv,ndev",
>                            VIRT_IRQCHIP_NUM_SOURCES - 1);
>      riscv_socket_fdt_write_id(ms, plic_name, socket);
> @@ -499,8 +503,8 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
>
>      if (!socket) {
>          platform_bus_add_all_fdt_nodes(ms->fdt, plic_name,
> -                                       memmap[VIRT_PLATFORM_BUS].base,
> -                                       memmap[VIRT_PLATFORM_BUS].size,
> +                                       s->memmap[VIRT_PLATFORM_BUS].base,
> +                                       s->memmap[VIRT_PLATFORM_BUS].size,
>                                         VIRT_PLATFORM_BUS_IRQ);
>      }
>  }
> @@ -587,7 +591,7 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
>      qemu_fdt_setprop_cell(ms->fdt, imsic_name, "phandle", msi_phandle);
>  }
>
> -static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
> +static void create_fdt_imsic(RISCVVirtState *s,
>                               uint32_t *phandle, uint32_t *intc_phandles,
>                               uint32_t *msi_m_phandle, uint32_t *msi_s_phandle)
>  {
> @@ -596,12 +600,12 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
>
>      if (!kvm_enabled()) {
>          /* M-level IMSIC node */
> -        create_fdt_one_imsic(s, memmap[VIRT_IMSIC_M].base, intc_phandles,
> +        create_fdt_one_imsic(s, s->memmap[VIRT_IMSIC_M].base, intc_phandles,
>                               *msi_m_phandle, true, 0);
>      }
>
>      /* S-level IMSIC node */
> -    create_fdt_one_imsic(s, memmap[VIRT_IMSIC_S].base, intc_phandles,
> +    create_fdt_one_imsic(s, s->memmap[VIRT_IMSIC_S].base, intc_phandles,
>                           *msi_s_phandle, false,
>                           imsic_num_bits(s->aia_guests + 1));
>
> @@ -678,7 +682,7 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket,
>  }
>
>  static void create_fdt_socket_aplic(RISCVVirtState *s,
> -                                    const MemMapEntry *memmap, int socket,
> +                                    int socket,
>                                      uint32_t msi_m_phandle,
>                                      uint32_t msi_s_phandle,
>                                      uint32_t *phandle,
> @@ -695,18 +699,19 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
>
>      if (!kvm_enabled()) {
>          /* M-level APLIC node */
> -        aplic_addr = memmap[VIRT_APLIC_M].base +
> -                     (memmap[VIRT_APLIC_M].size * socket);
> -        create_fdt_one_aplic(s, socket, aplic_addr, memmap[VIRT_APLIC_M].size,
> +        aplic_addr = s->memmap[VIRT_APLIC_M].base +
> +                     (s->memmap[VIRT_APLIC_M].size * socket);
> +        create_fdt_one_aplic(s, socket, aplic_addr,
> +                             s->memmap[VIRT_APLIC_M].size,
>                               msi_m_phandle, intc_phandles,
>                               aplic_m_phandle, aplic_s_phandle,
>                               true, num_harts);
>      }
>
>      /* S-level APLIC node */
> -    aplic_addr = memmap[VIRT_APLIC_S].base +
> -                 (memmap[VIRT_APLIC_S].size * socket);
> -    create_fdt_one_aplic(s, socket, aplic_addr, memmap[VIRT_APLIC_S].size,
> +    aplic_addr = s->memmap[VIRT_APLIC_S].base +
> +                 (s->memmap[VIRT_APLIC_S].size * socket);
> +    create_fdt_one_aplic(s, socket, aplic_addr, s->memmap[VIRT_APLIC_S].size,
>                           msi_s_phandle, intc_phandles,
>                           aplic_s_phandle, 0,
>                           false, num_harts);
> @@ -714,8 +719,8 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
>      if (!socket) {
>          g_autofree char *aplic_name = fdt_get_aplic_nodename(aplic_addr);
>          platform_bus_add_all_fdt_nodes(ms->fdt, aplic_name,
> -                                       memmap[VIRT_PLATFORM_BUS].base,
> -                                       memmap[VIRT_PLATFORM_BUS].size,
> +                                       s->memmap[VIRT_PLATFORM_BUS].base,
> +                                       s->memmap[VIRT_PLATFORM_BUS].size,
>                                         VIRT_PLATFORM_BUS_IRQ);
>      }
>
> @@ -733,7 +738,7 @@ static void create_fdt_pmu(RISCVVirtState *s)
>      riscv_pmu_generate_fdt_node(ms->fdt, hart.pmu_avail_ctrs, pmu_name);
>  }
>
> -static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
> +static void create_fdt_sockets(RISCVVirtState *s,
>                                 uint32_t *phandle,
>                                 uint32_t *irq_mmio_phandle,
>                                 uint32_t *irq_pcie_phandle,
> @@ -769,20 +774,20 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
>          create_fdt_socket_cpus(s, socket, clust_name, phandle,
>                                 &intc_phandles[phandle_pos]);
>
> -        create_fdt_socket_memory(s, memmap, socket);
> +        create_fdt_socket_memory(s, socket);
>
>          if (virt_aclint_allowed() && s->have_aclint) {
> -            create_fdt_socket_aclint(s, memmap, socket,
> +            create_fdt_socket_aclint(s, socket,
>                                       &intc_phandles[phandle_pos]);
>          } else if (tcg_enabled()) {
> -            create_fdt_socket_clint(s, memmap, socket,
> +            create_fdt_socket_clint(s, socket,
>                                      &intc_phandles[phandle_pos]);
>          }
>      }
>
>      if (s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
> -        create_fdt_imsic(s, memmap, phandle, intc_phandles,
> -            &msi_m_phandle, &msi_s_phandle);
> +        create_fdt_imsic(s, phandle, intc_phandles,
> +                         &msi_m_phandle, &msi_s_phandle);
>          *msi_pcie_phandle = msi_s_phandle;
>      }
>
> @@ -791,7 +796,7 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
>       * mode, we'll use only one APLIC instance.
>       */
>      if (!virt_use_emulated_aplic(s->aia_type)) {
> -        create_fdt_socket_aplic(s, memmap, 0,
> +        create_fdt_socket_aplic(s, 0,
>                                  msi_m_phandle, msi_s_phandle, phandle,
>                                  &intc_phandles[0], xplic_phandles,
>                                  ms->smp.cpus);
> @@ -805,11 +810,11 @@ static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
>              phandle_pos -= s->soc[socket].num_harts;
>
>              if (s->aia_type == VIRT_AIA_TYPE_NONE) {
> -                create_fdt_socket_plic(s, memmap, socket, phandle,
> +                create_fdt_socket_plic(s, socket, phandle,
>                                         &intc_phandles[phandle_pos],
>                                         xplic_phandles);
>              } else {
> -                create_fdt_socket_aplic(s, memmap, socket,
> +                create_fdt_socket_aplic(s, socket,
>                                          msi_m_phandle, msi_s_phandle, phandle,
>                                          &intc_phandles[phandle_pos],
>                                          xplic_phandles,
> @@ -1125,7 +1130,7 @@ static void finalize_fdt(RISCVVirtState *s)
>      uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
>      uint32_t iommu_sys_phandle = 1;
>
> -    create_fdt_sockets(s, s->memmap, &phandle, &irq_mmio_phandle,
> +    create_fdt_sockets(s, &phandle, &irq_mmio_phandle,
>                         &irq_pcie_phandle, &irq_virtio_phandle,
>                         &msi_pcie_phandle);
>
> --
> 2.49.0
>
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 7/9] hw/riscv/virt.c: use s->memmap in create_fdt_virtio()
  2025-04-29 12:58 ` [PATCH v2 7/9] hw/riscv/virt.c: use s->memmap in create_fdt_virtio() Daniel Henrique Barboza
@ 2025-05-01  0:11   ` Alistair Francis
  0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2025-05-01  0:11 UTC (permalink / raw)
  To: Daniel Henrique Barboza
  Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
	palmer, joel

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
>
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 8/9] hw/riscv/virt.c: use s->memmap in finalize_fdt() functions
  2025-04-29 12:58 ` [PATCH v2 8/9] hw/riscv/virt.c: use s->memmap in finalize_fdt() functions Daniel Henrique Barboza
@ 2025-05-01  0:12   ` Alistair Francis
  0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2025-05-01  0:12 UTC (permalink / raw)
  To: Daniel Henrique Barboza
  Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
	palmer, joel

On Tue, Apr 29, 2025 at 11:01 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Change create_fdt_pcie(), create_fdt_reset(), create_fdt_uart() and
> create_fdt_rtc() to use s->memmap in their logic.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  hw/riscv/virt.c | 44 ++++++++++++++++++++++----------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 46ac42058e..f38b64d836 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -871,7 +871,7 @@ static void create_fdt_virtio(RISCVVirtState *s, uint32_t irq_virtio_phandle)
>      }
>  }
>
> -static void create_fdt_pcie(RISCVVirtState *s, const MemMapEntry *memmap,
> +static void create_fdt_pcie(RISCVVirtState *s,
>                              uint32_t irq_pcie_phandle,
>                              uint32_t msi_pcie_phandle,
>                              uint32_t iommu_sys_phandle)
> @@ -880,7 +880,7 @@ static void create_fdt_pcie(RISCVVirtState *s, const MemMapEntry *memmap,
>      MachineState *ms = MACHINE(s);
>
>      name = g_strdup_printf("/soc/pci@%lx",
> -        (long) memmap[VIRT_PCIE_ECAM].base);
> +        (long) s->memmap[VIRT_PCIE_ECAM].base);
>      qemu_fdt_setprop_cell(ms->fdt, name, "#address-cells",
>          FDT_PCI_ADDR_CELLS);
>      qemu_fdt_setprop_cell(ms->fdt, name, "#interrupt-cells",
> @@ -891,19 +891,19 @@ static void create_fdt_pcie(RISCVVirtState *s, const MemMapEntry *memmap,
>      qemu_fdt_setprop_string(ms->fdt, name, "device_type", "pci");
>      qemu_fdt_setprop_cell(ms->fdt, name, "linux,pci-domain", 0);
>      qemu_fdt_setprop_cells(ms->fdt, name, "bus-range", 0,
> -        memmap[VIRT_PCIE_ECAM].size / PCIE_MMCFG_SIZE_MIN - 1);
> +        s->memmap[VIRT_PCIE_ECAM].size / PCIE_MMCFG_SIZE_MIN - 1);
>      qemu_fdt_setprop(ms->fdt, name, "dma-coherent", NULL, 0);
>      if (s->aia_type == VIRT_AIA_TYPE_APLIC_IMSIC) {
>          qemu_fdt_setprop_cell(ms->fdt, name, "msi-parent", msi_pcie_phandle);
>      }
>      qemu_fdt_setprop_cells(ms->fdt, name, "reg", 0,
> -        memmap[VIRT_PCIE_ECAM].base, 0, memmap[VIRT_PCIE_ECAM].size);
> +        s->memmap[VIRT_PCIE_ECAM].base, 0, s->memmap[VIRT_PCIE_ECAM].size);
>      qemu_fdt_setprop_sized_cells(ms->fdt, name, "ranges",
>          1, FDT_PCI_RANGE_IOPORT, 2, 0,
> -        2, memmap[VIRT_PCIE_PIO].base, 2, memmap[VIRT_PCIE_PIO].size,
> +        2, s->memmap[VIRT_PCIE_PIO].base, 2, s->memmap[VIRT_PCIE_PIO].size,
>          1, FDT_PCI_RANGE_MMIO,
> -        2, memmap[VIRT_PCIE_MMIO].base,
> -        2, memmap[VIRT_PCIE_MMIO].base, 2, memmap[VIRT_PCIE_MMIO].size,
> +        2, s->memmap[VIRT_PCIE_MMIO].base,
> +        2, s->memmap[VIRT_PCIE_MMIO].base, 2, s->memmap[VIRT_PCIE_MMIO].size,
>          1, FDT_PCI_RANGE_MMIO_64BIT,
>          2, virt_high_pcie_memmap.base,
>          2, virt_high_pcie_memmap.base, 2, virt_high_pcie_memmap.size);
> @@ -917,8 +917,7 @@ static void create_fdt_pcie(RISCVVirtState *s, const MemMapEntry *memmap,
>      create_pcie_irq_map(s, ms->fdt, name, irq_pcie_phandle);
>  }
>
> -static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
> -                             uint32_t *phandle)
> +static void create_fdt_reset(RISCVVirtState *s, uint32_t *phandle)
>  {
>      char *name;
>      uint32_t test_phandle;
> @@ -926,7 +925,7 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
>
>      test_phandle = (*phandle)++;
>      name = g_strdup_printf("/soc/test@%lx",
> -        (long)memmap[VIRT_TEST].base);
> +        (long)s->memmap[VIRT_TEST].base);
>      qemu_fdt_add_subnode(ms->fdt, name);
>      {
>          static const char * const compat[3] = {
> @@ -936,7 +935,7 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
>                                        (char **)&compat, ARRAY_SIZE(compat));
>      }
>      qemu_fdt_setprop_cells(ms->fdt, name, "reg",
> -        0x0, memmap[VIRT_TEST].base, 0x0, memmap[VIRT_TEST].size);
> +        0x0, s->memmap[VIRT_TEST].base, 0x0, s->memmap[VIRT_TEST].size);
>      qemu_fdt_setprop_cell(ms->fdt, name, "phandle", test_phandle);
>      test_phandle = qemu_fdt_get_phandle(ms->fdt, name);
>      g_free(name);
> @@ -958,18 +957,19 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
>      g_free(name);
>  }
>
> -static void create_fdt_uart(RISCVVirtState *s, const MemMapEntry *memmap,
> +static void create_fdt_uart(RISCVVirtState *s,
>                              uint32_t irq_mmio_phandle)
>  {
>      g_autofree char *name = NULL;
>      MachineState *ms = MACHINE(s);
>
> -    name = g_strdup_printf("/soc/serial@%lx", (long)memmap[VIRT_UART0].base);
> +    name = g_strdup_printf("/soc/serial@%lx",
> +                           (long)s->memmap[VIRT_UART0].base);
>      qemu_fdt_add_subnode(ms->fdt, name);
>      qemu_fdt_setprop_string(ms->fdt, name, "compatible", "ns16550a");
>      qemu_fdt_setprop_cells(ms->fdt, name, "reg",
> -        0x0, memmap[VIRT_UART0].base,
> -        0x0, memmap[VIRT_UART0].size);
> +        0x0, s->memmap[VIRT_UART0].base,
> +        0x0, s->memmap[VIRT_UART0].size);
>      qemu_fdt_setprop_cell(ms->fdt, name, "clock-frequency", 3686400);
>      qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent", irq_mmio_phandle);
>      if (s->aia_type == VIRT_AIA_TYPE_NONE) {
> @@ -982,18 +982,18 @@ static void create_fdt_uart(RISCVVirtState *s, const MemMapEntry *memmap,
>      qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial0", name);
>  }
>
> -static void create_fdt_rtc(RISCVVirtState *s, const MemMapEntry *memmap,
> +static void create_fdt_rtc(RISCVVirtState *s,
>                             uint32_t irq_mmio_phandle)
>  {
>      g_autofree char *name = NULL;
>      MachineState *ms = MACHINE(s);
>
> -    name = g_strdup_printf("/soc/rtc@%lx", (long)memmap[VIRT_RTC].base);
> +    name = g_strdup_printf("/soc/rtc@%lx", (long)s->memmap[VIRT_RTC].base);
>      qemu_fdt_add_subnode(ms->fdt, name);
>      qemu_fdt_setprop_string(ms->fdt, name, "compatible",
>          "google,goldfish-rtc");
>      qemu_fdt_setprop_cells(ms->fdt, name, "reg",
> -        0x0, memmap[VIRT_RTC].base, 0x0, memmap[VIRT_RTC].size);
> +        0x0, s->memmap[VIRT_RTC].base, 0x0, s->memmap[VIRT_RTC].size);
>      qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent",
>          irq_mmio_phandle);
>      if (s->aia_type == VIRT_AIA_TYPE_NONE) {
> @@ -1143,14 +1143,14 @@ static void finalize_fdt(RISCVVirtState *s)
>          create_fdt_iommu_sys(s, irq_mmio_phandle, msi_pcie_phandle,
>                               &iommu_sys_phandle);
>      }
> -    create_fdt_pcie(s, s->memmap, irq_pcie_phandle, msi_pcie_phandle,
> +    create_fdt_pcie(s, irq_pcie_phandle, msi_pcie_phandle,
>                      iommu_sys_phandle);
>
> -    create_fdt_reset(s, s->memmap, &phandle);
> +    create_fdt_reset(s, &phandle);
>
> -    create_fdt_uart(s, s->memmap, irq_mmio_phandle);
> +    create_fdt_uart(s, irq_mmio_phandle);
>
> -    create_fdt_rtc(s, s->memmap, irq_mmio_phandle);
> +    create_fdt_rtc(s, irq_mmio_phandle);
>  }
>
>  static void create_fdt(RISCVVirtState *s)
> --
> 2.49.0
>
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 9/9] hw/riscv/virt.c: remove 'long' casts in fmt strings
  2025-04-29 12:58 ` [PATCH v2 9/9] hw/riscv/virt.c: remove 'long' casts in fmt strings Daniel Henrique Barboza
@ 2025-05-01  0:13   ` Alistair Francis
  0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2025-05-01  0:13 UTC (permalink / raw)
  To: Daniel Henrique Barboza
  Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
	palmer, joel, Philippe Mathieu-Daudé

On Tue, Apr 29, 2025 at 11:01 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> We can avoid the 'long' casts by using PRIx64 and HWADDR_PRIx on the fmt
> strings for uint64_t and hwaddr types.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

Alistair

> ---
>  hw/riscv/virt.c | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index f38b64d836..0020d8f404 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -303,12 +303,13 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
>  static void create_fdt_socket_memory(RISCVVirtState *s, int socket)
>  {
>      g_autofree char *mem_name = NULL;
> -    uint64_t addr, size;
> +    hwaddr addr;
> +    uint64_t size;
>      MachineState *ms = MACHINE(s);
>
>      addr = s->memmap[VIRT_DRAM].base + riscv_socket_mem_offset(ms, socket);
>      size = riscv_socket_mem_size(ms, socket);
> -    mem_name = g_strdup_printf("/memory@%lx", (long)addr);
> +    mem_name = g_strdup_printf("/memory@%"HWADDR_PRIx, addr);
>      qemu_fdt_add_subnode(ms->fdt, mem_name);
>      qemu_fdt_setprop_cells(ms->fdt, mem_name, "reg",
>          addr >> 32, addr, size >> 32, size);
> @@ -879,8 +880,8 @@ static void create_fdt_pcie(RISCVVirtState *s,
>      g_autofree char *name = NULL;
>      MachineState *ms = MACHINE(s);
>
> -    name = g_strdup_printf("/soc/pci@%lx",
> -        (long) s->memmap[VIRT_PCIE_ECAM].base);
> +    name = g_strdup_printf("/soc/pci@%"HWADDR_PRIx,
> +                           s->memmap[VIRT_PCIE_ECAM].base);
>      qemu_fdt_setprop_cell(ms->fdt, name, "#address-cells",
>          FDT_PCI_ADDR_CELLS);
>      qemu_fdt_setprop_cell(ms->fdt, name, "#interrupt-cells",
> @@ -924,8 +925,8 @@ static void create_fdt_reset(RISCVVirtState *s, uint32_t *phandle)
>      MachineState *ms = MACHINE(s);
>
>      test_phandle = (*phandle)++;
> -    name = g_strdup_printf("/soc/test@%lx",
> -        (long)s->memmap[VIRT_TEST].base);
> +    name = g_strdup_printf("/soc/test@%"HWADDR_PRIx,
> +                           s->memmap[VIRT_TEST].base);
>      qemu_fdt_add_subnode(ms->fdt, name);
>      {
>          static const char * const compat[3] = {
> @@ -963,8 +964,8 @@ static void create_fdt_uart(RISCVVirtState *s,
>      g_autofree char *name = NULL;
>      MachineState *ms = MACHINE(s);
>
> -    name = g_strdup_printf("/soc/serial@%lx",
> -                           (long)s->memmap[VIRT_UART0].base);
> +    name = g_strdup_printf("/soc/serial@%"HWADDR_PRIx,
> +                           s->memmap[VIRT_UART0].base);
>      qemu_fdt_add_subnode(ms->fdt, name);
>      qemu_fdt_setprop_string(ms->fdt, name, "compatible", "ns16550a");
>      qemu_fdt_setprop_cells(ms->fdt, name, "reg",
> @@ -988,7 +989,8 @@ static void create_fdt_rtc(RISCVVirtState *s,
>      g_autofree char *name = NULL;
>      MachineState *ms = MACHINE(s);
>
> -    name = g_strdup_printf("/soc/rtc@%lx", (long)s->memmap[VIRT_RTC].base);
> +    name = g_strdup_printf("/soc/rtc@%"HWADDR_PRIx,
> +                           s->memmap[VIRT_RTC].base);
>      qemu_fdt_add_subnode(ms->fdt, name);
>      qemu_fdt_setprop_string(ms->fdt, name, "compatible",
>          "google,goldfish-rtc");
> @@ -1041,8 +1043,8 @@ static void create_fdt_virtio_iommu(RISCVVirtState *s, uint16_t bdf)
>      g_autofree char *iommu_node = NULL;
>      g_autofree char *pci_node = NULL;
>
> -    pci_node = g_strdup_printf("/soc/pci@%lx",
> -                               (long) s->memmap[VIRT_PCIE_ECAM].base);
> +    pci_node = g_strdup_printf("/soc/pci@%"HWADDR_PRIx,
> +                               s->memmap[VIRT_PCIE_ECAM].base);
>      iommu_node = g_strdup_printf("%s/virtio_iommu@%x,%x", pci_node,
>                                   PCI_SLOT(bdf), PCI_FUNC(bdf));
>      iommu_phandle = qemu_fdt_alloc_phandle(fdt);
> @@ -1110,8 +1112,8 @@ static void create_fdt_iommu(RISCVVirtState *s, uint16_t bdf)
>      g_autofree char *iommu_node = NULL;
>      g_autofree char *pci_node = NULL;
>
> -    pci_node = g_strdup_printf("/soc/pci@%lx",
> -                               (long) s->memmap[VIRT_PCIE_ECAM].base);
> +    pci_node = g_strdup_printf("/soc/pci@%"HWADDR_PRIx,
> +                               s->memmap[VIRT_PCIE_ECAM].base);
>      iommu_node = g_strdup_printf("%s/iommu@%x", pci_node, bdf);
>      iommu_phandle = qemu_fdt_alloc_phandle(fdt);
>      qemu_fdt_add_subnode(fdt, iommu_node);
> @@ -1180,8 +1182,8 @@ static void create_fdt(RISCVVirtState *s)
>       * The "/soc/pci@..." node is needed for PCIE hotplugs
>       * that might happen before finalize_fdt().
>       */
> -    name = g_strdup_printf("/soc/pci@%lx",
> -                           (long) s->memmap[VIRT_PCIE_ECAM].base);
> +    name = g_strdup_printf("/soc/pci@%"HWADDR_PRIx,
> +                           s->memmap[VIRT_PCIE_ECAM].base);
>      qemu_fdt_add_subnode(ms->fdt, name);
>
>      qemu_fdt_add_subnode(ms->fdt, "/chosen");
> --
> 2.49.0
>
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup
  2025-04-29 12:58 [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Daniel Henrique Barboza
                   ` (8 preceding siblings ...)
  2025-04-29 12:58 ` [PATCH v2 9/9] hw/riscv/virt.c: remove 'long' casts in fmt strings Daniel Henrique Barboza
@ 2025-05-01  0:33 ` Alistair Francis
  9 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2025-05-01  0:33 UTC (permalink / raw)
  To: Daniel Henrique Barboza
  Cc: qemu-devel, qemu-riscv, alistair.francis, liwei1518, zhiwei_liu,
	palmer, joel

On Tue, Apr 29, 2025 at 11:00 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Hi,
>
> In this new version a small change was made in patch 9 after
> discussions with Joel during v1 [1]. The idea is that we want to be
> consistent (as close as we can) with the idea that a
> memory address is a hwaddr type.
>
> This new version does not conflict with "hw/riscv/virt: device tree reg
> cleanups" from Joel [2].
>
> No other changes made. Patches based on alistair/riscv-to-apply.next.
>
> Changes from v2:
> - patch 9:
>   - in create_fdt_socket_memory(), change 'addr' to hwaddr and use the
>     HWADDR_PRIx fmt type
> - v1 link: https://lore.kernel.org/qemu-riscv/20250423110630.2249904-1-dbarboza@ventanamicro.com/
>
> [1] https://lore.kernel.org/qemu-riscv/d404d535-fc04-43ac-a7a7-2f216cad993c@ventanamicro.com/
> [2] https://lore.kernel.org/qemu-riscv/20250429061223.1457166-1-joel@jms.id.au/
>
> Daniel Henrique Barboza (9):
>   hw/riscv/virt.c: enforce s->memmap use in machine_init()
>   hw/riscv/virt.c: remove trivial virt_memmap references
>   hw/riscv/virt.c: use s->memmap in virt_machine_done()
>   hw/riscv/virt.c: add 'base' arg in create_fw_cfg()
>   hw/riscv/virt.c: use s->memmap in create_fdt() path
>   hw/riscv/virt.c: use s->memmap in create_fdt_sockets() path
>   hw/riscv/virt.c: use s->memmap in create_fdt_virtio()
>   hw/riscv/virt.c: use s->memmap in finalize_fdt() functions
>   hw/riscv/virt.c: remove 'long' casts in fmt strings

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  hw/riscv/virt.c | 272 +++++++++++++++++++++++++-----------------------
>  1 file changed, 140 insertions(+), 132 deletions(-)
>
> --
> 2.49.0
>
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2025-05-01  0:34 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-29 12:58 [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Daniel Henrique Barboza
2025-04-29 12:58 ` [PATCH v2 1/9] hw/riscv/virt.c: enforce s->memmap use in machine_init() Daniel Henrique Barboza
2025-04-29 12:58 ` [PATCH v2 2/9] hw/riscv/virt.c: remove trivial virt_memmap references Daniel Henrique Barboza
2025-04-30 23:59   ` Alistair Francis
2025-04-29 12:58 ` [PATCH v2 3/9] hw/riscv/virt.c: use s->memmap in virt_machine_done() Daniel Henrique Barboza
2025-05-01  0:02   ` Alistair Francis
2025-04-29 12:58 ` [PATCH v2 4/9] hw/riscv/virt.c: add 'base' arg in create_fw_cfg() Daniel Henrique Barboza
2025-05-01  0:03   ` Alistair Francis
2025-04-29 12:58 ` [PATCH v2 5/9] hw/riscv/virt.c: use s->memmap in create_fdt() path Daniel Henrique Barboza
2025-05-01  0:08   ` Alistair Francis
2025-04-29 12:58 ` [PATCH v2 6/9] hw/riscv/virt.c: use s->memmap in create_fdt_sockets() path Daniel Henrique Barboza
2025-05-01  0:10   ` Alistair Francis
2025-04-29 12:58 ` [PATCH v2 7/9] hw/riscv/virt.c: use s->memmap in create_fdt_virtio() Daniel Henrique Barboza
2025-05-01  0:11   ` Alistair Francis
2025-04-29 12:58 ` [PATCH v2 8/9] hw/riscv/virt.c: use s->memmap in finalize_fdt() functions Daniel Henrique Barboza
2025-05-01  0:12   ` Alistair Francis
2025-04-29 12:58 ` [PATCH v2 9/9] hw/riscv/virt.c: remove 'long' casts in fmt strings Daniel Henrique Barboza
2025-05-01  0:13   ` Alistair Francis
2025-05-01  0:33 ` [PATCH v2 0/9] hw/riscv/virt.c: memmap usage cleanup Alistair Francis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).