xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/4] iommu improvements
@ 2018-12-17  9:22 Paul Durrant
  2018-12-17  9:22 ` [PATCH v5 1/4] amd-iommu: add flush iommu_ops Paul Durrant
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Paul Durrant @ 2018-12-17  9:22 UTC (permalink / raw)
  To: xen-devel; +Cc: Paul Durrant

Paul Durrant (4):
  amd-iommu: add flush iommu_ops
  iommu: rename wrapper functions
  iommu: elide flushing for higher order map/unmap operations
  x86/mm/p2m: stop checking for IOMMU shared page tables in mmio_order()

 xen/arch/arm/p2m.c                            |  11 ++-
 xen/arch/x86/mm.c                             |  11 ++-
 xen/arch/x86/mm/p2m-ept.c                     |   4 +-
 xen/arch/x86/mm/p2m-pt.c                      |   5 +-
 xen/arch/x86/mm/p2m.c                         |  17 ++--
 xen/arch/x86/x86_64/mm.c                      |   9 +-
 xen/common/grant_table.c                      |  14 +--
 xen/common/memory.c                           |   6 +-
 xen/drivers/passthrough/amd/iommu_map.c       | 135 ++++++++++++++++++++------
 xen/drivers/passthrough/amd/pci_amd_iommu.c   |   2 +
 xen/drivers/passthrough/arm/smmu.c            |  11 ++-
 xen/drivers/passthrough/iommu.c               |  86 +++++++++++++---
 xen/drivers/passthrough/vtd/iommu.c           |  34 ++++---
 xen/drivers/passthrough/x86/iommu.c           |  25 +++--
 xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |  10 +-
 xen/include/xen/iommu.h                       |  56 +++++++++--
 16 files changed, 323 insertions(+), 113 deletions(-)

-- 
2.11.0


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

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

* [PATCH v5 1/4] amd-iommu: add flush iommu_ops
  2018-12-17  9:22 [PATCH v5 0/4] iommu improvements Paul Durrant
@ 2018-12-17  9:22 ` Paul Durrant
  2018-12-20 19:45   ` Woods, Brian
  2018-12-17  9:22 ` [PATCH v5 2/4] iommu: rename wrapper functions Paul Durrant
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Paul Durrant @ 2018-12-17  9:22 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Andrew Cooper, Paul Durrant, Suravee Suthikulpanit,
	Brian Woods, Roger Pau Monné

The iommu_ops structure contains two methods for flushing: 'iotlb_flush' and
'iotlb_flush_all'. This patch adds implementations of these for AMD IOMMUs.

The iotlb_flush method takes a base DFN and a (4k) page count, but the
flush needs to be done by page order (i.e. 0, 9 or 18). Because a flush
operation is fairly expensive to perform, the code calculates the minimum
order single flush that will cover the specified page range rather than
performing multiple flushes.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Brian Woods <brian.woods@amd.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v4:
 - Fix flush_count() properly this time.

v3:
 - Really get rid of dfn_lt().
 - Fix flush_count().

v2:
 - Treat passing INVALID_DFN to iommu_iotlb_flush() as an error, and a zero
   page_count as a no-op.
 - Get rid of dfn_lt().
---
 xen/drivers/passthrough/amd/iommu_map.c       | 50 +++++++++++++++++++++++++++
 xen/drivers/passthrough/amd/pci_amd_iommu.c   |  2 ++
 xen/drivers/passthrough/iommu.c               |  6 +++-
 xen/drivers/passthrough/vtd/iommu.c           |  2 ++
 xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |  3 ++
 5 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passthrough/amd/iommu_map.c
index 2429e01bb4..de5a880070 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -634,6 +634,56 @@ int amd_iommu_unmap_page(struct domain *d, dfn_t dfn)
     spin_unlock(&hd->arch.mapping_lock);
 
     amd_iommu_flush_pages(d, dfn_x(dfn), 0);
+    return 0;
+}
+
+static unsigned long flush_count(unsigned long dfn, unsigned int page_count,
+                                 unsigned int order)
+{
+    unsigned long start = dfn >> order;
+    unsigned long end = ((dfn + page_count - 1) >> order) + 1;
+
+    ASSERT(end > start);
+    return end - start;
+}
+
+int amd_iommu_flush_iotlb_pages(struct domain *d, dfn_t dfn,
+                                unsigned int page_count)
+{
+    unsigned long dfn_l = dfn_x(dfn);
+
+    ASSERT(page_count && !dfn_eq(dfn, INVALID_DFN));
+
+    /* If the range wraps then just flush everything */
+    if ( dfn_l + page_count < dfn_l )
+    {
+        amd_iommu_flush_all_pages(d);
+        return 0;
+    }
+
+    /*
+     * Flushes are expensive so find the minimal single flush that will
+     * cover the page range.
+     *
+     * NOTE: It is unnecessary to round down the DFN value to align with
+     *       the flush order here. This is done by the internals of the
+     *       flush code.
+     */
+    if ( page_count == 1 ) /* order 0 flush count */
+        amd_iommu_flush_pages(d, dfn_l, 0);
+    else if ( flush_count(dfn_l, page_count, 9) == 1 )
+        amd_iommu_flush_pages(d, dfn_l, 9);
+    else if ( flush_count(dfn_l, page_count, 18) == 1 )
+        amd_iommu_flush_pages(d, dfn_l, 18);
+    else
+        amd_iommu_flush_all_pages(d);
+
+    return 0;
+}
+
+int amd_iommu_flush_iotlb_all(struct domain *d)
+{
+    amd_iommu_flush_all_pages(d);
 
     return 0;
 }
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 900136390d..33a3798f36 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -579,6 +579,8 @@ static const struct iommu_ops __initconstrel amd_iommu_ops = {
     .teardown = amd_iommu_domain_destroy,
     .map_page = amd_iommu_map_page,
     .unmap_page = amd_iommu_unmap_page,
+    .iotlb_flush = amd_iommu_flush_iotlb_pages,
+    .iotlb_flush_all = amd_iommu_flush_iotlb_all,
     .free_page_table = deallocate_page_table,
     .reassign_device = reassign_device,
     .get_device_group_id = amd_iommu_group_id,
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index ac62d7f52a..c1cce08551 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -414,9 +414,13 @@ int iommu_iotlb_flush(struct domain *d, dfn_t dfn, 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 )
+    if ( !iommu_enabled || !hd->platform_ops ||
+         !hd->platform_ops->iotlb_flush || !page_count )
         return 0;
 
+    if ( dfn_eq(dfn, INVALID_DFN) )
+        return -EINVAL;
+
     rc = hd->platform_ops->iotlb_flush(d, dfn, page_count);
     if ( unlikely(rc) )
     {
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 1601278b07..d2fa5e2b25 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -635,6 +635,8 @@ static int __must_check iommu_flush_iotlb_pages(struct domain *d,
                                                 dfn_t dfn,
                                                 unsigned int page_count)
 {
+    ASSERT(page_count && !dfn_eq(dfn, INVALID_DFN));
+
     return iommu_flush_iotlb(d, dfn, 1, page_count);
 }
 
diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
index 718a44f956..88715329ca 100644
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -60,6 +60,9 @@ int __must_check amd_iommu_alloc_root(struct domain_iommu *hd);
 int amd_iommu_reserve_domain_unity_map(struct domain *domain,
                                        paddr_t phys_addr, unsigned long size,
                                        int iw, int ir);
+int __must_check amd_iommu_flush_iotlb_pages(struct domain *d, dfn_t dfn,
+                                             unsigned int page_count);
+int __must_check amd_iommu_flush_iotlb_all(struct domain *d);
 
 /* Share p2m table with iommu */
 void amd_iommu_share_p2m(struct domain *d);
-- 
2.11.0


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

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

* [PATCH v5 2/4] iommu: rename wrapper functions
  2018-12-17  9:22 [PATCH v5 0/4] iommu improvements Paul Durrant
  2018-12-17  9:22 ` [PATCH v5 1/4] amd-iommu: add flush iommu_ops Paul Durrant
@ 2018-12-17  9:22 ` Paul Durrant
  2018-12-19  9:05   ` Paul Durrant
  2018-12-25  2:29   ` Tian, Kevin
  2018-12-17  9:22 ` [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations Paul Durrant
  2018-12-17  9:22 ` [PATCH v5 4/4] x86/mm/p2m: stop checking for IOMMU shared page tables in mmio_order() Paul Durrant
  3 siblings, 2 replies; 16+ messages in thread
From: Paul Durrant @ 2018-12-17  9:22 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Paul Durrant, Jun Nakajima, Roger Pau Monné

A subsequent patch will add semantically different versions of
iommu_map/unmap() so, in advance of that change, this patch renames the
existing functions to iommu_legacy_map/unmap() and modifies all call-sites.
It also adjusts a comment that refers to iommu_map_page(), which was re-
named by a previous patch.

This patch is purely cosmetic. No functional change.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>A
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>

v2:
 - New in v2.

v3:
 - Leave iommu_iotlb_flush[_all] alone.
 - Make patch purely cosmetic.
 - Fix comment in xen/iommu.h.
---
 xen/arch/x86/mm.c                   | 11 ++++++-----
 xen/arch/x86/mm/p2m-ept.c           |  4 ++--
 xen/arch/x86/mm/p2m-pt.c            |  5 +++--
 xen/arch/x86/mm/p2m.c               | 12 ++++++------
 xen/arch/x86/x86_64/mm.c            |  9 +++++----
 xen/common/grant_table.c            | 14 +++++++-------
 xen/common/memory.c                 |  4 ++--
 xen/drivers/passthrough/iommu.c     |  6 +++---
 xen/drivers/passthrough/x86/iommu.c |  4 ++--
 xen/include/xen/iommu.h             | 16 +++++++++++-----
 10 files changed, 47 insertions(+), 38 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 1431f347f3..0ad3940bb3 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2801,12 +2801,13 @@ static int _get_page_type(struct page_info *page, unsigned long type,
             mfn_t mfn = page_to_mfn(page);
 
             if ( (x & PGT_type_mask) == PGT_writable_page )
-                iommu_ret = iommu_unmap(d, _dfn(mfn_x(mfn)),
-                                        PAGE_ORDER_4K);
+                iommu_ret = iommu_legacy_unmap(d, _dfn(mfn_x(mfn)),
+                                               PAGE_ORDER_4K);
             else if ( type == PGT_writable_page )
-                iommu_ret = iommu_map(d, _dfn(mfn_x(mfn)), mfn,
-                                      PAGE_ORDER_4K,
-                                      IOMMUF_readable | IOMMUF_writable);
+                iommu_ret = iommu_legacy_map(d, _dfn(mfn_x(mfn)), mfn,
+                                             PAGE_ORDER_4K,
+                                             IOMMUF_readable |
+                                             IOMMUF_writable);
         }
     }
 
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 6e4e375bad..64a49c07b7 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -882,8 +882,8 @@ out:
             rc = iommu_pte_flush(d, gfn, &ept_entry->epte, order, vtd_pte_present);
         else if ( need_iommu_pt_sync(d) )
             rc = iommu_flags ?
-                iommu_map(d, _dfn(gfn), mfn, order, iommu_flags) :
-                iommu_unmap(d, _dfn(gfn), order);
+                iommu_legacy_map(d, _dfn(gfn), mfn, order, iommu_flags) :
+                iommu_legacy_unmap(d, _dfn(gfn), order);
     }
 
     unmap_domain_page(table);
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index 17a6b61f12..69ffb08179 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -686,8 +686,9 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
 
         if ( need_iommu_pt_sync(p2m->domain) )
             rc = iommu_pte_flags ?
-                iommu_map(d, _dfn(gfn), mfn, page_order, iommu_pte_flags) :
-                iommu_unmap(d, _dfn(gfn), page_order);
+                iommu_legacy_map(d, _dfn(gfn), mfn, page_order,
+                                 iommu_pte_flags) :
+                iommu_legacy_unmap(d, _dfn(gfn), page_order);
         else if ( iommu_use_hap_pt(d) && iommu_old_flags )
             amd_iommu_flush_pages(p2m->domain, gfn, page_order);
     }
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index fea4497910..ed76e96d33 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -733,7 +733,7 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn_l, unsigned long mfn,
 
     if ( !paging_mode_translate(p2m->domain) )
         return need_iommu_pt_sync(p2m->domain) ?
-            iommu_unmap(p2m->domain, _dfn(mfn), page_order) : 0;
+            iommu_legacy_unmap(p2m->domain, _dfn(mfn), page_order) : 0;
 
     ASSERT(gfn_locked_by_me(p2m, gfn));
     P2M_DEBUG("removing gfn=%#lx mfn=%#lx\n", gfn_l, mfn);
@@ -780,8 +780,8 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mfn_t mfn,
 
     if ( !paging_mode_translate(d) )
         return (need_iommu_pt_sync(d) && t == p2m_ram_rw) ?
-            iommu_map(d, _dfn(mfn_x(mfn)), mfn, page_order,
-                      IOMMUF_readable | IOMMUF_writable) : 0;
+            iommu_legacy_map(d, _dfn(mfn_x(mfn)), mfn, page_order,
+                             IOMMUF_readable | IOMMUF_writable) : 0;
 
     /* foreign pages are added thru p2m_add_foreign */
     if ( p2m_is_foreign(t) )
@@ -1151,8 +1151,8 @@ int set_identity_p2m_entry(struct domain *d, unsigned long gfn_l,
     {
         if ( !need_iommu_pt_sync(d) )
             return 0;
-        return iommu_map(d, _dfn(gfn_l), _mfn(gfn_l), PAGE_ORDER_4K,
-                         IOMMUF_readable | IOMMUF_writable);
+        return iommu_legacy_map(d, _dfn(gfn_l), _mfn(gfn_l), PAGE_ORDER_4K,
+                                IOMMUF_readable | IOMMUF_writable);
     }
 
     gfn_lock(p2m, gfn, 0);
@@ -1242,7 +1242,7 @@ int clear_identity_p2m_entry(struct domain *d, unsigned long gfn_l)
     {
         if ( !need_iommu_pt_sync(d) )
             return 0;
-        return iommu_unmap(d, _dfn(gfn_l), PAGE_ORDER_4K);
+        return iommu_legacy_unmap(d, _dfn(gfn_l), PAGE_ORDER_4K);
     }
 
     gfn_lock(p2m, gfn, 0);
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 126a486d2e..d8f558bc3a 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1436,15 +1436,16 @@ int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm)
          !need_iommu_pt_sync(hardware_domain) )
     {
         for ( i = spfn; i < epfn; i++ )
-            if ( iommu_map(hardware_domain, _dfn(i), _mfn(i),
-                           PAGE_ORDER_4K,
-                           IOMMUF_readable | IOMMUF_writable) )
+            if ( iommu_legacy_map(hardware_domain, _dfn(i), _mfn(i),
+                                  PAGE_ORDER_4K,
+                                  IOMMUF_readable | IOMMUF_writable) )
                 break;
         if ( i != epfn )
         {
             while (i-- > old_max)
                 /* If statement to satisfy __must_check. */
-                if ( iommu_unmap(hardware_domain, _dfn(i), PAGE_ORDER_4K) )
+                if ( iommu_legacy_unmap(hardware_domain, _dfn(i),
+                                        PAGE_ORDER_4K) )
                     continue;
 
             goto destroy_m2p;
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index b67ae9e3f5..fd099a8f25 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1134,14 +1134,14 @@ map_grant_ref(
              !(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
         {
             if ( !(kind & MAPKIND_WRITE) )
-                err = iommu_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
-                                IOMMUF_readable | IOMMUF_writable);
+                err = iommu_legacy_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
+                                       IOMMUF_readable | IOMMUF_writable);
         }
         else if ( act_pin && !old_pin )
         {
             if ( !kind )
-                err = iommu_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
-                                IOMMUF_readable);
+                err = iommu_legacy_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
+                                       IOMMUF_readable);
         }
         if ( err )
         {
@@ -1389,10 +1389,10 @@ unmap_common(
 
         kind = mapkind(lgt, rd, op->mfn);
         if ( !kind )
-            err = iommu_unmap(ld, _dfn(mfn_x(op->mfn)), 0);
+            err = iommu_legacy_unmap(ld, _dfn(mfn_x(op->mfn)), 0);
         else if ( !(kind & MAPKIND_WRITE) )
-            err = iommu_map(ld, _dfn(mfn_x(op->mfn)), op->mfn, 0,
-                            IOMMUF_readable);
+            err = iommu_legacy_map(ld, _dfn(mfn_x(op->mfn)), op->mfn, 0,
+                                   IOMMUF_readable);
 
         double_gt_unlock(lgt, rgt);
 
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 5f7d081c61..f37eb288d4 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -853,11 +853,11 @@ int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap *xatp,
 
         this_cpu(iommu_dont_flush_iotlb) = 0;
 
-        ret = iommu_iotlb_flush(d, _dfn(xatp->idx - done), done);
+        ret = iommu_flush(d, _dfn(xatp->idx - done), done);
         if ( unlikely(ret) && rc >= 0 )
             rc = ret;
 
-        ret = iommu_iotlb_flush(d, _dfn(xatp->gpfn - done), done);
+        ret = iommu_flush(d, _dfn(xatp->gpfn - done), done);
         if ( unlikely(ret) && rc >= 0 )
             rc = ret;
     }
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index c1cce08551..105995a343 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -304,8 +304,8 @@ void iommu_domain_destroy(struct domain *d)
     arch_iommu_domain_destroy(d);
 }
 
-int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
-              unsigned int page_order, unsigned int flags)
+int iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
+                     unsigned int page_order, unsigned int flags)
 {
     const struct domain_iommu *hd = dom_iommu(d);
     unsigned long i;
@@ -345,7 +345,7 @@ int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
     return rc;
 }
 
-int iommu_unmap(struct domain *d, dfn_t dfn, unsigned int page_order)
+int iommu_legacy_unmap(struct domain *d, dfn_t dfn, unsigned int page_order)
 {
     const struct domain_iommu *hd = dom_iommu(d);
     unsigned long i;
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index c68a72279d..b12289a18f 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -241,8 +241,8 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
         if ( paging_mode_translate(d) )
             rc = set_identity_p2m_entry(d, pfn, p2m_access_rw, 0);
         else
-            rc = iommu_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
-                           IOMMUF_readable | IOMMUF_writable);
+            rc = iommu_legacy_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
+                                  IOMMUF_readable | IOMMUF_writable);
         if ( rc )
             printk(XENLOG_WARNING " d%d: IOMMU mapping failed: %d\n",
                    d->domain_id, rc);
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 3d78126801..1f875aa328 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -83,15 +83,21 @@ int iommu_construct(struct domain *d);
 /* Function used internally, use iommu_domain_destroy */
 void iommu_teardown(struct domain *d);
 
