xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: [PATCH v4 17/24] x86/pv: Avoid raising faults behind the emulators back
Date: Thu, 1 Dec 2016 16:56:00 +0000	[thread overview]
Message-ID: <1480611361-15294-5-git-send-email-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <1480611361-15294-1-git-send-email-andrew.cooper3@citrix.com>

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.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Tim Deegan <tim@xen.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
v4:
 * Tweak comment wording.
 * Drop default cases.
v3:
 * Split out #DB handling to an earlier part of the series
 * Don't raise #GP faults for unexpected events, but do return back to the
   guest.
v2:
 * New
---
 xen/arch/x86/mm.c | 101 ++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 63 insertions(+), 38 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 5d59479..14552a1 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,38 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned long addr,
     page_unlock(page);
     put_page(page);
 
-    /*
-     * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
-     * by the emulator itself to become X86EMUL_UNHANDLEABLE.  Such exceptions
-     * now set event_pending instead.  Exceptions raised behind the back of
-     * the emulator don't yet set event_pending.
-     *
-     * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
-     * for no functional change from before.  Future patches will fix this
-     * properly.
-     */
-    if ( rc == X86EMUL_EXCEPTION && ptwr_ctxt.ctxt.event_pending )
-        rc = X86EMUL_UNHANDLEABLE;
+    /* More strict than x86_emulate_wrapper(), as this is now true for PV. */
+    ASSERT(ptwr_ctxt.ctxt.event_pending == (rc == X86EMUL_EXCEPTION));
 
-    if ( rc == X86EMUL_UNHANDLEABLE )
-        goto bail;
+    switch ( rc )
+    {
+    case X86EMUL_EXCEPTION:
+        /*
+         * This emulation only covers writes to pagetables which are marked
+         * read-only by Xen.  We tolerate #PF (in case a concurrent pagetable
+         * update has succeeded on a different vcpu).  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_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);
 
-    if ( ptwr_ctxt.ctxt.retire.singlestep )
-        pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+        /* Fallthrough */
+    case X86EMUL_OKAY:
 
-    perfc_incr(ptwr_emulations);
-    return EXCRET_fault_fixed;
+        if ( ptwr_ctxt.ctxt.retire.singlestep )
+            pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+
+        /* Fallthrough */
+    case X86EMUL_RETRY:
+        perfc_incr(ptwr_emulations);
+        return EXCRET_fault_fixed;
+    }
 
  bail:
     return 0;
@@ -5519,26 +5531,39 @@ int mmio_ro_do_page_fault(struct vcpu *v, unsigned long addr,
     else
         rc = x86_emulate(&ctxt, &mmio_ro_emulate_ops);
 
-    /*
-     * The previous lack of inject_{sw,hw}*() hooks caused exceptions raised
-     * by the emulator itself to become X86EMUL_UNHANDLEABLE.  Such exceptions
-     * now set event_pending instead.  Exceptions raised behind the back of
-     * the emulator don't yet set event_pending.
-     *
-     * For now, cause such cases to return to the X86EMUL_UNHANDLEABLE path,
-     * for no functional change from before.  Future patches will fix this
-     * properly.
-     */
-    if ( rc == X86EMUL_EXCEPTION && ctxt.event_pending )
-        rc = X86EMUL_UNHANDLEABLE;
+    /* More strict than x86_emulate_wrapper(), as this is now true for PV. */
+    ASSERT(ctxt.event_pending == (rc == X86EMUL_EXCEPTION));
 
-    if ( rc == X86EMUL_UNHANDLEABLE )
-        return 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 or a successful
+         * concurrent pagetable update).  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_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);
+
+        /* Fallthrough */
+    case X86EMUL_OKAY:
 
-    if ( ctxt.retire.singlestep )
-        pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+        if ( ctxt.retire.singlestep )
+            pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
 
-    return EXCRET_fault_fixed;
+        /* Fallthrough */
+    case X86EMUL_RETRY:
+        perfc_incr(ptwr_emulations);
+        return EXCRET_fault_fixed;
+    }
+
+    return 0;
 }
 
 void *alloc_xen_pagetable(void)
-- 
2.1.4


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

  parent reply	other threads:[~2016-12-01 16:56 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-01 16:55 [PATCH v4 for-4.9 00/24] [SUBSET] XSA-191 followup Andrew Cooper
2016-12-01 16:55 ` [PATCH v4 09/24] x86/emul: Provide a wrapper to x86_emulate() to ASSERT() certain behaviour Andrew Cooper
2016-12-02 11:26   ` Jan Beulich
2016-12-01 16:55 ` [PATCH v4 10/24] x86/emul: Always use fault semantics for software events Andrew Cooper
2016-12-01 19:07   ` Boris Ostrovsky
2016-12-02 11:28   ` Jan Beulich
2016-12-01 16:55 ` [PATCH v4 11/24] x86/emul: Implement singlestep as a retire flag Andrew Cooper
2016-12-02 11:32   ` Jan Beulich
2016-12-01 16:56 ` Andrew Cooper [this message]
2016-12-01 16:56 ` [PATCH v4 18/24] x86/shadow: Avoid raising faults behind the emulators back Andrew Cooper
2016-12-02 11:37   ` Jan Beulich
2016-12-02 11:48     ` Andrew Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1480611361-15294-5-git-send-email-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).