[PATCH v14 0/3] VT-d Device-TLB flush issue

[PATCH v14 0/3] VT-d Device-TLB flush issue

[Xu, Quan]

From: Quan Xu <quan.xu@intel.com>

these patches fix current timeout concern and also allow limited ATS support.

these patches are the rest ones:
1. move the domain crash logic up to the generic IOMMU layer

2. If Device-TLB flush timed out, we hide the target ATS device
   immediately. By hiding the device, we make sure it can't be
   assigned to any domain any longer (see device_assigned).

---
Not covered in this series:

    a) Eliminate the panic() in IOMMU_WAIT_OP, used only in VT-d register read/write.
       Further discussion is required on whether and how to improve it.
    b) Handle IOTLB/Context/IEC flush timeout.
---
Quan Xu (3):
  IOMMU/x86: use a struct pci_dev* instead of SBDF
  IOMMU: add domain crash logic
  IOMMU: fix vt-d Device-TLB flush timeout issue

 xen/drivers/passthrough/amd/iommu_cmd.c     | 19 ++++----
 xen/drivers/passthrough/amd/pci_amd_iommu.c |  4 +-
 xen/drivers/passthrough/ats.h               | 10 ++---
 xen/drivers/passthrough/iommu.c             | 54 ++++++++++++++++++++++-
 xen/drivers/passthrough/pci.c               |  6 +--
 xen/drivers/passthrough/vtd/extern.h        |  5 ++-
 xen/drivers/passthrough/vtd/intremap.c      |  8 ++--
 xen/drivers/passthrough/vtd/iommu.c         | 25 ++++++++---
 xen/drivers/passthrough/vtd/qinval.c        | 56 ++++++++++++++++++------
 xen/drivers/passthrough/vtd/x86/ats.c       | 21 +++++----
 xen/drivers/passthrough/x86/ats.c           | 67 ++++++++++++++++++-----------
 xen/include/xen/iommu.h                     |  3 ++
 xen/include/xen/pci.h                       |  1 +
 13 files changed, 195 insertions(+), 84 deletions(-)

-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v14 1/3] IOMMU/x86: use a struct pci_dev* instead of SBDF

[Xu, Quan]

From: Quan Xu <quan.xu@intel.com>

A struct pci_dev* instead of SBDF is stored inside struct
pci_ats_dev and parameter to *_ats_device().

Also use ats_dev for "struct pci_ats_dev" variable, while
pdev (_pdev, if there is already a pdev) for "struct pci_dev"
in the scope of IOMMU.

Signed-off-by: Quan Xu <quan.xu@intel.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>

CC: Jan Beulich <jbeulich@suse.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Feng Wu <feng.wu@intel.com>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

---
v14: change 'ats_pdev' to 'ats_dev'.
---
 xen/drivers/passthrough/amd/iommu_cmd.c     | 19 ++++----
 xen/drivers/passthrough/amd/pci_amd_iommu.c |  4 +-
 xen/drivers/passthrough/ats.h               | 10 ++---
 xen/drivers/passthrough/vtd/intremap.c      |  8 ++--
 xen/drivers/passthrough/vtd/iommu.c         | 14 +++---
 xen/drivers/passthrough/vtd/x86/ats.c       | 24 +++++++----
 xen/drivers/passthrough/x86/ats.c           | 67 ++++++++++++++++++-----------
 7 files changed, 84 insertions(+), 62 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c b/xen/drivers/passthrough/amd/iommu_cmd.c
index 7c9d9be..7e010e6 100644
--- a/xen/drivers/passthrough/amd/iommu_cmd.c
+++ b/xen/drivers/passthrough/amd/iommu_cmd.c
@@ -289,35 +289,34 @@ void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
     unsigned long flags;
     struct amd_iommu *iommu;
     unsigned int req_id, queueid, maxpend;
-    struct pci_ats_dev *ats_pdev;
+    struct pci_ats_dev *ats_dev;
 
     if ( !ats_enabled )
         return;
 
-    ats_pdev = get_ats_device(pdev->seg, pdev->bus, pdev->devfn);
-    if ( ats_pdev == NULL )
+    ats_dev = get_ats_device(pdev);
+    if ( ats_dev == NULL )
         return;
 
-    if ( !pci_ats_enabled(ats_pdev->seg, ats_pdev->bus, ats_pdev->devfn) )
+    if ( !pci_ats_enabled(pdev->seg, pdev->bus, pdev->devfn) )
         return;
 
-    iommu = find_iommu_for_device(ats_pdev->seg,
-                                  PCI_BDF2(ats_pdev->bus, ats_pdev->devfn));
+    iommu = find_iommu_for_device(pdev->seg, PCI_BDF2(pdev->bus, pdev->devfn));
 
     if ( !iommu )
     {
         AMD_IOMMU_DEBUG("%s: Can't find iommu for %04x:%02x:%02x.%u\n",
-                        __func__, ats_pdev->seg, ats_pdev->bus,
-                        PCI_SLOT(ats_pdev->devfn), PCI_FUNC(ats_pdev->devfn));
+                        __func__, pdev->seg, pdev->bus,
+                        PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
         return;
     }
 
     if ( !iommu_has_cap(iommu, PCI_CAP_IOTLB_SHIFT) )
         return;
 
-    req_id = get_dma_requestor_id(iommu->seg, PCI_BDF2(ats_pdev->bus, devfn));
+    req_id = get_dma_requestor_id(iommu->seg, PCI_BDF2(pdev->bus, pdev->devfn));
     queueid = req_id;