-/* iommu_map_page() takes flags to direct the mapping operation. */
+/*
+ * The following flags are passed to map operations and passed by lookup
+ * operations.
+ */
 #define _IOMMUF_readable 0
 #define IOMMUF_readable  (1u<<_IOMMUF_readable)
 #define _IOMMUF_writable 1
 #define IOMMUF_writable  (1u<<_IOMMUF_writable)
-int __must_check iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
-                           unsigned int page_order, unsigned int flags);
-int __must_check iommu_unmap(struct domain *d, dfn_t dfn,
-                             unsigned int page_order);
+
+int __must_check iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
+                                  unsigned int page_order,
+                                  unsigned int flags);
+int __must_check iommu_legacy_unmap(struct domain *d, dfn_t dfn,
+                                    unsigned int page_order);
+
 int __must_check iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t *mfn,
                                    unsigned int *flags);
 
-- 
2.11.0


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

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

* [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations
  2018-12-17  9:22 [PATCH v5 0/4] iommu improvements Paul Durrant
  2018-12-17  9:22 ` [PATCH v5 1/4] amd-iommu: add flush iommu_ops Paul Durrant
  2018-12-17  9:22 ` [PATCH v5 2/4] iommu: rename wrapper functions Paul Durrant
@ 2018-12-17  9:22 ` Paul Durrant
  2018-12-19  9:05   ` Paul Durrant
                     ` (2 more replies)
  2018-12-17  9:22 ` [PATCH v5 4/4] x86/mm/p2m: stop checking for IOMMU shared page tables in mmio_order() Paul Durrant
  3 siblings, 3 replies; 16+ messages in thread
From: Paul Durrant @ 2018-12-17  9:22 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Paul Durrant, Suravee Suthikulpanit, Brian Woods,
	Roger Pau Monné

This patch removes any implicit flushing that occurs in the implementation
of map and unmap operations and adds new iommu_map/unmap() wrapper
functions. To maintain semantics of the iommu_legacy_map/unmap() wrapper
functions, these are modified to call the new wrapper functions and then
perform an explicit flush operation.

Because VT-d currently performs two different types of flush dependent upon
whether a PTE is being modified versus merely added (i.e. replacing a non-
present PTE) 'iommu flush flags' are defined by this patch and the
iommu_ops map_page() and unmap_page() methods are modified to OR the type
of flush necessary for the PTE that has been populated or depopulated into
an accumulated flags value. The accumulated value can then be passed into
the explicit flush operation.

The ARM SMMU implementations of map_page() and unmap_page() currently
perform no implicit flushing and therefore the modified methods do not
adjust the flush flags.

NOTE: The per-cpu 'iommu_dont_flush_iotlb' is respected by the
      iommu_legacy_map/unmap() wrapper functions and therefore this now
      applies to all IOMMU implementations rather than just VT-d.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Brian Woods <brian.woods@amd.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v5:
 - Fix style issues and typo picked up by Julien.

v4:
 - Formatting fixes.
 - Respect flush flags even on a failed map or unmap.

v3:
 - Make AMD IOMMU and Intel VT-d map/unmap operations pass back accurate
   flush_flags.
 - Respect 'iommu_dont_flush_iotlb' in legacy unmap wrapper.
 - Pass flush_flags into iommu_iotlb_flush_all().
 - Improve comments and fix style issues.

v2:
 - Add the new iommu_map/unmap() and don't proliferate use of
   iommu_dont_flush_iotlb.
 - Use 'flush flags' instead of a 'iommu_flush_type'
 - Add a 'flush_flags' argument to iommu_flush() and modify the call-sites.

This code has only been compile tested for ARM.
---
 xen/arch/arm/p2m.c                            | 11 +++-
 xen/common/memory.c                           |  6 +-
 xen/drivers/passthrough/amd/iommu_map.c       | 87 ++++++++++++++++++---------
 xen/drivers/passthrough/arm/smmu.c            | 11 +++-
 xen/drivers/passthrough/iommu.c               | 84 ++++++++++++++++++++------
 xen/drivers/passthrough/vtd/iommu.c           | 32 +++++-----
 xen/drivers/passthrough/x86/iommu.c           | 27 ++++++---
 xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |  9 ++-
 xen/include/xen/iommu.h                       | 44 +++++++++++---
 9 files changed, 226 insertions(+), 85 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 17e2523fc1..1389515199 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -986,8 +986,17 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
 
     if ( need_iommu_pt_sync(p2m->domain) &&
          (lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
+    {
+        unsigned int flush_flags = 0;
+
+        if ( lpae_is_valid(orig_pte) )
+            flush_flags |= IOMMU_FLUSHF_modified;
+        if ( lpae_is_valid(*entry) )
+            flush_flags |= IOMMU_FLUSHF_added;
+
         rc = iommu_iotlb_flush(p2m->domain, _dfn(gfn_x(sgfn)),
-                               1UL << page_order);
+                               1UL << page_order, flush_flags);
+    }
     else
         rc = 0;
 
diff --git a/xen/common/memory.c b/xen/common/memory.c
index f37eb288d4..b6cf09585c 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -853,11 +853,13 @@ int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap *xatp,
 
         this_cpu(iommu_dont_flush_iotlb) = 0;
 
-        ret = iommu_flush(d, _dfn(xatp->idx - done), done);
+        ret = iommu_iotlb_flush(d, _dfn(xatp->idx - done), done,
+                                IOMMU_FLUSHF_added | IOMMU_FLUSHF_modified);
         if ( unlikely(ret) && rc >= 0 )
             rc = ret;
 
-        ret = iommu_flush(d, _dfn(xatp->gpfn - done), done);
+        ret = iommu_iotlb_flush(d, _dfn(xatp->gpfn - done), done,
+                                IOMMU_FLUSHF_added | IOMMU_FLUSHF_modified);
         if ( unlikely(ret) && rc >= 0 )
             rc = ret;
     }
diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passthrough/amd/iommu_map.c
index de5a880070..21d147411e 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -35,23 +35,37 @@ static unsigned int pfn_to_pde_idx(unsigned long pfn, unsigned int level)
     return idx;
 }
 
-static void clear_iommu_pte_present(unsigned long l1_mfn, unsigned long dfn)
+static unsigned int clear_iommu_pte_present(unsigned long l1_mfn,
+                                            unsigned long dfn)
 {
     uint64_t *table, *pte;
+    uint32_t entry;
+    unsigned int flush_flags;
 
     table = map_domain_page(_mfn(l1_mfn));
-    pte = table + pfn_to_pde_idx(dfn, 1);
+
+    pte = (table + pfn_to_pde_idx(dfn, 1));
+    entry = *pte >> 32;
+
+    flush_flags = get_field_from_reg_u32(entry, IOMMU_PTE_PRESENT_MASK,
+                                         IOMMU_PTE_PRESENT_SHIFT) ?
+                                         IOMMU_FLUSHF_modified : 0;
+
     *pte = 0;
     unmap_domain_page(table);
+
+    return flush_flags;
 }
 
-static bool set_iommu_pde_present(uint32_t *pde, unsigned long next_mfn,
-                                  unsigned int next_level,
-                                  bool iw, bool ir)
+static unsigned int set_iommu_pde_present(uint32_t *pde,
+                                          unsigned long next_mfn,
+                                          unsigned int next_level, bool iw,
+                                          bool ir)
 {
     uint64_t maddr_next;
     uint32_t addr_lo, addr_hi, entry;
-    bool need_flush = false, old_present;
+    bool old_present;
+    unsigned int flush_flags = IOMMU_FLUSHF_added;
 
     maddr_next = __pfn_to_paddr(next_mfn);
 
@@ -84,7 +98,7 @@ static bool set_iommu_pde_present(uint32_t *pde, unsigned long next_mfn,
 
         if ( maddr_old != maddr_next || iw != old_w || ir != old_r ||
              old_level != next_level )
-            need_flush = true;
+            flush_flags |= IOMMU_FLUSHF_modified;
     }
 
     addr_lo = maddr_next & DMA_32BIT_MASK;
@@ -121,24 +135,27 @@ static bool set_iommu_pde_present(uint32_t *pde, unsigned long next_mfn,
                          IOMMU_PDE_PRESENT_SHIFT, &entry);
     pde[0] = entry;
 
-    return need_flush;
+    return flush_flags;
 }
 
-static bool set_iommu_pte_present(unsigned long pt_mfn, unsigned long dfn,
-                                  unsigned long next_mfn, int pde_level,
-                                  bool iw, bool ir)
+static unsigned int set_iommu_pte_present(unsigned long pt_mfn,
+                                          unsigned long dfn,
+                                          unsigned long next_mfn,
+                                          int pde_level,
+                                          bool iw, bool ir)
 {
     uint64_t *table;
     uint32_t *pde;
-    bool need_flush;
+    unsigned int flush_flags;
 
     table = map_domain_page(_mfn(pt_mfn));
 
     pde = (uint32_t *)(table + pfn_to_pde_idx(dfn, pde_level));
 
-    need_flush = set_iommu_pde_present(pde, next_mfn, 0, iw, ir);
+    flush_flags = set_iommu_pde_present(pde, next_mfn, 0, iw, ir);
     unmap_domain_page(table);
-    return need_flush;
+
+    return flush_flags;
 }
 
 void amd_iommu_set_root_page_table(uint32_t *dte, uint64_t root_ptr,
@@ -525,9 +542,8 @@ static int update_paging_mode(struct domain *d, unsigned long dfn)
 }
 
 int amd_iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn,
-                       unsigned int flags)
+                       unsigned int flags, unsigned int *flush_flags)
 {
-    bool need_flush;
     struct domain_iommu *hd = dom_iommu(d);
     int rc;
     unsigned long pt_mfn[7];
@@ -573,18 +589,17 @@ int amd_iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn,
     }
 
     /* Install 4k mapping */
-    need_flush = set_iommu_pte_present(pt_mfn[1], dfn_x(dfn), mfn_x(mfn), 1,
-                                       !!(flags & IOMMUF_writable),
-                                       !!(flags & IOMMUF_readable));
-
-    if ( need_flush )
-        amd_iommu_flush_pages(d, dfn_x(dfn), 0);
+    *flush_flags |= set_iommu_pte_present(pt_mfn[1], dfn_x(dfn), mfn_x(mfn),
+                                          1, (flags & IOMMUF_writable),
+                                          (flags & IOMMUF_readable));
 
     spin_unlock(&hd->arch.mapping_lock);
+
     return 0;
 }
 
-int amd_iommu_unmap_page(struct domain *d, dfn_t dfn)
+int amd_iommu_unmap_page(struct domain *d, dfn_t dfn,
+                         unsigned int *flush_flags)
 {
     unsigned long pt_mfn[7];
     struct domain_iommu *hd = dom_iommu(d);
@@ -629,11 +644,10 @@ int amd_iommu_unmap_page(struct domain *d, dfn_t dfn)
     }
 
     /* mark PTE as 'page not present' */
-    clear_iommu_pte_present(pt_mfn[1], dfn_x(dfn));
+    *flush_flags |= clear_iommu_pte_present(pt_mfn[1], dfn_x(dfn));
 
     spin_unlock(&hd->arch.mapping_lock);
 
-    amd_iommu_flush_pages(d, dfn_x(dfn), 0);
     return 0;
 }
 
@@ -648,11 +662,17 @@ static unsigned long flush_count(unsigned long dfn, unsigned int page_count,
 }
 
 int amd_iommu_flush_iotlb_pages(struct domain *d, dfn_t dfn,
-                                unsigned int page_count)
+                                unsigned int page_count,
+                                unsigned int flush_flags)
 {
     unsigned long dfn_l = dfn_x(dfn);
 
     ASSERT(page_count && !dfn_eq(dfn, INVALID_DFN));
+    ASSERT(flush_flags);
+
+    /* Unless a PTE was modified, no flush is required */
+    if ( !(flush_flags & IOMMU_FLUSHF_modified) )
+        return 0;
 
     /* If the range wraps then just flush everything */
     if ( dfn_l + page_count < dfn_l )
@@ -695,6 +715,7 @@ int amd_iommu_reserve_domain_unity_map(struct domain *domain,
     unsigned long npages, i;
     unsigned long gfn;
     unsigned int flags = !!ir;
+    unsigned int flush_flags = 0;
     int rt = 0;
 
     if ( iw )
@@ -706,11 +727,19 @@ int amd_iommu_reserve_domain_unity_map(struct domain *domain,
     {
         unsigned long frame = gfn + i;
 
-        rt = amd_iommu_map_page(domain, _dfn(frame), _mfn(frame), flags);
+        rt = amd_iommu_map_page(domain, _dfn(frame), _mfn(frame), flags,
+                                &flush_flags);
         if ( rt != 0 )
-            return rt;
+            break;
     }
-    return 0;
+
+    /* Use while-break to avoid compiler warning */
+    while ( flush_flags && amd_iommu_flush_iotlb_pages(domain, _dfn(gfn),
+                                                       npages,
+                                                       flush_flags) )
+        break;
+
+    return rt;
 }
 
 /* Share p2m table with iommu. */
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 9612c0fddc..73c8048504 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2535,8 +2535,11 @@ static int __must_check arm_smmu_iotlb_flush_all(struct domain *d)
 }
 
 static int __must_check arm_smmu_iotlb_flush(struct domain *d, dfn_t dfn,
-                                             unsigned int page_count)
+					     unsigned int page_count,
+					     unsigned int flush_flags)
 {
+	ASSERT(flush_flags);
+
 	/* ARM SMMU v1 doesn't have flush by VMA and VMID */
 	return arm_smmu_iotlb_flush_all(d);
 }
@@ -2732,7 +2735,8 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d)
 }
 
 static int __must_check arm_smmu_map_page(struct domain *d, dfn_t dfn,
-					  mfn_t mfn, unsigned int flags)
+					  mfn_t mfn, unsigned int flags,
+					  unsigned int *flush_flags)
 {
 	p2m_type_t t;
 
@@ -2761,7 +2765,8 @@ static int __must_check arm_smmu_map_page(struct domain *d, dfn_t dfn,
 				       0, t);
 }
 
-static int __must_check arm_smmu_unmap_page(struct domain *d, dfn_t dfn)
+static int __must_check arm_smmu_unmap_page(struct domain *d, dfn_t dfn,
+                                            unsigned int *flush_flags)
 {
 	/*
 	 * This function should only be used by gnttab code when the domain
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 105995a343..caff3ab243 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -211,7 +211,7 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
     if ( need_iommu_pt_sync(d) )
     {
         struct page_info *page;
-        unsigned int i = 0;
+        unsigned int i = 0, flush_flags = 0;
         int rc = 0;
 
         page_list_for_each ( page, &d->page_list )
@@ -226,8 +226,9 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
                   == PGT_writable_page) )
                 mapping |= IOMMUF_writable;
 
-            ret = hd->platform_ops->map_page(d, _dfn(dfn), _mfn(mfn),
-                                             mapping);
+            ret = iommu_map(d, _dfn(dfn), _mfn(mfn), mapping, 0,
+                            &flush_flags);
+
             if ( !rc )
                 rc = ret;
 
@@ -235,6 +236,10 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
                 process_pending_softirqs();
         }
 
+        /* Use while-break to avoid compiler warning */
+        while ( iommu_iotlb_flush_all(d, flush_flags) )
+            break;
+
         if ( rc )
             printk(XENLOG_WARNING "d%d: IOMMU mapping failed: %d\n",
                    d->domain_id, rc);
@@ -304,8 +309,9 @@ void iommu_domain_destroy(struct domain *d)
     arch_iommu_domain_destroy(d);
 }
 
-int iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
-                     unsigned int page_order, unsigned int flags)
+int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
+              unsigned int page_order, unsigned int flags,
+              unsigned int *flush_flags)
 {
     const struct domain_iommu *hd = dom_iommu(d);
     unsigned long i;
@@ -319,8 +325,8 @@ int iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
 
     for ( i = 0; i < (1ul << page_order); i++ )
     {
-        rc = hd->platform_ops->map_page(d, dfn_add(dfn, i),
-                                        mfn_add(mfn, i), flags);
+        rc = hd->platform_ops->map_page(d, dfn_add(dfn, i), mfn_add(mfn, i),
+                                        flags, flush_flags);
 
         if ( likely(!rc) )
             continue;
@@ -333,7 +339,8 @@ int iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
 
         while ( i-- )
             /* if statement to satisfy __must_check */
-            if ( hd->platform_ops->unmap_page(d, dfn_add(dfn, i)) )
+            if ( hd->platform_ops->unmap_page(d, dfn_add(dfn, i),
+                                              flush_flags) )
                 continue;
 
         if ( !is_hardware_domain(d) )
@@ -345,7 +352,26 @@ int iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
     return rc;
 }
 
-int iommu_legacy_unmap(struct domain *d, dfn_t dfn, unsigned int page_order)
+int iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
+                     unsigned int page_order, unsigned int flags)
+{
+    unsigned int flush_flags = 0;
+    int rc = iommu_map(d, dfn, mfn, page_order, flags, &flush_flags);
+
+    if ( !this_cpu(iommu_dont_flush_iotlb) )
+    {
+        int err = iommu_iotlb_flush(d, dfn, (1u << page_order),
+                                    flush_flags);
+
+        if ( !rc )
+            rc = err;
+    }
+
+    return rc;
+}
+
+int iommu_unmap(struct domain *d, dfn_t dfn, unsigned int page_order,
+                unsigned int *flush_flags)
 {
     const struct domain_iommu *hd = dom_iommu(d);
     unsigned long i;
@@ -358,7 +384,8 @@ int iommu_legacy_unmap(struct domain *d, dfn_t dfn, unsigned int page_order)
 
     for ( i = 0; i < (1ul << page_order); i++ )
     {
-        int err = hd->platform_ops->unmap_page(d, dfn_add(dfn, i));
+        int err = hd->platform_ops->unmap_page(d, dfn_add(dfn, i),
+                                               flush_flags);
 
         if ( likely(!err) )
             continue;
@@ -381,6 +408,23 @@ int iommu_legacy_unmap(struct domain *d, dfn_t dfn, unsigned int page_order)
     return rc;
 }
 
+int iommu_legacy_unmap(struct domain *d, dfn_t dfn, unsigned int page_order)
+{
+    unsigned int flush_flags = 0;
+    int rc = iommu_unmap(d, dfn, page_order, &flush_flags);
+
+    if ( !this_cpu(iommu_dont_flush_iotlb) )
+    {
+        int err = iommu_iotlb_flush(d, dfn, (1u << page_order),
+                                    flush_flags);
+
+        if ( !rc )
+            rc = err;
+    }
+
+    return rc;
+}
+
 int iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t *mfn,
                       unsigned int *flags)
 {
@@ -409,25 +453,26 @@ static void iommu_free_pagetables(unsigned long unused)
                             cpumask_cycle(smp_processor_id(), &cpu_online_map));
 }
 
