[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/8] pci: Rework PCI iommu lifetime assumptions
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 3/8] pci: Rework PCI iommu lifetime assumptions |
Date: |
Mon, 13 May 2013 14:15:43 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 |
Il 13/05/2013 12:54, David Gibson ha scritto:
> 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.
Good idea, applying this too.
Paolo
> Signed-off-by: David Gibson <address@hidden>
> ---
> 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;
>
- Re: [Qemu-devel] [PATCH 6/8] memory: Sanity check that no listeners remain on a destroyed AddressSpace, (continued)
[Qemu-devel] [PATCH 5/8] pci: Introduce helper to retrieve a PCI device's DMA address space, David Gibson, 2013/05/13
[Qemu-devel] [PATCH 8/8] vfio: Create VFIOAddressSpace objects as needed, David Gibson, 2013/05/13
[Qemu-devel] [PATCH 3/8] pci: Rework PCI iommu lifetime assumptions, David Gibson, 2013/05/13
- Re: [Qemu-devel] [PATCH 3/8] pci: Rework PCI iommu lifetime assumptions,
Paolo Bonzini <=
Re: [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited, Paolo Bonzini, 2013/05/13
- Re: [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited, David Gibson, 2013/05/13
- Re: [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited, Paolo Bonzini, 2013/05/13
- Re: [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited, David Gibson, 2013/05/13
- Re: [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited, Paolo Bonzini, 2013/05/14
- Re: [Qemu-devel] [0/8] RFC: VFIO and guest side IOMMUs, revisited, David Gibson, 2013/05/14