-    maxpend = ats_pdev->ats_queue_depth & 0xff;
+    maxpend = ats_dev->ats_queue_depth & 0xff;
 
     /* send INVALIDATE_IOTLB_PAGES command */
     spin_lock_irqsave(&iommu->lock, flags);
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 7761241..dad4a71 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -162,7 +162,7 @@ static void amd_iommu_setup_domain_device(
          !pci_ats_enabled(iommu->seg, bus, pdev->devfn) )
     {
         if ( devfn == pdev->devfn )
-            enable_ats_device(iommu->seg, bus, devfn, iommu);
+            enable_ats_device(iommu, pdev);
 
         amd_iommu_flush_iotlb(devfn, pdev, INV_IOMMU_ALL_PAGES_ADDRESS, 0);
     }
@@ -356,7 +356,7 @@ void amd_iommu_disable_domain_device(struct domain *domain,
     if ( devfn == pdev->devfn &&
          pci_ats_device(iommu->seg, bus, devfn) &&
          pci_ats_enabled(iommu->seg, bus, devfn) )
-        disable_ats_device(iommu->seg, bus, devfn);
+        disable_ats_device(pdev);
 }
 
 static int reassign_device(struct domain *source, struct domain *target,
diff --git a/xen/drivers/passthrough/ats.h b/xen/drivers/passthrough/ats.h
index 5c91572..47ff22d 100644
--- a/xen/drivers/passthrough/ats.h
+++ b/xen/drivers/passthrough/ats.h
@@ -19,9 +19,7 @@
 
 struct pci_ats_dev {
     struct list_head list;
-    u16 seg;
-    u8 bus;
-    u8 devfn;
+    struct pci_dev *pdev;
     u16 ats_queue_depth;    /* ATS device invalidation queue depth */
     const void *iommu;      /* No common IOMMU struct so use void pointer */
 };
@@ -34,9 +32,9 @@ 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, 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);
+int enable_ats_device(const void *iommu, struct pci_dev *pdev);
+void disable_ats_device(struct pci_dev *pdev);
+struct pci_ats_dev *get_ats_device(const struct pci_dev *pdev);
 
 static inline int pci_ats_enabled(int seg, int bus, int devfn)
 {
diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c
index 7c11565..28e3cf4 100644
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -952,7 +952,7 @@ int pi_update_irte(const struct vcpu *v, const struct pirq *pirq,
     const struct msi_desc *msi_desc;
     int remap_index;
     int rc = 0;
-    const struct pci_dev *pci_dev;
+    const struct pci_dev *pdev;
     const struct acpi_drhd_unit *drhd;
     struct iommu *iommu;
     struct ir_ctrl *ir_ctrl;
@@ -972,8 +972,8 @@ int pi_update_irte(const struct vcpu *v, const struct pirq *pirq,
         goto unlock_out;
     }
 
-    pci_dev = msi_desc->dev;
-    if ( !pci_dev )
+    pdev = msi_desc->dev;
+    if ( !pdev )
     {
         rc = -ENODEV;
         goto unlock_out;
@@ -990,7 +990,7 @@ int pi_update_irte(const struct vcpu *v, const struct pirq *pirq,
      * 'struct msi_desc' in some other place, so we don't need to waste
      * time searching it here.
      */
-    drhd = acpi_find_matched_drhd_unit(pci_dev);
+    drhd = acpi_find_matched_drhd_unit(pdev);
     if ( !drhd )
         return -ENODEV;
 
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index f010612..cc34497 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1461,8 +1461,8 @@ int domain_context_mapping_one(
     return rc;
 }
 
-static int domain_context_mapping(
-    struct domain *domain, u8 devfn, const struct pci_dev *pdev)
+static int domain_context_mapping(struct domain *domain, u8 devfn,
+                                  struct pci_dev *pdev)
 {
     struct acpi_drhd_unit *drhd;
     int ret = 0;
@@ -1498,7 +1498,7 @@ static int domain_context_mapping(
         ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn,
                                          pdev);
         if ( !ret && devfn == pdev->devfn && ats_device(pdev, drhd) > 0 )
-            enable_ats_device(seg, bus, devfn, drhd->iommu);
+            enable_ats_device(drhd->iommu, pdev);
 
         break;
 
@@ -1611,8 +1611,8 @@ int domain_context_unmap_one(
     return rc;
 }
 
-static int domain_context_unmap(
-    struct domain *domain, u8 devfn, const struct pci_dev *pdev)
+static int domain_context_unmap(struct domain *domain, u8 devfn,
+                                struct pci_dev *pdev)
 {
     struct acpi_drhd_unit *drhd;
     struct iommu *iommu;
@@ -1648,7 +1648,7 @@ static int domain_context_unmap(
                    PCI_SLOT(devfn), PCI_FUNC(devfn));
         ret = domain_context_unmap_one(domain, iommu, bus, devfn);
         if ( !ret && devfn == pdev->devfn && ats_device(pdev, drhd) > 0 )
-            disable_ats_device(seg, bus, devfn);
+            disable_ats_device(pdev);
 
         break;
 
@@ -1994,7 +1994,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, drhd->iommu);
+    ret = enable_ats_device(drhd->iommu, pdev);
 
     return ret >= 0 ? 0 : ret;
 }
diff --git a/xen/drivers/passthrough/vtd/x86/ats.c b/xen/drivers/passthrough/vtd/x86/ats.c
index dfa4d30..11fe9bb 100644
--- a/xen/drivers/passthrough/vtd/x86/ats.c
+++ b/xen/drivers/passthrough/vtd/x86/ats.c
@@ -71,12 +71,14 @@ int ats_device(const struct pci_dev *pdev, const struct acpi_drhd_unit *drhd)
     return pos;
 }
 
-static int device_in_domain(struct iommu *iommu, struct pci_ats_dev *pdev, u16 did)
+static int device_in_domain(struct iommu *iommu, struct pci_ats_dev *ats_dev, u16 did)
 {
     struct root_entry *root_entry = NULL;
     struct context_entry *ctxt_entry = NULL;
     int tt, found = 0;
+    struct pci_dev *pdev = ats_dev->pdev;
 
+    ASSERT(pdev);
     root_entry = (struct root_entry *) map_vtd_domain_page(iommu->root_maddr);
     if ( !root_entry || !root_present(root_entry[pdev->bus]) )
         goto out;
@@ -108,37 +110,41 @@ out:
 int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
     u64 addr, unsigned int size_order, u64 type)
 {
-    struct pci_ats_dev *pdev;
+    struct pci_ats_dev *ats_dev;
     int ret = 0;
 
     if ( !ecap_dev_iotlb(iommu->ecap) )
         return ret;
 
-    list_for_each_entry( pdev, &ats_devices, list )
+    list_for_each_entry( ats_dev, &ats_devices, list )
     {
-        u16 sid = PCI_BDF2(pdev->bus, pdev->devfn);
+        struct pci_dev *pdev = ats_dev->pdev;
+        u16 sid;
         bool_t sbit;
         int rc = 0;
 
+        ASSERT(pdev);
+        sid = PCI_BDF2(pdev->bus, pdev->devfn);
+
         /* Only invalidate devices that belong to this IOMMU */
-        if ( pdev->iommu != iommu )
+        if ( ats_dev->iommu != iommu )
             continue;
 
         switch ( type )
         {
         case DMA_TLB_DSI_FLUSH:
-            if ( !device_in_domain(iommu, pdev, did) )
+            if ( !device_in_domain(iommu, ats_dev, did) )
                 break;
             /* fall through if DSI condition met */
         case DMA_TLB_GLOBAL_FLUSH:
             /* invalidate all translations: sbit=1,bit_63=0,bit[62:12]=1 */
             sbit = 1;
             addr = (~0UL << PAGE_SHIFT_4K) & 0x7FFFFFFFFFFFFFFF;
-            rc = qinval_device_iotlb_sync(iommu, pdev->ats_queue_depth,
+            rc = qinval_device_iotlb_sync(iommu, ats_dev->ats_queue_depth,
                                           sid, sbit, addr);
             break;
         case DMA_TLB_PSI_FLUSH:
-            if ( !device_in_domain(iommu, pdev, did) )
+            if ( !device_in_domain(iommu, ats_dev, did) )
                 break;
 
             /* if size <= 4K, set sbit = 0, else set sbit = 1 */
@@ -154,7 +160,7 @@ int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
                 addr |= (((u64)1 << (size_order - 1)) - 1) << PAGE_SHIFT_4K;
             }
 
-            rc = qinval_device_iotlb_sync(iommu, pdev->ats_queue_depth,
+            rc = qinval_device_iotlb_sync(iommu, ats_dev->ats_queue_depth,
                                           sid, sbit, addr);
             break;
         default:
diff --git a/xen/drivers/passthrough/x86/ats.c b/xen/drivers/passthrough/x86/ats.c
index 40c9f40..627b9e3 100644
--- a/xen/drivers/passthrough/x86/ats.c
+++ b/xen/drivers/passthrough/x86/ats.c
@@ -22,10 +22,12 @@ LIST_HEAD(ats_devices);
 bool_t __read_mostly ats_enabled = 0;
 boolean_param("ats", ats_enabled);
 
-int enable_ats_device(int seg, int bus, int devfn, const void *iommu)
+int enable_ats_device(const void *iommu, struct pci_dev *pdev)
 {
-    struct pci_ats_dev *pdev = NULL;
+    struct pci_ats_dev *ats_dev = NULL;
     u32 value;
+    u16 seg = pdev->seg;
+    u8 bus = pdev->bus, devfn = pdev->devfn;
     int pos;
 
     pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS);
@@ -39,9 +41,14 @@ int enable_ats_device(int seg, int bus, int devfn, const void *iommu)
                             PCI_FUNC(devfn), pos + ATS_REG_CTL);
     if ( value & ATS_ENABLE )
     {
-        list_for_each_entry ( pdev, &ats_devices, list )
+        list_for_each_entry ( ats_dev, &ats_devices, list )
         {
-            if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == devfn )
+            struct pci_dev *_pdev = ats_dev->pdev;
+
+            ASSERT(_pdev);
+            if ( _pdev->seg == seg &&
+                 _pdev->bus == bus &&
+                 _pdev->devfn == devfn )
             {
                 pos = 0;
                 break;
@@ -49,8 +56,8 @@ int enable_ats_device(int seg, int bus, int devfn, const void *iommu)
         }
     }
     if ( pos )
-        pdev = xmalloc(struct pci_ats_dev);
-    if ( !pdev )
+        ats_dev = xmalloc(struct pci_ats_dev);
+    if ( !ats_dev )
         return -ENOMEM;
 
     if ( !(value & ATS_ENABLE) )
@@ -62,15 +69,13 @@ int enable_ats_device(int seg, int bus, int devfn, const void *iommu)
 
     if ( pos )
     {
-        pdev->seg = seg;
-        pdev->bus = bus;
-        pdev->devfn = devfn;
-        pdev->iommu = iommu;
+        ats_dev->pdev = pdev;
+        ats_dev->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 ?:
-                                ATS_QUEUE_DEPTH_MASK + 1;
-        list_add(&pdev->list, &ats_devices);
+        ats_dev->ats_queue_depth = value & ATS_QUEUE_DEPTH_MASK ?:
+                                   ATS_QUEUE_DEPTH_MASK + 1;
+        list_add(&ats_dev->list, &ats_devices);
     }
 
     if ( iommu_verbose )
@@ -81,10 +86,12 @@ int enable_ats_device(int seg, int bus, int devfn, const void *iommu)
     return pos;
 }
 
-void disable_ats_device(int seg, int bus, int devfn)
+void disable_ats_device(struct pci_dev *pdev)
 {
-    struct pci_ats_dev *pdev;
+    struct pci_ats_dev *ats_dev;
     u32 value;
+    u16 seg = pdev->seg;
+    u8 bus = pdev->bus, devfn = pdev->devfn;
     int pos;
 
     pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS);
@@ -96,12 +103,17 @@ void disable_ats_device(int seg, int bus, int devfn)
     pci_conf_write16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
                      pos + ATS_REG_CTL, value);
 
-    list_for_each_entry ( pdev, &ats_devices, list )
+    list_for_each_entry ( ats_dev, &ats_devices, list )
     {
-        if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == devfn )
+        struct pci_dev *_pdev = ats_dev->pdev;
+
+        ASSERT(_pdev);
+        if ( _pdev->seg == seg &&
+             _pdev->bus == bus &&
+             _pdev->devfn == devfn )
         {
-            list_del(&pdev->list);
-            xfree(pdev);
+            list_del(&ats_dev->list);
+            xfree(ats_dev);
             break;
         }
     }
@@ -111,17 +123,24 @@ void disable_ats_device(int seg, int bus, int devfn)
                 seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
 }
 
-struct pci_ats_dev *get_ats_device(int seg, int bus, int devfn)
+struct pci_ats_dev *get_ats_device(const struct pci_dev *pdev)
 {
-    struct pci_ats_dev *pdev;
+    struct pci_ats_dev *ats_dev;
+    u16 seg = pdev->seg;
+    u8 bus = pdev->bus, devfn = pdev->devfn;
 
     if ( !pci_ats_device(seg, bus, devfn) )
         return NULL;
 
-    list_for_each_entry ( pdev, &ats_devices, list )
+    list_for_each_entry ( ats_dev, &ats_devices, list )
     {
-        if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == devfn )
-            return pdev;
+        struct pci_dev *_pdev = ats_dev->pdev;
+
+        ASSERT(_pdev);
+        if ( _pdev->seg == seg &&
+             _pdev->bus == bus &&
+             _pdev->devfn == devfn )
+            return ats_dev;
     }
 
     return NULL;
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v14 2/3] IOMMU: add domain crash logic

[Xu, Quan]

From: Quan Xu <quan.xu@intel.com>

Add domain crash logic to the generic IOMMU layer to benefit
all platforms.

No spamming of the log can occur. For DomU, we avoid logging any
message for already dying domains. For Dom0, that'll still be more
verbose than we'd really like, but it at least wouldn't outright
flood the console.

Signed-off-by: Quan Xu <quan.xu@intel.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>

CC: Julien Grall <julien.grall@arm.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Feng Wu <feng.wu@intel.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 xen/drivers/passthrough/iommu.c     | 30 ++++++++++++++++++++++++++++--
 xen/drivers/passthrough/vtd/iommu.c | 11 +++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 7656aeb..d793f5d 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -318,21 +318,47 @@ int iommu_iotlb_flush(struct domain *d, unsigned long gfn,
                       unsigned int page_count)
 {
     const struct domain_iommu *hd = dom_iommu(d);
+    int rc;
 
     if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops->iotlb_flush )
         return 0;
 
-    return hd->platform_ops->iotlb_flush(d, gfn, page_count);
+    rc = hd->platform_ops->iotlb_flush(d, gfn, page_count);
+    if ( unlikely(rc) )
+    {
+        if ( !d->is_shutting_down && printk_ratelimit() )
+            printk(XENLOG_ERR
+                   "d%d: IOMMU IOTLB flush failed: %d, gfn %#lx, page count %u\n",
+                   d->domain_id, rc, gfn, page_count);
+
+        if ( !is_hardware_domain(d) )
+            domain_crash(d);
+    }
+
+    return rc;
 }
 
 int iommu_iotlb_flush_all(struct domain *d)
 {
     const struct domain_iommu *hd = dom_iommu(d);
+    int rc;
 
     if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops->iotlb_flush_all )
         return 0;
 
