Message ID | 1369133851-1894-28-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
On 21 May 2013 11:57, Paolo Bonzini <pbonzini@redhat.com> wrote: > From: Avi Kivity <avi.kivity@gmail.com> > > Use the new iommu support in the memory core for iommu support. The only > user, spapr, is also converted, but it still provides a DMAContext > interface until the non-PCI bits switch to AddressSpace. > > Cc: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Avi Kivity <avi.kivity@gmail.com> > [ Do not calls memory_region_del_subregion() on the device's > bus_master_enable_region, it is an alias; return an AddressSpace > from the IOMMU hook and remove the destructor hook. - David Gibson ] > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> More >80column excesses. Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> -- PMM
On Tue, May 21, 2013 at 12:57:28PM +0200, Paolo Bonzini wrote: > From: Avi Kivity <avi.kivity@gmail.com> > > Use the new iommu support in the memory core for iommu support. The only > user, spapr, is also converted, but it still provides a DMAContext > interface until the non-PCI bits switch to AddressSpace. > > Cc: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Avi Kivity <avi.kivity@gmail.com> > [ Do not calls memory_region_del_subregion() on the device's > bus_master_enable_region, it is an alias; return an AddressSpace > from the IOMMU hook and remove the destructor hook. - David Gibson ] > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Looks good to me. Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/pci/pci.c | 46 ++++++++++++++++++++++----------------------- > hw/ppc/spapr_pci.c | 9 +++++---- > include/hw/pci-host/spapr.h | 1 + > include/hw/pci/pci.h | 4 ++-- > include/hw/pci/pci_bus.h | 4 ++-- > 5 files changed, 33 insertions(+), 31 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index c9d585c..9e6bd77 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -279,6 +279,12 @@ int pci_find_domain(const PCIBus *bus) > return -1; > } > > +static AddressSpace *pci_default_address_space(PCIBus *bus, void *opaque, int devfn) > +{ > + /* FIXME: inherit memory region from bus creator */ > + return &address_space_memory; > +} > + > static void pci_bus_init(PCIBus *bus, DeviceState *parent, > const char *name, > MemoryRegion *address_space_mem, > @@ -289,6 +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_address_space, NULL); > > /* host bridge */ > QLIST_INIT(&bus->child); > @@ -786,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; > + AddressSpace *dma_as; > > if (devfn < 0) { > for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices); > @@ -801,21 +809,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, > PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name); > return NULL; > } > + > pci_dev->bus = bus; > - if (bus->dma_context_fn) { > - pci_dev->dma = bus->dma_context_fn(bus, bus->dma_context_opaque, devfn); > - } else { > - /* FIXME: Make dma_context_fn use MemoryRegions instead, so this path is > - * taken unconditionally */ > - /* FIXME: inherit memory region from bus creator */ > - memory_region_init_alias(&pci_dev->bus_master_enable_region, "bus master", > - get_system_memory(), 0, > - memory_region_size(get_system_memory())); > - 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->dma = g_new(DMAContext, 1); > - dma_context_init(pci_dev->dma, &pci_dev->bus_master_as); > - } > + dma_as = bus->iommu_fn(bus, bus->iommu_opaque, devfn); > + memory_region_init_alias(&pci_dev->bus_master_enable_region, "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->dma = g_new(DMAContext, 1); > + dma_context_init(pci_dev->dma, &pci_dev->bus_master_as); > > pci_dev->devfn = devfn; > pstrcpy(pci_dev->name, sizeof(pci_dev->name), name); > @@ -870,12 +872,10 @@ static void do_pci_unregister_device(PCIDevice *pci_dev) > pci_dev->bus->devices[pci_dev->devfn] = NULL; > pci_config_free(pci_dev); > > - if (!pci_dev->bus->dma_context_fn) { > - address_space_destroy(&pci_dev->bus_master_as); > - memory_region_destroy(&pci_dev->bus_master_enable_region); > - g_free(pci_dev->dma); > - pci_dev->dma = NULL; > - } > + address_space_destroy(&pci_dev->bus_master_as); > + memory_region_destroy(&pci_dev->bus_master_enable_region); > + g_free(pci_dev->dma); > + pci_dev->dma = NULL; > } > > static void pci_unregister_io_regions(PCIDevice *pci_dev) > @@ -2232,10 +2232,10 @@ static void pci_device_class_init(ObjectClass *klass, void *data) > k->props = pci_props; > } > > -void pci_setup_iommu(PCIBus *bus, PCIDMAContextFunc fn, void *opaque) > +void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque) > { > - bus->dma_context_fn = fn; > - bus->dma_context_opaque = opaque; > + bus->iommu_fn = fn; > + bus->iommu_opaque = opaque; > } > > static const TypeInfo pci_device_type_info = { > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index eb64a8f..762db62 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -506,12 +506,11 @@ static const MemoryRegionOps spapr_msi_ops = { > /* > * PHB PCI device > */ > -static DMAContext *spapr_pci_dma_context_fn(PCIBus *bus, void *opaque, > - int devfn) > +static AddressSpace *spapr_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn) > { > sPAPRPHBState *phb = opaque; > > - return spapr_tce_get_dma(phb->tcet); > + return &phb->iommu_as; > } > > static int spapr_phb_init(SysBusDevice *s) > @@ -651,7 +650,9 @@ 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_context_fn, sphb); > + address_space_init(&sphb->iommu_as, spapr_tce_get_iommu(sphb->tcet), > + sphb->dtbusname); > + pci_setup_iommu(bus, spapr_pci_dma_iommu, sphb); > > QLIST_INSERT_HEAD(&spapr->phbs, sphb, list); > > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h > index 653dd40..1e23dbf 100644 > --- a/include/hw/pci-host/spapr.h > +++ b/include/hw/pci-host/spapr.h > @@ -50,6 +50,7 @@ typedef struct sPAPRPHBState { > uint64_t dma_window_start; > uint64_t dma_window_size; > sPAPRTCETable *tcet; > + AddressSpace iommu_as; > > struct { > uint32_t irq; > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index 8d075ab..3a85bce 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -401,9 +401,9 @@ int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp, > > void pci_device_deassert_intx(PCIDevice *dev); > > -typedef DMAContext *(*PCIDMAContextFunc)(PCIBus *, void *, int); > +typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int); > > -void pci_setup_iommu(PCIBus *bus, PCIDMAContextFunc fn, 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 6ee443c..66762f6 100644 > --- a/include/hw/pci/pci_bus.h > +++ b/include/hw/pci/pci_bus.h > @@ -10,8 +10,8 @@ > > struct PCIBus { > BusState qbus; > - PCIDMAContextFunc dma_context_fn; > - void *dma_context_opaque; > + PCIIOMMUFunc iommu_fn; > + void *iommu_opaque; > uint8_t devfn_min; > pci_set_irq_fn set_irq; > pci_map_irq_fn map_irq; > -- > 1.8.1.4 >
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index c9d585c..9e6bd77 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -279,6 +279,12 @@ int pci_find_domain(const PCIBus *bus) return -1; } +static AddressSpace *pci_default_address_space(PCIBus *bus, void *opaque, int devfn) +{ + /* FIXME: inherit memory region from bus creator */ + return &address_space_memory; +} + static void pci_bus_init(PCIBus *bus, DeviceState *parent, const char *name, MemoryRegion *address_space_mem, @@ -289,6 +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_address_space, NULL); /* host bridge */ QLIST_INIT(&bus->child); @@ -786,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; + AddressSpace *dma_as; if (devfn < 0) { for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices); @@ -801,21 +809,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name); return NULL; } + pci_dev->bus = bus; - if (bus->dma_context_fn) { - pci_dev->dma = bus->dma_context_fn(bus, bus->dma_context_opaque, devfn); - } else { - /* FIXME: Make dma_context_fn use MemoryRegions instead, so this path is - * taken unconditionally */ - /* FIXME: inherit memory region from bus creator */ - memory_region_init_alias(&pci_dev->bus_master_enable_region, "bus master", - get_system_memory(), 0, - memory_region_size(get_system_memory())); - 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->dma = g_new(DMAContext, 1); - dma_context_init(pci_dev->dma, &pci_dev->bus_master_as); - } + dma_as = bus->iommu_fn(bus, bus->iommu_opaque, devfn); + memory_region_init_alias(&pci_dev->bus_master_enable_region, "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->dma = g_new(DMAContext, 1); + dma_context_init(pci_dev->dma, &pci_dev->bus_master_as); pci_dev->devfn = devfn; pstrcpy(pci_dev->name, sizeof(pci_dev->name), name); @@ -870,12 +872,10 @@ static void do_pci_unregister_device(PCIDevice *pci_dev) pci_dev->bus->devices[pci_dev->devfn] = NULL; pci_config_free(pci_dev); - if (!pci_dev->bus->dma_context_fn) { - address_space_destroy(&pci_dev->bus_master_as); - memory_region_destroy(&pci_dev->bus_master_enable_region); - g_free(pci_dev->dma); - pci_dev->dma = NULL; - } + address_space_destroy(&pci_dev->bus_master_as); + memory_region_destroy(&pci_dev->bus_master_enable_region); + g_free(pci_dev->dma); + pci_dev->dma = NULL; } static void pci_unregister_io_regions(PCIDevice *pci_dev) @@ -2232,10 +2232,10 @@ static void pci_device_class_init(ObjectClass *klass, void *data) k->props = pci_props; } -void pci_setup_iommu(PCIBus *bus, PCIDMAContextFunc fn, void *opaque) +void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque) { - bus->dma_context_fn = fn; - bus->dma_context_opaque = opaque; + bus->iommu_fn = fn; + bus->iommu_opaque = opaque; } static const TypeInfo pci_device_type_info = { diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index eb64a8f..762db62 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -506,12 +506,11 @@ static const MemoryRegionOps spapr_msi_ops = { /* * PHB PCI device */ -static DMAContext *spapr_pci_dma_context_fn(PCIBus *bus, void *opaque, - int devfn) +static AddressSpace *spapr_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn) { sPAPRPHBState *phb = opaque; - return spapr_tce_get_dma(phb->tcet); + return &phb->iommu_as; } static int spapr_phb_init(SysBusDevice *s) @@ -651,7 +650,9 @@ 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_context_fn, sphb); + address_space_init(&sphb->iommu_as, spapr_tce_get_iommu(sphb->tcet), + sphb->dtbusname); + pci_setup_iommu(bus, spapr_pci_dma_iommu, sphb); QLIST_INSERT_HEAD(&spapr->phbs, sphb, list); diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h index 653dd40..1e23dbf 100644 --- a/include/hw/pci-host/spapr.h +++ b/include/hw/pci-host/spapr.h @@ -50,6 +50,7 @@ typedef struct sPAPRPHBState { uint64_t dma_window_start; uint64_t dma_window_size; sPAPRTCETable *tcet; + AddressSpace iommu_as; struct { uint32_t irq; diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 8d075ab..3a85bce 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -401,9 +401,9 @@ int pci_read_devaddr(Monitor *mon, const char *addr, int *domp, int *busp, void pci_device_deassert_intx(PCIDevice *dev); -typedef DMAContext *(*PCIDMAContextFunc)(PCIBus *, void *, int); +typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int); -void pci_setup_iommu(PCIBus *bus, PCIDMAContextFunc fn, 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 6ee443c..66762f6 100644 --- a/include/hw/pci/pci_bus.h +++ b/include/hw/pci/pci_bus.h @@ -10,8 +10,8 @@ struct PCIBus { BusState qbus; - PCIDMAContextFunc dma_context_fn; - void *dma_context_opaque; + PCIIOMMUFunc iommu_fn; + void *iommu_opaque; uint8_t devfn_min; pci_set_irq_fn set_irq; pci_map_irq_fn map_irq;