The current bus callbacks to assign and destroy an iommu memory region for a PCI device tacitly assume that the lifetime of that address space is tied to the lifetime of the PCI device. Although that's certainly possible, it's much more likely that the region will be (at least potentially) shared between several devices and have a lifetime instead tied to the PCI host bridge, or the IOMMU itself. This is demonstrated in the fact that there are no existing users of the destructor hook.
This patch simplifies the code by moving to the more likely use model. This means we can eliminate the destructor hook entirely, and the iommu_fn hook is now assumed to inform us of the device's pre-existing DMA adddress space, rather than creating or assigning it. That in turn means we have no need to keep a reference to the region around in the PCIDevice structure, which saves a little space. Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> --- hw/pci/pci.c | 16 +++++----------- hw/ppc/spapr_pci.c | 2 +- include/hw/pci/pci.h | 5 +---- include/hw/pci/pci_bus.h | 1 - 4 files changed, 7 insertions(+), 17 deletions(-) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 58d3f69..3c947b3 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -285,10 +285,6 @@ static MemoryRegion *pci_default_iommu(PCIBus *bus, void *opaque, int devfn) return get_system_memory(); } -static void pci_default_iommu_dtor(MemoryRegion *mr) -{ -} - static void pci_bus_init(PCIBus *bus, DeviceState *parent, const char *name, MemoryRegion *address_space_mem, @@ -299,7 +295,7 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent, bus->devfn_min = devfn_min; bus->address_space_mem = address_space_mem; bus->address_space_io = address_space_io; - pci_setup_iommu(bus, pci_default_iommu, NULL, NULL); + pci_setup_iommu(bus, pci_default_iommu, NULL); /* host bridge */ QLIST_INIT(&bus->child); @@ -797,6 +793,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev); PCIConfigReadFunc *config_read = pc->config_read; PCIConfigWriteFunc *config_write = pc->config_write; + MemoryRegion *dma_mr; if (devfn < 0) { for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices); @@ -814,9 +811,9 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, } pci_dev->bus = bus; - pci_dev->iommu = bus->iommu_fn(bus, bus->iommu_opaque, devfn); + dma_mr = bus->iommu_fn(bus, bus->iommu_opaque, devfn); memory_region_init_alias(&pci_dev->bus_master_enable_region, "bus master", - pci_dev->iommu, 0, memory_region_size(pci_dev->iommu)); + dma_mr, 0, memory_region_size(dma_mr)); 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); @@ -875,7 +872,6 @@ static void do_pci_unregister_device(PCIDevice *pci_dev) pci_config_free(pci_dev); address_space_destroy(&pci_dev->bus_master_as); - pci_dev->bus->iommu_dtor_fn(pci_dev->iommu); memory_region_destroy(&pci_dev->bus_master_enable_region); } @@ -2237,11 +2233,9 @@ static void pci_device_class_init(ObjectClass *klass, void *data) k->props = pci_props; } -void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, PCIIOMMUDestructorFunc dtor, - void *opaque) +void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque) { bus->iommu_fn = fn; - bus->iommu_dtor_fn = dtor ? dtor : pci_default_iommu_dtor; bus->iommu_opaque = opaque; } diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 7014b61..eb1d9e7 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -650,7 +650,7 @@ static int spapr_phb_init(SysBusDevice *s) fprintf(stderr, "Unable to create TCE table for %s\n", sphb->dtbusname); return -1; } - pci_setup_iommu(bus, spapr_pci_dma_iommu, NULL, sphb); + pci_setup_iommu(bus, spapr_pci_dma_iommu, sphb); QLIST_INSERT_HEAD(&spapr->phbs, sphb, list); diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 400e772..61fe51e 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -242,7 +242,6 @@ struct PCIDevice { PCIIORegion io_regions[PCI_NUM_REGIONS]; AddressSpace bus_master_as; MemoryRegion bus_master_enable_region; - MemoryRegion *iommu; /* do not access the following fields */ PCIConfigReadFunc *config_read; @@ -402,10 +401,8 @@ int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp, void pci_device_deassert_intx(PCIDevice *dev); typedef MemoryRegion *(*PCIIOMMUFunc)(PCIBus *, void *, int); -typedef void (*PCIIOMMUDestructorFunc)(MemoryRegion *mr); -void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, PCIIOMMUDestructorFunc dtor, - void *opaque); +void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque); static inline void pci_set_byte(uint8_t *config, uint8_t val) diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h index fada8f5..66762f6 100644 --- a/include/hw/pci/pci_bus.h +++ b/include/hw/pci/pci_bus.h @@ -11,7 +11,6 @@ struct PCIBus { BusState qbus; PCIIOMMUFunc iommu_fn; - PCIIOMMUDestructorFunc iommu_dtor_fn; void *iommu_opaque; uint8_t devfn_min; pci_set_irq_fn set_irq; -- 1.7.10.4