xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 0/3] Check VT-d Device-TLB flush error
@ 2016-06-17  3:37 Xu, Quan
  2016-06-17  3:37 ` [PATCH v9 1/3] vt-d: fix the IOMMU flush issue Xu, Quan
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Xu, Quan @ 2016-06-17  3:37 UTC (permalink / raw)
  To: xen-devel; +Cc: dario.faggioli, Quan Xu

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

This patch set is a prereq patch set for Patch:'VT-d Device-TLB flush issue'.

While IOMMU Device-TLB flush timed out, xen calls panic() at present. However the existing panic()
is going to be eliminated, so we must propagate the IOMMU Device-TLB flush error up to the call trees.

This patch set is also based on the discussion of 'abstract model of IOMMU unmaping/mapping failures'.

---
Quan Xu (3):
  vt-d: fix the IOMMU flush issue
  vt-d: propagate the IOMMU Device-TLB flush error up to ME phantom
    functions
  vt-d: add __must_check annotation to IOMMU flush pointers and handlers

 xen/drivers/passthrough/vtd/extern.h |   3 +-
 xen/drivers/passthrough/vtd/iommu.c  | 184 +++++++++++++++++++++++------------
 xen/drivers/passthrough/vtd/iommu.h  |  11 ++-
 xen/drivers/passthrough/vtd/qinval.c |  14 +--
 xen/drivers/passthrough/vtd/quirks.c |  27 +++--
 5 files changed, 153 insertions(+), 86 deletions(-)

-- 
1.9.1


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v9 1/3] vt-d: fix the IOMMU flush issue
  2016-06-17  3:37 [PATCH v9 0/3] Check VT-d Device-TLB flush error Xu, Quan
@ 2016-06-17  3:37 ` Xu, Quan
  2016-06-21 13:25   ` Jan Beulich
  2016-06-17  3:37 ` [PATCH v9 2/3] vt-d: propagate the IOMMU Device-TLB flush error up to ME phantom functions Xu, Quan
  2016-06-17  3:37 ` [PATCH v9 3/3] vt-d: add __must_check annotation to IOMMU flush pointers and handlers Xu, Quan
  2 siblings, 1 reply; 6+ messages in thread
From: Xu, Quan @ 2016-06-17  3:37 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Quan Xu, Andrew Cooper, dario.faggioli,
	Jan Beulich, Feng Wu

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

The propagation value from IOMMU flush interfaces may be positive, which
indicates callers need to flush cache, not one of faliures.

when the propagation value is positive, this patch fixes this flush issue
as follows:
  - call iommu_flush_write_buffer() to flush cache.
  - return zero.

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

CC: Kevin Tian <kevin.tian@intel.com>
CC: Feng Wu <feng.wu@intel.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

---
v9: fix naming issue, changing 'iommu_rc / iommu_ret' to 'context_rc / iotlb_rc'
---
 xen/drivers/passthrough/vtd/iommu.c | 150 +++++++++++++++++++++++++-----------
 1 file changed, 103 insertions(+), 47 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 48edb67..2679ef6 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -388,17 +388,18 @@ static int flush_context_reg(
     return 0;
 }
 
-static int iommu_flush_context_global(
-    struct iommu *iommu, int flush_non_present_entry)
+static int __must_check iommu_flush_context_global(struct iommu *iommu,
+                                                   int flush_non_present_entry)
 {
     struct iommu_flush *flush = iommu_get_flush(iommu);
     return flush->context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL,
                                  flush_non_present_entry);
 }
 
-static int iommu_flush_context_device(
-    struct iommu *iommu, u16 did, u16 source_id,
-    u8 function_mask, int flush_non_present_entry)
+static int __must_check iommu_flush_context_device(struct iommu *iommu,
+                                                   u16 did, u16 source_id,
+                                                   u8 function_mask,
+                                                   int flush_non_present_entry)
 {
     struct iommu_flush *flush = iommu_get_flush(iommu);
     return flush->context(iommu, did, source_id, function_mask,
@@ -473,8 +474,9 @@ static int flush_iotlb_reg(void *_iommu, u16 did,
     return 0;
 }
 
-static int iommu_flush_iotlb_global(struct iommu *iommu,
-    int flush_non_present_entry, int flush_dev_iotlb)
+static int __must_check iommu_flush_iotlb_global(struct iommu *iommu,
+                                                 int flush_non_present_entry,
+                                                 int flush_dev_iotlb)
 {
     struct iommu_flush *flush = iommu_get_flush(iommu);
     int status;
@@ -491,8 +493,9 @@ static int iommu_flush_iotlb_global(struct iommu *iommu,
     return status;
 }
 
-static int iommu_flush_iotlb_dsi(struct iommu *iommu, u16 did,
-    int flush_non_present_entry, int flush_dev_iotlb)
+static int __must_check iommu_flush_iotlb_dsi(struct iommu *iommu, u16 did,
+                                              int flush_non_present_entry,
+                                              int flush_dev_iotlb)
 {
     struct iommu_flush *flush = iommu_get_flush(iommu);
     int status;
@@ -509,9 +512,10 @@ static int iommu_flush_iotlb_dsi(struct iommu *iommu, u16 did,
     return status;
 }
 
-static int iommu_flush_iotlb_psi(
-    struct iommu *iommu, u16 did, u64 addr, unsigned int order,
-    int flush_non_present_entry, int flush_dev_iotlb)
+static int __must_check iommu_flush_iotlb_psi(struct iommu *iommu, u16 did,
+                                              u64 addr, unsigned int order,
+                                              int flush_non_present_entry,
+                                              int flush_dev_iotlb)
 {
     struct iommu_flush *flush = iommu_get_flush(iommu);
     int status;
@@ -546,17 +550,37 @@ static int __must_check iommu_flush_all(void)
     struct acpi_drhd_unit *drhd;
     struct iommu *iommu;
     int flush_dev_iotlb;
+    int rc = 0;
 
     flush_all_cache();
     for_each_drhd_unit ( drhd )
     {
+        int context_rc, iotlb_rc;
+
         iommu = drhd->iommu;
-        iommu_flush_context_global(iommu, 0);
+        context_rc = iommu_flush_context_global(iommu, 0);
         flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-        iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
+        iotlb_rc = iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
+
+        /*
+         * The current logic for returns:
+         *   - positive  invoke iommu_flush_write_buffer to flush cache.
+         *   - zero      on success.
+         *   - negative  on failure. Continue to flush IOMMU IOTLB on a
+         *               best effort basis.
+         */
+        if ( context_rc > 0 || iotlb_rc > 0 )
+            iommu_flush_write_buffer(iommu);
+        if ( context_rc >= 0 )
+            rc = context_rc;
+        if ( rc >= 0 )
+            rc = iotlb_rc;
     }
 
-    return 0;
+    if ( rc > 0 )
+        rc = 0;
+
+    return rc;
 }
 
 static int __must_check iommu_flush_iotlb(struct domain *d,
@@ -569,6 +593,7 @@ static int __must_check iommu_flush_iotlb(struct domain *d,
     struct iommu *iommu;
     int flush_dev_iotlb;
     int iommu_domid;
+    int rc = 0;
 
     /*
      * No need pcideves_lock here because we have flush
@@ -587,21 +612,23 @@ static int __must_check iommu_flush_iotlb(struct domain *d,
             continue;
 
         if ( page_count != 1 || gfn == INVALID_GFN )
-        {
-            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
-                        0, flush_dev_iotlb) )
-                iommu_flush_write_buffer(iommu);
-        }
+            rc = iommu_flush_iotlb_dsi(iommu, iommu_domid,
+                                       0, flush_dev_iotlb);
         else
+            rc = iommu_flush_iotlb_psi(iommu, iommu_domid,
+                                       (paddr_t)gfn << PAGE_SHIFT_4K,
+                                       PAGE_ORDER_4K,
+                                       !dma_old_pte_present,
+                                       flush_dev_iotlb);
+
+        if ( rc > 0 )
         {
-            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
-                        (paddr_t)gfn << PAGE_SHIFT_4K, PAGE_ORDER_4K,
-                        !dma_old_pte_present, flush_dev_iotlb) )
-                iommu_flush_write_buffer(iommu);
+            iommu_flush_write_buffer(iommu);
+            rc = 0;
         }
     }
 
-    return 0;
+    return rc;
 }
 
 static int __must_check iommu_flush_iotlb_pages(struct domain *d,
@@ -1290,7 +1317,8 @@ int domain_context_mapping_one(
     struct context_entry *context, *context_entries;
     u64 maddr, pgd_maddr;
     u16 seg = iommu->intel->drhd->segment;
-    int agaw;
+    int agaw, rc, ret;
+    int flush_dev_iotlb;
 
     ASSERT(pcidevs_locked());
     spin_lock(&iommu->lock);
@@ -1404,14 +1432,24 @@ int domain_context_mapping_one(
     spin_unlock(&iommu->lock);
 
     /* Context entry was previously non-present (with domid 0). */
-    if ( iommu_flush_context_device(iommu, 0, (((u16)bus) << 8) | devfn,
-                                    DMA_CCMD_MASK_NOBIT, 1) )
+    rc = iommu_flush_context_device(iommu, 0, PCI_BDF2(bus, devfn),
+                                    DMA_CCMD_MASK_NOBIT, 1);
+    flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+    ret = iommu_flush_iotlb_dsi(iommu, 0, 1, flush_dev_iotlb);
+
+    /*
+     * The current logic for returns:
+     *   - positive  invoke iommu_flush_write_buffer to flush cache.
+     *   - zero      on success.
+     *   - negative  on failure. Continue to flush IOMMU IOTLB on a
+     *               best effort basis.
+     */
+    if ( rc > 0 || ret > 0 )
         iommu_flush_write_buffer(iommu);
-    else
-    {
-        int flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-        iommu_flush_iotlb_dsi(iommu, 0, 1, flush_dev_iotlb);
-    }
+    if ( rc >= 0 )
+        rc = ret;
+    if ( rc > 0 )
+        rc = 0;
 
     set_bit(iommu->index, &hd->arch.iommu_bitmap);
 
@@ -1420,7 +1458,7 @@ int domain_context_mapping_one(
     if ( !seg )
         me_wifi_quirk(domain, bus, devfn, MAP_ME_PHANTOM_FUNC);
 
-    return 0;
+    return rc;
 }
 
 static int domain_context_mapping(
@@ -1514,7 +1552,8 @@ int domain_context_unmap_one(
 {
     struct context_entry *context, *context_entries;
     u64 maddr;
-    int iommu_domid;
+    int iommu_domid, rc, ret;
+    int flush_dev_iotlb;
 
     ASSERT(pcidevs_locked());
     spin_lock(&iommu->lock);
@@ -1542,15 +1581,26 @@ int domain_context_unmap_one(
         return -EINVAL;
     }
 
-    if ( iommu_flush_context_device(iommu, iommu_domid,
-                                    (((u16)bus) << 8) | devfn,
-                                    DMA_CCMD_MASK_NOBIT, 0) )
+    rc = iommu_flush_context_device(iommu, iommu_domid,
+                                    PCI_BDF2(bus, devfn),
+                                    DMA_CCMD_MASK_NOBIT, 0);
+
+    flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+    ret = iommu_flush_iotlb_dsi(iommu, iommu_domid, 0, flush_dev_iotlb);
+
+    /*
+     * The current logic for returns:
+     *   - positive  invoke iommu_flush_write_buffer to flush cache.
+     *   - zero      on success.
+     *   - negative  on failure. Continue to flush IOMMU IOTLB on a
+     *               best effort basis.
+     */
+    if ( rc > 0 || ret > 0 )
         iommu_flush_write_buffer(iommu);
-    else
-    {
-        int flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-        iommu_flush_iotlb_dsi(iommu, iommu_domid, 0, flush_dev_iotlb);
-    }
+    if ( rc >= 0 )
+        rc = ret;
+    if ( rc > 0 )
+        rc = 0;
 
     spin_unlock(&iommu->lock);
     unmap_vtd_domain_page(context_entries);
@@ -1558,7 +1608,7 @@ int domain_context_unmap_one(
     if ( !iommu->intel->drhd->segment )
         me_wifi_quirk(domain, bus, devfn, UNMAP_ME_PHANTOM_FUNC);
 
-    return 0;
+    return rc;
 }
 
 static int domain_context_unmap(
@@ -1772,6 +1822,7 @@ int iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
     struct domain_iommu *hd = dom_iommu(d);
     int flush_dev_iotlb;
     int iommu_domid;
+    int rc = 0;
 
     iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
 
@@ -1785,13 +1836,18 @@ int iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
         iommu_domid= domain_iommu_domid(d, iommu);
         if ( iommu_domid == -1 )
             continue;
-        if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
+
+        rc = iommu_flush_iotlb_psi(iommu, iommu_domid,
                                    (paddr_t)gfn << PAGE_SHIFT_4K,
-                                   order, !present, flush_dev_iotlb) )
+                                   order, !present, flush_dev_iotlb);
+        if ( rc > 0 )
+        {
             iommu_flush_write_buffer(iommu);
+            rc = 0;
+        }
     }
 
-    return 0;
+    return rc;
 }
 
 static int __init vtd_ept_page_compatible(struct iommu *iommu)
-- 
1.9.1


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v9 2/3] vt-d: propagate the IOMMU Device-TLB flush error up to ME phantom functions
  2016-06-17  3:37 [PATCH v9 0/3] Check VT-d Device-TLB flush error Xu, Quan
  2016-06-17  3:37 ` [PATCH v9 1/3] vt-d: fix the IOMMU flush issue Xu, Quan
