[Xen-devel] [PATCH v2 12/19] x86/pv: Avoid raising faults behind the emulators back

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

Use x86_emul_pagefault() rather than pv_inject_page_fault() to cause raised
pagefaults to be known to the emulator.  This requires altering the callers of
x86_emulate() to properly re-inject the event.

While fixing this, fix the singlestep behaviour.  Previously, an otherwise
successful emulation would fail if singlestepping was active, as the emulator
couldn't raise #DB.  This is unreasonable from the point of view of the guest.

We therefore tolerate either #PF or #DB being raised by the emulator, but
reject anything else as unexpected.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Tim Deegan <tim@xxxxxxx>

v2:
 * New
---
 xen/arch/x86/mm.c | 96 ++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 64 insertions(+), 32 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 8a1e7b4..5b60b59 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5136,7 +5136,7 @@ static int ptwr_emulated_read(
     if ( !__addr_ok(addr) ||
          (rc = __copy_from_user(p_data, (void *)addr, bytes)) )
     {
-        pv_inject_page_fault(0, addr + bytes - rc); /* Read fault. */
+        x86_emul_pagefault(0, addr + bytes - rc, ctxt);  /* Read fault. */
         return X86EMUL_EXCEPTION;
     }
 
@@ -5177,8 +5177,9 @@ static int ptwr_emulated_update(
         addr &= ~(sizeof(paddr_t)-1);
         if ( (rc = copy_from_user(&full, (void *)addr, sizeof(paddr_t))) != 0 )
         {
-            pv_inject_page_fault(0, /* Read fault. */
-                                 addr + sizeof(paddr_t) - rc);
+            x86_emul_pagefault(0, /* Read fault. */
+                               addr + sizeof(paddr_t) - rc,
+                               &ptwr_ctxt->ctxt);
             return X86EMUL_EXCEPTION;
         }
         /* Mask out bits provided by caller. */
@@ -5379,27 +5380,40 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned long 
addr,
     page_unlock(page);
     put_page(page);
 
-    /*
-     * TODO: Make this true:
-     *
     ASSERT(ptwr_ctxt.ctxt.event_pending == (rc == X86EMUL_EXCEPTION));
-     *
-     * Some codepaths still raise exceptions behind the back of the
-     * emulator. (i.e. return X86EMUL_EXCEPTION but without
-     * event_pending being set).  In the meantime, use a slightly
-     * relaxed check...
-     */
-    if ( ptwr_ctxt.ctxt.event_pending )
-        ASSERT(rc == X86EMUL_EXCEPTION);
 
-    if ( rc == X86EMUL_UNHANDLEABLE || ptwr_ctxt.ctxt.event_pending )
-        goto bail;
+    switch ( rc )
+    {
+    case X86EMUL_EXCEPTION:
+        /*
+         * This emulation only covers writes to pagetables which marked
+         * read-only by Xen.  We tolerate #PF (from hitting an adjacent page)
+         * and #DB (from singlestepping).  Anything else is an emulation bug,
+         * or a guest playing with the instruction stream under Xen's feet.
+         */
+        if ( ptwr_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
+             (ptwr_ctxt.ctxt.event.vector == TRAP_debug ||
+              ptwr_ctxt.ctxt.event.vector == TRAP_page_fault) )
+            pv_inject_event(&ptwr_ctxt.ctxt.event);
+        else
+        {
+            gdprintk(XENLOG_WARNING,
+                     "Unexpected event (type %u, vector %#x) from emulation\n",
+                     ptwr_ctxt.ctxt.event.type, ptwr_ctxt.ctxt.event.vector);
+
+            pv_inject_hw_exception(TRAP_gp_fault, 0);
+        }
 
-    perfc_incr(ptwr_emulations);
-    return EXCRET_fault_fixed;
+        /* Fallthrough */
+    case X86EMUL_OKAY:
+    case X86EMUL_RETRY:
+        perfc_incr(ptwr_emulations);
+        return EXCRET_fault_fixed;
 
  bail:
-    return 0;
+    default:
+        return 0;
+    }
 }
 
 /*************************
@@ -5516,21 +5530,39 @@ int mmio_ro_do_page_fault(struct vcpu *v, unsigned long 
addr,
     else
         rc = x86_emulate(&ctxt, &mmio_ro_emulate_ops);
 
-    /*
-     * TODO: Make this true:
-     *
     ASSERT(ctxt.event_pending == (rc == X86EMUL_EXCEPTION));
-     *
-     * Some codepaths still raise exceptions behind the back of the
-     * emulator. (i.e. return X86EMUL_EXCEPTION but without
-     * event_pending being set).  In the meantime, use a slightly
-     * relaxed check...
-     */
-    if ( ctxt.event_pending )
-        ASSERT(rc == X86EMUL_EXCEPTION);
 
-    return ((rc != X86EMUL_UNHANDLEABLE && !ctxt.event_pending)
-            ? EXCRET_fault_fixed : 0);
+    switch ( rc )
+    {
+    case X86EMUL_EXCEPTION:
+        /*
+         * This emulation only covers writes to MMCFG space or read-only MFNs.
+         * We tolerate #PF (from hitting an adjacent page) and #DB (from
+         * singlestepping).  Anything else is an emulation bug, or a guest
+         * playing with the instruction stream under Xen's feet.
+         */
+        if ( ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
+             (ctxt.event.vector == TRAP_debug ||
+              ctxt.event.vector == TRAP_page_fault) )
+            pv_inject_event(&ctxt.event);
+        else
+        {
+            gdprintk(XENLOG_WARNING,
+                     "Unexpected event (type %u, vector %#x) from emulation\n",
+                     ctxt.event.type, ctxt.event.vector);
+
+            pv_inject_hw_exception(TRAP_gp_fault, 0);
+        }
+
+        /* Fallthrough */
+    case X86EMUL_OKAY:
+    case X86EMUL_RETRY:
+        perfc_incr(ptwr_emulations);
+        return EXCRET_fault_fixed;
+
+    default:
+        return 0;
+    }
 }
 
 void *alloc_xen_pagetable(void)
-- 
2.1.4


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