From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47885) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zg8ru-0000D8-7g for qemu-devel@nongnu.org; Sun, 27 Sep 2015 06:07:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zg8rq-0006yJ-VG for qemu-devel@nongnu.org; Sun, 27 Sep 2015 06:07:42 -0400 Date: Sun, 27 Sep 2015 13:07:27 +0300 From: "Michael S. Tsirkin" Message-ID: <20150927130653-mutt-send-email-mst@redhat.com> References: <1443247796-11622-1-git-send-email-knut.omang@oracle.com> <1443247796-11622-2-git-send-email-knut.omang@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1443247796-11622-2-git-send-email-knut.omang@oracle.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 1/1] intel_iommu: Add support for translation for devices behind bridges List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Knut Omang Cc: Alex Williamson , Eduardo Habkost , qemu-devel@nongnu.org, Alexander Graf , Andreas =?iso-8859-1?Q?F=E4rber?= , qemu-ppc@nongnu.org, Le Tan , Jan Kiszka , Paolo Bonzini , Richard Henderson , David Gibson On Sat, Sep 26, 2015 at 08:09:56AM +0200, Knut Omang wrote: > - Use a hash table indexed on bus pointers to store information about b= uses > instead of using the bus numbers. > Bus pointers are stored in a new VTDBus struct together with the vect= or > of device address space pointers indexed by devfn. > - The bus number is still used for lookup for selective SID based inval= idate, > in which case the bus number is lazily resolved from the bus hash tab= le and > cached in a separate index. >=20 > Signed-off-by: Knut Omang Fails on 32 bit: /scm/qemu/hw/i386/intel_iommu.c: In function =E2=80=98vtd_find_add_as=E2=80= =99: /scm/qemu/hw/i386/intel_iommu.c:1869:20: error: cast from pointer to integer of different size [-Werror=3Dpointer-to-int-cast] uint64_t key =3D (uint64_t)bus; ^ /scm/qemu/hw/i386/intel_iommu.c:1877:15: error: cast from pointer to integer of different size [-Werror=3Dpointer-to-int-cast] key =3D (uint64_t)bus; ^ You need to cast things to uintptr_t. > --- > hw/i386/intel_iommu.c | 90 +++++++++++++++++++++++++++++++++++= -------- > hw/pci-host/q35.c | 25 ++---------- > include/hw/i386/intel_iommu.h | 16 +++++++- > 3 files changed, 91 insertions(+), 40 deletions(-) >=20 > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 08055a8..d677a28 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -22,6 +22,7 @@ > #include "hw/sysbus.h" > #include "exec/address-spaces.h" > #include "intel_iommu_internal.h" > +#include "hw/pci/pci.h" > =20 > /*#define DEBUG_INTEL_IOMMU*/ > #ifdef DEBUG_INTEL_IOMMU > @@ -166,19 +167,17 @@ static gboolean vtd_hash_remove_by_page(gpointer = key, gpointer value, > */ > static void vtd_reset_context_cache(IntelIOMMUState *s) > { > - VTDAddressSpace **pvtd_as; > VTDAddressSpace *vtd_as; > - uint32_t bus_it; > + VTDBus *vtd_bus; > + GHashTableIter bus_it; > uint32_t devfn_it; > =20 > + g_hash_table_iter_init(&bus_it, s->vtd_as_by_busptr); > + > VTD_DPRINTF(CACHE, "global context_cache_gen=3D1"); > - for (bus_it =3D 0; bus_it < VTD_PCI_BUS_MAX; ++bus_it) { > - pvtd_as =3D s->address_spaces[bus_it]; > - if (!pvtd_as) { > - continue; > - } > + while (g_hash_table_iter_next (&bus_it, NULL, (void**)&vtd_bus)) { > for (devfn_it =3D 0; devfn_it < VTD_PCI_DEVFN_MAX; ++devfn_it)= { > - vtd_as =3D pvtd_as[devfn_it]; > + vtd_as =3D vtd_bus->dev_as[devfn_it]; > if (!vtd_as) { > continue; > } > @@ -754,12 +753,13 @@ static inline bool vtd_is_interrupt_addr(hwaddr a= ddr) > * @is_write: The access is a write operation > * @entry: IOMMUTLBEntry that contain the addr to be translated and re= sult > */ > -static void vtd_do_iommu_translate(VTDAddressSpace *vtd_as, uint8_t bu= s_num, > +static void vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bu= s, > uint8_t devfn, hwaddr addr, bool is= _write, > IOMMUTLBEntry *entry) > { > IntelIOMMUState *s =3D vtd_as->iommu_state; > VTDContextEntry ce; > + uint8_t bus_num =3D pci_bus_num(bus); > VTDContextCacheEntry *cc_entry =3D &vtd_as->context_cache_entry; > uint64_t slpte; > uint32_t level; > @@ -874,6 +874,30 @@ static void vtd_context_global_invalidate(IntelIOM= MUState *s) > } > } > =20 > + > +/* Find the VTD address space currently associated with a given bus nu= mber, > + */ > +static VTDBus *vtd_find_as_from_bus_num(IntelIOMMUState *s, uint8_t bu= s_num) > +{ > + VTDBus *vtd_bus =3D s->vtd_as_by_bus_num[bus_num]; > + if (!vtd_bus) { > + /* Iterate over the registered buses to find the one > + * which currently hold this bus number, and update the bus_nu= m lookup table: > + */ > + GHashTableIter iter; > + uint64_t key; > + > + g_hash_table_iter_init(&iter, s->vtd_as_by_busptr); > + while (g_hash_table_iter_next (&iter, (void**)&key, (void**)&v= td_bus)) { > + if (pci_bus_num(vtd_bus->bus) =3D=3D bus_num) { > + s->vtd_as_by_bus_num[bus_num] =3D vtd_bus; > + return vtd_bus; > + } > + } > + } > + return vtd_bus; > +} > + > /* Do a context-cache device-selective invalidation. > * @func_mask: FM field after shifting > */ > @@ -882,7 +906,7 @@ static void vtd_context_device_invalidate(IntelIOMM= UState *s, > uint16_t func_mask) > { > uint16_t mask; > - VTDAddressSpace **pvtd_as; > + VTDBus *vtd_bus; > VTDAddressSpace *vtd_as; > uint16_t devfn; > uint16_t devfn_it; > @@ -903,11 +927,11 @@ static void vtd_context_device_invalidate(IntelIO= MMUState *s, > } > VTD_DPRINTF(INV, "device-selective invalidation source 0x%"PRIx16 > " mask %"PRIu16, source_id, mask); > - pvtd_as =3D s->address_spaces[VTD_SID_TO_BUS(source_id)]; > - if (pvtd_as) { > + vtd_bus =3D vtd_find_as_from_bus_num(s, VTD_SID_TO_BUS(source_id))= ; > + if (vtd_bus) { > devfn =3D VTD_SID_TO_DEVFN(source_id); > for (devfn_it =3D 0; devfn_it < VTD_PCI_DEVFN_MAX; ++devfn_it)= { > - vtd_as =3D pvtd_as[devfn_it]; > + vtd_as =3D vtd_bus->dev_as[devfn_it]; > if (vtd_as && ((devfn_it & mask) =3D=3D (devfn & mask))) { > VTD_DPRINTF(INV, "invalidate context-cahce of devfn 0x= %"PRIx16, > devfn_it); > @@ -1805,11 +1829,11 @@ static IOMMUTLBEntry vtd_iommu_translate(Memory= Region *iommu, hwaddr addr, > return ret; > } > =20 > - vtd_do_iommu_translate(vtd_as, vtd_as->bus_num, vtd_as->devfn, add= r, > + vtd_do_iommu_translate(vtd_as, vtd_as->bus, vtd_as->devfn, addr, > is_write, &ret); > VTD_DPRINTF(MMU, > "bus %"PRIu8 " slot %"PRIu8 " func %"PRIu8 " devfn %"P= RIu8 > - " gpa 0x%"PRIx64 " hpa 0x%"PRIx64, vtd_as->bus_num, > + " gpa 0x%"PRIx64 " hpa 0x%"PRIx64, pci_bus_num(vtd_as-= >bus), > VTD_PCI_SLOT(vtd_as->devfn), VTD_PCI_FUNC(vtd_as->devf= n), > vtd_as->devfn, addr, ret.translated_addr); > return ret; > @@ -1839,6 +1863,38 @@ static Property vtd_properties[] =3D { > DEFINE_PROP_END_OF_LIST(), > }; > =20 > + > +VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int = devfn) > +{ > + uint64_t key =3D (uint64_t)bus; > + VTDBus *vtd_bus =3D g_hash_table_lookup(s->vtd_as_by_busptr, &key)= ; > + VTDAddressSpace *vtd_dev_as; > + > + if (!vtd_bus) { > + /* No corresponding free() */ > + vtd_bus =3D g_malloc0(sizeof(VTDBus) + sizeof(VTDAddressSpace = *) * VTD_PCI_DEVFN_MAX); > + vtd_bus->bus =3D bus; > + key =3D (uint64_t)bus; > + g_hash_table_insert(s->vtd_as_by_busptr, &key, vtd_bus); > + } > + > + vtd_dev_as =3D vtd_bus->dev_as[devfn]; > + > + if (!vtd_dev_as) { > + vtd_bus->dev_as[devfn] =3D vtd_dev_as =3D g_malloc0(sizeof(VTD= AddressSpace)); > + > + vtd_dev_as->bus =3D bus; > + vtd_dev_as->devfn =3D (uint8_t)devfn; > + vtd_dev_as->iommu_state =3D s; > + vtd_dev_as->context_cache_entry.context_cache_gen =3D 0; > + memory_region_init_iommu(&vtd_dev_as->iommu, OBJECT(s), > + &s->iommu_ops, "intel_iommu", UINT64_= MAX); > + address_space_init(&vtd_dev_as->as, > + &vtd_dev_as->iommu, "intel_iommu"); > + } > + return vtd_dev_as; > +} > + > /* Do the initialization. It will also be called when reset, so pay > * attention when adding new initialization stuff. > */ > @@ -1931,13 +1987,15 @@ static void vtd_realize(DeviceState *dev, Error= **errp) > IntelIOMMUState *s =3D INTEL_IOMMU_DEVICE(dev); > =20 > VTD_DPRINTF(GENERAL, ""); > - memset(s->address_spaces, 0, sizeof(s->address_spaces)); > + memset(s->vtd_as_by_bus_num, 0, sizeof(s->vtd_as_by_bus_num)); > memory_region_init_io(&s->csrmem, OBJECT(s), &vtd_mem_ops, s, > "intel_iommu", DMAR_REG_SIZE); > sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->csrmem); > /* No corresponding destroy */ > s->iotlb =3D g_hash_table_new_full(vtd_uint64_hash, vtd_uint64_equ= al, > g_free, g_free); > + s->vtd_as_by_busptr =3D g_hash_table_new_full(vtd_uint64_hash, vtd= _uint64_equal, > + g_free, g_free); > vtd_init(s); > } > =20 > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c > index bd74094..c81507d 100644 > --- a/hw/pci-host/q35.c > +++ b/hw/pci-host/q35.c > @@ -426,31 +426,12 @@ static void mch_reset(DeviceState *qdev) > static AddressSpace *q35_host_dma_iommu(PCIBus *bus, void *opaque, int= devfn) > { > IntelIOMMUState *s =3D opaque; > - VTDAddressSpace **pvtd_as; > - int bus_num =3D pci_bus_num(bus); > + VTDAddressSpace *vtd_as; > =20 > - assert(0 <=3D bus_num && bus_num <=3D VTD_PCI_BUS_MAX); > assert(0 <=3D devfn && devfn <=3D VTD_PCI_DEVFN_MAX); > =20 > - pvtd_as =3D s->address_spaces[bus_num]; > - if (!pvtd_as) { > - /* No corresponding free() */ > - pvtd_as =3D g_malloc0(sizeof(VTDAddressSpace *) * VTD_PCI_DEVF= N_MAX); > - s->address_spaces[bus_num] =3D pvtd_as; > - } > - if (!pvtd_as[devfn]) { > - pvtd_as[devfn] =3D g_malloc0(sizeof(VTDAddressSpace)); > - > - pvtd_as[devfn]->bus_num =3D (uint8_t)bus_num; > - pvtd_as[devfn]->devfn =3D (uint8_t)devfn; > - pvtd_as[devfn]->iommu_state =3D s; > - pvtd_as[devfn]->context_cache_entry.context_cache_gen =3D 0; > - memory_region_init_iommu(&pvtd_as[devfn]->iommu, OBJECT(s), > - &s->iommu_ops, "intel_iommu", UINT64_= MAX); > - address_space_init(&pvtd_as[devfn]->as, > - &pvtd_as[devfn]->iommu, "intel_iommu"); > - } > - return &pvtd_as[devfn]->as; > + vtd_as =3D vtd_find_add_as(s, bus, devfn); > + return &vtd_as->as; > } > =20 > static void mch_init_dmar(MCHPCIState *mch) > diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iomm= u.h > index e321ee4..5dbadb7 100644 > --- a/include/hw/i386/intel_iommu.h > +++ b/include/hw/i386/intel_iommu.h > @@ -49,6 +49,7 @@ typedef struct VTDContextCacheEntry VTDContextCacheEn= try; > typedef struct IntelIOMMUState IntelIOMMUState; > typedef struct VTDAddressSpace VTDAddressSpace; > typedef struct VTDIOTLBEntry VTDIOTLBEntry; > +typedef struct VTDBus VTDBus; > =20 > /* Context-Entry */ > struct VTDContextEntry { > @@ -65,7 +66,7 @@ struct VTDContextCacheEntry { > }; > =20 > struct VTDAddressSpace { > - uint8_t bus_num; > + PCIBus *bus; > uint8_t devfn; > AddressSpace as; > MemoryRegion iommu; > @@ -73,6 +74,11 @@ struct VTDAddressSpace { > VTDContextCacheEntry context_cache_entry; > }; > =20 > +struct VTDBus { > + PCIBus* bus; /* A reference to the bus to provide translation for= */ > + VTDAddressSpace *dev_as[0]; /* A table of VTDAddressSpace objects = indexed by devfn */ > +}; > + > struct VTDIOTLBEntry { > uint64_t gfn; > uint16_t domain_id; > @@ -114,7 +120,13 @@ struct IntelIOMMUState { > GHashTable *iotlb; /* IOTLB */ > =20 > MemoryRegionIOMMUOps iommu_ops; > - VTDAddressSpace **address_spaces[VTD_PCI_BUS_MAX]; > + GHashTable *vtd_as_by_busptr; /* VTDBus objects indexed by PCIBu= s* reference */ > + VTDBus *vtd_as_by_bus_num[VTD_PCI_BUS_MAX]; /* VTDBus objects inde= xed by bus number */ > }; > =20 > +/* Find the VTD Address space associated with the given bus pointer, > + * create a new one if none exists > + */ > +VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int = devfn); > + > #endif > --=20 > 2.4.3