@ 2016-06-17  3:37 ` Xu, Quan
  2016-06-17  3:37 ` [PATCH v9 3/3] vt-d: add __must_check annotation to IOMMU flush pointers and handlers Xu, Quan
  2 siblings, 0 replies; 6+ messages in thread
From: Xu, Quan @ 2016-06-17  3:37 UTC (permalink / raw)
  To: xen-devel; +Cc: Kevin Tian, dario.faggioli, Feng Wu, Jan Beulich, Quan Xu

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

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

CC: Jan Beulich <jbeulich@suse.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Feng Wu <feng.wu@intel.com>
---
 xen/drivers/passthrough/vtd/extern.h |  3 ++-
 xen/drivers/passthrough/vtd/iommu.c  |  8 ++++----
 xen/drivers/passthrough/vtd/quirks.c | 27 +++++++++++++++++----------
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/extern.h b/xen/drivers/passthrough/vtd/extern.h
index cbe0286..6772839 100644
--- a/xen/drivers/passthrough/vtd/extern.h
+++ b/xen/drivers/passthrough/vtd/extern.h
@@ -91,7 +91,8 @@ int is_igd_vt_enabled_quirk(void);
 void platform_quirks_init(void);
 void vtd_ops_preamble_quirk(struct iommu* iommu);
 void vtd_ops_postamble_quirk(struct iommu* iommu);
