Re: [PATCH 11/16] hw/pci-host/raven: Do not use parent object for mmcfg region

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 4/5/25 18:01, BALATON Zoltan wrote:
> The mmcfg field in PCIHostState is only used by raven for the PCI
> config direct access but is not actually needed as the memory region
> lifetime can be managed by the object given during init so use that
> and remove the unused field from PCIHostState.
> 

Well, this is the recommended way to avoid leaking MemoryRegions.

If QOM object allocates something, it should keep a reference to it,
allowing simpler eventual implementation of DeviceUnrealize handler.

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/pci-host/raven.c       | 7 ++++---
>   include/hw/pci/pci_host.h | 1 -
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
> index c39e95b45f..7550c291c6 100644
> --- a/hw/pci-host/raven.c
> +++ b/hw/pci-host/raven.c
> @@ -212,7 +212,7 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
>       SysBusDevice *dev = SYS_BUS_DEVICE(d);
>       PCIHostState *h = PCI_HOST_BRIDGE(dev);
>       PREPPCIState *s = RAVEN_PCI_HOST_BRIDGE(dev);
> -    MemoryRegion *address_space_mem = get_system_memory();
> +    MemoryRegion *mr, *address_space_mem = get_system_memory();
>   
>       qdev_init_gpio_in(d, raven_change_gpio, 1);
>   
> @@ -229,9 +229,10 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
>                             "pci-conf-data", 4);
>       memory_region_add_subregion(&s->pci_io, 0xcfc, &h->data_mem);
>   
> -    memory_region_init_io(&h->mmcfg, OBJECT(h), &raven_mmcfg_ops, h->bus,
> +    mr = g_new0(MemoryRegion, 1);
> +    memory_region_init_io(mr, OBJECT(h), &raven_mmcfg_ops, h->bus,
>                             "pci-mmcfg", 0x00400000);
> -    memory_region_add_subregion(address_space_mem, 0x80800000, &h->mmcfg);
> +    memory_region_add_subregion(address_space_mem, 0x80800000, mr);
>   
>       memory_region_init_io(&s->pci_intack, OBJECT(s), &raven_intack_ops, s,
>                             "pci-intack", 1);
> diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
> index e52d8ec2cd..7c0285e2ff 100644
> --- a/include/hw/pci/pci_host.h
> +++ b/include/hw/pci/pci_host.h
> @@ -41,7 +41,6 @@ struct PCIHostState {
>   
>       MemoryRegion conf_mem;
>       MemoryRegion data_mem;
> -    MemoryRegion mmcfg;
>       uint32_t config_reg;
>       bool mig_enabled;
>       PCIBus *bus;