-    return hd->platform_ops->iotlb_flush_all(d);
+    rc = hd->platform_ops->iotlb_flush_all(d);
+    if ( unlikely(rc) )
+    {
+        if ( !d->is_shutting_down && printk_ratelimit() )
+            printk(XENLOG_ERR
+                   "d%d: IOMMU IOTLB flush all failed: %d\n",
+                   d->domain_id, rc);
+
+        if ( !is_hardware_domain(d) )
+            domain_crash(d);
+    }
+
+    return rc;
 }
 
 int __init iommu_setup(void)
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index cc34497..a02b4c40 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1847,6 +1847,17 @@ int iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
         }
     }
 
+    if ( unlikely(rc) )
+    {
+        if ( !d->is_shutting_down && printk_ratelimit() )
+            printk(XENLOG_ERR VTDPREFIX
+                   " d%d: IOMMU pages flush failed: %d\n",
+                   d->domain_id, rc);
+
+        if ( !is_hardware_domain(d) )
+            domain_crash(d);
+    }
+
     return rc;
 }
 
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Xu, Quan]

From: Quan Xu <quan.xu@intel.com>

If Device-TLB flush timed out, we hide the target ATS device
immediately. By hiding the device, we make sure it can't be
assigned to any domain any longer (see device_assigned).