-void me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map);
+int __must_check me_wifi_quirk(struct domain *domain,
+                               u8 bus, u8 devfn, int map);
 void pci_vtd_quirk(const struct pci_dev *);
 bool_t platform_supports_intremap(void);
 bool_t platform_supports_x2apic(void);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 2679ef6..cc47c30 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1455,8 +1455,8 @@ int domain_context_mapping_one(
 
     unmap_vtd_domain_page(context_entries);
 
-    if ( !seg )
-        me_wifi_quirk(domain, bus, devfn, MAP_ME_PHANTOM_FUNC);
+    if ( !seg && !rc )
+        rc = me_wifi_quirk(domain, bus, devfn, MAP_ME_PHANTOM_FUNC);
 
     return rc;
 }
@@ -1605,8 +1605,8 @@ int domain_context_unmap_one(
     spin_unlock(&iommu->lock);
     unmap_vtd_domain_page(context_entries);
 
-    if ( !iommu->intel->drhd->segment )
-        me_wifi_quirk(domain, bus, devfn, UNMAP_ME_PHANTOM_FUNC);
+    if ( !iommu->intel->drhd->segment && !rc )
+        rc = me_wifi_quirk(domain, bus, devfn, UNMAP_ME_PHANTOM_FUNC);
 
     return rc;
 }
diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c
index 473d1fc..91f96ac 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -331,10 +331,12 @@ void __init platform_quirks_init(void)
  * assigning Intel integrated wifi device to a guest.
  */
 
-static void map_me_phantom_function(struct domain *domain, u32 dev, int map)
+static int __must_check map_me_phantom_function(struct domain *domain,
+                                                u32 dev, int map)
 {
     struct acpi_drhd_unit *drhd;
     struct pci_dev *pdev;
+    int rc;
 
     /* find ME VT-d engine base on a real ME device */
     pdev = pci_get_pdev(0, 0, PCI_DEVFN(dev, 0));
@@ -342,23 +344,26 @@ static void map_me_phantom_function(struct domain *domain, u32 dev, int map)
 
     /* map or unmap ME phantom function */
     if ( map )
-        domain_context_mapping_one(domain, drhd->iommu, 0,
-                                   PCI_DEVFN(dev, 7), NULL);
+        rc = domain_context_mapping_one(domain, drhd->iommu, 0,
+                                        PCI_DEVFN(dev, 7), NULL);
     else
-        domain_context_unmap_one(domain, drhd->iommu, 0,
-                                 PCI_DEVFN(dev, 7));
+        rc = domain_context_unmap_one(domain, drhd->iommu, 0,
+                                      PCI_DEVFN(dev, 7));
+
+    return rc;
 }
 
