x86/svm: Fix svm_update_guest_efer() for domains using shadow paging

When using shadow paging, EFER.NX is a Xen controlled bit, and is required by
the shadow pagefault handler to distinguish instruction fetches from data
accesses.

This can be observed by a guest which has NX and SMEP clear but SMAP active by
attempting to execute code on a user mapping.  The first attempt to build the
target shadow will #PF so is handled by the shadow code, but when walking the
the guest pagetables, the lack of PFEC_insn_fetch being signalled causes the
shadow code to mistake the instruction fetch for a data fetch, and believe
that it is a real guest fault.  As a result, the guest receives #PF[-d-srP]
for an action which should complete successfully.

The suspicious-looking gymnastics with LME is actually a subtle corner case
with shadow paging.  When dropping out of Long Mode, a guests choice of LME
and Xen's choice of CR0.PG cause hardware to operate in Long Mode, but the
shadow code to operate in 2-on-3 mode.

In addition to describing this corner case in the SVM side, extend the comment
for the same fix on the VT-x side.  (I have a suspicion that I've just worked
out why VT-x doesn't tolerate LMA != LME when Unrestricted Guest is clear.)

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index fa18cc07fd5b3f7437598ffe98b19a9eaf438469..dd0aca4f53abe78954b1b112c4ed8f5ed769cf76 100644 (file)
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -638,13 +638,32 @@ void svm_update_guest_cr(struct vcpu *v, unsigned int cr, unsigned int flags)
 static void svm_update_guest_efer(struct vcpu *v)
 {
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
-    bool lma = v->arch.hvm.guest_efer & EFER_LMA;
-    uint64_t new_efer;
+    unsigned long guest_efer = v->arch.hvm.guest_efer,
+        xen_efer = read_efer();
 
-    new_efer = (v->arch.hvm.guest_efer | EFER_SVME) & ~EFER_LME;
-    if ( lma )
-        new_efer |= EFER_LME;
-    vmcb_set_efer(vmcb, new_efer);
+    if ( paging_mode_shadow(v->domain) )
+    {
+        /* EFER.NX is a Xen-owned bit and is not under guest control. */
+        guest_efer &= ~EFER_NX;
+        guest_efer |= xen_efer & EFER_NX;
+
+        /*
+         * CR0.PG is a Xen-owned bit, and remains set even when the guest has
+         * logically disabled paging.
+         *
+         * LMA was calculated using the guest CR0.PG setting, but LME needs
+         * clearing to avoid interacting with Xen's CR0.PG setting.  As writes
+         * to CR0 are intercepted, it is safe to leave LME clear at this
+         * point, and fix up both LME and LMA when CR0.PG is set.
+         */
+        if ( !(guest_efer & EFER_LMA) )
+            guest_efer &= ~EFER_LME;
+    }
+
+    /* SVME must remain set in non-root mode. */
+    guest_efer |= EFER_SVME;
+
+    vmcb_set_efer(vmcb, guest_efer);
 
     ASSERT(nestedhvm_enabled(v->domain) ||
            !(v->arch.hvm.guest_efer & EFER_SVME));

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index c85aa62ce7dc727685cc55301388acd7b2f6f44f..d16129fb59c108b672b2ea2b8973f820aa4eaad4 100644 (file)
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1654,6 +1654,12 @@ static void vmx_update_guest_efer(struct vcpu *v)
          * not tolerate the LME and LMA settings being different.  As writes
          * to CR0 are intercepted, it is safe to leave LME clear at this
          * point, and fix up both LME and LMA when CR0.PG is set.
+         *
+         * Furthermore, when using shadow pagetables (subsumed by the
+         * Unrestricted Guest check), CR0.PG is a Xen-owned bit, and remains
+         * set even when the guest has logically disabled paging.  LMA was
+         * calculated using the guest CR0.PG setting, but LME needs clearing
+         * to avoid interacting with Xen's CR0.PG setting.
          */
         if ( !(guest_efer & EFER_LMA) )
             guest_efer &= ~EFER_LME;