-int iommu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned int page_count)
+int iommu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned int page_count,
+                      unsigned int flush_flags)
 {
     const struct domain_iommu *hd = dom_iommu(d);
     int rc;
 
     if ( !iommu_enabled || !hd->platform_ops ||
-         !hd->platform_ops->iotlb_flush || !page_count )
+         !hd->platform_ops->iotlb_flush || !page_count || !flush_flags )
         return 0;
 
     if ( dfn_eq(dfn, INVALID_DFN) )
         return -EINVAL;
 
-    rc = hd->platform_ops->iotlb_flush(d, dfn, page_count);
+    rc = hd->platform_ops->iotlb_flush(d, dfn, page_count, flush_flags);
     if ( unlikely(rc) )
     {
         if ( !d->is_shutting_down && printk_ratelimit() )
             printk(XENLOG_ERR
-                   "d%d: IOMMU IOTLB flush failed: %d, dfn %"PRI_dfn", page count %u\n",
-                   d->domain_id, rc, dfn_x(dfn), page_count);
+                   "d%d: IOMMU IOTLB flush failed: %d, dfn %"PRI_dfn", page count %u flags %x\n",
+                   d->domain_id, rc, dfn_x(dfn), page_count, flush_flags);
 
         if ( !is_hardware_domain(d) )
             domain_crash(d);
@@ -436,14 +481,19 @@ int iommu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned int page_count)
     return rc;
 }
 
-int iommu_iotlb_flush_all(struct domain *d)
+int iommu_iotlb_flush_all(struct domain *d, unsigned int flush_flags)
 {
     const struct domain_iommu *hd = dom_iommu(d);
     int rc;
 
-    if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops->iotlb_flush_all )
+    if ( !iommu_enabled || !hd->platform_ops ||
+         !hd->platform_ops->iotlb_flush_all || !flush_flags )
         return 0;
 
+    /*
+     * The operation does a full flush so we don't need to pass the
+     * flush_flags in.
+     */
     rc = hd->platform_ops->iotlb_flush_all(d);
     if ( unlikely(rc) )
     {
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index d2fa5e2b25..50a0e25224 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -633,11 +633,14 @@ static int __must_check iommu_flush_iotlb(struct domain *d, dfn_t dfn,
 
 static int __must_check iommu_flush_iotlb_pages(struct domain *d,
                                                 dfn_t dfn,
-                                                unsigned int page_count)
+                                                unsigned int page_count,
+                                                unsigned int flush_flags)
 {
     ASSERT(page_count && !dfn_eq(dfn, INVALID_DFN));
+    ASSERT(flush_flags);
 
-    return iommu_flush_iotlb(d, dfn, 1, page_count);
+    return iommu_flush_iotlb(d, dfn, flush_flags & IOMMU_FLUSHF_modified,
+                             page_count);
 }
 
 static int __must_check iommu_flush_iotlb_all(struct domain *d)
@@ -646,7 +649,8 @@ static int __must_check iommu_flush_iotlb_all(struct domain *d)
 }
 
 /* clear one page's page table */
-static int __must_check dma_pte_clear_one(struct domain *domain, u64 addr)
+static int __must_check dma_pte_clear_one(struct domain *domain, u64 addr,
+                                          unsigned int *flush_flags)
 {
     struct domain_iommu *hd = dom_iommu(domain);
     struct dma_pte *page = NULL, *pte = NULL;
@@ -673,12 +677,11 @@ static int __must_check dma_pte_clear_one(struct domain *domain, u64 addr)
     }
 
     dma_clear_pte(*pte);
+    *flush_flags |= IOMMU_FLUSHF_modified;
+
     spin_unlock(&hd->arch.mapping_lock);
     iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
 
-    if ( !this_cpu(iommu_dont_flush_iotlb) )
-        rc = iommu_flush_iotlb_pages(domain, daddr_to_dfn(addr), 1);
-
     unmap_vtd_domain_page(page);
 
     return rc;
@@ -1773,9 +1776,9 @@ static void iommu_domain_teardown(struct domain *d)
     spin_unlock(&hd->arch.mapping_lock);
 }
 
-static int __must_check intel_iommu_map_page(struct domain *d,
-                                             dfn_t dfn, mfn_t mfn,
-                                             unsigned int flags)
+static int __must_check intel_iommu_map_page(struct domain *d, dfn_t dfn,
+                                             mfn_t mfn, unsigned int flags,
+                                             unsigned int *flush_flags)
 {
     struct domain_iommu *hd = dom_iommu(d);
     struct dma_pte *page, *pte, old, new = {};
@@ -1825,14 +1828,15 @@ static int __must_check intel_iommu_map_page(struct domain *d,
     spin_unlock(&hd->arch.mapping_lock);
     unmap_vtd_domain_page(page);
 
-    if ( !this_cpu(iommu_dont_flush_iotlb) )
-        rc = iommu_flush_iotlb(d, dfn, dma_pte_present(old), 1);
+    *flush_flags |= IOMMU_FLUSHF_added;
+    if ( dma_pte_present(old) )
+        *flush_flags |= IOMMU_FLUSHF_modified;
 
     return rc;
 }
 
-static int __must_check intel_iommu_unmap_page(struct domain *d,
-                                               dfn_t dfn)
+static int __must_check intel_iommu_unmap_page(struct domain *d, dfn_t dfn,
+                                               unsigned int *flush_flags)
 {
     /* Do nothing if VT-d shares EPT page table */
     if ( iommu_use_hap_pt(d) )
@@ -1842,7 +1846,7 @@ static int __must_check intel_iommu_unmap_page(struct domain *d,
     if ( iommu_hwdom_passthrough && is_hardware_domain(d) )
         return 0;
 
-    return dma_pte_clear_one(d, dfn_to_daddr(dfn));
+    return dma_pte_clear_one(d, dfn_to_daddr(dfn), flush_flags);
 }
 
 static int intel_iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t *mfn,
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index b12289a18f..e40d7a7d7b 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -46,11 +46,9 @@ int __init iommu_setup_hpet_msi(struct msi_desc *msi)
 
 int arch_iommu_populate_page_table(struct domain *d)
 {
-    const struct domain_iommu *hd = dom_iommu(d);
     struct page_info *page;
     int rc = 0, n = 0;
 
-    this_cpu(iommu_dont_flush_iotlb) = 1;
     spin_lock(&d->page_alloc_lock);
 
     if ( unlikely(d->is_dying) )
@@ -63,14 +61,15 @@ int arch_iommu_populate_page_table(struct domain *d)
         {
             unsigned long mfn = mfn_x(page_to_mfn(page));
             unsigned long gfn = mfn_to_gmfn(d, mfn);
+            unsigned int flush_flags = 0;
 
             if ( gfn != gfn_x(INVALID_GFN) )
             {
                 ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
                 BUG_ON(SHARED_M2P(gfn));
-                rc = hd->platform_ops->map_page(d, _dfn(gfn), _mfn(mfn),
-                                                IOMMUF_readable |
-                                                IOMMUF_writable);
+                rc = iommu_map(d, _dfn(gfn), _mfn(mfn), PAGE_ORDER_4K,
+                               IOMMUF_readable | IOMMUF_writable,
+                               &flush_flags);
             }
             if ( rc )
             {
@@ -104,10 +103,14 @@ int arch_iommu_populate_page_table(struct domain *d)
     }
 
     spin_unlock(&d->page_alloc_lock);
-    this_cpu(iommu_dont_flush_iotlb) = 0;
 
     if ( !rc )
-        rc = iommu_iotlb_flush_all(d);
+        /*
+         * flush_flags are not tracked across hypercall pre-emption so
+         * assume a full flush is necessary.
+         */
+        rc = iommu_iotlb_flush_all(
+            d, IOMMU_FLUSHF_added | IOMMU_FLUSHF_modified);
 
     if ( rc && rc != -ERESTART )
         iommu_teardown(d);
@@ -207,6 +210,7 @@ static bool __hwdom_init hwdom_iommu_map(const struct domain *d,
 void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
 {
     unsigned long i, top, max_pfn;
+    unsigned int flush_flags = 0;
 
     BUG_ON(!is_hardware_domain(d));
 
@@ -241,8 +245,9 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
         if ( paging_mode_translate(d) )
             rc = set_identity_p2m_entry(d, pfn, p2m_access_rw, 0);
         else
-            rc = iommu_legacy_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
-                                  IOMMUF_readable | IOMMUF_writable);
+            rc = iommu_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
+                           IOMMUF_readable | IOMMUF_writable, &flush_flags);
+
         if ( rc )
             printk(XENLOG_WARNING " d%d: IOMMU mapping failed: %d\n",
                    d->domain_id, rc);
@@ -250,6 +255,10 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
         if (!(i & 0xfffff))
             process_pending_softirqs();
     }
+
+    /* Use if to avoid compiler warning */
+    if ( iommu_iotlb_flush_all(d, flush_flags) )
+        return;
 }
 
 /*
diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
index 88715329ca..c5697565d6 100644
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -53,15 +53,18 @@ int amd_iommu_update_ivrs_mapping_acpi(void);
 
 /* mapping functions */
 int __must_check amd_iommu_map_page(struct domain *d, dfn_t dfn,
-                                    mfn_t mfn, unsigned int flags);
-int __must_check amd_iommu_unmap_page(struct domain *d, dfn_t dfn);
+                                    mfn_t mfn, unsigned int flags,
+                                    unsigned int *flush_flags);
+int __must_check amd_iommu_unmap_page(struct domain *d, dfn_t dfn,
+                                      unsigned int *flush_flags);
 uint64_t amd_iommu_get_address_from_pte(void *entry);
 int __must_check amd_iommu_alloc_root(struct domain_iommu *hd);
 int amd_iommu_reserve_domain_unity_map(struct domain *domain,
                                        paddr_t phys_addr, unsigned long size,
                                        int iw, int ir);
 int __must_check amd_iommu_flush_iotlb_pages(struct domain *d, dfn_t dfn,
-                                             unsigned int page_count);
+                                             unsigned int page_count,
+                                             unsigned int flush_flags);
 int __must_check amd_iommu_flush_iotlb_all(struct domain *d);
 
 /* Share p2m table with iommu */
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 1f875aa328..cdc8021cbd 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -92,6 +92,31 @@ void iommu_teardown(struct domain *d);
 #define _IOMMUF_writable 1
 #define IOMMUF_writable  (1u<<_IOMMUF_writable)
 
+/*
+ * flush_flags:
+ *
+ * IOMMU_FLUSHF_added -> A new 'present' PTE has been inserted.
+ * IOMMU_FLUSHF_modified -> An existing 'present' PTE has been modified
+ *                          (whether the new PTE value is 'present' or not).
+ *
+ * These flags are passed back from map/unmap operations and passed into
+ * flush operations.
+ */
+enum
+{
+    _IOMMU_FLUSHF_added,
+    _IOMMU_FLUSHF_modified,
+};
+#define IOMMU_FLUSHF_added (1u << _IOMMU_FLUSHF_added)
+#define IOMMU_FLUSHF_modified (1u << _IOMMU_FLUSHF_modified)
+
+int __must_check iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
+                           unsigned int page_order, unsigned int flags,
+                           unsigned int *flush_flags);
+int __must_check iommu_unmap(struct domain *d, dfn_t dfn,
+                             unsigned int page_order,
+                             unsigned int *flush_flags);
+
 int __must_check iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
                                   unsigned int page_order,
                                   unsigned int flags);
@@ -101,6 +126,12 @@ int __must_check iommu_legacy_unmap(struct domain *d, dfn_t dfn,
 int __must_check iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t *mfn,
                                    unsigned int *flags);
 
+int __must_check iommu_iotlb_flush(struct domain *d, dfn_t dfn,
+                                   unsigned int page_count,
+                                   unsigned int flush_flags);
+int __must_check iommu_iotlb_flush_all(struct domain *d,
+                                       unsigned int flush_flags);
+
 enum iommu_feature
 {
     IOMMU_FEAT_COHERENT_WALK,
@@ -178,8 +209,10 @@ struct iommu_ops {
      * other by the caller in order to have meaningful results.
      */
     int __must_check (*map_page)(struct domain *d, dfn_t dfn, mfn_t mfn,
-                                 unsigned int flags);
-    int __must_check (*unmap_page)(struct domain *d, dfn_t dfn);
+                                 unsigned int flags,
+                                 unsigned int *flush_flags);
+    int __must_check (*unmap_page)(struct domain *d, dfn_t dfn,
+                                   unsigned int *flush_flags);
     int __must_check (*lookup_page)(struct domain *d, dfn_t dfn, mfn_t *mfn,
                                     unsigned int *flags);
 
@@ -194,7 +227,8 @@ struct iommu_ops {
     void (*share_p2m)(struct domain *d);
     void (*crash_shutdown)(void);
     int __must_check (*iotlb_flush)(struct domain *d, dfn_t dfn,
-                                    unsigned int page_count);
+                                    unsigned int page_count,
+                                    unsigned int flush_flags);
     int __must_check (*iotlb_flush_all)(struct domain *d);
     int (*get_reserved_device_memory)(iommu_grdm_t *, void *);
     void (*dump_p2m_table)(struct domain *d);
@@ -253,10 +287,6 @@ int iommu_do_pci_domctl(struct xen_domctl *, struct domain *d,
 int iommu_do_domctl(struct xen_domctl *, struct domain *d,
                     XEN_GUEST_HANDLE_PARAM(xen_domctl_t));
 
-int __must_check iommu_iotlb_flush(struct domain *d, dfn_t dfn,
-                                   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);
 
 /*
-- 
2.11.0


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

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

* [PATCH v5 4/4] x86/mm/p2m: stop checking for IOMMU shared page tables in mmio_order()
  2018-12-17  9:22 [PATCH v5 0/4] iommu improvements Paul Durrant
                   ` (2 preceding siblings ...)
  2018-12-17  9:22 ` [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations Paul Durrant
@ 2018-12-17  9:22 ` Paul Durrant
  3 siblings, 0 replies; 16+ messages in thread
From: Paul Durrant @ 2018-12-17  9:22 UTC (permalink / raw)
  To: xen-devel
  Cc: George Dunlap, Andrew Cooper, Paul Durrant, Wei Liu,
	Roger Pau Monné

Now that the iommu_map() and iommu_unmap() operations take an order
parameter and elide flushing there's no strong reason why modifying MMIO
ranges in the p2m should be restricted to a 4k granularity simply because
the IOMMU is enabled but shared page tables are not in operation.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v2:
 - New in v2. (Adapted from a previously independent patch).
---
 xen/arch/x86/mm/p2m.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index ed76e96d33..a9cfd1b2e4 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2059,13 +2059,12 @@ static unsigned int mmio_order(const struct domain *d,
                                unsigned long start_fn, unsigned long nr)
 {
     /*
-     * Note that the !iommu_use_hap_pt() here has three effects:
-     * - cover iommu_{,un}map_page() not having an "order" input yet,
+     * Note that the !hap_enabled() here has two effects:
      * - exclude shadow mode (which doesn't support large MMIO mappings),
      * - exclude PV guests, should execution reach this code for such.
      * So be careful when altering this.
      */
-    if ( !iommu_use_hap_pt(d) ||
+    if ( !hap_enabled(d) ||
          (start_fn & ((1UL << PAGE_ORDER_2M) - 1)) || !(nr >> PAGE_ORDER_2M) )
         return PAGE_ORDER_4K;
 
-- 
2.11.0


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

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

* Re: [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations
  2018-12-17  9:22 ` [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations Paul Durrant
@ 2018-12-19  9:05   ` Paul Durrant
  2018-12-19 15:43   ` Julien Grall
  2018-12-20 19:27   ` Woods, Brian
  2 siblings, 0 replies; 16+ messages in thread
From: Paul Durrant @ 2018-12-19  9:05 UTC (permalink / raw)
  To: Paul Durrant, xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	Andrew Cooper, Tim (Xen.org),
	George Dunlap, Julien Grall, Suravee Suthikulpanit, Ian Jackson,
	Brian Woods, Roger Pau Monne

Julien, ping? 

> -----Original Message-----
> From: Paul Durrant [mailto:paul.durrant@citrix.com]
> Sent: 17 December 2018 09:23
> To: xen-devel@lists.xenproject.org
> Cc: Paul Durrant <Paul.Durrant@citrix.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Julien Grall <julien.grall@arm.com>; Andrew
> Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Konrad
> Rzeszutek Wilk <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>; Wei
> Liu <wei.liu2@citrix.com>; Suravee Suthikulpanit
> <suravee.suthikulpanit@amd.com>; Brian Woods <brian.woods@amd.com>; Roger
> Pau Monne <roger.pau@citrix.com>
> Subject: [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap
> operations
> 
> This patch removes any implicit flushing that occurs in the implementation
> of map and unmap operations and adds new iommu_map/unmap() wrapper
> functions. To maintain semantics of the iommu_legacy_map/unmap() wrapper
> functions, these are modified to call the new wrapper functions and then
> perform an explicit flush operation.
> 
> Because VT-d currently performs two different types of flush dependent
> upon
> whether a PTE is being modified versus merely added (i.e. replacing a non-
> present PTE) 'iommu flush flags' are defined by this patch and the
> iommu_ops map_page() and unmap_page() methods are modified to OR the type
> of flush necessary for the PTE that has been populated or depopulated into
> an accumulated flags value. The accumulated value can then be passed into
> the explicit flush operation.
> 
> The ARM SMMU implementations of map_page() and unmap_page() currently
> perform no implicit flushing and therefore the modified methods do not
> adjust the flush flags.
> 
> NOTE: The per-cpu 'iommu_dont_flush_iotlb' is respected by the
>       iommu_legacy_map/unmap() wrapper functions and therefore this now
>       applies to all IOMMU implementations rather than just VT-d.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Cc: Brian Woods <brian.woods@amd.com>
> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> 
> v5:
>  - Fix style issues and typo picked up by Julien.
> 
> v4:
>  - Formatting fixes.
>  - Respect flush flags even on a failed map or unmap.
> 
> v3:
>  - Make AMD IOMMU and Intel VT-d map/unmap operations pass back accurate
>    flush_flags.
>  - Respect 'iommu_dont_flush_iotlb' in legacy unmap wrapper.
>  - Pass flush_flags into iommu_iotlb_flush_all().
>  - Improve comments and fix style issues.
> 
> v2:
>  - Add the new iommu_map/unmap() and don't proliferate use of
>    iommu_dont_flush_iotlb.
>  - Use 'flush flags' instead of a 'iommu_flush_type'
>  - Add a 'flush_flags' argument to iommu_flush() and modify the call-
> sites.
> 
> This code has only been compile tested for ARM.
> ---
>  xen/arch/arm/p2m.c                            | 11 +++-
>  xen/common/memory.c                           |  6 +-
>  xen/drivers/passthrough/amd/iommu_map.c       | 87 ++++++++++++++++++----
> -----
>  xen/drivers/passthrough/arm/smmu.c            | 11 +++-
>  xen/drivers/passthrough/iommu.c               | 84 ++++++++++++++++++++--
> ----
>  xen/drivers/passthrough/vtd/iommu.c           | 32 +++++-----
>  xen/drivers/passthrough/x86/iommu.c           | 27 ++++++---
>  xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |  9 ++-
>  xen/include/xen/iommu.h                       | 44 +++++++++++---
>  9 files changed, 226 insertions(+), 85 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 17e2523fc1..1389515199 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -986,8 +986,17 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
> 
>      if ( need_iommu_pt_sync(p2m->domain) &&
>           (lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
> +    {
> +        unsigned int flush_flags = 0;
> +
> +        if ( lpae_is_valid(orig_pte) )
> +            flush_flags |= IOMMU_FLUSHF_modified;
> +        if ( lpae_is_valid(*entry) )
> +            flush_flags |= IOMMU_FLUSHF_added;
> +
>          rc = iommu_iotlb_flush(p2m->domain, _dfn(gfn_x(sgfn)),
> -                               1UL << page_order);
> +                               1UL << page_order, flush_flags);
> +    }
>      else
>          rc = 0;
> 
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index f37eb288d4..b6cf09585c 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -853,11 +853,13 @@ int xenmem_add_to_physmap(struct domain *d, struct
> xen_add_to_physmap *xatp,
> 
>          this_cpu(iommu_dont_flush_iotlb) = 0;
> 
> -        ret = iommu_flush(d, _dfn(xatp->idx - done), done);
> +        ret = iommu_iotlb_flush(d, _dfn(xatp->idx - done), done,
> +                                IOMMU_FLUSHF_added |
> IOMMU_FLUSHF_modified);
>          if ( unlikely(ret) && rc >= 0 )
>              rc = ret;
> 
> -        ret = iommu_flush(d, _dfn(xatp->gpfn - done), done);
> +        ret = iommu_iotlb_flush(d, _dfn(xatp->gpfn - done), done,
> +                                IOMMU_FLUSHF_added |
> IOMMU_FLUSHF_modified);
>          if ( unlikely(ret) && rc >= 0 )
>              rc = ret;
>      }
> diff --git a/xen/drivers/passthrough/amd/iommu_map.c
> b/xen/drivers/passthrough/amd/iommu_map.c
> index de5a880070..21d147411e 100644
> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -35,23 +35,37 @@ static unsigned int pfn_to_pde_idx(unsigned long pfn,
> unsigned int level)
>      return idx;
>  }
> 
> -static void clear_iommu_pte_present(unsigned long l1_mfn, unsigned long
> dfn)
> +static unsigned int clear_iommu_pte_present(unsigned long l1_mfn,
> +                                            unsigned long dfn)
>  {
>      uint64_t *table, *pte;
> +    uint32_t entry;
> +    unsigned int flush_flags;
> 
>      table = map_domain_page(_mfn(l1_mfn));
> -    pte = table + pfn_to_pde_idx(dfn, 1);
> +
> +    pte = (table + pfn_to_pde_idx(dfn, 1));
> +    entry = *pte >> 32;
> +
> +    flush_flags = get_field_from_reg_u32(entry, IOMMU_PTE_PRESENT_MASK,
> +                                         IOMMU_PTE_PRESENT_SHIFT) ?
> +                                         IOMMU_FLUSHF_modified : 0;
> +
>      *pte = 0;
>      unmap_domain_page(table);
> +
> +    return flush_flags;
>  }
> 
> -static bool set_iommu_pde_present(uint32_t *pde, unsigned long next_mfn,
> -                                  unsigned int next_level,
> -                                  bool iw, bool ir)
> +static unsigned int set_iommu_pde_present(uint32_t *pde,
> +                                          unsigned long next_mfn,
> +                                          unsigned int next_level, bool
> iw,
> +                                          bool ir)
>  {
>      uint64_t maddr_next;
>      uint32_t addr_lo, addr_hi, entry;
> -    bool need_flush = false, old_present;
> +    bool old_present;
> +    unsigned int flush_flags = IOMMU_FLUSHF_added;
> 
>      maddr_next = __pfn_to_paddr(next_mfn);
> 
> @@ -84,7 +98,7 @@ static bool set_iommu_pde_present(uint32_t *pde,
> unsigned long next_mfn,
> 
>          if ( maddr_old != maddr_next || iw != old_w || ir != old_r ||
>               old_level != next_level )
> -            need_flush = true;
> +            flush_flags |= IOMMU_FLUSHF_modified;
>      }
> 
>      addr_lo = maddr_next & DMA_32BIT_MASK;
> @@ -121,24 +135,27 @@ static bool set_iommu_pde_present(uint32_t *pde,
> unsigned long next_mfn,
>                           IOMMU_PDE_PRESENT_SHIFT, &entry);
>      pde[0] = entry;
> 
> -    return need_flush;
> +    return flush_flags;
>  }
> 
> -static bool set_iommu_pte_present(unsigned long pt_mfn, unsigned long
> dfn,
> -                                  unsigned long next_mfn, int pde_level,
> -                                  bool iw, bool ir)
> +static unsigned int set_iommu_pte_present(unsigned long pt_mfn,
> +                                          unsigned long dfn,
> +                                          unsigned long next_mfn,
> +                                          int pde_level,
> +                                          bool iw, bool ir)
>  {
>      uint64_t *table;
>      uint32_t *pde;
> -    bool need_flush;
> +    unsigned int flush_flags;
> 
>      table = map_domain_page(_mfn(pt_mfn));
> 
>      pde = (uint32_t *)(table + pfn_to_pde_idx(dfn, pde_level));
> 
> -    need_flush = set_iommu_pde_present(pde, next_mfn, 0, iw, ir);
> +    flush_flags = set_iommu_pde_present(pde, next_mfn, 0, iw, ir);
>      unmap_domain_page(table);
> -    return need_flush;
> +
> +    return flush_flags;
>  }
> 
>  void amd_iommu_set_root_page_table(uint32_t *dte, uint64_t root_ptr,
> @@ -525,9 +542,8 @@ static int update_paging_mode(struct domain *d,
> unsigned long dfn)
>  }
> 
>  int amd_iommu_map_page(struct domain *d, dfn_t dfn, mfn_t mfn,
> -                       unsigned int flags)
> +                       unsigned int flags, unsigned int *flush_flags)
>  {
> -    bool need_flush;
>      struct domain_iommu *hd = dom_iommu(d);
>      int rc;
>      unsigned long pt_mfn[7];
> @@ -573,18 +589,17 @@ int amd_iommu_map_page(struct domain *d, dfn_t dfn,
> mfn_t mfn,
>      }
> 
>      /* Install 4k mapping */
> -    need_flush = set_iommu_pte_present(pt_mfn[1], dfn_x(dfn), mfn_x(mfn),
> 1,
> -                                       !!(flags & IOMMUF_writable),
> -                                       !!(flags & IOMMUF_readable));
> -
> -    if ( need_flush )
> -        amd_iommu_flush_pages(d, dfn_x(dfn), 0);
> +    *flush_flags |= set_iommu_pte_present(pt_mfn[1], dfn_x(dfn),
> mfn_x(mfn),
> +                                          1, (flags & IOMMUF_writable),
> +                                          (flags & IOMMUF_readable));
> 
>      spin_unlock(&hd->arch.mapping_lock);
> +
>      return 0;
>  }
> 
> -int amd_iommu_unmap_page(struct domain *d, dfn_t dfn)
> +int amd_iommu_unmap_page(struct domain *d, dfn_t dfn,
> +                         unsigned int *flush_flags)
>  {
>      unsigned long pt_mfn[7];
>      struct domain_iommu *hd = dom_iommu(d);
> @@ -629,11 +644,10 @@ int amd_iommu_unmap_page(struct domain *d, dfn_t
> dfn)
>      }
> 
>      /* mark PTE as 'page not present' */
> -    clear_iommu_pte_present(pt_mfn[1], dfn_x(dfn));
> +    *flush_flags |= clear_iommu_pte_present(pt_mfn[1], dfn_x(dfn));
> 
>      spin_unlock(&hd->arch.mapping_lock);
> 
> -    amd_iommu_flush_pages(d, dfn_x(dfn), 0);
>      return 0;
>  }
> 
> @@ -648,11 +662,17 @@ static unsigned long flush_count(unsigned long dfn,
> unsigned int page_count,
>  }
> 
>  int amd_iommu_flush_iotlb_pages(struct domain *d, dfn_t dfn,
> -                                unsigned int page_count)
> +                                unsigned int page_count,
> +                                unsigned int flush_flags)
>  {
>      unsigned long dfn_l = dfn_x(dfn);
> 
>      ASSERT(page_count && !dfn_eq(dfn, INVALID_DFN));
> +    ASSERT(flush_flags);
> +
> +    /* Unless a PTE was modified, no flush is required */
> +    if ( !(flush_flags & IOMMU_FLUSHF_modified) )
> +        return 0;
> 
>      /* If the range wraps then just flush everything */
>      if ( dfn_l + page_count < dfn_l )
> @@ -695,6 +715,7 @@ int amd_iommu_reserve_domain_unity_map(struct domain
> *domain,
>      unsigned long npages, i;
>      unsigned long gfn;
>      unsigned int flags = !!ir;
> +    unsigned int flush_flags = 0;
>      int rt = 0;
> 
>      if ( iw )
> @@ -706,11 +727,19 @@ int amd_iommu_reserve_domain_unity_map(struct domain
> *domain,
>      {
>          unsigned long frame = gfn + i;
> 
> -        rt = amd_iommu_map_page(domain, _dfn(frame), _mfn(frame), flags);
> +        rt = amd_iommu_map_page(domain, _dfn(frame), _mfn(frame), flags,
> +                                &flush_flags);
>          if ( rt != 0 )
> -            return rt;
> +            break;
>      }
> -    return 0;
> +
> +    /* Use while-break to avoid compiler warning */
> +    while ( flush_flags && amd_iommu_flush_iotlb_pages(domain, _dfn(gfn),
> +                                                       npages,
> +                                                       flush_flags) )
> +        break;
> +
> +    return rt;
>  }
> 
>  /* Share p2m table with iommu. */
> diff --git a/xen/drivers/passthrough/arm/smmu.c
> b/xen/drivers/passthrough/arm/smmu.c
> index 9612c0fddc..73c8048504 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -2535,8 +2535,11 @@ static int __must_check
> arm_smmu_iotlb_flush_all(struct domain *d)
>  }
> 
>  static int __must_check arm_smmu_iotlb_flush(struct domain *d, dfn_t dfn,
> -                                             unsigned int page_count)
> +					     unsigned int page_count,
> +					     unsigned int flush_flags)
>  {
> +	ASSERT(flush_flags);
> +
>  	/* ARM SMMU v1 doesn't have flush by VMA and VMID */
>  	return arm_smmu_iotlb_flush_all(d);
>  }
> @@ -2732,7 +2735,8 @@ static void arm_smmu_iommu_domain_teardown(struct
> domain *d)
>  }
> 
>  static int __must_check arm_smmu_map_page(struct domain *d, dfn_t dfn,
> -					  mfn_t mfn, unsigned int flags)
> +					  mfn_t mfn, unsigned int flags,
> +					  unsigned int *flush_flags)
>  {
>  	p2m_type_t t;
> 
> @@ -2761,7 +2765,8 @@ static int __must_check arm_smmu_map_page(struct
> domain *d, dfn_t dfn,
>  				       0, t);
>  }
> 
> -static int __must_check arm_smmu_unmap_page(struct domain *d, dfn_t dfn)
> +static int __must_check arm_smmu_unmap_page(struct domain *d, dfn_t dfn,
> +                                            unsigned int *flush_flags)
>  {
>  	/*
>  	 * This function should only be used by gnttab code when the domain
> diff --git a/xen/drivers/passthrough/iommu.c
> b/xen/drivers/passthrough/iommu.c
> index 105995a343..caff3ab243 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -211,7 +211,7 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
>      if ( need_iommu_pt_sync(d) )
>      {
>          struct page_info *page;
> -        unsigned int i = 0;
> +        unsigned int i = 0, flush_flags = 0;
>          int rc = 0;
> 
>          page_list_for_each ( page, &d->page_list )
> @@ -226,8 +226,9 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
>                    == PGT_writable_page) )
>                  mapping |= IOMMUF_writable;
> 
> -            ret = hd->platform_ops->map_page(d, _dfn(dfn), _mfn(mfn),
> -                                             mapping);
> +            ret = iommu_map(d, _dfn(dfn), _mfn(mfn), mapping, 0,
> +                            &flush_flags);
> +
>              if ( !rc )
>                  rc = ret;
> 
> @@ -235,6 +236,10 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
>                  process_pending_softirqs();
>          }
> 
> +        /* Use while-break to avoid compiler warning */
> +        while ( iommu_iotlb_flush_all(d, flush_flags) )
> +            break;
> +
>          if ( rc )
>              printk(XENLOG_WARNING "d%d: IOMMU mapping failed: %d\n",
>                     d->domain_id, rc);
> @@ -304,8 +309,9 @@ void iommu_domain_destroy(struct domain *d)
>      arch_iommu_domain_destroy(d);
>  }
> 
> -int iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> -                     unsigned int page_order, unsigned int flags)
> +int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> +              unsigned int page_order, unsigned int flags,
> +              unsigned int *flush_flags)
>  {
>      const struct domain_iommu *hd = dom_iommu(d);
>      unsigned long i;
> @@ -319,8 +325,8 @@ int iommu_legacy_map(struct domain *d, dfn_t dfn,
> mfn_t mfn,
> 
>      for ( i = 0; i < (1ul << page_order); i++ )
>      {
> -        rc = hd->platform_ops->map_page(d, dfn_add(dfn, i),
> -                                        mfn_add(mfn, i), flags);
> +        rc = hd->platform_ops->map_page(d, dfn_add(dfn, i), mfn_add(mfn,
> i),
> +                                        flags, flush_flags);
> 
>          if ( likely(!rc) )
>              continue;
> @@ -333,7 +339,8 @@ int iommu_legacy_map(struct domain *d, dfn_t dfn,
> mfn_t mfn,
> 
>          while ( i-- )
>              /* if statement to satisfy __must_check */
> -            if ( hd->platform_ops->unmap_page(d, dfn_add(dfn, i)) )
> +            if ( hd->platform_ops->unmap_page(d, dfn_add(dfn, i),
> +                                              flush_flags) )
>                  continue;
> 
>          if ( !is_hardware_domain(d) )
> @@ -345,7 +352,26 @@ int iommu_legacy_map(struct domain *d, dfn_t dfn,
> mfn_t mfn,
>      return rc;
>  }
> 
> -int iommu_legacy_unmap(struct domain *d, dfn_t dfn, unsigned int
> page_order)
> +int iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> +                     unsigned int page_order, unsigned int flags)
> +{
> +    unsigned int flush_flags = 0;
> +    int rc = iommu_map(d, dfn, mfn, page_order, flags, &flush_flags);
> +
> +    if ( !this_cpu(iommu_dont_flush_iotlb) )
> +    {
> +        int err = iommu_iotlb_flush(d, dfn, (1u << page_order),
> +                                    flush_flags);
> +
> +        if ( !rc )
> +            rc = err;
> +    }
> +
> +    return rc;
> +}
> +
> +int iommu_unmap(struct domain *d, dfn_t dfn, unsigned int page_order,
> +                unsigned int *flush_flags)
>  {
>      const struct domain_iommu *hd = dom_iommu(d);
>      unsigned long i;
> @@ -358,7 +384,8 @@ int iommu_legacy_unmap(struct domain *d, dfn_t dfn,
> unsigned int page_order)
> 
>      for ( i = 0; i < (1ul << page_order); i++ )
>      {
> -        int err = hd->platform_ops->unmap_page(d, dfn_add(dfn, i));
> +        int err = hd->platform_ops->unmap_page(d, dfn_add(dfn, i),
> +                                               flush_flags);
> 
>          if ( likely(!err) )
>              continue;
> @@ -381,6 +408,23 @@ int iommu_legacy_unmap(struct domain *d, dfn_t dfn,
> unsigned int page_order)
>      return rc;
>  }
> 
> +int iommu_legacy_unmap(struct domain *d, dfn_t dfn, unsigned int
> page_order)
> +{
> +    unsigned int flush_flags = 0;
> +    int rc = iommu_unmap(d, dfn, page_order, &flush_flags);
> +
> +    if ( !this_cpu(iommu_dont_flush_iotlb) )
> +    {
> +        int err = iommu_iotlb_flush(d, dfn, (1u << page_order),
> +                                    flush_flags);
> +
> +        if ( !rc )
> +            rc = err;
> +    }
> +
> +    return rc;
> +}
> +
>  int iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t *mfn,
>                        unsigned int *flags)
>  {
> @@ -409,25 +453,26 @@ static void iommu_free_pagetables(unsigned long
> unused)
>                              cpumask_cycle(smp_processor_id(),
> &cpu_online_map));
>  }
> 
> -int iommu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned int
> page_count)
> +int iommu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned int
> page_count,
> +                      unsigned int flush_flags)
>  {
>      const struct domain_iommu *hd = dom_iommu(d);
>      int rc;
> 
>      if ( !iommu_enabled || !hd->platform_ops ||
> -         !hd->platform_ops->iotlb_flush || !page_count )
> +         !hd->platform_ops->iotlb_flush || !page_count || !flush_flags )
>          return 0;
> 
>      if ( dfn_eq(dfn, INVALID_DFN) )
>          return -EINVAL;
> 
> -    rc = hd->platform_ops->iotlb_flush(d, dfn, page_count);
> +    rc = hd->platform_ops->iotlb_flush(d, dfn, page_count, flush_flags);
>      if ( unlikely(rc) )
>      {
>          if ( !d->is_shutting_down && printk_ratelimit() )
>              printk(XENLOG_ERR
> -                   "d%d: IOMMU IOTLB flush failed: %d, dfn %"PRI_dfn",
> page count %u\n",
> -                   d->domain_id, rc, dfn_x(dfn), page_count);
> +                   "d%d: IOMMU IOTLB flush failed: %d, dfn %"PRI_dfn",
> page count %u flags %x\n",
> +                   d->domain_id, rc, dfn_x(dfn), page_count,
> flush_flags);
> 
>          if ( !is_hardware_domain(d) )
>              domain_crash(d);
> @@ -436,14 +481,19 @@ int iommu_iotlb_flush(struct domain *d, dfn_t dfn,
> unsigned int page_count)
>      return rc;
>  }
> 
> -int iommu_iotlb_flush_all(struct domain *d)
> +int iommu_iotlb_flush_all(struct domain *d, unsigned int flush_flags)
>  {
>      const struct domain_iommu *hd = dom_iommu(d);
>      int rc;
> 
> -    if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops-
> >iotlb_flush_all )
> +    if ( !iommu_enabled || !hd->platform_ops ||
> +         !hd->platform_ops->iotlb_flush_all || !flush_flags )
>          return 0;
> 
> +    /*
> +     * The operation does a full flush so we don't need to pass the
> +     * flush_flags in.
> +     */
>      rc = hd->platform_ops->iotlb_flush_all(d);
>      if ( unlikely(rc) )
>      {
> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> index d2fa5e2b25..50a0e25224 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -633,11 +633,14 @@ static int __must_check iommu_flush_iotlb(struct
> domain *d, dfn_t dfn,
> 
>  static int __must_check iommu_flush_iotlb_pages(struct domain *d,
>                                                  dfn_t dfn,
> -                                                unsigned int page_count)
> +                                                unsigned int page_count,
> +                                                unsigned int flush_flags)
>  {
>      ASSERT(page_count && !dfn_eq(dfn, INVALID_DFN));
> +    ASSERT(flush_flags);
> 
> -    return iommu_flush_iotlb(d, dfn, 1, page_count);
> +    return iommu_flush_iotlb(d, dfn, flush_flags & IOMMU_FLUSHF_modified,
> +                             page_count);
>  }
> 
>  static int __must_check iommu_flush_iotlb_all(struct domain *d)
> @@ -646,7 +649,8 @@ static int __must_check iommu_flush_iotlb_all(struct
> domain *d)
>  }
> 
>  /* clear one page's page table */
> -static int __must_check dma_pte_clear_one(struct domain *domain, u64
> addr)
> +static int __must_check dma_pte_clear_one(struct domain *domain, u64
> addr,
> +                                          unsigned int *flush_flags)
>  {
>      struct domain_iommu *hd = dom_iommu(domain);
>      struct dma_pte *page = NULL, *pte = NULL;
> @@ -673,12 +677,11 @@ static int __must_check dma_pte_clear_one(struct
> domain *domain, u64 addr)
>      }
> 
>      dma_clear_pte(*pte);
> +    *flush_flags |= IOMMU_FLUSHF_modified;
> +
>      spin_unlock(&hd->arch.mapping_lock);
>      iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
> 
> -    if ( !this_cpu(iommu_dont_flush_iotlb) )
> -        rc = iommu_flush_iotlb_pages(domain, daddr_to_dfn(addr), 1);
> -
>      unmap_vtd_domain_page(page);
> 
>      return rc;
> @@ -1773,9 +1776,9 @@ static void iommu_domain_teardown(struct domain *d)
>      spin_unlock(&hd->arch.mapping_lock);
>  }
> 
> -static int __must_check intel_iommu_map_page(struct domain *d,
> -                                             dfn_t dfn, mfn_t mfn,
> -                                             unsigned int flags)
> +static int __must_check intel_iommu_map_page(struct domain *d, dfn_t dfn,
> +                                             mfn_t mfn, unsigned int
> flags,
> +                                             unsigned int *flush_flags)
>  {
>      struct domain_iommu *hd = dom_iommu(d);
>      struct dma_pte *page, *pte, old, new = {};
> @@ -1825,14 +1828,15 @@ static int __must_check
> intel_iommu_map_page(struct domain *d,
>      spin_unlock(&hd->arch.mapping_lock);
>      unmap_vtd_domain_page(page);
> 
> -    if ( !this_cpu(iommu_dont_flush_iotlb) )
> -        rc = iommu_flush_iotlb(d, dfn, dma_pte_present(old), 1);
> +    *flush_flags |= IOMMU_FLUSHF_added;
> +    if ( dma_pte_present(old) )
> +        *flush_flags |= IOMMU_FLUSHF_modified;
> 
>      return rc;
>  }
> 
> -static int __must_check intel_iommu_unmap_page(struct domain *d,
> -                                               dfn_t dfn)
> +static int __must_check intel_iommu_unmap_page(struct domain *d, dfn_t
> dfn,
> +                                               unsigned int *flush_flags)
>  {
>      /* Do nothing if VT-d shares EPT page table */
>      if ( iommu_use_hap_pt(d) )
> @@ -1842,7 +1846,7 @@ static int __must_check
> intel_iommu_unmap_page(struct domain *d,
>      if ( iommu_hwdom_passthrough && is_hardware_domain(d) )
>          return 0;
> 
> -    return dma_pte_clear_one(d, dfn_to_daddr(dfn));
> +    return dma_pte_clear_one(d, dfn_to_daddr(dfn), flush_flags);
>  }
> 
>  static int intel_iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t
> *mfn,
> diff --git a/xen/drivers/passthrough/x86/iommu.c
> b/xen/drivers/passthrough/x86/iommu.c
> index b12289a18f..e40d7a7d7b 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -46,11 +46,9 @@ int __init iommu_setup_hpet_msi(struct msi_desc *msi)
> 
>  int arch_iommu_populate_page_table(struct domain *d)
>  {
> -    const struct domain_iommu *hd = dom_iommu(d);
>      struct page_info *page;
>      int rc = 0, n = 0;
> 
> -    this_cpu(iommu_dont_flush_iotlb) = 1;
>      spin_lock(&d->page_alloc_lock);
> 
>      if ( unlikely(d->is_dying) )
> @@ -63,14 +61,15 @@ int arch_iommu_populate_page_table(struct domain *d)
>          {
>              unsigned long mfn = mfn_x(page_to_mfn(page));
>              unsigned long gfn = mfn_to_gmfn(d, mfn);
> +            unsigned int flush_flags = 0;
> 
>              if ( gfn != gfn_x(INVALID_GFN) )
>              {
>                  ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
>                  BUG_ON(SHARED_M2P(gfn));
> -                rc = hd->platform_ops->map_page(d, _dfn(gfn), _mfn(mfn),
> -                                                IOMMUF_readable |
> -                                                IOMMUF_writable);
> +                rc = iommu_map(d, _dfn(gfn), _mfn(mfn), PAGE_ORDER_4K,
> +                               IOMMUF_readable | IOMMUF_writable,
> +                               &flush_flags);
>              }
>              if ( rc )
>              {
> @@ -104,10 +103,14 @@ int arch_iommu_populate_page_table(struct domain *d)
>      }
> 
>      spin_unlock(&d->page_alloc_lock);
> -    this_cpu(iommu_dont_flush_iotlb) = 0;
> 
>      if ( !rc )
> -        rc = iommu_iotlb_flush_all(d);
> +        /*
> +         * flush_flags are not tracked across hypercall pre-emption so
> +         * assume a full flush is necessary.
> +         */
> +        rc = iommu_iotlb_flush_all(
> +            d, IOMMU_FLUSHF_added | IOMMU_FLUSHF_modified);
> 
>      if ( rc && rc != -ERESTART )
>          iommu_teardown(d);
> @@ -207,6 +210,7 @@ static bool __hwdom_init hwdom_iommu_map(const struct
> domain *d,
>  void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
>  {
>      unsigned long i, top, max_pfn;
> +    unsigned int flush_flags = 0;
> 
>      BUG_ON(!is_hardware_domain(d));
> 
> @@ -241,8 +245,9 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain
> *d)
>          if ( paging_mode_translate(d) )
>              rc = set_identity_p2m_entry(d, pfn, p2m_access_rw, 0);
>          else
> -            rc = iommu_legacy_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
> -                                  IOMMUF_readable | IOMMUF_writable);
> +            rc = iommu_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
> +                           IOMMUF_readable | IOMMUF_writable,
> &flush_flags);
> +
>          if ( rc )
>              printk(XENLOG_WARNING " d%d: IOMMU mapping failed: %d\n",
>                     d->domain_id, rc);
> @@ -250,6 +255,10 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain
> *d)
>          if (!(i & 0xfffff))
>              process_pending_softirqs();
>      }
> +
> +    /* Use if to avoid compiler warning */
> +    if ( iommu_iotlb_flush_all(d, flush_flags) )
> +        return;
>  }
> 
>  /*
> diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> index 88715329ca..c5697565d6 100644
> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> @@ -53,15 +53,18 @@ int amd_iommu_update_ivrs_mapping_acpi(void);
> 
>  /* mapping functions */
>  int __must_check amd_iommu_map_page(struct domain *d, dfn_t dfn,
> -                                    mfn_t mfn, unsigned int flags);
> -int __must_check amd_iommu_unmap_page(struct domain *d, dfn_t dfn);
> +                                    mfn_t mfn, unsigned int flags,
> +                                    unsigned int *flush_flags);
> +int __must_check amd_iommu_unmap_page(struct domain *d, dfn_t dfn,
> +                                      unsigned int *flush_flags);
>  uint64_t amd_iommu_get_address_from_pte(void *entry);
>  int __must_check amd_iommu_alloc_root(struct domain_iommu *hd);
>  int amd_iommu_reserve_domain_unity_map(struct domain *domain,
>                                         paddr_t phys_addr, unsigned long
> size,
>                                         int iw, int ir);
>  int __must_check amd_iommu_flush_iotlb_pages(struct domain *d, dfn_t dfn,
> -                                             unsigned int page_count);
> +                                             unsigned int page_count,
> +                                             unsigned int flush_flags);
>  int __must_check amd_iommu_flush_iotlb_all(struct domain *d);
> 
>  /* Share p2m table with iommu */
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 1f875aa328..cdc8021cbd 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -92,6 +92,31 @@ void iommu_teardown(struct domain *d);
>  #define _IOMMUF_writable 1
>  #define IOMMUF_writable  (1u<<_IOMMUF_writable)
> 
> +/*
> + * flush_flags:
> + *
> + * IOMMU_FLUSHF_added -> A new 'present' PTE has been inserted.
> + * IOMMU_FLUSHF_modified -> An existing 'present' PTE has been modified
> + *                          (whether the new PTE value is 'present' or
> not).
> + *
> + * These flags are passed back from map/unmap operations and passed into
> + * flush operations.
> + */
> +enum
> +{
> +    _IOMMU_FLUSHF_added,
> +    _IOMMU_FLUSHF_modified,
> +};
> +#define IOMMU_FLUSHF_added (1u << _IOMMU_FLUSHF_added)
> +#define IOMMU_FLUSHF_modified (1u << _IOMMU_FLUSHF_modified)
> +
> +int __must_check iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> +                           unsigned int page_order, unsigned int flags,
> +                           unsigned int *flush_flags);
> +int __must_check iommu_unmap(struct domain *d, dfn_t dfn,
> +                             unsigned int page_order,
> +                             unsigned int *flush_flags);
> +
>  int __must_check iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
>                                    unsigned int page_order,
>                                    unsigned int flags);
> @@ -101,6 +126,12 @@ int __must_check iommu_legacy_unmap(struct domain *d,
> dfn_t dfn,
>  int __must_check iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t
> *mfn,
>                                     unsigned int *flags);
> 
> +int __must_check iommu_iotlb_flush(struct domain *d, dfn_t dfn,
> +                                   unsigned int page_count,
> +                                   unsigned int flush_flags);
> +int __must_check iommu_iotlb_flush_all(struct domain *d,
> +                                       unsigned int flush_flags);
> +
>  enum iommu_feature
>  {
>      IOMMU_FEAT_COHERENT_WALK,
> @@ -178,8 +209,10 @@ struct iommu_ops {
>       * other by the caller in order to have meaningful results.
>       */
>      int __must_check (*map_page)(struct domain *d, dfn_t dfn, mfn_t mfn,
> -                                 unsigned int flags);
> -    int __must_check (*unmap_page)(struct domain *d, dfn_t dfn);
> +                                 unsigned int flags,
> +                                 unsigned int *flush_flags);
> +    int __must_check (*unmap_page)(struct domain *d, dfn_t dfn,
> +                                   unsigned int *flush_flags);
>      int __must_check (*lookup_page)(struct domain *d, dfn_t dfn, mfn_t
> *mfn,
>                                      unsigned int *flags);
> 
> @@ -194,7 +227,8 @@ struct iommu_ops {
>      void (*share_p2m)(struct domain *d);
>      void (*crash_shutdown)(void);
>      int __must_check (*iotlb_flush)(struct domain *d, dfn_t dfn,
> -                                    unsigned int page_count);
> +                                    unsigned int page_count,
> +                                    unsigned int flush_flags);
>      int __must_check (*iotlb_flush_all)(struct domain *d);
>      int (*get_reserved_device_memory)(iommu_grdm_t *, void *);
>      void (*dump_p2m_table)(struct domain *d);
> @@ -253,10 +287,6 @@ int iommu_do_pci_domctl(struct xen_domctl *, struct
> domain *d,
>  int iommu_do_domctl(struct xen_domctl *, struct domain *d,
>                      XEN_GUEST_HANDLE_PARAM(xen_domctl_t));
> 
> -int __must_check iommu_iotlb_flush(struct domain *d, dfn_t dfn,
> -                                   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);
> 
>  /*
> --
> 2.11.0

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

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

* Re: [PATCH v5 2/4] iommu: rename wrapper functions
  2018-12-17  9:22 ` [PATCH v5 2/4] iommu: rename wrapper functions Paul Durrant
@ 2018-12-19  9:05   ` Paul Durrant
  2018-12-19 15:39     ` Julien Grall
  2018-12-19 16:23     ` Jan Beulich
  2018-12-25  2:29   ` Tian, Kevin
  1 sibling, 2 replies; 16+ messages in thread
From: Paul Durrant @ 2018-12-19  9:05 UTC (permalink / raw)
  To: Paul Durrant, xen-devel
  Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	Andrew Cooper, Tim (Xen.org),
	George Dunlap, Julien Grall, Jun Nakajima, Ian Jackson,
	Roger Pau Monne

Julien, ping?

> -----Original Message-----
> From: Paul Durrant [mailto:paul.durrant@citrix.com]
> Sent: 17 December 2018 09:23
> To: xen-devel@lists.xenproject.org
> Cc: Paul Durrant <Paul.Durrant@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Roger Pau
> Monne <roger.pau@citrix.com>; George Dunlap <George.Dunlap@citrix.com>;
> Ian Jackson <Ian.Jackson@citrix.com>; Julien Grall <julien.grall@arm.com>;
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Tim (Xen.org) <tim@xen.org>; Jun Nakajima
> <jun.nakajima@intel.com>; Kevin Tian <kevin.tian@intel.com>; George Dunlap
> <George.Dunlap@citrix.com>
> Subject: [PATCH v5 2/4] iommu: rename wrapper functions
> 
> A subsequent patch will add semantically different versions of
> iommu_map/unmap() so, in advance of that change, this patch renames the
> existing functions to iommu_legacy_map/unmap() and modifies all call-
> sites.
> It also adjusts a comment that refers to iommu_map_page(), which was re-
> named by a previous patch.
> 
> This patch is purely cosmetic. No functional change.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>A
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> 
> v2:
>  - New in v2.
> 
> v3:
>  - Leave iommu_iotlb_flush[_all] alone.
>  - Make patch purely cosmetic.
>  - Fix comment in xen/iommu.h.
> ---
>  xen/arch/x86/mm.c                   | 11 ++++++-----
>  xen/arch/x86/mm/p2m-ept.c           |  4 ++--
>  xen/arch/x86/mm/p2m-pt.c            |  5 +++--
>  xen/arch/x86/mm/p2m.c               | 12 ++++++------
>  xen/arch/x86/x86_64/mm.c            |  9 +++++----
>  xen/common/grant_table.c            | 14 +++++++-------
>  xen/common/memory.c                 |  4 ++--
>  xen/drivers/passthrough/iommu.c     |  6 +++---
>  xen/drivers/passthrough/x86/iommu.c |  4 ++--
>  xen/include/xen/iommu.h             | 16 +++++++++++-----
>  10 files changed, 47 insertions(+), 38 deletions(-)
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 1431f347f3..0ad3940bb3 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -2801,12 +2801,13 @@ static int _get_page_type(struct page_info *page,
> unsigned long type,
>              mfn_t mfn = page_to_mfn(page);
> 
>              if ( (x & PGT_type_mask) == PGT_writable_page )
> -                iommu_ret = iommu_unmap(d, _dfn(mfn_x(mfn)),
> -                                        PAGE_ORDER_4K);
> +                iommu_ret = iommu_legacy_unmap(d, _dfn(mfn_x(mfn)),
> +                                               PAGE_ORDER_4K);
>              else if ( type == PGT_writable_page )
> -                iommu_ret = iommu_map(d, _dfn(mfn_x(mfn)), mfn,
> -                                      PAGE_ORDER_4K,
> -                                      IOMMUF_readable | IOMMUF_writable);
> +                iommu_ret = iommu_legacy_map(d, _dfn(mfn_x(mfn)), mfn,
> +                                             PAGE_ORDER_4K,
> +                                             IOMMUF_readable |
> +                                             IOMMUF_writable);
>          }
>      }
> 
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index 6e4e375bad..64a49c07b7 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -882,8 +882,8 @@ out:
>              rc = iommu_pte_flush(d, gfn, &ept_entry->epte, order,
> vtd_pte_present);
>          else if ( need_iommu_pt_sync(d) )
>              rc = iommu_flags ?
> -                iommu_map(d, _dfn(gfn), mfn, order, iommu_flags) :
> -                iommu_unmap(d, _dfn(gfn), order);
> +                iommu_legacy_map(d, _dfn(gfn), mfn, order, iommu_flags) :
> +                iommu_legacy_unmap(d, _dfn(gfn), order);
>      }
> 
>      unmap_domain_page(table);
> diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
> index 17a6b61f12..69ffb08179 100644
> --- a/xen/arch/x86/mm/p2m-pt.c
> +++ b/xen/arch/x86/mm/p2m-pt.c
> @@ -686,8 +686,9 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_,
> mfn_t mfn,
> 
>          if ( need_iommu_pt_sync(p2m->domain) )
>              rc = iommu_pte_flags ?
> -                iommu_map(d, _dfn(gfn), mfn, page_order, iommu_pte_flags)
> :
> -                iommu_unmap(d, _dfn(gfn), page_order);
> +                iommu_legacy_map(d, _dfn(gfn), mfn, page_order,
> +                                 iommu_pte_flags) :
> +                iommu_legacy_unmap(d, _dfn(gfn), page_order);
>          else if ( iommu_use_hap_pt(d) && iommu_old_flags )
>              amd_iommu_flush_pages(p2m->domain, gfn, page_order);
>      }
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index fea4497910..ed76e96d33 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -733,7 +733,7 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long
> gfn_l, unsigned long mfn,
> 
>      if ( !paging_mode_translate(p2m->domain) )
>          return need_iommu_pt_sync(p2m->domain) ?
> -            iommu_unmap(p2m->domain, _dfn(mfn), page_order) : 0;
> +            iommu_legacy_unmap(p2m->domain, _dfn(mfn), page_order) : 0;
> 
>      ASSERT(gfn_locked_by_me(p2m, gfn));
>      P2M_DEBUG("removing gfn=%#lx mfn=%#lx\n", gfn_l, mfn);
> @@ -780,8 +780,8 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn,
> mfn_t mfn,
> 
>      if ( !paging_mode_translate(d) )
>          return (need_iommu_pt_sync(d) && t == p2m_ram_rw) ?
> -            iommu_map(d, _dfn(mfn_x(mfn)), mfn, page_order,
> -                      IOMMUF_readable | IOMMUF_writable) : 0;
> +            iommu_legacy_map(d, _dfn(mfn_x(mfn)), mfn, page_order,
> +                             IOMMUF_readable | IOMMUF_writable) : 0;
> 
>      /* foreign pages are added thru p2m_add_foreign */
>      if ( p2m_is_foreign(t) )
> @@ -1151,8 +1151,8 @@ int set_identity_p2m_entry(struct domain *d,
> unsigned long gfn_l,
>      {
>          if ( !need_iommu_pt_sync(d) )
>              return 0;
> -        return iommu_map(d, _dfn(gfn_l), _mfn(gfn_l), PAGE_ORDER_4K,
> -                         IOMMUF_readable | IOMMUF_writable);
> +        return iommu_legacy_map(d, _dfn(gfn_l), _mfn(gfn_l),
> PAGE_ORDER_4K,
> +                                IOMMUF_readable | IOMMUF_writable);
>      }
> 
>      gfn_lock(p2m, gfn, 0);
> @@ -1242,7 +1242,7 @@ int clear_identity_p2m_entry(struct domain *d,
> unsigned long gfn_l)
>      {
>          if ( !need_iommu_pt_sync(d) )
>              return 0;
> -        return iommu_unmap(d, _dfn(gfn_l), PAGE_ORDER_4K);
> +        return iommu_legacy_unmap(d, _dfn(gfn_l), PAGE_ORDER_4K);
>      }
> 
>      gfn_lock(p2m, gfn, 0);
> diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
> index 126a486d2e..d8f558bc3a 100644
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -1436,15 +1436,16 @@ int memory_add(unsigned long spfn, unsigned long
> epfn, unsigned int pxm)
>           !need_iommu_pt_sync(hardware_domain) )
>      {
>          for ( i = spfn; i < epfn; i++ )
> -            if ( iommu_map(hardware_domain, _dfn(i), _mfn(i),
> -                           PAGE_ORDER_4K,
> -                           IOMMUF_readable | IOMMUF_writable) )
> +            if ( iommu_legacy_map(hardware_domain, _dfn(i), _mfn(i),
> +                                  PAGE_ORDER_4K,
> +                                  IOMMUF_readable | IOMMUF_writable) )
>                  break;
>          if ( i != epfn )
>          {
>              while (i-- > old_max)
>                  /* If statement to satisfy __must_check. */
> -                if ( iommu_unmap(hardware_domain, _dfn(i), PAGE_ORDER_4K)
> )
> +                if ( iommu_legacy_unmap(hardware_domain, _dfn(i),
> +                                        PAGE_ORDER_4K) )
>                      continue;
> 
>              goto destroy_m2p;
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index b67ae9e3f5..fd099a8f25 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1134,14 +1134,14 @@ map_grant_ref(
>               !(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
>          {
>              if ( !(kind & MAPKIND_WRITE) )
> -                err = iommu_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
> -                                IOMMUF_readable | IOMMUF_writable);
> +                err = iommu_legacy_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
> +                                       IOMMUF_readable |
> IOMMUF_writable);
>          }
>          else if ( act_pin && !old_pin )
>          {
>              if ( !kind )
> -                err = iommu_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
> -                                IOMMUF_readable);
> +                err = iommu_legacy_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
> +                                       IOMMUF_readable);
>          }
>          if ( err )
>          {
> @@ -1389,10 +1389,10 @@ unmap_common(
> 
>          kind = mapkind(lgt, rd, op->mfn);
>          if ( !kind )
> -            err = iommu_unmap(ld, _dfn(mfn_x(op->mfn)), 0);
> +            err = iommu_legacy_unmap(ld, _dfn(mfn_x(op->mfn)), 0);
>          else if ( !(kind & MAPKIND_WRITE) )
> -            err = iommu_map(ld, _dfn(mfn_x(op->mfn)), op->mfn, 0,
> -                            IOMMUF_readable);
> +            err = iommu_legacy_map(ld, _dfn(mfn_x(op->mfn)), op->mfn, 0,
> +                                   IOMMUF_readable);
> 
>          double_gt_unlock(lgt, rgt);
> 
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index 5f7d081c61..f37eb288d4 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -853,11 +853,11 @@ int xenmem_add_to_physmap(struct domain *d, struct
> xen_add_to_physmap *xatp,
> 
>          this_cpu(iommu_dont_flush_iotlb) = 0;
> 
> -        ret = iommu_iotlb_flush(d, _dfn(xatp->idx - done), done);
> +        ret = iommu_flush(d, _dfn(xatp->idx - done), done);
>          if ( unlikely(ret) && rc >= 0 )
>              rc = ret;
> 
> -        ret = iommu_iotlb_flush(d, _dfn(xatp->gpfn - done), done);
> +        ret = iommu_flush(d, _dfn(xatp->gpfn - done), done);
>          if ( unlikely(ret) && rc >= 0 )
>              rc = ret;
>      }
> diff --git a/xen/drivers/passthrough/iommu.c
> b/xen/drivers/passthrough/iommu.c
> index c1cce08551..105995a343 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -304,8 +304,8 @@ void iommu_domain_destroy(struct domain *d)
>      arch_iommu_domain_destroy(d);
>  }
> 
> -int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> -              unsigned int page_order, unsigned int flags)
> +int iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> +                     unsigned int page_order, unsigned int flags)
>  {
>      const struct domain_iommu *hd = dom_iommu(d);
>      unsigned long i;
> @@ -345,7 +345,7 @@ int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
>      return rc;
>  }
> 
> -int iommu_unmap(struct domain *d, dfn_t dfn, unsigned int page_order)
> +int iommu_legacy_unmap(struct domain *d, dfn_t dfn, unsigned int
> page_order)
>  {
>      const struct domain_iommu *hd = dom_iommu(d);
>      unsigned long i;
> diff --git a/xen/drivers/passthrough/x86/iommu.c
> b/xen/drivers/passthrough/x86/iommu.c
> index c68a72279d..b12289a18f 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -241,8 +241,8 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain
> *d)
>          if ( paging_mode_translate(d) )
>              rc = set_identity_p2m_entry(d, pfn, p2m_access_rw, 0);
>          else
> -            rc = iommu_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
> -                           IOMMUF_readable | IOMMUF_writable);
> +            rc = iommu_legacy_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
> +                                  IOMMUF_readable | IOMMUF_writable);
>          if ( rc )
>              printk(XENLOG_WARNING " d%d: IOMMU mapping failed: %d\n",
>                     d->domain_id, rc);
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 3d78126801..1f875aa328 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -83,15 +83,21 @@ int iommu_construct(struct domain *d);
>  /* Function used internally, use iommu_domain_destroy */
>  void iommu_teardown(struct domain *d);
> 
> -/* iommu_map_page() takes flags to direct the mapping operation. */
> +/*
> + * The following flags are passed to map operations and passed by lookup
> + * operations.
> + */
>  #define _IOMMUF_readable 0
>  #define IOMMUF_readable  (1u<<_IOMMUF_readable)
>  #define _IOMMUF_writable 1
>  #define IOMMUF_writable  (1u<<_IOMMUF_writable)
> -int __must_check iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> -                           unsigned int page_order, unsigned int flags);
> -int __must_check iommu_unmap(struct domain *d, dfn_t dfn,
> -                             unsigned int page_order);
> +
> +int __must_check iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> +                                  unsigned int page_order,
> +                                  unsigned int flags);
> +int __must_check iommu_legacy_unmap(struct domain *d, dfn_t dfn,
> +                                    unsigned int page_order);
> +
>  int __must_check iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t
> *mfn,
>                                     unsigned int *flags);
> 
> --
> 2.11.0

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

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