-void me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map)
+int me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map)
 {
     u32 id;
+    int rc = 0;
 
     id = pci_conf_read32(0, 0, 0, 0, 0);
     if ( IS_CTG(id) )
     {
         /* quit if ME does not exist */
         if ( pci_conf_read32(0, 0, 3, 0, 0) == 0xffffffff )
-            return;
+            return 0;
 
         /* if device is WLAN device, map ME phantom device 0:3.7 */
         id = pci_conf_read32(0, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), 0);
@@ -372,7 +377,7 @@ void me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map)
             case 0x423b8086:
             case 0x423c8086:
             case 0x423d8086:
-                map_me_phantom_function(domain, 3, map);
+                rc = map_me_phantom_function(domain, 3, map);
                 break;
             default:
                 break;
@@ -382,7 +387,7 @@ void me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map)
     {
         /* quit if ME does not exist */
         if ( pci_conf_read32(0, 0, 22, 0, 0) == 0xffffffff )
-            return;
+            return 0;
 
         /* if device is WLAN device, map ME phantom device 0:22.7 */
         id = pci_conf_read32(0, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), 0);
@@ -398,12 +403,14 @@ void me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map)
             case 0x42388086:        /* Puma Peak */
             case 0x422b8086:
             case 0x422c8086:
-                map_me_phantom_function(domain, 22, map);
+                rc = map_me_phantom_function(domain, 22, map);
                 break;
             default:
                 break;
         }
     }
+
+    return rc;
 }
 
 void pci_vtd_quirk(const struct pci_dev *pdev)
-- 
1.9.1


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v9 3/3] vt-d: add __must_check annotation to IOMMU flush pointers and handlers
  2016-06-17  3:37 [PATCH v9 0/3] Check VT-d Device-TLB flush error Xu, Quan
  2016-06-17  3:37 ` [PATCH v9 1/3] vt-d: fix the IOMMU flush issue Xu, Quan
  2016-06-17  3:37 ` [PATCH v9 2/3] vt-d: propagate the IOMMU Device-TLB flush error up to ME phantom functions Xu, Quan