Signed-off-by: Quan Xu <quan.xu@intel.com>

CC: Jan Beulich <jbeulich@suse.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Feng Wu <feng.wu@intel.com>

---
v14: release the lock before return.
---
 xen/drivers/passthrough/iommu.c       | 24 +++++++++++++++
 xen/drivers/passthrough/pci.c         |  6 ++--
 xen/drivers/passthrough/vtd/extern.h  |  5 ++--
 xen/drivers/passthrough/vtd/qinval.c  | 56 +++++++++++++++++++++++++++--------
 xen/drivers/passthrough/vtd/x86/ats.c | 11 ++-----
 xen/include/xen/iommu.h               |  3 ++
 xen/include/xen/pci.h                 |  1 +
 7 files changed, 79 insertions(+), 27 deletions(-)

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index d793f5d..2353c7d 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -361,6 +361,30 @@ int iommu_iotlb_flush_all(struct domain *d)
     return rc;
 }
 
+void iommu_dev_iotlb_flush_timeout(struct domain *d,
+                                   struct pci_dev *pdev)
+{
+    pcidevs_lock();
+
+    ASSERT(pdev->domain);
+    if ( d != pdev->domain )
+    {
+        pcidevs_unlock();
+        return;
+    }
+
+    list_del(&pdev->domain_list);
+    pdev->domain = NULL;
+    pci_hide_existing_device(pdev);
+    if ( !d->is_shutting_down && printk_ratelimit() )
+        printk(XENLOG_ERR
+               "dom%d: ATS device %04x:%02x:%02x.%u flush failed\n",
+               d->domain_id, pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
+               PCI_FUNC(pdev->devfn));
+
+    pcidevs_unlock();
+}
+
 int __init iommu_setup(void)
 {
     int rc = -ENODEV;
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index bb5f344..58bfb79 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -419,7 +419,7 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
     xfree(pdev);
 }
 
