On Tue, Jan 27, 2015 at 01:55:32PM +0000, Peter Maydell wrote:
> On 21 January 2015 at 16:18, Alexander Graf <ag...@suse.de> wrote:
> > The mmcfg space is a memory region that allows access to PCI config space
> > in the PCIe world. To maintain abstraction layers, I would like to expose
> > the mmcfg space as a sysbus mmio region rather than have it mapped straight
> > into the system's memory address space though.
> >
> > So this patch splits the initialization of the mmcfg space from the actual
> > mapping, allowing us to only have an mmfg memory region without the map.
> >
> > Signed-off-by: Alexander Graf <ag...@suse.de>
> > Reviewed-by: Claudio Fontana <claudio.font...@huawei.com>
> > Tested-by: Claudio Fontana <claudio.font...@huawei.com>
>
> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
> ...as far as it goes, but:
>
> Really the pcie_host_mmcfg_map/unmap/update() function is just totally
> misguided. This functionality should be pushed upwards into
> hw/pci-host/q35.c which can handle its own mapping of the MMIO region
> into the system address space at the appropriate location/size.
>
> In particular, at the moment q35.c will leak a bunch of stuff
> every time the guest unmaps and remaps the mmcfg space, because
> we call memory_region_init_io() over and over again on the same
> MMIO object (which isn't valid).
I used to be fine before the QOM conversion I think?
Take a look at this one (and previous patch):
commit 469b046ead0671932ff3af8d6f95045b19b186ef
Author: Paolo Bonzini <pbonz...@redhat.com>
Date: Wed Jun 11 12:50:43 2014 +0200
memory: remove memory_region_destroy
The function is empty after the previous patch, so remove it.
Reviewed-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
>
> Any time you see a device with its own base address in its
> device struct it's a red flag that the design's probably wrong...
I suspect this is not the only device that leaks memory now.
Paolo?
> The size of the MMCFG region should probably be a device property.
> Then the subclass realize could just rely on the baseclass realize
> to always create the mmio region, rather than having to explicitly
> call a function to get it to do the right thing.
>
> thanks
> -- PMM
--
MST