@ 2016-06-17  3:37 ` Xu, Quan
  2 siblings, 0 replies; 6+ messages in thread
From: Xu, Quan @ 2016-06-17  3:37 UTC (permalink / raw)
  To: xen-devel; +Cc: Kevin Tian, dario.faggioli, Feng Wu, Jan Beulich, Quan Xu

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

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

CC: Jan Beulich <jbeulich@suse.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Feng Wu <feng.wu@intel.com>
---
 xen/drivers/passthrough/vtd/iommu.c  | 50 ++++++++++++++++++------------------
 xen/drivers/passthrough/vtd/iommu.h  | 11 +++++---
 xen/drivers/passthrough/vtd/qinval.c | 14 +++++-----
 3 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index cc47c30..7d413f1 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -335,10 +335,9 @@ static void iommu_flush_write_buffer(struct iommu *iommu)
 }
 
 /* return value determine if we need a write buffer flush */
-static int flush_context_reg(
-    void *_iommu,
-    u16 did, u16 source_id, u8 function_mask, u64 type,
-    int flush_non_present_entry)
+static int __must_check flush_context_reg(void *_iommu, u16 did, u16 source_id,
+                                          u8 function_mask, u64 type,
+                                          bool_t flush_non_present_entry)
 {
     struct iommu *iommu = (struct iommu *) _iommu;
     u64 val = 0;
@@ -389,7 +388,7 @@ static int flush_context_reg(
 }
 
 static int __must_check iommu_flush_context_global(struct iommu *iommu,
-                                                   int flush_non_present_entry)
+                                                   bool_t flush_non_present_entry)
 {
     struct iommu_flush *flush = iommu_get_flush(iommu);
     return flush->context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL,
@@ -399,7 +398,7 @@ static int __must_check iommu_flush_context_global(struct iommu *iommu,
 static int __must_check iommu_flush_context_device(struct iommu *iommu,
                                                    u16 did, u16 source_id,
                                                    u8 function_mask,
-                                                   int flush_non_present_entry)
+                                                   bool_t flush_non_present_entry)
 {
     struct iommu_flush *flush = iommu_get_flush(iommu);
     return flush->context(iommu, did, source_id, function_mask,
@@ -408,9 +407,10 @@ static int __must_check iommu_flush_context_device(struct iommu *iommu,
 }
 
 /* return value determine if we need a write buffer flush */
-static int flush_iotlb_reg(void *_iommu, u16 did,
-                           u64 addr, unsigned int size_order, u64 type,
-                           int flush_non_present_entry, int flush_dev_iotlb)
+static int __must_check flush_iotlb_reg(void *_iommu, u16 did, u64 addr,
+                                        unsigned int size_order, u64 type,
+                                        bool_t flush_non_present_entry,
+                                        bool_t flush_dev_iotlb)
 {
     struct iommu *iommu = (struct iommu *) _iommu;
     int tlb_offset = ecap_iotlb_offset(iommu->ecap);
@@ -475,8 +475,8 @@ static int flush_iotlb_reg(void *_iommu, u16 did,
 }
 
 static int __must_check iommu_flush_iotlb_global(struct iommu *iommu,
-                                                 int flush_non_present_entry,
-                                                 int flush_dev_iotlb)
+                                                 bool_t flush_non_present_entry,
+                                                 bool_t flush_dev_iotlb)
 {
     struct iommu_flush *flush = iommu_get_flush(iommu);
     int status;
@@ -494,8 +494,8 @@ static int __must_check iommu_flush_iotlb_global(struct iommu *iommu,
 }
 
 static int __must_check iommu_flush_iotlb_dsi(struct iommu *iommu, u16 did,
-                                              int flush_non_present_entry,
-                                              int flush_dev_iotlb)
+                                              bool_t flush_non_present_entry,
+                                              bool_t flush_dev_iotlb)
 {
     struct iommu_flush *flush = iommu_get_flush(iommu);
     int status;
@@ -514,8 +514,8 @@ static int __must_check iommu_flush_iotlb_dsi(struct iommu *iommu, u16 did,
 
 static int __must_check iommu_flush_iotlb_psi(struct iommu *iommu, u16 did,
                                               u64 addr, unsigned int order,
-                                              int flush_non_present_entry,
-                                              int flush_dev_iotlb)
+                                              bool_t flush_non_present_entry,
+                                              bool_t flush_dev_iotlb)
 {
     struct iommu_flush *flush = iommu_get_flush(iommu);
     int status;
@@ -549,7 +549,7 @@ static int __must_check iommu_flush_all(void)
 {
     struct acpi_drhd_unit *drhd;
     struct iommu *iommu;
-    int flush_dev_iotlb;
+    bool_t flush_dev_iotlb;
     int rc = 0;
 
     flush_all_cache();
@@ -559,7 +559,7 @@ static int __must_check iommu_flush_all(void)
 
         iommu = drhd->iommu;
         context_rc = iommu_flush_context_global(iommu, 0);
-        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+        flush_dev_iotlb = !!find_ats_dev_drhd(iommu);
         iotlb_rc = iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
 
         /*
@@ -591,7 +591,7 @@ static int __must_check iommu_flush_iotlb(struct domain *d,
     struct domain_iommu *hd = dom_iommu(d);
     struct acpi_drhd_unit *drhd;
     struct iommu *iommu;
-    int flush_dev_iotlb;
+    bool_t flush_dev_iotlb;
     int iommu_domid;
     int rc = 0;
 
@@ -606,7 +606,7 @@ static int __must_check iommu_flush_iotlb(struct domain *d,
         if ( !test_bit(iommu->index, &hd->arch.iommu_bitmap) )
             continue;
 
-        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+        flush_dev_iotlb = !!find_ats_dev_drhd(iommu);
         iommu_domid= domain_iommu_domid(d, iommu);
         if ( iommu_domid == -1 )
             continue;
@@ -1318,7 +1318,7 @@ int domain_context_mapping_one(
     u64 maddr, pgd_maddr;
     u16 seg = iommu->intel->drhd->segment;
     int agaw, rc, ret;
-    int flush_dev_iotlb;
+    bool_t flush_dev_iotlb;
 
     ASSERT(pcidevs_locked());
     spin_lock(&iommu->lock);
@@ -1434,7 +1434,7 @@ int domain_context_mapping_one(
     /* Context entry was previously non-present (with domid 0). */
     rc = iommu_flush_context_device(iommu, 0, PCI_BDF2(bus, devfn),
                                     DMA_CCMD_MASK_NOBIT, 1);
-    flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+    flush_dev_iotlb = !!find_ats_dev_drhd(iommu);
     ret = iommu_flush_iotlb_dsi(iommu, 0, 1, flush_dev_iotlb);
 
     /*
@@ -1553,7 +1553,7 @@ int domain_context_unmap_one(
     struct context_entry *context, *context_entries;
     u64 maddr;
     int iommu_domid, rc, ret;
-    int flush_dev_iotlb;
+    bool_t flush_dev_iotlb;
 
     ASSERT(pcidevs_locked());
     spin_lock(&iommu->lock);
@@ -1585,7 +1585,7 @@ int domain_context_unmap_one(
                                     PCI_BDF2(bus, devfn),
                                     DMA_CCMD_MASK_NOBIT, 0);
 
-    flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+    flush_dev_iotlb = !!find_ats_dev_drhd(iommu);
     ret = iommu_flush_iotlb_dsi(iommu, iommu_domid, 0, flush_dev_iotlb);
 
     /*
@@ -1820,7 +1820,7 @@ int iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
     struct acpi_drhd_unit *drhd;
     struct iommu *iommu = NULL;
     struct domain_iommu *hd = dom_iommu(d);
-    int flush_dev_iotlb;
+    bool_t flush_dev_iotlb;
     int iommu_domid;
     int rc = 0;
 
@@ -1832,7 +1832,7 @@ int iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
         if ( !test_bit(iommu->index, &hd->arch.iommu_bitmap) )
             continue;
 
-        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+        flush_dev_iotlb = !!find_ats_dev_drhd(iommu);
         iommu_domid= domain_iommu_domid(d, iommu);
         if ( iommu_domid == -1 )
             continue;
diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h
index c55ee08..e6cf738 100644
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -514,10 +514,13 @@ struct ir_ctrl {
 };
 
 struct iommu_flush {
-    int (*context)(void *iommu, u16 did, u16 source_id,
-                   u8 function_mask, u64 type, int non_present_entry_flush);
-    int (*iotlb)(void *iommu, u16 did, u64 addr, unsigned int size_order,
-                 u64 type, int flush_non_present_entry, int flush_dev_iotlb);
+    int __must_check (*context)(void *iommu, u16 did, u16 source_id,
+                                u8 function_mask, u64 type,
+                                bool_t non_present_entry_flush);
+    int __must_check (*iotlb)(void *iommu, u16 did, u64 addr,
+                              unsigned int size_order, u64 type,
+                              bool_t flush_non_present_entry,
+                              bool_t flush_dev_iotlb);
 };
 
 struct intel_iommu {
diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
index b81b0bd..aa7841a 100644
--- a/xen/drivers/passthrough/vtd/qinval.c
+++ b/xen/drivers/passthrough/vtd/qinval.c
@@ -274,9 +274,9 @@ int iommu_flush_iec_index(struct iommu *iommu, u8 im, u16 iidx)
    return __iommu_flush_iec(iommu, IEC_INDEX_INVL, im, iidx);
 }
 
-static int flush_context_qi(
-    void *_iommu, u16 did, u16 sid, u8 fm, u64 type,
-    int flush_non_present_entry)
+static int __must_check flush_context_qi(void *_iommu, u16 did,
+                                         u16 sid, u8 fm, u64 type,
+                                         bool_t flush_non_present_entry)
 {
     int ret = 0;
     struct iommu *iommu = (struct iommu *)_iommu;
@@ -305,10 +305,10 @@ static int flush_context_qi(
     return ret;
 }
 
-static int flush_iotlb_qi(
-    void *_iommu, u16 did,
-    u64 addr, unsigned int size_order, u64 type,
-    int flush_non_present_entry, int flush_dev_iotlb)
+static int __must_check flush_iotlb_qi(void *_iommu, u16 did, u64 addr,
+                                       unsigned int size_order, u64 type,
+                                       bool_t flush_non_present_entry,
+                                       bool_t flush_dev_iotlb)
 {
     u8 dr = 0, dw = 0;
     int ret = 0;
-- 
1.9.1


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v9 1/3] vt-d: fix the IOMMU flush issue
  2016-06-17  3:37 ` [PATCH v9 1/3] vt-d: fix the IOMMU flush issue Xu, Quan
