Re: [PATCH 2/4] hw/pci-host/articia: Map PCI memory windows in realize

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

On 25/10/25 01:31, BALATON Zoltan wrote:
> These memory windows are a result of the address decoding in the
> Articia S north bridge so better model it there and not in board code.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/pci-host/articia.c | 15 ++++++++++++++-
>   hw/ppc/amigaone.c     | 28 +++++-----------------------
>   hw/ppc/pegasos2.c     | 13 -------------
>   3 files changed, 19 insertions(+), 37 deletions(-)


> @@ -169,6 +174,7 @@ static void articia_realize(DeviceState *dev, Error **errp)
>   {
>       ArticiaState *s = ARTICIA(dev);
>       PCIHostState *h = PCI_HOST_BRIDGE(dev);
> +    MemoryRegion *mr;
>       PCIDevice *pdev;
>   
>       bitbang_i2c_init(&s->smbus, i2c_init_bus(dev, "smbus"));
> @@ -180,6 +186,14 @@ static void articia_realize(DeviceState *dev, Error **errp)
>       memory_region_init_io(&s->reg, OBJECT(s), &articia_reg_ops, s,
>                             TYPE_ARTICIA, 0x1000000);
>       memory_region_add_subregion_overlap(&s->reg, 0, &s->io, 1);
> +    mr = g_new(MemoryRegion, 1);

Won't Coverity or other analysis tools complain about the leak?
(this is why we usually keep a reference in the device state, here
ArticiaState). Otherwise:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +    memory_region_init_alias(mr, OBJECT(dev), "pci-mem-low", &s->mem,
> +                             0, PCI_LOW_SIZE);
> +    memory_region_add_subregion(get_system_memory(), PCI_LOW_ADDR, mr);
> +    mr = g_new(MemoryRegion, 1);
> +    memory_region_init_alias(mr, OBJECT(dev), "pci-mem-high", &s->mem,
> +                             PCI_HIGH_ADDR, PCI_HIGH_SIZE);
> +    memory_region_add_subregion(get_system_memory(), PCI_HIGH_ADDR, mr);