pci/ats: do not allow broken devices to be assigned to guests

Introduce a new field to mark devices as broken: having it set prevents
the device from being assigned to guests. Use the field in order to mark
ATS devices that have failed a flush when using VT-d as broken, thus
preventing them to be assigned to any guest.

This allows the device IOMMU context entry to be cleaned up properly, as
calling _pci_hide_device will just change the ownership of the device,
but the IOMMU context entry of the device would be left as-is. It would
also leak a VT-d Domain ID if using one, as removing the device from
its previous owner will allow releasing the IOMMU DID used by the device
without having cleaned up the context entry.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 22cb3872c22c061537f4ecebc60fa3cb32b43ffd..c6d99af5d47020a185f71bcf5fe84beb529aa8b1 100644 (file)
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -501,7 +501,7 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
     xfree(pdev);
 }
 
-static void _pci_hide_device(struct pci_dev *pdev)
+static void __init _pci_hide_device(struct pci_dev *pdev)
 {
     if ( pdev->domain )
         return;
@@ -1489,6 +1489,11 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
     ASSERT(pdev && (pdev->domain == hardware_domain ||
                     pdev->domain == dom_io));
 
+    /* Do not allow broken devices to be assigned to guests. */
+    rc = -EBADF;
+    if ( pdev->broken && d != hardware_domain && d != dom_io )
+        goto done;
+
     rc = pdev_msix_assign(d, pdev);
     if ( rc )
         goto done;
@@ -1587,9 +1592,7 @@ void iommu_dev_iotlb_flush_timeout(struct domain *d, struct pci_dev *pdev)
         return;
     }
 
-    list_del(&pdev->domain_list);
-    pdev->domain = NULL;
-    _pci_hide_device(pdev);
+    pdev->broken = true;
 
     if ( !d->is_shutting_down && printk_ratelimit() )
         printk(XENLOG_ERR "dom%d: ATS device %pp flush failed\n",

diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
index beeb65f0deec4d861254932284b60c7e65bb19f3..6a1c6bd7a9779cc362c5835da062d715e1f34003 100644 (file)
--- a/xen/drivers/passthrough/vtd/qinval.c
+++ b/xen/drivers/passthrough/vtd/qinval.c
@@ -227,7 +227,7 @@ static int __must_check dev_invalidate_sync(struct vtd_iommu *iommu,
 
     ASSERT(iommu->qinval_maddr);
     rc = queue_invalidate_wait(iommu, 0, 1, 1, 1);
-    if ( rc == -ETIMEDOUT )
+    if ( rc == -ETIMEDOUT && !pdev->broken )
     {
         struct domain *d = rcu_lock_domain_by_id(did_to_domain_id(iommu, did));
 
@@ -241,6 +241,12 @@ static int __must_check dev_invalidate_sync(struct vtd_iommu *iommu,
         iommu_dev_iotlb_flush_timeout(d, pdev);
         rcu_unlock_domain(d);
     }
+    else if ( rc == -ETIMEDOUT )
+        /*
+         * The device is already marked as broken, ignore the error in order to
+         * allow {de,}assign to succeed.
+         */
+        rc = 0;
 
     return rc;
 }

diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index b6d7e454f81465b3a1fcf740ba4c7123a7a08d3e..02b31f72593b53fc32c749c8693aa7fb94932bcf 100644 (file)
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -108,6 +108,9 @@ struct pci_dev {
     /* Device with errata, ignore the BARs. */
     bool ignore_bars;
 
+    /* Device misbehaving, prevent assigning it to guests. */
+    bool broken;
+
     enum pdev_type {
         DEV_TYPE_PCI_UNKNOWN,
         DEV_TYPE_PCIe_ENDPOINT,