From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38444) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V8W93-00069s-TN for qemu-devel@nongnu.org; Sun, 11 Aug 2013 09:57:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V8W8z-0000Qe-NQ for qemu-devel@nongnu.org; Sun, 11 Aug 2013 09:57:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:23195) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V8W8z-0000QU-FL for qemu-devel@nongnu.org; Sun, 11 Aug 2013 09:57:17 -0400 Date: Sun, 11 Aug 2013 16:58:47 +0300 From: "Michael S. Tsirkin" Message-ID: <20130811135847.GD15442@redhat.com> References: <1376060948-28790-1-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1376060948-28790-1-git-send-email-aik@ozlabs.ru> Subject: Re: [Qemu-devel] [PATCH v3] pci: Introduce helper to retrieve a PCI device's DMA address space List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Anthony Liguori , qemu-devel@nongnu.org, Alex Williamson , Paolo Bonzini , David Gibson On Sat, Aug 10, 2013 at 01:09:08AM +1000, Alexey Kardashevskiy wrote: > A PCI device's DMA address space (possibly an IOMMU) is returned by a > method on the PCIBus. At the moment that only has one caller, so the > method is simply open coded. We'll need another caller for VFIO, so > this patch introduces a helper/wrapper function. > > If IOMMU is not set, the pci_device_iommu_address_space() function > returns the parent's IOMMU skipping the "bus master" address space as > otherwise proper emulation would require more effort for no benefit. > > Signed-off-by: David Gibson > [aik: added inheritance from parent if iommu is not set for the current bus] > Signed-off-by: Alexey Kardashevskiy > > --- > Changes: > v3: > * added comment about ignoring bus master address space > > v2: > * added inheritance, needed for a pci-bridge on spapr-ppc64 > * pci_iommu_as renamed to pci_device_iommu_address_space > --- > hw/pci/pci.c | 24 ++++++++++++++++++------ > include/hw/pci/pci.h | 1 + > 2 files changed, 19 insertions(+), 6 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 4c004f5..dbfa395 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -812,12 +812,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, > } > > pci_dev->bus = bus; > - if (bus->iommu_fn) { > - dma_as = bus->iommu_fn(bus, bus->iommu_opaque, devfn); > - } else { > - /* FIXME: inherit memory region from bus creator */ > - dma_as = &address_space_memory; > - } > + dma_as = pci_device_iommu_address_space(pci_dev); > > memory_region_init_alias(&pci_dev->bus_master_enable_region, > OBJECT(pci_dev), "bus master", > @@ -2239,6 +2234,23 @@ static void pci_device_class_init(ObjectClass *klass, void *data) > k->props = pci_props; > } > > +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > +{ > + PCIBus *bus = PCI_BUS(dev->bus); > + > + if (bus->iommu_fn) { > + return bus->iommu_fn(bus, bus->iommu_opaque, dev->devfn); > + } > + > + if (bus->parent_dev) { > + /** We are ignoring the bus master DMA bit of the bridge > + * as it would complicate things such as VFIO for no good reason */ /* * Always * like * this */ /** Never * like this */ The comment should be improved I think. I would put it like this: /* * Note: this does not check bus master enable bit on device or * any of the pci to pci bridges above it, it's up to the caller to * check that before initiating the transaction. * * TODO: design a mechanism for callers to do this without * doing bus scans on data path. */ Would you like me to queue this on the pci tree? If yes I can tweak the comment myself, no need to repost. > + return pci_device_iommu_address_space(bus->parent_dev); > + } > + > + return &address_space_memory; > +} > + > void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque) > { > bus->iommu_fn = fn; > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index ccec2ba..2374aa9 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -405,6 +405,7 @@ void pci_device_deassert_intx(PCIDevice *dev); > > typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int); > > +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev); > void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque); > > static inline void > -- > 1.8.3.2