[Xen-devel] [PATCH V3 3/3] x86/altp2m: fix display frozen when switching to a new view early

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

When an new altp2m view is created very early on guest boot, the
display will freeze (although the guest will run normally). This
may also happen on resizing the display. The reason is the way
Xen currently (mis)handles logdirty VGA: it intentionally
misconfigures VGA pages so that they will fault.

The problem is that it only does this in the host p2m. Once we
switch to a new altp2m, the misconfigured entries will no longer
fault, so the display will not be updated.

This patch:
* updates ept_handle_misconfig() to use the active altp2m instead
  of the hostp2m;
* modifies p2m_change_entry_type_global(), p2m_memory_type_changed
  and p2m_change_type_range() to propagate their changes to all
  valid altp2ms.

Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
Suggested-by: George Dunlap <george.dunlap@xxxxxxxxxx>

---
CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
CC: Kevin Tian <kevin.tian@xxxxxxxxx>
CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>

---
Changes since V2:
 - RFC: We need George's opinion on Jan's suggestion to update
   p2m-pt.c as well.
 - Dropped the p2m_ prefix from the static helpers instead of
   adding a leading underscore (except for _memory_type_changed(),
   as there's a memory_type_changed() in asm/mtrr.h, which would
   break the build.
 - Used hostp2m in p2m_change_entry_type_global() where previously
   I had missed doing so for
   _p2m_change_entry_type_global(p2m_get_hostp2m(d), ot, nt);
   (although it had been already latched).
 - Now consistenly using the "update the hostp2m first, then the
   altp2ms" pattern.
---
 xen/arch/x86/mm/p2m-ept.c |  8 +++++
 xen/arch/x86/mm/p2m.c     | 83 +++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 78 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index fabcd06..e6fa85f 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -657,6 +657,9 @@ bool_t ept_handle_misconfig(uint64_t gpa)
     bool_t spurious;
     int rc;
 
+    if ( altp2m_active(curr->domain) )
+        p2m = p2m_get_altp2m(curr);
+
     p2m_lock(p2m);
 
     spurious = curr->arch.hvm.vmx.ept_spurious_misconfig;
@@ -1440,6 +1443,11 @@ void p2m_init_altp2m_ept(struct domain *d, unsigned int 
i)
     struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
     struct ept_data *ept;
 
+    p2m->max_mapped_pfn = hostp2m->max_mapped_pfn;
+    p2m->default_access = hostp2m->default_access;
+    p2m->domain = hostp2m->domain;
+
+    p2m->global_logdirty = hostp2m->global_logdirty;
     p2m->ept.ad = hostp2m->ept.ad;
     p2m->min_remapped_gfn = gfn_x(INVALID_GFN);
     p2m->max_remapped_gfn = 0;
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index c6b17e6..5fbee82 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -281,7 +281,6 @@ int p2m_init(struct domain *d)
 int p2m_is_logdirty_range(struct p2m_domain *p2m, unsigned long start,
                           unsigned long end)
 {
-    ASSERT(p2m_is_hostp2m(p2m));
     if ( p2m->global_logdirty ||
          rangeset_contains_range(p2m->logdirty_ranges, start, end) )
         return 1;
@@ -290,24 +289,48 @@ int p2m_is_logdirty_range(struct p2m_domain *p2m, 
unsigned long start,
     return 0;
 }
 
+static void change_entry_type_global(struct p2m_domain *p2m,
+                                          p2m_type_t ot, p2m_type_t nt)
+{
+    p2m->change_entry_type_global(p2m, ot, nt);
+    p2m->global_logdirty = (nt == p2m_ram_logdirty);
+}
+
 void p2m_change_entry_type_global(struct domain *d,
                                   p2m_type_t ot, p2m_type_t nt)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
+    struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
 
     ASSERT(ot != nt);
     ASSERT(p2m_is_changeable(ot) && p2m_is_changeable(nt));
 
-    p2m_lock(p2m);
-    p2m->change_entry_type_global(p2m, ot, nt);
-    p2m->global_logdirty = (nt == p2m_ram_logdirty);
-    p2m_unlock(p2m);
+    p2m_lock(hostp2m);
+
+    change_entry_type_global(hostp2m, ot, nt);
+
+#ifdef CONFIG_HVM
+    if ( unlikely(altp2m_active(d)) )
+    {
+        unsigned int i;
+
+        for ( i = 0; i < MAX_ALTP2M; i++ )
+            if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
+            {
+                struct p2m_domain *p2m = d->arch.altp2m_p2m[i];
+
+                p2m_lock(p2m);
+                change_entry_type_global(p2m, ot, nt);
+                p2m_unlock(p2m);
+            }
+    }
+#endif
+
+    p2m_unlock(hostp2m);
 }
 
-void p2m_memory_type_changed(struct domain *d)
+/* There's already a memory_type_changed() in asm/mtrr.h. */
+static void _memory_type_changed(struct p2m_domain *p2m)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
-
     if ( p2m->memory_type_changed )
     {
         p2m_lock(p2m);
@@ -316,6 +339,22 @@ void p2m_memory_type_changed(struct domain *d)
     }
 }
 
+void p2m_memory_type_changed(struct domain *d)
+{
+    _memory_type_changed(p2m_get_hostp2m(d));
+
+#ifdef CONFIG_HVM
+    if ( unlikely(altp2m_active(d)) )
+    {
+        unsigned int i;
+
+        for ( i = 0; i < MAX_ALTP2M; i++ )
+            if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
+                _memory_type_changed(d->arch.altp2m_p2m[i]);
+    }
+#endif
+}
+
 int p2m_set_ioreq_server(struct domain *d,
                          unsigned int flags,
                          struct hvm_ioreq_server *s)
@@ -996,12 +1035,12 @@ int p2m_change_type_one(struct domain *d, unsigned long 
gfn_l,
 }
 
 /* Modify the p2m type of a range of gfns from ot to nt. */
-void p2m_change_type_range(struct domain *d, 
-                           unsigned long start, unsigned long end,
-                           p2m_type_t ot, p2m_type_t nt)
+static void change_type_range(struct p2m_domain *p2m,
+                              unsigned long start, unsigned long end,
+                              p2m_type_t ot, p2m_type_t nt)
 {
     unsigned long gfn = start;
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
+       struct domain *d = p2m->domain;
     int rc = 0;
 
     ASSERT(ot != nt);
@@ -1054,6 +1093,24 @@ void p2m_change_type_range(struct domain *d,
     p2m_unlock(p2m);
 }
 
+void p2m_change_type_range(struct domain *d,
+                           unsigned long start, unsigned long end,
+                           p2m_type_t ot, p2m_type_t nt)
+{
+    change_type_range(p2m_get_hostp2m(d), start, end, ot, nt);
+
+#ifdef CONFIG_HVM
+    if ( unlikely(altp2m_active(d)) )
+    {
+        unsigned int i;
+
+        for ( i = 0; i < MAX_ALTP2M; i++ )
+            if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
+                change_type_range(d->arch.altp2m_p2m[i], start, end, ot, nt);
+    }
+#endif
+}
+
 /*
  * Finish p2m type change for gfns which are marked as need_recalc in a range.
  * Returns: 0/1 for success, negative for failure
-- 
2.7.4


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