-static void _pci_hide_device(struct pci_dev *pdev)
+void pci_hide_existing_device(struct pci_dev *pdev)
 {
     if ( pdev->domain )
         return;
@@ -436,7 +436,7 @@ int __init pci_hide_device(int bus, int devfn)
     pdev = alloc_pdev(get_pseg(0), bus, devfn);
     if ( pdev )
     {
-        _pci_hide_device(pdev);
+        pci_hide_existing_device(pdev);
         rc = 0;
     }
     pcidevs_unlock();
@@ -466,7 +466,7 @@ int __init pci_ro_device(int seg, int bus, int devfn)
     }
 
     __set_bit(PCI_BDF2(bus, devfn), pseg->ro_map);
-    _pci_hide_device(pdev);
+    pci_hide_existing_device(pdev);
 
     return 0;
 }
diff --git a/xen/drivers/passthrough/vtd/extern.h b/xen/drivers/passthrough/vtd/extern.h
index 45357f2..efaff28 100644
--- a/xen/drivers/passthrough/vtd/extern.h
+++ b/xen/drivers/passthrough/vtd/extern.h
@@ -25,6 +25,7 @@
 
 #define VTDPREFIX "[VT-D]"
 
+struct pci_ats_dev;
 extern bool_t rwbf_quirk;
 
 void print_iommu_regs(struct acpi_drhd_unit *drhd);
@@ -60,8 +61,8 @@ int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
                          u64 addr, unsigned int size_order, u64 type);
 
 int __must_check qinval_device_iotlb_sync(struct iommu *iommu,
-                                          u32 max_invs_pend,
-                                          u16 sid, u16 size, u64 addr);
+                                          struct pci_ats_dev *ats_dev,
+                                          u16 did, u16 size, u64 addr);
 
 unsigned int get_cache_line_size(void);
 void cacheline_flush(char *);
diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
index 4492b29..7a5c433 100644
--- a/xen/drivers/passthrough/vtd/qinval.c
+++ b/xen/drivers/passthrough/vtd/qinval.c
@@ -27,11 +27,11 @@
 #include "dmar.h"
 #include "vtd.h"
 #include "extern.h"
+#include "../ats.h"
 
 #define VTD_QI_TIMEOUT	1
 
