From patchwork Tue May 14 09:13:49 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Gibson X-Patchwork-Id: 243644 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id F216E2C0109 for ; Tue, 14 May 2013 19:35:04 +1000 (EST) Received: from localhost ([::1]:52129 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UcBLb-0007CD-KO for incoming@patchwork.ozlabs.org; Tue, 14 May 2013 05:16:39 -0400 Received: from eggs.gnu.org ([208.118.235.92]:39520) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UcBJI-0004dy-5h for qemu-devel@nongnu.org; Tue, 14 May 2013 05:14:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UcBJ4-0001Lz-Af for qemu-devel@nongnu.org; Tue, 14 May 2013 05:14:15 -0400 Received: from ozlabs.org ([2402:b800:7003:1:1::1]:49302) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UcBJ3-0001L2-MC; Tue, 14 May 2013 05:14:02 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 89C6D2C00C2; Tue, 14 May 2013 19:13:59 +1000 (EST) From: David Gibson To: alex.williamson@redhat.com, pbonzini@redhat.com Date: Tue, 14 May 2013 19:13:49 +1000 Message-Id: <1368522837-20747-4-git-send-email-david@gibson.dropbear.id.au> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1368522837-20747-1-git-send-email-david@gibson.dropbear.id.au> References: <1368522837-20747-1-git-send-email-david@gibson.dropbear.id.au> X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2402:b800:7003:1:1::1 Cc: aik@ozlabs.ru, David Gibson , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, mst@redhat.com Subject: [Qemu-devel] [PATCH 03/11] pci: Rework PCI iommu lifetime assumptions X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 --- 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;