@ 2016-06-21 13:25   ` Jan Beulich
  2016-06-22  3:07     ` Xu, Quan
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2016-06-21 13:25 UTC (permalink / raw)
  To: Quan Xu
  Cc: Kevin Tian, Keir Fraser, Andrew Cooper, dario.faggioli,
	xen-devel, Feng Wu

>>> On 17.06.16 at 05:37, <quan.xu@intel.com> wrote:
> @@ -546,17 +550,37 @@ static int __must_check iommu_flush_all(void)
>      struct acpi_drhd_unit *drhd;
>      struct iommu *iommu;
>      int flush_dev_iotlb;
> +    int rc = 0;
>  
>      flush_all_cache();
>      for_each_drhd_unit ( drhd )
>      {
> +        int context_rc, iotlb_rc;
> +
>          iommu = drhd->iommu;
> -        iommu_flush_context_global(iommu, 0);
> +        context_rc = iommu_flush_context_global(iommu, 0);
>          flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
> -        iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
> +        iotlb_rc = iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
> +
> +        /*
> +         * The current logic for returns:
> +         *   - positive  invoke iommu_flush_write_buffer to flush cache.
> +         *   - zero      on success.
> +         *   - negative  on failure. Continue to flush IOMMU IOTLB on a
> +         *               best effort basis.
> +         */
> +        if ( context_rc > 0 || iotlb_rc > 0 )
> +            iommu_flush_write_buffer(iommu);
> +        if ( context_rc >= 0 )

Wasn't this meant to be just "rc"? (I can't, btw, imagine Kevin's ack
to be rightfully retained with a change like this.)