* Re: [PATCH v5 2/4] iommu: rename wrapper functions
  2018-12-19  9:05   ` Paul Durrant
@ 2018-12-19 15:39     ` Julien Grall
  2018-12-19 16:23     ` Jan Beulich
  1 sibling, 0 replies; 16+ messages in thread
From: Julien Grall @ 2018-12-19 15:39 UTC (permalink / raw)
  To: Paul Durrant, xen-devel
  Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	Andrew Cooper, Tim (Xen.org),
	George Dunlap, Jun Nakajima, Ian Jackson, Roger Pau Monne

Hi,

On 19/12/2018 09:05, Paul Durrant wrote:
> Julien, ping?

My ack should not be necessary for this one :). Anyway:

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

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

* Re: [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations
  2018-12-17  9:22 ` [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations Paul Durrant
  2018-12-19  9:05   ` Paul Durrant
@ 2018-12-19 15:43   ` Julien Grall
  2018-12-20 19:27   ` Woods, Brian
  2 siblings, 0 replies; 16+ messages in thread
From: Julien Grall @ 2018-12-19 15:43 UTC (permalink / raw)
  To: Paul Durrant, xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Tim Deegan, Suravee Suthikulpanit,
	Brian Woods, Ian Jackson, Roger Pau Monné

Hi Paul,

On 17/12/2018 09:22, Paul Durrant wrote:
> This patch removes any implicit flushing that occurs in the implementation
> of map and unmap operations and adds new iommu_map/unmap() wrapper
> functions. To maintain semantics of the iommu_legacy_map/unmap() wrapper
> functions, these are modified to call the new wrapper functions and then
> perform an explicit flush operation.
> 
> Because VT-d currently performs two different types of flush dependent upon
> whether a PTE is being modified versus merely added (i.e. replacing a non-
> present PTE) 'iommu flush flags' are defined by this patch and the
> iommu_ops map_page() and unmap_page() methods are modified to OR the type
> of flush necessary for the PTE that has been populated or depopulated into
> an accumulated flags value. The accumulated value can then be passed into
> the explicit flush operation.
> 
> The ARM SMMU implementations of map_page() and unmap_page() currently
> perform no implicit flushing and therefore the modified methods do not
> adjust the flush flags.
> 
> NOTE: The per-cpu 'iommu_dont_flush_iotlb' is respected by the
>        iommu_legacy_map/unmap() wrapper functions and therefore this now
>        applies to all IOMMU implementations rather than just VT-d.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

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

* Re: [PATCH v5 2/4] iommu: rename wrapper functions
  2018-12-19  9:05   ` Paul Durrant
  2018-12-19 15:39     ` Julien Grall
@ 2018-12-19 16:23     ` Jan Beulich
  2018-12-19 16:26       ` Paul Durrant
  1 sibling, 1 reply; 16+ messages in thread
From: Jan Beulich @ 2018-12-19 16:23 UTC (permalink / raw)
  To: Paul Durrant
  Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	Andrew Cooper, Tim Deegan, george.dunlap, Julien Grall,
	Jun Nakajima, xen-devel, Ian Jackson, Roger Pau Monne

>>> On 19.12.18 at 10:05, <Paul.Durrant@citrix.com> wrote:
> Julien, ping?

Seeing Julien's reply, did you perhaps mean to ping George instead
(the minimal change to p2m-ept.c probably doesn't need a
separate EPT maintainer ack)?

Jan

>> -----Original Message-----
>> From: Paul Durrant [mailto:paul.durrant@citrix.com]
>> Sent: 17 December 2018 09:23
>> To: xen-devel@lists.xenproject.org 
>> Cc: Paul Durrant <Paul.Durrant@citrix.com>; Andrew Cooper
>> <Andrew.Cooper3@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Roger Pau
>> Monne <roger.pau@citrix.com>; George Dunlap <George.Dunlap@citrix.com>;
>> Ian Jackson <Ian.Jackson@citrix.com>; Julien Grall <julien.grall@arm.com>;
>> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini
>> <sstabellini@kernel.org>; Tim (Xen.org) <tim@xen.org>; Jun Nakajima
>> <jun.nakajima@intel.com>; Kevin Tian <kevin.tian@intel.com>; George Dunlap
>> <George.Dunlap@citrix.com>
>> Subject: [PATCH v5 2/4] iommu: rename wrapper functions
>> 
>> A subsequent patch will add semantically different versions of
>> iommu_map/unmap() so, in advance of that change, this patch renames the
>> existing functions to iommu_legacy_map/unmap() and modifies all call-
>> sites.
>> It also adjusts a comment that refers to iommu_map_page(), which was re-
>> named by a previous patch.
>> 
>> This patch is purely cosmetic. No functional change.
>> 
>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>> Acked-by: Jan Beulich <jbeulich@suse.com>A
>> ---
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: Wei Liu <wei.liu2@citrix.com>
>> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
>> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Julien Grall <julien.grall@arm.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Tim Deegan <tim@xen.org>
>> Cc: Jun Nakajima <jun.nakajima@intel.com>
>> Cc: Kevin Tian <kevin.tian@intel.com>
>> Cc: George Dunlap <george.dunlap@eu.citrix.com>
>> 
>> v2:
>>  - New in v2.
>> 
>> v3:
>>  - Leave iommu_iotlb_flush[_all] alone.
>>  - Make patch purely cosmetic.
>>  - Fix comment in xen/iommu.h.
>> ---
>>  xen/arch/x86/mm.c                   | 11 ++++++-----
>>  xen/arch/x86/mm/p2m-ept.c           |  4 ++--
>>  xen/arch/x86/mm/p2m-pt.c            |  5 +++--
>>  xen/arch/x86/mm/p2m.c               | 12 ++++++------
>>  xen/arch/x86/x86_64/mm.c            |  9 +++++----
>>  xen/common/grant_table.c            | 14 +++++++-------
>>  xen/common/memory.c                 |  4 ++--
>>  xen/drivers/passthrough/iommu.c     |  6 +++---
>>  xen/drivers/passthrough/x86/iommu.c |  4 ++--
>>  xen/include/xen/iommu.h             | 16 +++++++++++-----
>>  10 files changed, 47 insertions(+), 38 deletions(-)
>> 
>> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
>> index 1431f347f3..0ad3940bb3 100644
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -2801,12 +2801,13 @@ static int _get_page_type(struct page_info *page,
>> unsigned long type,
>>              mfn_t mfn = page_to_mfn(page);
>> 
>>              if ( (x & PGT_type_mask) == PGT_writable_page )
>> -                iommu_ret = iommu_unmap(d, _dfn(mfn_x(mfn)),
>> -                                        PAGE_ORDER_4K);
>> +                iommu_ret = iommu_legacy_unmap(d, _dfn(mfn_x(mfn)),
>> +                                               PAGE_ORDER_4K);
>>              else if ( type == PGT_writable_page )
>> -                iommu_ret = iommu_map(d, _dfn(mfn_x(mfn)), mfn,
>> -                                      PAGE_ORDER_4K,
>> -                                      IOMMUF_readable | IOMMUF_writable);
>> +                iommu_ret = iommu_legacy_map(d, _dfn(mfn_x(mfn)), mfn,
>> +                                             PAGE_ORDER_4K,
>> +                                             IOMMUF_readable |
>> +                                             IOMMUF_writable);
>>          }
>>      }
>> 
>> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
>> index 6e4e375bad..64a49c07b7 100644
>> --- a/xen/arch/x86/mm/p2m-ept.c
>> +++ b/xen/arch/x86/mm/p2m-ept.c
>> @@ -882,8 +882,8 @@ out:
>>              rc = iommu_pte_flush(d, gfn, &ept_entry->epte, order,
>> vtd_pte_present);
>>          else if ( need_iommu_pt_sync(d) )
>>              rc = iommu_flags ?
>> -                iommu_map(d, _dfn(gfn), mfn, order, iommu_flags) :
>> -                iommu_unmap(d, _dfn(gfn), order);
>> +                iommu_legacy_map(d, _dfn(gfn), mfn, order, iommu_flags) :
>> +                iommu_legacy_unmap(d, _dfn(gfn), order);
>>      }
>> 
>>      unmap_domain_page(table);
>> diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
>> index 17a6b61f12..69ffb08179 100644
>> --- a/xen/arch/x86/mm/p2m-pt.c
>> +++ b/xen/arch/x86/mm/p2m-pt.c
>> @@ -686,8 +686,9 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_,
>> mfn_t mfn,
>> 
>>          if ( need_iommu_pt_sync(p2m->domain) )
>>              rc = iommu_pte_flags ?
>> -                iommu_map(d, _dfn(gfn), mfn, page_order, iommu_pte_flags)
>> :
>> -                iommu_unmap(d, _dfn(gfn), page_order);
>> +                iommu_legacy_map(d, _dfn(gfn), mfn, page_order,
>> +                                 iommu_pte_flags) :
>> +                iommu_legacy_unmap(d, _dfn(gfn), page_order);
>>          else if ( iommu_use_hap_pt(d) && iommu_old_flags )
>>              amd_iommu_flush_pages(p2m->domain, gfn, page_order);
>>      }
>> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
>> index fea4497910..ed76e96d33 100644
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -733,7 +733,7 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long
>> gfn_l, unsigned long mfn,
>> 
>>      if ( !paging_mode_translate(p2m->domain) )
>>          return need_iommu_pt_sync(p2m->domain) ?
>> -            iommu_unmap(p2m->domain, _dfn(mfn), page_order) : 0;
>> +            iommu_legacy_unmap(p2m->domain, _dfn(mfn), page_order) : 0;
>> 
>>      ASSERT(gfn_locked_by_me(p2m, gfn));
>>      P2M_DEBUG("removing gfn=%#lx mfn=%#lx\n", gfn_l, mfn);
>> @@ -780,8 +780,8 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn,
>> mfn_t mfn,
>> 
>>      if ( !paging_mode_translate(d) )
>>          return (need_iommu_pt_sync(d) && t == p2m_ram_rw) ?
>> -            iommu_map(d, _dfn(mfn_x(mfn)), mfn, page_order,
>> -                      IOMMUF_readable | IOMMUF_writable) : 0;
>> +            iommu_legacy_map(d, _dfn(mfn_x(mfn)), mfn, page_order,
>> +                             IOMMUF_readable | IOMMUF_writable) : 0;
>> 
>>      /* foreign pages are added thru p2m_add_foreign */
>>      if ( p2m_is_foreign(t) )
>> @@ -1151,8 +1151,8 @@ int set_identity_p2m_entry(struct domain *d,
>> unsigned long gfn_l,
>>      {
>>          if ( !need_iommu_pt_sync(d) )
>>              return 0;
>> -        return iommu_map(d, _dfn(gfn_l), _mfn(gfn_l), PAGE_ORDER_4K,
>> -                         IOMMUF_readable | IOMMUF_writable);
>> +        return iommu_legacy_map(d, _dfn(gfn_l), _mfn(gfn_l),
>> PAGE_ORDER_4K,
>> +                                IOMMUF_readable | IOMMUF_writable);
>>      }
>> 
>>      gfn_lock(p2m, gfn, 0);
>> @@ -1242,7 +1242,7 @@ int clear_identity_p2m_entry(struct domain *d,
>> unsigned long gfn_l)
>>      {
>>          if ( !need_iommu_pt_sync(d) )
>>              return 0;
>> -        return iommu_unmap(d, _dfn(gfn_l), PAGE_ORDER_4K);
>> +        return iommu_legacy_unmap(d, _dfn(gfn_l), PAGE_ORDER_4K);
>>      }
>> 
>>      gfn_lock(p2m, gfn, 0);
>> diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
>> index 126a486d2e..d8f558bc3a 100644
>> --- a/xen/arch/x86/x86_64/mm.c
>> +++ b/xen/arch/x86/x86_64/mm.c
>> @@ -1436,15 +1436,16 @@ int memory_add(unsigned long spfn, unsigned long
>> epfn, unsigned int pxm)
>>           !need_iommu_pt_sync(hardware_domain) )
>>      {
>>          for ( i = spfn; i < epfn; i++ )
>> -            if ( iommu_map(hardware_domain, _dfn(i), _mfn(i),
>> -                           PAGE_ORDER_4K,
>> -                           IOMMUF_readable | IOMMUF_writable) )
>> +            if ( iommu_legacy_map(hardware_domain, _dfn(i), _mfn(i),
>> +                                  PAGE_ORDER_4K,
>> +                                  IOMMUF_readable | IOMMUF_writable) )
>>                  break;
>>          if ( i != epfn )
>>          {
>>              while (i-- > old_max)
>>                  /* If statement to satisfy __must_check. */
>> -                if ( iommu_unmap(hardware_domain, _dfn(i), PAGE_ORDER_4K)
>> )
>> +                if ( iommu_legacy_unmap(hardware_domain, _dfn(i),
>> +                                        PAGE_ORDER_4K) )
>>                      continue;
>> 
>>              goto destroy_m2p;
>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
>> index b67ae9e3f5..fd099a8f25 100644
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -1134,14 +1134,14 @@ map_grant_ref(
>>               !(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
>>          {
>>              if ( !(kind & MAPKIND_WRITE) )
>> -                err = iommu_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
>> -                                IOMMUF_readable | IOMMUF_writable);
>> +                err = iommu_legacy_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
>> +                                       IOMMUF_readable |
>> IOMMUF_writable);
>>          }
>>          else if ( act_pin && !old_pin )
>>          {
>>              if ( !kind )
>> -                err = iommu_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
>> -                                IOMMUF_readable);
>> +                err = iommu_legacy_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
>> +                                       IOMMUF_readable);
>>          }
>>          if ( err )
>>          {
>> @@ -1389,10 +1389,10 @@ unmap_common(
>> 
>>          kind = mapkind(lgt, rd, op->mfn);
>>          if ( !kind )
>> -            err = iommu_unmap(ld, _dfn(mfn_x(op->mfn)), 0);
>> +            err = iommu_legacy_unmap(ld, _dfn(mfn_x(op->mfn)), 0);
>>          else if ( !(kind & MAPKIND_WRITE) )
>> -            err = iommu_map(ld, _dfn(mfn_x(op->mfn)), op->mfn, 0,
>> -                            IOMMUF_readable);
>> +            err = iommu_legacy_map(ld, _dfn(mfn_x(op->mfn)), op->mfn, 0,
>> +                                   IOMMUF_readable);
>> 
>>          double_gt_unlock(lgt, rgt);
>> 
>> diff --git a/xen/common/memory.c b/xen/common/memory.c
>> index 5f7d081c61..f37eb288d4 100644
>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -853,11 +853,11 @@ int xenmem_add_to_physmap(struct domain *d, struct
>> xen_add_to_physmap *xatp,
>> 
>>          this_cpu(iommu_dont_flush_iotlb) = 0;
>> 
>> -        ret = iommu_iotlb_flush(d, _dfn(xatp->idx - done), done);
>> +        ret = iommu_flush(d, _dfn(xatp->idx - done), done);
>>          if ( unlikely(ret) && rc >= 0 )
>>              rc = ret;
>> 
>> -        ret = iommu_iotlb_flush(d, _dfn(xatp->gpfn - done), done);
>> +        ret = iommu_flush(d, _dfn(xatp->gpfn - done), done);
>>          if ( unlikely(ret) && rc >= 0 )
>>              rc = ret;
>>      }
>> diff --git a/xen/drivers/passthrough/iommu.c
>> b/xen/drivers/passthrough/iommu.c
>> index c1cce08551..105995a343 100644
>> --- a/xen/drivers/passthrough/iommu.c
>> +++ b/xen/drivers/passthrough/iommu.c
>> @@ -304,8 +304,8 @@ void iommu_domain_destroy(struct domain *d)
>>      arch_iommu_domain_destroy(d);
>>  }
>> 
>> -int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
>> -              unsigned int page_order, unsigned int flags)
>> +int iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
>> +                     unsigned int page_order, unsigned int flags)
>>  {
>>      const struct domain_iommu *hd = dom_iommu(d);
>>      unsigned long i;
>> @@ -345,7 +345,7 @@ int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
>>      return rc;
>>  }
>> 
>> -int iommu_unmap(struct domain *d, dfn_t dfn, unsigned int page_order)
>> +int iommu_legacy_unmap(struct domain *d, dfn_t dfn, unsigned int
>> page_order)
>>  {
>>      const struct domain_iommu *hd = dom_iommu(d);
>>      unsigned long i;
>> diff --git a/xen/drivers/passthrough/x86/iommu.c
>> b/xen/drivers/passthrough/x86/iommu.c
>> index c68a72279d..b12289a18f 100644
>> --- a/xen/drivers/passthrough/x86/iommu.c
>> +++ b/xen/drivers/passthrough/x86/iommu.c
>> @@ -241,8 +241,8 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain
>> *d)
>>          if ( paging_mode_translate(d) )
>>              rc = set_identity_p2m_entry(d, pfn, p2m_access_rw, 0);
>>          else
>> -            rc = iommu_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
>> -                           IOMMUF_readable | IOMMUF_writable);
>> +            rc = iommu_legacy_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
>> +                                  IOMMUF_readable | IOMMUF_writable);
>>          if ( rc )
>>              printk(XENLOG_WARNING " d%d: IOMMU mapping failed: %d\n",
>>                     d->domain_id, rc);
>> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
>> index 3d78126801..1f875aa328 100644
>> --- a/xen/include/xen/iommu.h
>> +++ b/xen/include/xen/iommu.h
>> @@ -83,15 +83,21 @@ int iommu_construct(struct domain *d);
>>  /* Function used internally, use iommu_domain_destroy */
>>  void iommu_teardown(struct domain *d);
>> 
>> -/* iommu_map_page() takes flags to direct the mapping operation. */
>> +/*
>> + * The following flags are passed to map operations and passed by lookup
>> + * operations.
>> + */
>>  #define _IOMMUF_readable 0
>>  #define IOMMUF_readable  (1u<<_IOMMUF_readable)
>>  #define _IOMMUF_writable 1
>>  #define IOMMUF_writable  (1u<<_IOMMUF_writable)
>> -int __must_check iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
>> -                           unsigned int page_order, unsigned int flags);
>> -int __must_check iommu_unmap(struct domain *d, dfn_t dfn,
>> -                             unsigned int page_order);
>> +
>> +int __must_check iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
>> +                                  unsigned int page_order,
>> +                                  unsigned int flags);
>> +int __must_check iommu_legacy_unmap(struct domain *d, dfn_t dfn,
>> +                                    unsigned int page_order);
>> +
>>  int __must_check iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t
>> *mfn,
>>                                     unsigned int *flags);
>> 
>> --
>> 2.11.0
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org 
> https://lists.xenproject.org/mailman/listinfo/xen-devel 




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

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

* Re: [PATCH v5 2/4] iommu: rename wrapper functions
  2018-12-19 16:23     ` Jan Beulich
@ 2018-12-19 16:26       ` Paul Durrant
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Durrant @ 2018-12-19 16:26 UTC (permalink / raw)
  To: 'Jan Beulich', George Dunlap
  Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	Andrew Cooper, Tim (Xen.org),
	Julien Grall, Jun Nakajima, xen-devel, Ian Jackson,
	Roger Pau Monne

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 19 December 2018 16:24
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; George Dunlap <George.Dunlap@citrix.com>; Ian
> Jackson <Ian.Jackson@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>;
> Wei Liu <wei.liu2@citrix.com>; Jun Nakajima <jun.nakajima@intel.com>;
> Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini
> <sstabellini@kernel.org>; xen-devel <xen-devel@lists.xenproject.org>;
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Tim (Xen.org)
> <tim@xen.org>
> Subject: Re: [Xen-devel] [PATCH v5 2/4] iommu: rename wrapper functions
> 
> >>> On 19.12.18 at 10:05, <Paul.Durrant@citrix.com> wrote:
> > Julien, ping?
> 
> Seeing Julien's reply, did you perhaps mean to ping George instead
> (the minimal change to p2m-ept.c probably doesn't need a
> separate EPT maintainer ack)?

No, I mistakenly remembered this patch touching ARM code. But, yes I guess it does need an ack from George because of the memory hunks.

  Paul

> 
> Jan
> 
> >> -----Original Message-----
> >> From: Paul Durrant [mailto:paul.durrant@citrix.com]
> >> Sent: 17 December 2018 09:23
> >> To: xen-devel@lists.xenproject.org
> >> Cc: Paul Durrant <Paul.Durrant@citrix.com>; Andrew Cooper
> >> <Andrew.Cooper3@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Roger Pau
> >> Monne <roger.pau@citrix.com>; George Dunlap <George.Dunlap@citrix.com>;
> >> Ian Jackson <Ian.Jackson@citrix.com>; Julien Grall
> <julien.grall@arm.com>;
> >> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini
> >> <sstabellini@kernel.org>; Tim (Xen.org) <tim@xen.org>; Jun Nakajima
> >> <jun.nakajima@intel.com>; Kevin Tian <kevin.tian@intel.com>; George
> Dunlap
> >> <George.Dunlap@citrix.com>
> >> Subject: [PATCH v5 2/4] iommu: rename wrapper functions
> >>
> >> A subsequent patch will add semantically different versions of
> >> iommu_map/unmap() so, in advance of that change, this patch renames the
> >> existing functions to iommu_legacy_map/unmap() and modifies all call-
> >> sites.
> >> It also adjusts a comment that refers to iommu_map_page(), which was
> re-
> >> named by a previous patch.
> >>
> >> This patch is purely cosmetic. No functional change.
> >>
> >> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> >> Acked-by: Jan Beulich <jbeulich@suse.com>A
> >> ---
> >> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> >> Cc: Wei Liu <wei.liu2@citrix.com>
> >> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> >> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> >> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> >> Cc: Julien Grall <julien.grall@arm.com>
> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >> Cc: Stefano Stabellini <sstabellini@kernel.org>
> >> Cc: Tim Deegan <tim@xen.org>
> >> Cc: Jun Nakajima <jun.nakajima@intel.com>
> >> Cc: Kevin Tian <kevin.tian@intel.com>
> >> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> >>
> >> v2:
> >>  - New in v2.
> >>
> >> v3:
> >>  - Leave iommu_iotlb_flush[_all] alone.
> >>  - Make patch purely cosmetic.
> >>  - Fix comment in xen/iommu.h.
> >> ---
> >>  xen/arch/x86/mm.c                   | 11 ++++++-----
> >>  xen/arch/x86/mm/p2m-ept.c           |  4 ++--
> >>  xen/arch/x86/mm/p2m-pt.c            |  5 +++--
> >>  xen/arch/x86/mm/p2m.c               | 12 ++++++------
> >>  xen/arch/x86/x86_64/mm.c            |  9 +++++----
> >>  xen/common/grant_table.c            | 14 +++++++-------
> >>  xen/common/memory.c                 |  4 ++--
> >>  xen/drivers/passthrough/iommu.c     |  6 +++---
> >>  xen/drivers/passthrough/x86/iommu.c |  4 ++--
> >>  xen/include/xen/iommu.h             | 16 +++++++++++-----
> >>  10 files changed, 47 insertions(+), 38 deletions(-)
> >>
> >> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> >> index 1431f347f3..0ad3940bb3 100644
> >> --- a/xen/arch/x86/mm.c
> >> +++ b/xen/arch/x86/mm.c
> >> @@ -2801,12 +2801,13 @@ static int _get_page_type(struct page_info
> *page,
> >> unsigned long type,
> >>              mfn_t mfn = page_to_mfn(page);
> >>
> >>              if ( (x & PGT_type_mask) == PGT_writable_page )
> >> -                iommu_ret = iommu_unmap(d, _dfn(mfn_x(mfn)),
> >> -                                        PAGE_ORDER_4K);
> >> +                iommu_ret = iommu_legacy_unmap(d, _dfn(mfn_x(mfn)),
> >> +                                               PAGE_ORDER_4K);
> >>              else if ( type == PGT_writable_page )
> >> -                iommu_ret = iommu_map(d, _dfn(mfn_x(mfn)), mfn,
> >> -                                      PAGE_ORDER_4K,
> >> -                                      IOMMUF_readable |
> IOMMUF_writable);
> >> +                iommu_ret = iommu_legacy_map(d, _dfn(mfn_x(mfn)), mfn,
> >> +                                             PAGE_ORDER_4K,
> >> +                                             IOMMUF_readable |
> >> +                                             IOMMUF_writable);
> >>          }
> >>      }
> >>
> >> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> >> index 6e4e375bad..64a49c07b7 100644
> >> --- a/xen/arch/x86/mm/p2m-ept.c
> >> +++ b/xen/arch/x86/mm/p2m-ept.c
> >> @@ -882,8 +882,8 @@ out:
> >>              rc = iommu_pte_flush(d, gfn, &ept_entry->epte, order,
> >> vtd_pte_present);
> >>          else if ( need_iommu_pt_sync(d) )
> >>              rc = iommu_flags ?
> >> -                iommu_map(d, _dfn(gfn), mfn, order, iommu_flags) :
> >> -                iommu_unmap(d, _dfn(gfn), order);
> >> +                iommu_legacy_map(d, _dfn(gfn), mfn, order,
> iommu_flags) :
> >> +                iommu_legacy_unmap(d, _dfn(gfn), order);
> >>      }
> >>
> >>      unmap_domain_page(table);
> >> diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
> >> index 17a6b61f12..69ffb08179 100644
> >> --- a/xen/arch/x86/mm/p2m-pt.c
> >> +++ b/xen/arch/x86/mm/p2m-pt.c
> >> @@ -686,8 +686,9 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t
> gfn_,
> >> mfn_t mfn,
> >>
> >>          if ( need_iommu_pt_sync(p2m->domain) )
> >>              rc = iommu_pte_flags ?
> >> -                iommu_map(d, _dfn(gfn), mfn, page_order,
> iommu_pte_flags)
> >> :
> >> -                iommu_unmap(d, _dfn(gfn), page_order);
> >> +                iommu_legacy_map(d, _dfn(gfn), mfn, page_order,
> >> +                                 iommu_pte_flags) :
> >> +                iommu_legacy_unmap(d, _dfn(gfn), page_order);
> >>          else if ( iommu_use_hap_pt(d) && iommu_old_flags )
> >>              amd_iommu_flush_pages(p2m->domain, gfn, page_order);
> >>      }
> >> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> >> index fea4497910..ed76e96d33 100644
> >> --- a/xen/arch/x86/mm/p2m.c
> >> +++ b/xen/arch/x86/mm/p2m.c
> >> @@ -733,7 +733,7 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned
> long
> >> gfn_l, unsigned long mfn,
> >>
> >>      if ( !paging_mode_translate(p2m->domain) )
> >>          return need_iommu_pt_sync(p2m->domain) ?
> >> -            iommu_unmap(p2m->domain, _dfn(mfn), page_order) : 0;
> >> +            iommu_legacy_unmap(p2m->domain, _dfn(mfn), page_order) :
> 0;
> >>
> >>      ASSERT(gfn_locked_by_me(p2m, gfn));
> >>      P2M_DEBUG("removing gfn=%#lx mfn=%#lx\n", gfn_l, mfn);
> >> @@ -780,8 +780,8 @@ guest_physmap_add_entry(struct domain *d, gfn_t
> gfn,
> >> mfn_t mfn,
> >>
> >>      if ( !paging_mode_translate(d) )
> >>          return (need_iommu_pt_sync(d) && t == p2m_ram_rw) ?
> >> -            iommu_map(d, _dfn(mfn_x(mfn)), mfn, page_order,
> >> -                      IOMMUF_readable | IOMMUF_writable) : 0;
> >> +            iommu_legacy_map(d, _dfn(mfn_x(mfn)), mfn, page_order,
> >> +                             IOMMUF_readable | IOMMUF_writable) : 0;
> >>
> >>      /* foreign pages are added thru p2m_add_foreign */
> >>      if ( p2m_is_foreign(t) )
> >> @@ -1151,8 +1151,8 @@ int set_identity_p2m_entry(struct domain *d,
> >> unsigned long gfn_l,
> >>      {
> >>          if ( !need_iommu_pt_sync(d) )
> >>              return 0;
> >> -        return iommu_map(d, _dfn(gfn_l), _mfn(gfn_l), PAGE_ORDER_4K,
> >> -                         IOMMUF_readable | IOMMUF_writable);
> >> +        return iommu_legacy_map(d, _dfn(gfn_l), _mfn(gfn_l),
> >> PAGE_ORDER_4K,
> >> +                                IOMMUF_readable | IOMMUF_writable);
> >>      }
> >>
> >>      gfn_lock(p2m, gfn, 0);
> >> @@ -1242,7 +1242,7 @@ int clear_identity_p2m_entry(struct domain *d,
> >> unsigned long gfn_l)
> >>      {
> >>          if ( !need_iommu_pt_sync(d) )
> >>              return 0;
> >> -        return iommu_unmap(d, _dfn(gfn_l), PAGE_ORDER_4K);
> >> +        return iommu_legacy_unmap(d, _dfn(gfn_l), PAGE_ORDER_4K);
> >>      }
> >>
> >>      gfn_lock(p2m, gfn, 0);
> >> diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
> >> index 126a486d2e..d8f558bc3a 100644
> >> --- a/xen/arch/x86/x86_64/mm.c
> >> +++ b/xen/arch/x86/x86_64/mm.c
> >> @@ -1436,15 +1436,16 @@ int memory_add(unsigned long spfn, unsigned
> long
> >> epfn, unsigned int pxm)
> >>           !need_iommu_pt_sync(hardware_domain) )
> >>      {
> >>          for ( i = spfn; i < epfn; i++ )
> >> -            if ( iommu_map(hardware_domain, _dfn(i), _mfn(i),
> >> -                           PAGE_ORDER_4K,
> >> -                           IOMMUF_readable | IOMMUF_writable) )
> >> +            if ( iommu_legacy_map(hardware_domain, _dfn(i), _mfn(i),
> >> +                                  PAGE_ORDER_4K,
> >> +                                  IOMMUF_readable | IOMMUF_writable) )
> >>                  break;
> >>          if ( i != epfn )
> >>          {
> >>              while (i-- > old_max)
> >>                  /* If statement to satisfy __must_check. */
> >> -                if ( iommu_unmap(hardware_domain, _dfn(i),
> PAGE_ORDER_4K)
> >> )
> >> +                if ( iommu_legacy_unmap(hardware_domain, _dfn(i),
> >> +                                        PAGE_ORDER_4K) )
> >>                      continue;
> >>
> >>              goto destroy_m2p;
> >> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> >> index b67ae9e3f5..fd099a8f25 100644
> >> --- a/xen/common/grant_table.c
> >> +++ b/xen/common/grant_table.c
> >> @@ -1134,14 +1134,14 @@ map_grant_ref(
> >>               !(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
> >>          {
> >>              if ( !(kind & MAPKIND_WRITE) )
> >> -                err = iommu_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
> >> -                                IOMMUF_readable | IOMMUF_writable);
> >> +                err = iommu_legacy_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
> >> +                                       IOMMUF_readable |
> >> IOMMUF_writable);
> >>          }
> >>          else if ( act_pin && !old_pin )
> >>          {
> >>              if ( !kind )
> >> -                err = iommu_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
> >> -                                IOMMUF_readable);
> >> +                err = iommu_legacy_map(ld, _dfn(mfn_x(mfn)), mfn, 0,
> >> +                                       IOMMUF_readable);
> >>          }
> >>          if ( err )
> >>          {
> >> @@ -1389,10 +1389,10 @@ unmap_common(
> >>
> >>          kind = mapkind(lgt, rd, op->mfn);
> >>          if ( !kind )
> >> -            err = iommu_unmap(ld, _dfn(mfn_x(op->mfn)), 0);
> >> +            err = iommu_legacy_unmap(ld, _dfn(mfn_x(op->mfn)), 0);
> >>          else if ( !(kind & MAPKIND_WRITE) )
> >> -            err = iommu_map(ld, _dfn(mfn_x(op->mfn)), op->mfn, 0,
> >> -                            IOMMUF_readable);
> >> +            err = iommu_legacy_map(ld, _dfn(mfn_x(op->mfn)), op->mfn,
> 0,
> >> +                                   IOMMUF_readable);
> >>
> >>          double_gt_unlock(lgt, rgt);
> >>
> >> diff --git a/xen/common/memory.c b/xen/common/memory.c
> >> index 5f7d081c61..f37eb288d4 100644
> >> --- a/xen/common/memory.c
> >> +++ b/xen/common/memory.c
> >> @@ -853,11 +853,11 @@ int xenmem_add_to_physmap(struct domain *d,
> struct
> >> xen_add_to_physmap *xatp,
> >>
> >>          this_cpu(iommu_dont_flush_iotlb) = 0;
> >>
> >> -        ret = iommu_iotlb_flush(d, _dfn(xatp->idx - done), done);
> >> +        ret = iommu_flush(d, _dfn(xatp->idx - done), done);
> >>          if ( unlikely(ret) && rc >= 0 )
> >>              rc = ret;
> >>
> >> -        ret = iommu_iotlb_flush(d, _dfn(xatp->gpfn - done), done);
> >> +        ret = iommu_flush(d, _dfn(xatp->gpfn - done), done);
> >>          if ( unlikely(ret) && rc >= 0 )
> >>              rc = ret;
> >>      }
> >> diff --git a/xen/drivers/passthrough/iommu.c
> >> b/xen/drivers/passthrough/iommu.c
> >> index c1cce08551..105995a343 100644
> >> --- a/xen/drivers/passthrough/iommu.c
> >> +++ b/xen/drivers/passthrough/iommu.c
> >> @@ -304,8 +304,8 @@ void iommu_domain_destroy(struct domain *d)
> >>      arch_iommu_domain_destroy(d);
> >>  }
> >>
> >> -int iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> >> -              unsigned int page_order, unsigned int flags)
> >> +int iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> >> +                     unsigned int page_order, unsigned int flags)
> >>  {
> >>      const struct domain_iommu *hd = dom_iommu(d);
> >>      unsigned long i;
> >> @@ -345,7 +345,7 @@ int iommu_map(struct domain *d, dfn_t dfn, mfn_t
> mfn,
> >>      return rc;
> >>  }
> >>
> >> -int iommu_unmap(struct domain *d, dfn_t dfn, unsigned int page_order)
> >> +int iommu_legacy_unmap(struct domain *d, dfn_t dfn, unsigned int
> >> page_order)
> >>  {
> >>      const struct domain_iommu *hd = dom_iommu(d);
> >>      unsigned long i;
> >> diff --git a/xen/drivers/passthrough/x86/iommu.c
> >> b/xen/drivers/passthrough/x86/iommu.c
> >> index c68a72279d..b12289a18f 100644
> >> --- a/xen/drivers/passthrough/x86/iommu.c
> >> +++ b/xen/drivers/passthrough/x86/iommu.c
> >> @@ -241,8 +241,8 @@ void __hwdom_init arch_iommu_hwdom_init(struct
> domain
> >> *d)
> >>          if ( paging_mode_translate(d) )
> >>              rc = set_identity_p2m_entry(d, pfn, p2m_access_rw, 0);
> >>          else
> >> -            rc = iommu_map(d, _dfn(pfn), _mfn(pfn), PAGE_ORDER_4K,
> >> -                           IOMMUF_readable | IOMMUF_writable);
> >> +            rc = iommu_legacy_map(d, _dfn(pfn), _mfn(pfn),
> PAGE_ORDER_4K,
> >> +                                  IOMMUF_readable | IOMMUF_writable);
> >>          if ( rc )
> >>              printk(XENLOG_WARNING " d%d: IOMMU mapping failed: %d\n",
> >>                     d->domain_id, rc);
> >> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> >> index 3d78126801..1f875aa328 100644
> >> --- a/xen/include/xen/iommu.h
> >> +++ b/xen/include/xen/iommu.h
> >> @@ -83,15 +83,21 @@ int iommu_construct(struct domain *d);
> >>  /* Function used internally, use iommu_domain_destroy */
> >>  void iommu_teardown(struct domain *d);
> >>
> >> -/* iommu_map_page() takes flags to direct the mapping operation. */
> >> +/*
> >> + * The following flags are passed to map operations and passed by
> lookup
> >> + * operations.
> >> + */
> >>  #define _IOMMUF_readable 0
> >>  #define IOMMUF_readable  (1u<<_IOMMUF_readable)
> >>  #define _IOMMUF_writable 1
> >>  #define IOMMUF_writable  (1u<<_IOMMUF_writable)
> >> -int __must_check iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
> >> -                           unsigned int page_order, unsigned int
> flags);
> >> -int __must_check iommu_unmap(struct domain *d, dfn_t dfn,
> >> -                             unsigned int page_order);
> >> +
> >> +int __must_check iommu_legacy_map(struct domain *d, dfn_t dfn, mfn_t
> mfn,
> >> +                                  unsigned int page_order,
> >> +                                  unsigned int flags);
> >> +int __must_check iommu_legacy_unmap(struct domain *d, dfn_t dfn,
> >> +                                    unsigned int page_order);
> >> +
> >>  int __must_check iommu_lookup_page(struct domain *d, dfn_t dfn, mfn_t
> >> *mfn,
> >>                                     unsigned int *flags);
> >>
> >> --
> >> 2.11.0
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xenproject.org
> > https://lists.xenproject.org/mailman/listinfo/xen-devel
> 
> 

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

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

* Re: [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations
  2018-12-17  9:22 ` [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations Paul Durrant
  2018-12-19  9:05   ` Paul Durrant
  2018-12-19 15:43   ` Julien Grall
@ 2018-12-20 19:27   ` Woods, Brian
  2018-12-20 22:24     ` Paul Durrant
  2 siblings, 1 reply; 16+ messages in thread
From: Woods, Brian @ 2018-12-20 19:27 UTC (permalink / raw)
  To: Paul Durrant, xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Suthikulpanit, Suravee, Roger Pau Monné

From: Paul Durrant <paul.durrant@citrix.com>
Sent: Monday, December 17, 2018 3:22 AM
To: xen-devel@lists.xenproject.org
Cc: Paul Durrant; Stefano Stabellini; Julien Grall; Andrew Cooper; George Dunlap; Ian Jackson; Konrad Rzeszutek Wilk; Tim Deegan; Wei Liu; Suthikulpanit, Suravee; Woods, Brian; Roger Pau Monné
Subject: [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations

This patch removes any implicit flushing that occurs in the implementation
of map and unmap operations and adds new iommu_map/unmap() wrapper
functions. To maintain semantics of the iommu_legacy_map/unmap() wrapper
functions, these are modified to call the new wrapper functions and then
perform an explicit flush operation.

Because VT-d currently performs two different types of flush dependent upon
whether a PTE is being modified versus merely added (i.e. replacing a non-
present PTE) 'iommu flush flags' are defined by this patch and the
iommu_ops map_page() and unmap_page() methods are modified to OR the type
of flush necessary for the PTE that has been populated or depopulated into
an accumulated flags value. The accumulated value can then be passed into
the explicit flush operation.

The ARM SMMU implementations of map_page() and unmap_page() currently
perform no implicit flushing and therefore the modified methods do not
adjust the flush flags.

NOTE: The per-cpu 'iommu_dont_flush_iotlb' is respected by the
      iommu_legacy_map/unmap() wrapper functions and therefore this now
      applies to all IOMMU implementations rather than just VT-d.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>

Acked-by: Brian Woods <brian.woods@amd.com>

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

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

* Re: [PATCH v5 1/4] amd-iommu: add flush iommu_ops
  2018-12-17  9:22 ` [PATCH v5 1/4] amd-iommu: add flush iommu_ops Paul Durrant
@ 2018-12-20 19:45   ` Woods, Brian
  2018-12-20 22:24     ` Paul Durrant
  0 siblings, 1 reply; 16+ messages in thread
From: Woods, Brian @ 2018-12-20 19:45 UTC (permalink / raw)
  To: Paul Durrant, xen-devel
  Cc: Andrew Cooper, Wei Liu, Suthikulpanit, Suravee, Roger Pau Monné

From: Paul Durrant <paul.durrant@citrix.com>
Sent: Monday, December 17, 2018 3:22 AM
To: xen-devel@lists.xenproject.org
Cc: Paul Durrant; Suthikulpanit, Suravee; Woods, Brian; Andrew Cooper; Wei Liu; Roger Pau Monné
Subject: [PATCH v5 1/4] amd-iommu: add flush iommu_ops

The iommu_ops structure contains two methods for flushing: 'iotlb_flush' and
'iotlb_flush_all'. This patch adds implementations of these for AMD IOMMUs.

The iotlb_flush method takes a base DFN and a (4k) page count, but the
flush needs to be done by page order (i.e. 0, 9 or 18). Because a flush
operation is fairly expensive to perform, the code calculates the minimum
order single flush that will cover the specified page range rather than
performing multiple flushes.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Brian Woods <brian.woods@amd.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

* Re: [PATCH v5 1/4] amd-iommu: add flush iommu_ops
  2018-12-20 19:45   ` Woods, Brian
@ 2018-12-20 22:24     ` Paul Durrant
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Durrant @ 2018-12-20 22:24 UTC (permalink / raw)
  To: 'Woods, Brian', xen-devel
  Cc: Andrew Cooper, Wei Liu, Suthikulpanit, Suravee, Roger Pau Monne

> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
> Of Woods, Brian
> Sent: 20 December 2018 19:46
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu
> <wei.liu2@citrix.com>; Suthikulpanit, Suravee
> <Suravee.Suthikulpanit@amd.com>; Roger Pau Monne <roger.pau@citrix.com>
> Subject: Re: [Xen-devel] [PATCH v5 1/4] amd-iommu: add flush iommu_ops
> 
> From: Paul Durrant <paul.durrant@citrix.com>
> Sent: Monday, December 17, 2018 3:22 AM
> To: xen-devel@lists.xenproject.org
> Cc: Paul Durrant; Suthikulpanit, Suravee; Woods, Brian; Andrew Cooper; Wei
> Liu; Roger Pau Monné
> Subject: [PATCH v5 1/4] amd-iommu: add flush iommu_ops
> 
> The iommu_ops structure contains two methods for flushing: 'iotlb_flush'
> and
> 'iotlb_flush_all'. This patch adds implementations of these for AMD
> IOMMUs.
> 
> The iotlb_flush method takes a base DFN and a (4k) page count, but the
> flush needs to be done by page order (i.e. 0, 9 or 18). Because a flush
> operation is fairly expensive to perform, the code calculates the minimum
> order single flush that will cover the specified page range rather than
> performing multiple flushes.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Brian Woods <brian.woods@amd.com>

Thanks Brian,

  Paul

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

* Re: [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations
  2018-12-20 19:27   ` Woods, Brian
@ 2018-12-20 22:24     ` Paul Durrant
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Durrant @ 2018-12-20 22:24 UTC (permalink / raw)
  To: 'Woods, Brian', xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	Andrew Cooper, Tim (Xen.org),
	George Dunlap, Julien Grall, Suthikulpanit, Suravee, Ian Jackson,
	Roger Pau Monne

> -----Original Message-----
> From: Woods, Brian [mailto:Brian.Woods@amd.com]
> Sent: 20 December 2018 19:27
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
> <julien.grall@arm.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; George
> Dunlap <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Tim (Xen.org)
> <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>; Suthikulpanit, Suravee
> <Suravee.Suthikulpanit@amd.com>; Roger Pau Monne <roger.pau@citrix.com>
> Subject: Re: [PATCH v5 3/4] iommu: elide flushing for higher order
> map/unmap operations
> 
> From: Paul Durrant <paul.durrant@citrix.com>
> Sent: Monday, December 17, 2018 3:22 AM
> To: xen-devel@lists.xenproject.org
> Cc: Paul Durrant; Stefano Stabellini; Julien Grall; Andrew Cooper; George
> Dunlap; Ian Jackson; Konrad Rzeszutek Wilk; Tim Deegan; Wei Liu;
> Suthikulpanit, Suravee; Woods, Brian; Roger Pau Monné
> Subject: [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap
> operations
> 
> This patch removes any implicit flushing that occurs in the implementation
> of map and unmap operations and adds new iommu_map/unmap() wrapper
> functions. To maintain semantics of the iommu_legacy_map/unmap() wrapper
> functions, these are modified to call the new wrapper functions and then
> perform an explicit flush operation.
> 
> Because VT-d currently performs two different types of flush dependent
> upon
> whether a PTE is being modified versus merely added (i.e. replacing a non-
> present PTE) 'iommu flush flags' are defined by this patch and the
> iommu_ops map_page() and unmap_page() methods are modified to OR the type
> of flush necessary for the PTE that has been populated or depopulated into
> an accumulated flags value. The accumulated value can then be passed into
> the explicit flush operation.
> 
> The ARM SMMU implementations of map_page() and unmap_page() currently
> perform no implicit flushing and therefore the modified methods do not
> adjust the flush flags.
> 
> NOTE: The per-cpu 'iommu_dont_flush_iotlb' is respected by the
>       iommu_legacy_map/unmap() wrapper functions and therefore this now
>       applies to all IOMMU implementations rather than just VT-d.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> 
> Acked-by: Brian Woods <brian.woods@amd.com>

Thanks Brian,

  Paul

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

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

* Re: [PATCH v5 2/4] iommu: rename wrapper functions
  2018-12-17  9:22 ` [PATCH v5 2/4] iommu: rename wrapper functions Paul Durrant
  2018-12-19  9:05   ` Paul Durrant
@ 2018-12-25  2:29   ` Tian, Kevin
  1 sibling, 0 replies; 16+ messages in thread
From: Tian, Kevin @ 2018-12-25  2:29 UTC (permalink / raw)
  To: Paul Durrant, xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Nakajima, Jun, Roger Pau Monné

> From: Paul Durrant [mailto:paul.durrant@citrix.com]
> Sent: Monday, December 17, 2018 5:23 PM
> 
> A subsequent patch will add semantically different versions of
> iommu_map/unmap() so, in advance of that change, this patch renames
> the
> existing functions to iommu_legacy_map/unmap() and modifies all call-
> sites.
> It also adjusts a comment that refers to iommu_map_page(), which was re-
> named by a previous patch.
> 
> This patch is purely cosmetic. No functional change.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>A

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

end of thread, other threads:[~2018-12-25  2:29 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-17  9:22 [PATCH v5 0/4] iommu improvements Paul Durrant
2018-12-17  9:22 ` [PATCH v5 1/4] amd-iommu: add flush iommu_ops Paul Durrant
2018-12-20 19:45   ` Woods, Brian
2018-12-20 22:24     ` Paul Durrant
2018-12-17  9:22 ` [PATCH v5 2/4] iommu: rename wrapper functions Paul Durrant
2018-12-19  9:05   ` Paul Durrant
2018-12-19 15:39     ` Julien Grall
2018-12-19 16:23     ` Jan Beulich
2018-12-19 16:26       ` Paul Durrant
2018-12-25  2:29   ` Tian, Kevin
2018-12-17  9:22 ` [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations Paul Durrant
2018-12-19  9:05   ` Paul Durrant
2018-12-19 15:43   ` Julien Grall
2018-12-20 19:27   ` Woods, Brian
2018-12-20 22:24     ` Paul Durrant
2018-12-17  9:22 ` [PATCH v5 4/4] x86/mm/p2m: stop checking for IOMMU shared page tables in mmio_order() Paul Durrant

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).