[4/8] pci: Use AddressSpace rather than MemoryRegion to represent PCI DMA space

Currently the PCI iommu_fn hook returns a MemoryRegion * to represent the
DMA address of this bus's IOMMU, although that MemoryRegion does have to
be a root MemoryRegion.  Several upcoming users of this need the extra
features of an AddressSpace object, rather than a MemoryRegion, and while
they could each construct their own AddressSpace wrapper for the iommu
MemoryRegion, that leads to unnecessary proliferation of essentially
identical AddressSpace objects.  This patch avoids that, by instead having
iommu_fn return an AddressSpace *, assuming the referenced AS's lifetime
is managed somewhere else (probably the PCI host bridge).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/pci/pci.c                |   10 +++++-----
 hw/ppc/spapr_pci.c          |    6 ++++--
 include/hw/pci-host/spapr.h |    1 +
 include/hw/pci/pci.h        |    2 +-
 4 files changed, 11 insertions(+), 8 deletions(-)

Il 13/05/2013 12:54, David Gibson ha scritto:
> Currently the PCI iommu_fn hook returns a MemoryRegion * to represent the
> DMA address of this bus's IOMMU, although that MemoryRegion does have to
> be a root MemoryRegion.  Several upcoming users of this need the extra
> features of an AddressSpace object, rather than a MemoryRegion, and while
> they could each construct their own AddressSpace wrapper for the iommu
> MemoryRegion, that leads to unnecessary proliferation of essentially
> identical AddressSpace objects.  This patch avoids that, by instead having
> iommu_fn return an AddressSpace *, assuming the referenced AS's lifetime
> is managed somewhere else (probably the PCI host bridge).

Makes sense too.

Paolo

> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/pci/pci.c                |   10 +++++-----
>  hw/ppc/spapr_pci.c          |    6 ++++--
>  include/hw/pci-host/spapr.h |    1 +
>  include/hw/pci/pci.h        |    2 +-
>  4 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 3c947b3..39085d8 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -279,10 +279,10 @@ int pci_find_domain(const PCIBus *bus)
>      return -1;
>  }
>  
> -static MemoryRegion *pci_default_iommu(PCIBus *bus, void *opaque, int devfn)
> +static AddressSpace *pci_default_iommu(PCIBus *bus, void *opaque, int devfn)
>  {
>      /* FIXME: inherit memory region from bus creator */
> -    return get_system_memory();
> +    return &address_space_memory;
>  }
>  
>  static void pci_bus_init(PCIBus *bus, DeviceState *parent,
> @@ -793,7 +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;
> +    AddressSpace *dma_as;
>  
>      if (devfn < 0) {
>          for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
> @@ -811,9 +811,9 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>      }
>  
>      pci_dev->bus = bus;
> -    dma_mr = bus->iommu_fn(bus, bus->iommu_opaque, devfn);
> +    dma_as = bus->iommu_fn(bus, bus->iommu_opaque, devfn);
>      memory_region_init_alias(&pci_dev->bus_master_enable_region, "bus master",
> -                             dma_mr, 0, memory_region_size(dma_mr));
> +                             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);
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index eb1d9e7..762db62 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -506,11 +506,11 @@ static const MemoryRegionOps spapr_msi_ops = {
>  /*
>   * PHB PCI device
>   */
> -static MemoryRegion *spapr_pci_dma_iommu(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_iommu(phb->tcet);
> +    return &phb->iommu_as;
>  }
>  
>  static int spapr_phb_init(SysBusDevice *s)
> @@ -650,6 +650,8 @@ static int spapr_phb_init(SysBusDevice *s)
>          fprintf(stderr, "Unable to create TCE table for %s\n", sphb->dtbusname);
>          return -1;
>      }
> +    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 61fe51e..6ef1f97 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -400,7 +400,7 @@ 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 AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
>  
>  void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque);
>  
>