On Sat, 6 Sep 2025, Akihiko Odaki wrote:
> raven-pcihost is not hotpluggable but its instance can still be created
> and finalized when processing the device-list-properties QMP command.
> Exposing such a temporary instance to AddressSpace should be
> avoided because it leaks the instance.
This may suggest there's an issue with freeing address spaces that should
be fixed there at one place if possible rather than adding a new rule to
remember not to create address spaces in init methods. Should adress space
be a child of the device too so it's freed with it? This series simplifies
some devices removing the rule to remember to unparent memory regions but
this now adds another rule to remember avoid creating address spaces in
init so overall we're in the same situation and still have to work around
issues. This trades one workaround for another so still cannot fix all
issues with freeing all created objects.
> Expose instances to the AddressSpace at their realization time so that
> it won't happen for the temporary instances.
I had a series here:
https://patchew.org/QEMU/cover.1751493467.git.balaton@eik.bme.hu/
that changes this for raven (among other clean ups) but I could not get
that merged in the last devel cycle because of PPC being a bit
unmaintained. I'd prefer that series to be taken first instead of this
patch so I don't have to rebase that.
Regards,
BALATON Zoltan
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
> hw/pci-host/raven.c | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
> index f8c0be5d21c351305742a696a65f70f87b546b0c..82f245c91cf267cdc6a518765a8c31d06eac7228 100644
> --- a/hw/pci-host/raven.c
> +++ b/hw/pci-host/raven.c
> @@ -233,6 +233,20 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
> MemoryRegion *address_space_mem = get_system_memory();
> int i;
>
> + address_space_init(&s->pci_io_as, &s->pci_io, "raven-io");
> +
> + /* CPU address space */
> + memory_region_add_subregion(address_space_mem, PCI_IO_BASE_ADDR,
> + &s->pci_io);
> + memory_region_add_subregion_overlap(address_space_mem, PCI_IO_BASE_ADDR,
> + &s->pci_io_non_contiguous, 1);
> + memory_region_add_subregion(address_space_mem, 0xc0000000, &s->pci_memory);
> + pci_root_bus_init(&s->pci_bus, sizeof(s->pci_bus), DEVICE(d), NULL,
> + &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
> +
> + address_space_init(&s->bm_as, &s->bm, "raven-bm");
> + pci_setup_iommu(&s->pci_bus, &raven_iommu_ops, s);
> +
> /*
> * According to PReP specification section 6.1.6 "System Interrupt
> * Assignments", all PCI interrupts are routed via IRQ 15
> @@ -276,14 +290,12 @@ static void raven_pcihost_initfn(Object *obj)
> {
> PCIHostState *h = PCI_HOST_BRIDGE(obj);
> PREPPCIState *s = RAVEN_PCI_HOST_BRIDGE(obj);
> - MemoryRegion *address_space_mem = get_system_memory();
> DeviceState *pci_dev;
>
> memory_region_init(&s->pci_io, obj, "pci-io", 0x3f800000);
> memory_region_init_io(&s->pci_io_non_contiguous, obj, &raven_io_ops, s,
> "pci-io-non-contiguous", 0x00800000);
> memory_region_init(&s->pci_memory, obj, "pci-memory", 0x3f000000);
> - address_space_init(&s->pci_io_as, &s->pci_io, "raven-io");
>
> /*
> * Raven's raven_io_ops use the address-space API to access pci-conf-idx
> @@ -292,15 +304,6 @@ static void raven_pcihost_initfn(Object *obj)
> */
> s->pci_io_non_contiguous.disable_reentrancy_guard = true;
>
> - /* CPU address space */
> - memory_region_add_subregion(address_space_mem, PCI_IO_BASE_ADDR,
> - &s->pci_io);
> - memory_region_add_subregion_overlap(address_space_mem, PCI_IO_BASE_ADDR,
> - &s->pci_io_non_contiguous, 1);
> - memory_region_add_subregion(address_space_mem, 0xc0000000, &s->pci_memory);
> - pci_root_bus_init(&s->pci_bus, sizeof(s->pci_bus), DEVICE(obj), NULL,
> - &s->pci_memory, &s->pci_io, 0, TYPE_PCI_BUS);
> -
> /* Bus master address space */
> memory_region_init(&s->bm, obj, "bm-raven", 4 * GiB);
> memory_region_init_alias(&s->bm_pci_memory_alias, obj, "bm-pci-memory",
> @@ -310,8 +313,6 @@ static void raven_pcihost_initfn(Object *obj)
> get_system_memory(), 0, 0x80000000);
> memory_region_add_subregion(&s->bm, 0 , &s->bm_pci_memory_alias);
> memory_region_add_subregion(&s->bm, 0x80000000, &s->bm_ram_alias);
> - address_space_init(&s->bm_as, &s->bm, "raven-bm");
> - pci_setup_iommu(&s->pci_bus, &raven_iommu_ops, s);
>
> h->bus = &s->pci_bus;
>
>
>