----- 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 > >