----- Original Message -----
> From: "Marcel Apfelbaum" <mar...@redhat.com>
> To: qemu-devel@nongnu.org
> Cc: mar...@redhat.com, m...@redhat.com, pbonz...@redhat.com,
> ehabk...@redhat.com, pet...@redhat.com,
> davidkiar...@gmail.com, "jan kiszka" <jan.kis...@web.de>, "bd aviv"
> <bd.a...@gmail.com>, "alex williamson"
> <alex.william...@redhat.com>
> Sent: Thursday, June 2, 2016 10:15:53 PM
> Subject: [Qemu-devel] [PATCH v2 1/3] hw/pci: delay bus_master_enable_region
> initialization
>
> Skip bus_master_enable region creation on PCI device init
> in order to be sure the IOMMU device (if present) would
> be created in advance. Add this memory region at machine_done time.
>
> Signed-off-by: Marcel Apfelbaum <mar...@redhat.com>
> ---
> hw/pci/pci.c | 46 ++++++++++++++++++++++++++++++++++++----------
> include/hw/pci/pci_bus.h | 2 ++
> 2 files changed, 38 insertions(+), 10 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index bb605ef..29dcbcf 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -127,11 +127,44 @@ static void pci_bus_class_init(ObjectClass *klass, void
> *data)
> pbc->numa_node = pcibus_numa_node;
> }
>
> +static void pci_init_bus_master(PCIDevice *pci_dev)
> +{
> + AddressSpace *dma_as = pci_device_iommu_address_space(pci_dev);
> +
> + memory_region_init_alias(&pci_dev->bus_master_enable_region,
> + OBJECT(pci_dev), "bus master",
> + dma_as->root, 0,
> memory_region_size(dma_as->root));
> + memory_region_set_enabled(&pci_dev->bus_master_enable_region, false);
> + address_space_init(&pci_dev->bus_master_as,
> + &pci_dev->bus_master_enable_region, pci_dev->name);
> +}
> +
> +static void pcibus_machine_done(Notifier *notifier, void *data)
> +{
> + PCIBus *bus = container_of(notifier, PCIBus, machine_done);
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
> + if (bus->devices[i]) {
> + pci_init_bus_master(bus->devices[i]);
> + }
> + }
> +}
> +
> +static void pcibus_initfn(Object *obj)
> +{
> + PCIBus *bus = PCI_BUS(obj);
> +
> + bus->machine_done.notify = pcibus_machine_done;
> + qemu_add_machine_init_done_notifier(&bus->machine_done);
> +}
This should be done at realize time, otherwise
object_unref(object_new(TYPE_PCI_BUS));
leaves a dangling reference.
On one hand I think it's fair to assume that there's no unrealize
between realize and machine done; if something fails to realize
QEMU will exit.
On the other hand it might break in weird ways in the future.
So if you could add a qemu_remove_machine_init_done_notifier and
call it from bus unrealize, it would be best.
Thanks,
Paolo
> static const TypeInfo pci_bus_info = {
> .name = TYPE_PCI_BUS,
> .parent = TYPE_BUS,
> .instance_size = sizeof(PCIBus),
> .class_size = sizeof(PCIBusClass),
> + .instance_init = pcibus_initfn,
> .class_init = pci_bus_class_init,
> };
>
> @@ -845,7 +878,6 @@ static PCIDevice *do_pci_register_device(PCIDevice
> *pci_dev, PCIBus *bus,
> PCIConfigReadFunc *config_read = pc->config_read;
> PCIConfigWriteFunc *config_write = pc->config_write;
> Error *local_err = NULL;
> - AddressSpace *dma_as;
> DeviceState *dev = DEVICE(pci_dev);
>
> pci_dev->bus = bus;
> @@ -885,15 +917,9 @@ static PCIDevice *do_pci_register_device(PCIDevice
> *pci_dev, PCIBus *bus,
> }
>
> pci_dev->devfn = devfn;
> - dma_as = pci_device_iommu_address_space(pci_dev);
> -
> - memory_region_init_alias(&pci_dev->bus_master_enable_region,
> - OBJECT(pci_dev), "bus master",
> - dma_as->root, 0,
> memory_region_size(dma_as->root));
> - memory_region_set_enabled(&pci_dev->bus_master_enable_region, false);
> - address_space_init(&pci_dev->bus_master_as,
> &pci_dev->bus_master_enable_region,
> - name);
> -
> + if (qdev_hotplug) {
> + pci_init_bus_master(pci_dev);
> + }
> pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
> pci_dev->irq_state = 0;
> pci_config_alloc(pci_dev);
> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
> index 403fec6..5484a9b 100644
> --- a/include/hw/pci/pci_bus.h
> +++ b/include/hw/pci/pci_bus.h
> @@ -39,6 +39,8 @@ struct PCIBus {
> Keep a count of the number of devices with raised IRQs. */
> int nirq;
> int *irq_count;
> +
> + Notifier machine_done;
> };
>
> typedef struct PCIBridgeWindows PCIBridgeWindows;
> --
> 2.4.3
>
>