Jan


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v9 1/3] vt-d: fix the IOMMU flush issue
  2016-06-21 13:25   ` Jan Beulich
@ 2016-06-22  3:07     ` Xu, Quan
  0 siblings, 0 replies; 6+ messages in thread
From: Xu, Quan @ 2016-06-22  3:07 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Tian, Kevin, Keir Fraser, Andrew Cooper, dario.faggioli,
	xen-devel, Wu, Feng

On June 21, 2016 9:25 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >>> On 17.06.16 at 05:37, <quan.xu@intel.com> wrote:
> > @@ -546,17 +550,37 @@ static int __must_check iommu_flush_all(void)
> >      struct acpi_drhd_unit *drhd;
> >      struct iommu *iommu;
> >      int flush_dev_iotlb;
> > +    int rc = 0;
> >
> >      flush_all_cache();
> >      for_each_drhd_unit ( drhd )
> >      {
> > +        int context_rc, iotlb_rc;
> > +
> >          iommu = drhd->iommu;
> > -        iommu_flush_context_global(iommu, 0);
> > +        context_rc = iommu_flush_context_global(iommu, 0);
> >          flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
> > -        iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
> > +        iotlb_rc = iommu_flush_iotlb_global(iommu, 0,
> > + flush_dev_iotlb);
> > +
> > +        /*
> > +         * The current logic for returns:
> > +         *   - positive  invoke iommu_flush_write_buffer to flush cache.
> > +         *   - zero      on success.
> > +         *   - negative  on failure. Continue to flush IOMMU IOTLB on a
> > +         *               best effort basis.
> > +         */
> > +        if ( context_rc > 0 || iotlb_rc > 0 )
> > +            iommu_flush_write_buffer(iommu);
> > +        if ( context_rc >= 0 )
> 
> Wasn't this meant to be just "rc"? (I can't, btw, imagine Kevin's ack to be
> rightfully retained with a change like this.)
>
SORRY, it is 'rc'. It is really my mistake here, but Kevin's ack is right as the previous v8 was:

+        if ( rc >= 0 )
+            rc = iommu_rc;
+        if ( rc >= 0 )
+            rc = iommu_ret;

,, I will send it out again with this fix.

Quan

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-06-22  3:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-17  3:37 [PATCH v9 0/3] Check VT-d Device-TLB flush error Xu, Quan
2016-06-17  3:37 ` [PATCH v9 1/3] vt-d: fix the IOMMU flush issue Xu, Quan
2016-06-21 13:25   ` Jan Beulich
2016-06-22  3:07     ` Xu, Quan
2016-06-17  3:37 ` [PATCH v9 2/3] vt-d: propagate the IOMMU Device-TLB flush error up to ME phantom functions Xu, Quan
2016-06-17  3:37 ` [PATCH v9 3/3] vt-d: add __must_check annotation to IOMMU flush pointers and handlers Xu, Quan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).