-static int __must_check invalidate_sync(struct iommu *iommu,
-                                        bool_t flush_dev_iotlb);
+static int __must_check invalidate_sync(struct iommu *iommu);
 
 static void print_qi_regs(struct iommu *iommu)
 {
@@ -103,7 +103,7 @@ static int __must_check queue_invalidate_context_sync(struct iommu *iommu,
 
     unmap_vtd_domain_page(qinval_entries);
 
-    return invalidate_sync(iommu, 0);
+    return invalidate_sync(iommu);
 }
 
 static int __must_check queue_invalidate_iotlb_sync(struct iommu *iommu,
@@ -140,7 +140,7 @@ static int __must_check queue_invalidate_iotlb_sync(struct iommu *iommu,
     qinval_update_qtail(iommu, index);
     spin_unlock_irqrestore(&iommu->register_lock, flags);
 
-    return invalidate_sync(iommu, 0);
+    return invalidate_sync(iommu);
 }
 
 static int __must_check queue_invalidate_wait(struct iommu *iommu,
@@ -199,25 +199,55 @@ static int __must_check queue_invalidate_wait(struct iommu *iommu,
     return -EOPNOTSUPP;
 }
 
-static int __must_check invalidate_sync(struct iommu *iommu,
-                                        bool_t flush_dev_iotlb)
+static int __must_check invalidate_sync(struct iommu *iommu)
 {
     struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
 
     ASSERT(qi_ctrl->qinval_maddr);
 
-    return queue_invalidate_wait(iommu, 0, 1, 1, flush_dev_iotlb);
+    return queue_invalidate_wait(iommu, 0, 1, 1, 0);
+}
+
+static int __must_check dev_invalidate_sync(struct iommu *iommu,
+                                            struct pci_dev *pdev, u16 did)
+{
+    struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
+    int rc;
+
+    ASSERT(qi_ctrl->qinval_maddr);
+    rc = queue_invalidate_wait(iommu, 0, 1, 1, 1);
+    if ( rc == -ETIMEDOUT )
+    {
+        struct domain *d = NULL;
+
+        if ( test_bit(did, iommu->domid_bitmap) )
+            d = rcu_lock_domain_by_id(iommu->domid_map[did]);
+
+        /*
+         * In case the domain has been freed or the IOMMU domid bitmap is
+         * not valid, the device no longer belongs to this domain.
+         */
+        if ( d == NULL )
+            return rc;
+
+        iommu_dev_iotlb_flush_timeout(d, pdev);
+        rcu_unlock_domain(d);
+    }
+
+    return rc;
 }
 
 int qinval_device_iotlb_sync(struct iommu *iommu,
-                             u32 max_invs_pend,
-                             u16 sid, u16 size, u64 addr)
+                             struct pci_ats_dev *ats_dev,
+                             u16 did, u16 size, u64 addr)
 {
     unsigned long flags;
     unsigned int index;
     u64 entry_base;
     struct qinval_entry *qinval_entry, *qinval_entries;
+    struct pci_dev *pdev = ats_dev->pdev;
 
+    ASSERT(pdev);
     spin_lock_irqsave(&iommu->register_lock, flags);
     index = qinval_next_index(iommu);
     entry_base = iommu_qi_ctrl(iommu)->qinval_maddr +
@@ -227,9 +257,9 @@ int qinval_device_iotlb_sync(struct iommu *iommu,
 
     qinval_entry->q.dev_iotlb_inv_dsc.lo.type = TYPE_INVAL_DEVICE_IOTLB;
     qinval_entry->q.dev_iotlb_inv_dsc.lo.res_1 = 0;
-    qinval_entry->q.dev_iotlb_inv_dsc.lo.max_invs_pend = max_invs_pend;
+    qinval_entry->q.dev_iotlb_inv_dsc.lo.max_invs_pend = ats_dev->ats_queue_depth;
     qinval_entry->q.dev_iotlb_inv_dsc.lo.res_2 = 0;
-    qinval_entry->q.dev_iotlb_inv_dsc.lo.sid = sid;
+    qinval_entry->q.dev_iotlb_inv_dsc.lo.sid = PCI_BDF2(pdev->bus, pdev->devfn);
     qinval_entry->q.dev_iotlb_inv_dsc.lo.res_3 = 0;
 
     qinval_entry->q.dev_iotlb_inv_dsc.hi.size = size;
@@ -240,7 +270,7 @@ int qinval_device_iotlb_sync(struct iommu *iommu,
     qinval_update_qtail(iommu, index);
     spin_unlock_irqrestore(&iommu->register_lock, flags);
 
-    return invalidate_sync(iommu, 1);
+    return dev_invalidate_sync(iommu, pdev, did);
 }
 
 static int __must_check queue_invalidate_iec_sync(struct iommu *iommu,
@@ -271,7 +301,7 @@ static int __must_check queue_invalidate_iec_sync(struct iommu *iommu,
     qinval_update_qtail(iommu, index);
     spin_unlock_irqrestore(&iommu->register_lock, flags);
 
-    ret = invalidate_sync(iommu, 0);
+    ret = invalidate_sync(iommu);
 
     /*
      * reading vt-d architecture register will ensure
diff --git a/xen/drivers/passthrough/vtd/x86/ats.c b/xen/drivers/passthrough/vtd/x86/ats.c
index 11fe9bb..23e46b1 100644
--- a/xen/drivers/passthrough/vtd/x86/ats.c
+++ b/xen/drivers/passthrough/vtd/x86/ats.c
@@ -118,14 +118,9 @@ int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
 
     list_for_each_entry( ats_dev, &ats_devices, list )
     {
-        struct pci_dev *pdev = ats_dev->pdev;
-        u16 sid;
         bool_t sbit;
         int rc = 0;
 
-        ASSERT(pdev);
-        sid = PCI_BDF2(pdev->bus, pdev->devfn);
-
         /* Only invalidate devices that belong to this IOMMU */
         if ( ats_dev->iommu != iommu )
             continue;
@@ -140,8 +135,7 @@ int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
             /* invalidate all translations: sbit=1,bit_63=0,bit[62:12]=1 */
             sbit = 1;
             addr = (~0UL << PAGE_SHIFT_4K) & 0x7FFFFFFFFFFFFFFF;
-            rc = qinval_device_iotlb_sync(iommu, ats_dev->ats_queue_depth,
-                                          sid, sbit, addr);
+            rc = qinval_device_iotlb_sync(iommu, ats_dev, did, sbit, addr);
             break;
         case DMA_TLB_PSI_FLUSH:
             if ( !device_in_domain(iommu, ats_dev, did) )
@@ -160,8 +154,7 @@ int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
                 addr |= (((u64)1 << (size_order - 1)) - 1) << PAGE_SHIFT_4K;
             }
 
-            rc = qinval_device_iotlb_sync(iommu, ats_dev->ats_queue_depth,
-                                          sid, sbit, addr);
+            rc = qinval_device_iotlb_sync(iommu, ats_dev, did, sbit, addr);
             break;
         default:
             dprintk(XENLOG_WARNING VTDPREFIX, "invalid vt-d flush type\n");
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index a759f2b..e0f7d52 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -207,6 +207,9 @@ int __must_check iommu_iotlb_flush(struct domain *d, unsigned long gfn,
                                    unsigned int page_count);
 int __must_check iommu_iotlb_flush_all(struct domain *d);
 
+void iommu_dev_iotlb_flush_timeout(struct domain *d,
+                                   struct pci_dev *pdev);
+
 /*
  * The purpose of the iommu_dont_flush_iotlb optional cpu flag is to
  * avoid unecessary iotlb_flush in the low level IOMMU code.
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 6ed29dd..e4940cd 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -118,6 +118,7 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
 int pci_remove_device(u16 seg, u8 bus, u8 devfn);
 int pci_ro_device(int seg, int bus, int devfn);
 int pci_hide_device(int bus, int devfn);
+void pci_hide_existing_device(struct pci_dev *pdev);
 struct pci_dev *pci_get_pdev(int seg, int bus, int devfn);
 struct pci_dev *pci_get_real_pdev(int seg, int bus, int devfn);
 struct pci_dev *pci_get_pdev_by_domain(
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Tian, Kevin]

> From: Xu, Quan
> Sent: Monday, July 04, 2016 5:12 PM
> 
> From: Quan Xu <quan.xu@intel.com>
> 
> If Device-TLB flush timed out, we hide the target ATS device
> immediately. By hiding the device, we make sure it can't be
> assigned to any domain any longer (see device_assigned).
> 
> Signed-off-by: Quan Xu <quan.xu@intel.com>
> 
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Kevin Tian <kevin.tian@intel.com>
> CC: Feng Wu <feng.wu@intel.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Xu, Quan]

On July 05, 2016 8:47 AM, Tian, Kevin <kevin.tian@intel.com> wrote:
> > From: Xu, Quan
> > Sent: Monday, July 04, 2016 5:12 PM
> >
> > From: Quan Xu <quan.xu@intel.com>
> >
> > If Device-TLB flush timed out, we hide the target ATS device
> > immediately. By hiding the device, we make sure it can't be assigned
> > to any domain any longer (see device_assigned).
> >
> > Signed-off-by: Quan Xu <quan.xu@intel.com>
> >
> > CC: Jan Beulich <jbeulich@suse.com>
> > CC: Kevin Tian <kevin.tian@intel.com>
> > CC: Feng Wu <feng.wu@intel.com>
> 
> Acked-by: Kevin Tian <kevin.tian@intel.com>

Jan,  now there patches  are all acked-by Kevin. Is it ready to go in?
If not,  could you send out your comments, I shall be very grateful and fix it today.

Quan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Jan Beulich]

>>> On 05.07.16 at 04:04, <quan.xu@intel.com> wrote:
> On July 05, 2016 8:47 AM, Tian, Kevin <kevin.tian@intel.com> wrote:
>> > From: Xu, Quan
>> > Sent: Monday, July 04, 2016 5:12 PM
>> >
>> > From: Quan Xu <quan.xu@intel.com>
>> >
>> > If Device-TLB flush timed out, we hide the target ATS device
>> > immediately. By hiding the device, we make sure it can't be assigned
>> > to any domain any longer (see device_assigned).
>> >
>> > Signed-off-by: Quan Xu <quan.xu@intel.com>
>> >
>> > CC: Jan Beulich <jbeulich@suse.com>
>> > CC: Kevin Tian <kevin.tian@intel.com>
>> > CC: Feng Wu <feng.wu@intel.com>
>> 
>> Acked-by: Kevin Tian <kevin.tian@intel.com>
> 
> Jan,  now there patches  are all acked-by Kevin. Is it ready to go in?
> If not,  could you send out your comments, I shall be very grateful and fix 
> it today.

You sent the most recent version yesterday. Please allow more than
just one day before you ping.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Xu, Quan]

On July 05, 2016 3:00 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >>> On 05.07.16 at 04:04, <quan.xu@intel.com> wrote:
> > On July 05, 2016 8:47 AM, Tian, Kevin <kevin.tian@intel.com> wrote:
> >> > From: Xu, Quan
> >> > Sent: Monday, July 04, 2016 5:12 PM
> >> >
> >> > From: Quan Xu <quan.xu@intel.com>
> >> >
> >> > If Device-TLB flush timed out, we hide the target ATS device
> >> > immediately. By hiding the device, we make sure it can't be
> >> > assigned to any domain any longer (see device_assigned).
> >> >
> >> > Signed-off-by: Quan Xu <quan.xu@intel.com>
> >> >
> >> > CC: Jan Beulich <jbeulich@suse.com>
> >> > CC: Kevin Tian <kevin.tian@intel.com>
> >> > CC: Feng Wu <feng.wu@intel.com>
> >>
> >> Acked-by: Kevin Tian <kevin.tian@intel.com>
> >
> > Jan,  now there patches  are all acked-by Kevin. Is it ready to go in?
> > If not,  could you send out your comments, I shall be very grateful
> > and fix it today.
> 
> You sent the most recent version yesterday. Please allow more than just one
> day before you ping.
> 

Sorry for that, as today is my last working day at Intel Corporation. 
I really want to close it as a  milestone (but if there is still some comment, I will continue to fix it).

Quan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v14 1/3] IOMMU/x86: use a struct pci_dev* instead of SBDF

[Jan Beulich]

>>> On 04.07.16 at 11:11, <quan.xu@intel.com> wrote:
> --- a/xen/drivers/passthrough/ats.h
> +++ b/xen/drivers/passthrough/ats.h
> @@ -19,9 +19,7 @@
>  
>  struct pci_ats_dev {
>      struct list_head list;
> -    u16 seg;
> -    u8 bus;
> -    u8 devfn;
> +    struct pci_dev *pdev;
>      u16 ats_queue_depth;    /* ATS device invalidation queue depth */
>      const void *iommu;      /* No common IOMMU struct so use void pointer */
>  };

Taking into consideration the single use the struct iommu * here has,
I think a different approach is needed to achieve the goal: Eliminate
the global "ats_devices" and link ATS devices on a per-IOMMU list.
Since that'll leave (besides the list member and the struct pci_dev
back pointer) only ats_queue_depth, I then don't see the existence
of the structure above warranted anymore: It should be subsumed
into struct pci_dev.

> @@ -34,9 +32,9 @@ 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, 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);
> +int enable_ats_device(const void *iommu, struct pci_dev *pdev);

The ordering of parameters is backwards here.

> --- a/xen/drivers/passthrough/x86/ats.c
> +++ b/xen/drivers/passthrough/x86/ats.c
> @@ -22,10 +22,12 @@ LIST_HEAD(ats_devices);
>  bool_t __read_mostly ats_enabled = 0;
>  boolean_param("ats", ats_enabled);
>  
> -int enable_ats_device(int seg, int bus, int devfn, const void *iommu)
> +int enable_ats_device(const void *iommu, struct pci_dev *pdev)
>  {
> -    struct pci_ats_dev *pdev = NULL;
> +    struct pci_ats_dev *ats_dev = NULL;
>      u32 value;
> +    u16 seg = pdev->seg;
> +    u8 bus = pdev->bus, devfn = pdev->devfn;
>      int pos;
>  
>      pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS);
> @@ -39,9 +41,14 @@ int enable_ats_device(int seg, int bus, int devfn, const void *iommu)
>                              PCI_FUNC(devfn), pos + ATS_REG_CTL);
>      if ( value & ATS_ENABLE )
>      {
> -        list_for_each_entry ( pdev, &ats_devices, list )
> +        list_for_each_entry ( ats_dev, &ats_devices, list )
>          {
> -            if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == devfn )
> +            struct pci_dev *_pdev = ats_dev->pdev;
> +
> +            ASSERT(_pdev);
> +            if ( _pdev->seg == seg &&
> +                 _pdev->bus == bus &&
> +                 _pdev->devfn == devfn )

Why do you do SBDF based compares, when you can simply compare
the two pointers now? (And btw., local variables should never begin
with an underscore.)

Since you had indicated that today is your last day at Intel, I guess
I'll see to pick up this series (or maybe Kevin could) to bring it to the
point where it can go in.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Jan Beulich]

>>> On 04.07.16 at 11:11, <quan.xu@intel.com> wrote:
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -361,6 +361,30 @@ int iommu_iotlb_flush_all(struct domain *d)
>      return rc;
>  }
>  
> +void iommu_dev_iotlb_flush_timeout(struct domain *d,

const

> +                                   struct pci_dev *pdev)
> +{
> +    pcidevs_lock();
> +
> +    ASSERT(pdev->domain);
> +    if ( d != pdev->domain )
> +    {
> +        pcidevs_unlock();
> +        return;
> +    }
> +
> +    list_del(&pdev->domain_list);
> +    pdev->domain = NULL;
> +    pci_hide_existing_device(pdev);
> +    if ( !d->is_shutting_down && printk_ratelimit() )
> +        printk(XENLOG_ERR
> +               "dom%d: ATS device %04x:%02x:%02x.%u flush failed\n",
> +               d->domain_id, pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> +               PCI_FUNC(pdev->devfn));
> +
> +    pcidevs_unlock();
> +}

I'm missing the domain_crash() part here (which would be the only
reason why the parameter above can't be const).

> +static int __must_check dev_invalidate_sync(struct iommu *iommu,
> +                                            struct pci_dev *pdev, u16 did)

const

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v14 1/3] IOMMU/x86: use a struct pci_dev* instead of SBDF

[Xu, Quan]

On July 05, 2016 9:38 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >>> On 04.07.16 at 11:11, <quan.xu@intel.com> wrote:
> Since you had indicated that today is your last day at Intel, I guess I'll see to
> pick up this series (or maybe Kevin could) to bring it to the point where it can
> go in.
> 

Jan,  this is the best idea if you can pick this series.  Thanks very much!!

I very much appreciated!!
-Quan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Xu, Quan]

Jan, 
Just check it, do you agree to pick up this series?

Quan

On July 05, 2016 9:46 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >>> On 04.07.16 at 11:11, <quan.xu@intel.com> wrote:
> > --- a/xen/drivers/passthrough/iommu.c
> > +++ b/xen/drivers/passthrough/iommu.c
> > @@ -361,6 +361,30 @@ int iommu_iotlb_flush_all(struct domain *d)
> >      return rc;
> >  }
> >
> > +void iommu_dev_iotlb_flush_timeout(struct domain *d,
> 
> const
> 
> > +                                   struct pci_dev *pdev) {
> > +    pcidevs_lock();
> > +
> > +    ASSERT(pdev->domain);
> > +    if ( d != pdev->domain )
> > +    {
> > +        pcidevs_unlock();
> > +        return;
> > +    }
> > +
> > +    list_del(&pdev->domain_list);
> > +    pdev->domain = NULL;
> > +    pci_hide_existing_device(pdev);
> > +    if ( !d->is_shutting_down && printk_ratelimit() )
> > +        printk(XENLOG_ERR
> > +               "dom%d: ATS device %04x:%02x:%02x.%u flush failed\n",
> > +               d->domain_id, pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> > +               PCI_FUNC(pdev->devfn));
> > +
> > +    pcidevs_unlock();
> > +}
> 
> I'm missing the domain_crash() part here (which would be the only reason
> why the parameter above can't be const).
> 
> > +static int __must_check dev_invalidate_sync(struct iommu *iommu,
> > +                                            struct pci_dev *pdev, u16
> > +did)
> 
> const
> 
> Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v14 3/3] IOMMU: fix vt-d Device-TLB flush timeout issue

[Jan Beulich]

>>> On 05.07.16 at 15:56, <quan.xu@intel.com> wrote:
> Just check it, do you agree to pick up this series?

Depends on Kevin's view; I wanted to send out review feedback
publicly regardless of who'd be picking it up.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel