From: <security@xenproject.org>
Subject: x86/ept: flush cache when modifying PTEs and sharing page tables

Modifications made to the page tables by EPT code need to be written
to memory when the page tables are shared with the IOMMU, as Intel
IOMMUs can be non-coherent and thus require changes to be written to
memory in order to be visible to the IOMMU.

In order to achieve this make sure data is written back to memory
after writing an EPT entry when the recalc bit is not set in
atomic_write_ept_entry. If such bit is set, the entry will be
adjusted and atomic_write_ept_entry will be called a second time
without the recalc bit set. Note that when splitting a super page the
new tables resulting of the split should also be written back.

Failure to do so can allow devices behind the IOMMU access to the
stale super page, or cause coherency issues as changes made by the
processor to the page tables are not visible to the IOMMU.

This allows to remove the VT-d specific iommu_pte_flush helper, since
the cache write back is now performed by atomic_write_ept_entry, and
hence iommu_iotlb_flush can be used to flush the IOMMU TLB. The newly
used method (iommu_iotlb_flush) can result in less flushes, since it
might sometimes be called rightly with 0 flags, in which case it
becomes a no-op.

This is part of XSA-321.

Reviewed-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -24,6 +24,7 @@
 #include <asm/types.h>
 #include <asm/domain.h>
 #include <asm/p2m.h>
+#include <asm/hvm/iommu.h>
 #include <asm/hvm/vmx/vmx.h>
 #include <asm/hvm/vmx/vmcs.h>
 #include <asm/hvm/nestedhvm.h>
@@ -37,8 +38,11 @@
 
 #define atomic_read_ept_entry(__pepte)                              \
     ( (ept_entry_t) { .epte = read_atomic(&(__pepte)->epte) } )
-#define atomic_write_ept_entry(__pepte, __epte)                     \
-    write_atomic(&(__pepte)->epte, (__epte).epte)
+#define atomic_write_ept_entry(__pepte, __epte) ({                  \
+    write_atomic(&(__pepte)->epte, (__epte).epte);                  \
+    if ( iommu_hap_pt_share )                                       \
+        iommu_sync_cache(__pepte, sizeof(*(__pepte)));              \
+})
 
 #define is_epte_present(ept_entry)      ((ept_entry)->epte & 0x7)
 #define is_epte_superpage(ept_entry)    ((ept_entry)->sp)
@@ -210,6 +214,9 @@ static int ept_split_super_page(struct p
             break;
     }
 
+    if ( iommu_hap_pt_share )
+        iommu_sync_cache(table, EPT_PAGETABLE_ENTRIES * sizeof(ept_entry_t));
+
     unmap_domain_page(table);
 
     /* Even failed we should install the newly allocated ept page. */
@@ -236,8 +243,7 @@ static int ept_next_level(struct p2m_dom
                           ept_entry_t **table, unsigned long *gfn_remainder,
                           int next_level)
 {
-    unsigned long mfn;
-    ept_entry_t *ept_entry, e;
+    ept_entry_t *ept_entry, *next = NULL, e;
     u32 shift, index;
 
     ASSERT(next_level);
@@ -267,6 +273,11 @@ static int ept_next_level(struct p2m_dom
         if ( !ept_set_middle_entry(p2m, &e) )
             return GUEST_TABLE_MAP_FAILED;
 
+        next = map_domain_page(e.mfn);
+
+        if ( iommu_hap_pt_share )
+            iommu_sync_cache(next, EPT_PAGETABLE_ENTRIES * sizeof(ept_entry_t));
+
         atomic_write_ept_entry(ept_entry, e);
     }
 
@@ -274,9 +285,8 @@ static int ept_next_level(struct p2m_dom
     if ( is_epte_superpage(&e) )
         return GUEST_TABLE_SUPER_PAGE;
 
-    mfn = e.mfn;
     unmap_domain_page(*table);
-    *table = map_domain_page(mfn);
+    *table = next ?: map_domain_page(e.mfn);
     *gfn_remainder &= (1UL << shift) - 1;
     return GUEST_TABLE_NORMAL_PAGE;
 }
@@ -456,7 +466,7 @@ out:
          need_iommu(p2m->domain) && need_modify_vtd_table )
     {
         if ( iommu_hap_pt_share )
-            iommu_pte_flush(d, gfn, (u64*)ept_entry, order, vtd_pte_present);
+            iommu_flush_iotlb(d, gfn, vtd_pte_present, 1u << order);
         else
         {
             if ( p2mt == p2m_ram_rw )
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -551,8 +551,8 @@ static void iommu_flush_all(void)
     }
 }
 
