IOMMU: prevent VT-d device IOTLB operations on wrong IOMMU

PCIe ATS allows for devices to contain IOTLBs, the VT-d code was iterating
around all ATS capable devices and issuing IOTLB operations for all IOMMUs,
even though each ATS device is only accessible via one particular IOMMU.

Issuing an IOMMU operation to a device not accessible via that IOMMU results
in an IOMMU timeout because the device does not reply. VT-d IOMMU timeouts
result in a Xen panic.

Therefore this bug prevents any Intel system with 2 or more ATS enabled IOMMUs,
each with an ATS device connected to them, from booting Xen.

The patch adds a IOMMU pointer to the ATS device struct so the VT-d code can
ensure it does not issue IOMMU ATS operations on the wrong IOMMU. A void
pointer has to be used because AMD and Intel IOMMU implementations do not have
a common IOMMU structure or indexing mechanism.

Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 84c340ba4c3eb99278b6ba885616bb183b88ad67
master date: 2014-06-18 15:50:02 +0200

diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index c3cbf887fedae8eeafa29e077775f4008dbe4c17..75b323bb299719b1522eefdc49baa5b090b9cff1 100644 (file)
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -137,7 +137,7 @@ static void amd_iommu_setup_domain_device(
     {
         struct pci_dev *pdev;
 
-        enable_ats_device(iommu->seg, bus, devfn);
+        enable_ats_device(iommu->seg, bus, devfn, iommu);
 
         ASSERT(spin_is_locked(&pcidevs_lock));
         pdev = pci_get_pdev(iommu->seg, bus, devfn);

diff --git a/xen/drivers/passthrough/ats.h b/xen/drivers/passthrough/ats.h
index c34fa2cad5ef19a3b56b8c20930dc77d338db90c..cf082afcc526bb0d94f42d6d0592924e4d17650e 100644 (file)
--- a/xen/drivers/passthrough/ats.h
+++ b/xen/drivers/passthrough/ats.h
@@ -24,6 +24,7 @@ struct pci_ats_dev {
     u8 bus;
     u8 devfn;
     u16 ats_queue_depth;    /* ATS device invalidation queue depth */
+    const void *iommu;      /* No common IOMMU struct so use void pointer */
 };
 
 #ifdef CONFIG_X86_64
@@ -36,7 +37,7 @@ struct pci_ats_dev {
 extern struct list_head ats_devices;
 extern bool_t ats_enabled;
 
-int enable_ats_device(int seg, int bus, int devfn);
+int enable_ats_device(int seg, int bus, int devfn, const void *iommu);
 void disable_ats_device(int seg, int bus, int devfn);
 struct pci_ats_dev *get_ats_device(int seg, int bus, int devfn);
 
@@ -64,7 +65,7 @@ static inline int pci_ats_device(int seg, int bus, int devfn)
 #else
 
 #define ats_enabled 0
-static inline int enable_ats_device(int seg, int bus, int devfn)
+static inline int enable_ats_device(int seg, int bus, int devfn, const void *iommu)
 {
     BUG();
     return -ENOSYS;

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index bb7c4443d527ba0fe8e66ca5c89d65ca0020f071..51c3f9807346c833aca7363452764d55530cd560 100644 (file)
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1475,7 +1475,7 @@ static int domain_context_mapping(
                     PCI_SLOT(devfn), PCI_FUNC(devfn));
         ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn);
         if ( !ret && ats_device(pdev, drhd) > 0 )
-            enable_ats_device(seg, bus, devfn);
+            enable_ats_device(seg, bus, devfn, drhd->iommu);
 
         break;
 
@@ -1961,7 +1961,7 @@ static int intel_iommu_enable_device(struct pci_dev *pdev)
     if ( ret <= 0 )
         return ret;
 
-    ret = enable_ats_device(pdev->seg, pdev->bus, pdev->devfn);
+    ret = enable_ats_device(pdev->seg, pdev->bus, pdev->devfn, drhd->iommu);
 
     return ret >= 0 ? 0 : ret;
 }

diff --git a/xen/drivers/passthrough/vtd/x86/ats.c b/xen/drivers/passthrough/vtd/x86/ats.c
index f3b8c2d9d300688e22da8c1166d6c469c602374b..ea57d7d29f77123a0bbc92afb1265f3aed4fa25d 100644 (file)
--- a/xen/drivers/passthrough/vtd/x86/ats.c
+++ b/xen/drivers/passthrough/vtd/x86/ats.c
@@ -120,6 +120,10 @@ int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
     {
         sid = (pdev->bus << 8) | pdev->devfn;
 
+        /* Only invalidate devices that belong to this IOMMU */
+        if ( pdev->iommu != iommu )
+            continue;
+
         switch ( type ) {
         case DMA_TLB_DSI_FLUSH:
             if ( !device_in_domain(iommu, pdev, did) )

diff --git a/xen/drivers/passthrough/x86/ats.c b/xen/drivers/passthrough/x86/ats.c
index bb7ee9ab68f0a844d70b6d673cf80511f630aea2..1e3e03ab404b0fb2469a368d106ec2149f3b6676 100644 (file)
--- a/xen/drivers/passthrough/x86/ats.c
+++ b/xen/drivers/passthrough/x86/ats.c
@@ -23,7 +23,7 @@ LIST_HEAD(ats_devices);
 bool_t __read_mostly ats_enabled = 1;
 boolean_param("ats", ats_enabled);
 
-int enable_ats_device(int seg, int bus, int devfn)
+int enable_ats_device(int seg, int bus, int devfn, const void *iommu)
 {
     struct pci_ats_dev *pdev = NULL;
     u32 value;
@@ -66,6 +66,7 @@ int enable_ats_device(int seg, int bus, int devfn)
         pdev->seg = seg;
         pdev->bus = bus;
         pdev->devfn = devfn;
+        pdev->iommu = iommu;
         value = pci_conf_read16(seg, bus, PCI_SLOT(devfn),
                                 PCI_FUNC(devfn), pos + ATS_REG_CAP);
         pdev->ats_queue_depth = value & ATS_QUEUE_DEPTH_MASK ?: