AMD/IOMMU: suppress PTE merging after initial table creation

The logic is not fit for this purpose, so simply disable its use until
it can be fixed / replaced. Note that this re-enables merging for the
table creation case, which was disabled as a (perhaps unintended) side
effect of the earlier "amd/iommu: fix flush checks". It relies on no
page getting mapped more than once (with different properties) in this
process, as that would still be beyond what the merging logic can cope
with. But arch_iommu_populate_page_table() guarantees this afaict.

This is part of XSA-275.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
master commit: 937ef32565fa3a81fdb37b9dd5aa99a1b87afa75
master date: 2018-11-20 14:55:14 +0100

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 047a489647843067125aeffa779f73f44a970c54..479e48ab98a6b6f31587be25bf77a6bf6566eff0 100644 (file)
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1023,6 +1023,20 @@ int domain_unpause_by_systemcontroller(struct domain *d)
         prev = cmpxchg(&d->controller_pause_count, old, new);
     } while ( prev != old );
 
+    /*
+     * d->controller_pause_count is initialised to 1, and the toolstack is
+     * responsible for making one unpause hypercall when it wishes the guest
+     * to start running.
+     *
+     * All other toolstack operations should make a pair of pause/unpause
+     * calls and rely on the reference counting here.
+     *
+     * Creation is considered finished when the controller reference count
+     * first drops to 0.
+     */
+    if ( new == 0 )
+        d->creation_finished = 1;
+
     domain_unpause(d);
 
     return 0;

diff --git a/xen/drivers/passthrough/amd/iommu_map.c b/xen/drivers/passthrough/amd/iommu_map.c
index 1c04ed65f5c7b569d79206b7f23dc42c039a260d..87b93c20742cd781f8d6a0214720e787b5743e1c 100644 (file)
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -695,11 +695,24 @@ int amd_iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn,
                                        !!(flags & IOMMUF_writable),
                                        !!(flags & IOMMUF_readable));
 
-    /* Do not increase pde count if io mapping has not been changed */
-    if ( !need_flush )
-        goto out;
+    if ( need_flush )
+    {
+        amd_iommu_flush_pages(d, gfn, 0);
+        /* No further merging, as the logic doesn't cope. */
+        hd->arch.no_merge = 1;
+    }
 
-    amd_iommu_flush_pages(d, gfn, 0);
+    /*
+     * Suppress merging of non-R/W mappings or after initial table creation,
+     * as the merge logic does not cope with this.
+     */
+    if ( hd->arch.no_merge || flags != (IOMMUF_writable | IOMMUF_readable) )
+        goto out;
+    if ( d->creation_finished )
+    {
+        hd->arch.no_merge = 1;
+        goto out;
+    }
 
     for ( merge_level = IOMMU_PAGING_MODE_LEVEL_2;
           merge_level <= hd->arch.paging_mode; merge_level++ )
@@ -769,6 +782,10 @@ int amd_iommu_unmap_page(struct domain *d, unsigned long gfn)
 
     /* mark PTE as 'page not present' */
     clear_iommu_pte_present(pt_mfn[1], gfn);
+
+    /* No further merging in amd_iommu_map_page(), as the logic doesn't cope. */
+    hd->arch.no_merge = 1;
+
     spin_unlock(&hd->arch.mapping_lock);
 
     amd_iommu_flush_pages(d, gfn, 0);

diff --git a/xen/include/asm-x86/hvm/iommu.h b/xen/include/asm-x86/hvm/iommu.h
index 6962cf6fe24f3cfb4ad4a2f139d28daefdde0929..95a7802a3ac60d8d7c83e4f6c7de6ddbd741b055 100644 (file)
--- a/xen/include/asm-x86/hvm/iommu.h
+++ b/xen/include/asm-x86/hvm/iommu.h
@@ -59,6 +59,7 @@ struct arch_iommu
 
     /* amd iommu support */
     int paging_mode;
+    bool_t no_merge;
     struct page_info *root_table;
     struct guest_iommu *g_iommu;
 };

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 4d8be547dd87a94556547f81c043a28decd7997a..0d45ba4d2c0f6ea23e30cdf57d639cf008fc3916 100644 (file)
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -387,6 +387,12 @@ struct domain
     bool_t           disable_migrate;
     /* Is this guest being debugged by dom0? */
     bool_t           debugger_attached;
+    /*
+     * Set to true at the very end of domain creation, when the domain is
+     * unpaused for the first time by the systemcontroller.
+     */
+    bool_t           creation_finished;
+
     /* Which guest this guest has privileges on */
     struct domain   *target;