-static void __intel_iommu_iotlb_flush(struct domain *d, unsigned long gfn,
-        int dma_old_pte_present, unsigned int page_count)
+void iommu_flush_iotlb(struct domain *d, unsigned long gfn,
+                       bool_t dma_old_pte_present, unsigned int page_count)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(d);
     struct acpi_drhd_unit *drhd;
@@ -596,12 +596,12 @@ static void __intel_iommu_iotlb_flush(st
 
 static void intel_iommu_iotlb_flush(struct domain *d, unsigned long gfn, unsigned int page_count)
 {
-    __intel_iommu_iotlb_flush(d, gfn, 1, page_count);
+    iommu_flush_iotlb(d, gfn, 1, page_count);
 }
 
 static void intel_iommu_iotlb_flush_all(struct domain *d)
 {
-    __intel_iommu_iotlb_flush(d, 0, 0, 0);
+    iommu_flush_iotlb(d, 0, 0, 0);
 }
 
 /* clear one page's page table */
@@ -635,7 +635,7 @@ static void dma_pte_clear_one(struct dom
     iommu_sync_cache(pte, sizeof(struct dma_pte));
 
     if ( !this_cpu(iommu_dont_flush_iotlb) )
-        __intel_iommu_iotlb_flush(domain, addr >> PAGE_SHIFT_4K, 1, 1);
+        intel_iommu_iotlb_flush(domain, addr >> PAGE_SHIFT_4K, 1);
 
     unmap_vtd_domain_page(page);
 }
@@ -1790,7 +1790,7 @@ static int intel_iommu_map_page(
     unmap_vtd_domain_page(page);
 
     if ( !this_cpu(iommu_dont_flush_iotlb) )
-        __intel_iommu_iotlb_flush(d, gfn, dma_pte_present(old), 1);
+        iommu_flush_iotlb(d, gfn, dma_pte_present(old), 1);
 
     return 0;
 }
@@ -1810,34 +1810,6 @@ static int intel_iommu_unmap_page(struct
     return 0;
 }
 
-void iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte,
-                     int order, int present)
-{
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu = NULL;
-    struct hvm_iommu *hd = domain_hvm_iommu(d);
-    int flush_dev_iotlb;
-    int iommu_domid;
-
-    iommu_sync_cache(pte, sizeof(struct dma_pte));
-
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
-            continue;
-
-        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-        iommu_domid= domain_iommu_domid(d, iommu);
-        if ( iommu_domid == -1 )
-            continue;
-        if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
-                                   (paddr_t)gfn << PAGE_SHIFT_4K,
-                                   order, !present, flush_dev_iotlb) )
-            iommu_flush_write_buffer(iommu);
-    }
-}
-
 static int vtd_ept_page_compatible(struct iommu *iommu)
 {
     u64 ept_cap, vtd_cap = iommu->cap;
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -67,7 +67,8 @@ int deassign_device(struct domain *d, u1
 int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
                    unsigned int flags);
 int iommu_unmap_page(struct domain *d, unsigned long gfn);
-void iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte, int order, int present);
+void iommu_flush_iotlb(struct domain *d, unsigned long gfn,
+                       bool_t dma_old_pte_present, unsigned int page_count);
 void iommu_set_pgd(struct domain *d);
 void iommu_domain_teardown(struct domain *d);
 
