From 7a87ba8956e59bec8cc4677c6aa5141e4c023a7d Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Fri, 10 Nov 2023 14:25:59 -0300 Subject: [PATCH] hw/riscv/virt.c: do create_fdt() earlier, add finalize_fdt() Commit 49554856f0 fixed a problem, where TPM devices were not appearing in the FDT, by delaying the FDT creation up until virt_machine_done(). This create a side effect (see gitlab #1925) - devices that need access to the '/chosen' FDT node during realize() stopped working because, at that point, we don't have a FDT. This happens because our FDT creation is monolithic, but it doesn't need to be. We can add the needed FDT components for realize() time and, at the same time, do another FDT round where we account for dynamic sysbus devices. In other words, the problem fixed by 49554856f0 could also be fixed by postponing only create_fdt_sockets() and its dependencies, leaving everything else from create_fdt() to be done during init(). Split the FDT creation in two parts: - create_fdt(), now moved back to virt_machine_init(), will create FDT nodes that doesn't depend on additional (dynamic) devices from the sysbus; - a new finalize_fdt() step is added, where create_fdt_sockets() and friends is executed, accounting for the dynamic sysbus devices that were added during realize(). This will make both use cases happy: TPM devices are still working as intended, and devices such as 'guest-loader' have a FDT to work on during realize(). Fixes: 49554856f0 ("riscv: Generate devicetree only after machine initialization is complete") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1925 Signed-off-by: Daniel Henrique Barboza Reviewed-by: Alistair Francis Message-ID: <20231110172559.73209-1-dbarboza@ventanamicro.com> Signed-off-by: Alistair Francis --- hw/riscv/virt.c | 71 ++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index c7fc97e273..d2eac24156 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -962,7 +962,6 @@ static void create_fdt_uart(RISCVVirtState *s, const MemMapEntry *memmap, qemu_fdt_setprop_cells(ms->fdt, name, "interrupts", UART0_IRQ, 0x4); } - qemu_fdt_add_subnode(ms->fdt, "/chosen"); qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", name); g_free(name); } @@ -1023,11 +1022,29 @@ static void create_fdt_fw_cfg(RISCVVirtState *s, const MemMapEntry *memmap) g_free(nodename); } -static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap) +static void finalize_fdt(RISCVVirtState *s) { - MachineState *ms = MACHINE(s); uint32_t phandle = 1, irq_mmio_phandle = 1, msi_pcie_phandle = 1; uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1; + + create_fdt_sockets(s, virt_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_pcie(s, virt_memmap, irq_pcie_phandle, msi_pcie_phandle); + + create_fdt_reset(s, virt_memmap, &phandle); + + create_fdt_uart(s, virt_memmap, irq_mmio_phandle); + + create_fdt_rtc(s, virt_memmap, irq_mmio_phandle); +} + +static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap) +{ + MachineState *ms = MACHINE(s); uint8_t rng_seed[32]; ms->fdt = create_device_tree(&s->fdt_size); @@ -1047,28 +1064,16 @@ static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap) qemu_fdt_setprop_cell(ms->fdt, "/soc", "#size-cells", 0x2); qemu_fdt_setprop_cell(ms->fdt, "/soc", "#address-cells", 0x2); - create_fdt_sockets(s, memmap, &phandle, &irq_mmio_phandle, - &irq_pcie_phandle, &irq_virtio_phandle, - &msi_pcie_phandle); - - create_fdt_virtio(s, memmap, irq_virtio_phandle); - - create_fdt_pcie(s, memmap, irq_pcie_phandle, msi_pcie_phandle); - - create_fdt_reset(s, memmap, &phandle); - - create_fdt_uart(s, memmap, irq_mmio_phandle); - - create_fdt_rtc(s, memmap, irq_mmio_phandle); - - create_fdt_flash(s, memmap); - create_fdt_fw_cfg(s, memmap); - create_fdt_pmu(s); + qemu_fdt_add_subnode(ms->fdt, "/chosen"); /* Pass seed to RNG */ qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed)); qemu_fdt_setprop(ms->fdt, "/chosen", "rng-seed", rng_seed, sizeof(rng_seed)); + + create_fdt_flash(s, memmap); + create_fdt_fw_cfg(s, memmap); + create_fdt_pmu(s); } static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem, @@ -1257,15 +1262,12 @@ static void virt_machine_done(Notifier *notifier, void *data) uint64_t kernel_entry = 0; BlockBackend *pflash_blk0; - /* load/create device tree */ - if (machine->dtb) { - machine->fdt = load_device_tree(machine->dtb, &s->fdt_size); - if (!machine->fdt) { - error_report("load_device_tree() failed"); - exit(1); - } - } else { - create_fdt(s, memmap); + /* + * An user provided dtb must include everything, including + * dynamic sysbus devices. Our FDT needs to be finalized. + */ + if (machine->dtb == NULL) { + finalize_fdt(s); } /* @@ -1541,6 +1543,17 @@ static void virt_machine_init(MachineState *machine) } virt_flash_map(s, system_memory); + /* load/create device tree */ + if (machine->dtb) { + machine->fdt = load_device_tree(machine->dtb, &s->fdt_size); + if (!machine->fdt) { + error_report("load_device_tree() failed"); + exit(1); + } + } else { + create_fdt(s, memmap); + } + s->machine_done.notify = virt_machine_done; qemu_add_machine_init_done_notifier(&s->machine_done); } -- 2.11.0