Changeset 71755 in vbox for trunk/src/VBox/VMM/VMMAll

Timestamp:

Apr 9, 2018 8:10:23 AM (7 years ago)

Author:

vboxsync

Message:

VMM: Nested Hw.virt: Fix overriding SVM nested-guest PAT MSR while executing with nested-guest w/ shadow paging.
Also fixes loading, validating and restoring the PAT MSR when nested-paging is used by the nested-hypervisor.

Location:

trunk/src/VBox/VMM/VMMAll

Files:

• CPUMAllMsrs.cpp (modified) (2 diffs)
• HMSVMAll.cpp (modified) (11 diffs)
• IEMAllCImplSvmInstr.cpp.h (modified) (4 diffs)

trunk/src/VBox/VMM/VMMAll/CPUMAllMsrs.cpp

@@ -551 +551 @@
 {
     RT_NOREF_PV(idMsr); RT_NOREF_PV(pRange); RT_NOREF_PV(uRawValue);
-
-    for (uint32_t cShift = 0; cShift < 63; cShift += 8)
+    if (CPUMIsPatMsrValid(uValue))
     {
-        /* Check all eight bits because the top 5 bits of each byte are reserved. */
-        uint8_t uType = (uint8_t)(uValue >> cShift);
-        if ((uType >= 8) || (uType == 2) || (uType == 3))
-        {
-            Log(("CPUM: Invalid PAT type at %u:%u in IA32_PAT: %#llx (%#llx)\n",
-                 cShift + 7, cShift, uValue, uType));
-            return VERR_CPUM_RAISE_GP_0;
-        }
+        pVCpu->cpum.s.Guest.msrPAT = uValue;
+        return VINF_SUCCESS;
     }
-
-    pVCpu->cpum.s.Guest.msrPAT = uValue;
-    return VINF_SUCCESS;
+    return VERR_CPUM_RAISE_GP_0;
 }
 
@@ -6209 +6200 @@
 
 /**
+ * Checks if a guest PAT MSR write is valid.
+ *
+ * @returns @c true if the PAT bit combination is valid, @c false otherwise.
+ * @param   uValue      The PAT MSR value.
+ */
+VMMDECL(bool) CPUMIsPatMsrValid(uint64_t uValue)
+{
+    for (uint32_t cShift = 0; cShift < 63; cShift += 8)
+    {
+        /* Check all eight bits because the top 5 bits of each byte are reserved. */
+        uint8_t uType = (uint8_t)(uValue >> cShift);
+        if ((uType >= 8) || (uType == 2) || (uType == 3))
+        {
+            Log(("CPUM: Invalid PAT type at %u:%u in IA32_PAT: %#llx (%#llx)\n", cShift + 7, cShift, uValue, uType));
+            return false;
+        }
+    }
+    return true;
+}
+
+
+/**
  * Validates an EFER MSR write.
  *

trunk/src/VBox/VMM/VMMAll/HMSVMAll.cpp

@@ -132 +132 @@
  * in IEM).
  *
- * @param   pVCpu           The cross context virtual CPU structure.
- * @param   pCtx            Pointer to the guest-CPU context.
+ * @param   pVCpu   The cross context virtual CPU structure.
+ * @param   pCtx    Pointer to the guest-CPU context.
  *
  * @sa      hmR0SvmVmRunCacheVmcb.
@@ -169 +169 @@
         pVmcbNstGstCtrl->IntCtrl.n.u1VIntrMasking      = pNstGstVmcbCache->fVIntrMasking;
         pVmcbNstGstCtrl->TLBCtrl                       = pNstGstVmcbCache->TLBCtrl;
+
+        /*
+         * If the nested-hypervisor isn't using nested-paging (and thus shadow paging
+         * is used by HM), we restore the original PAT MSR from the nested-guest VMCB.
+         * Otherwise, the nested-guest-CPU PAT MSR would've already been saved here by
+         * hardware-assisted SVM or by IEM.
+         */
+        if (!pNstGstVmcbCache->u1NestedPaging)
+            pVmcbNstGstState->u64PAT = pNstGstVmcbCache->u64PAT;
+
         pVmcbNstGstCtrl->NestedPaging.n.u1NestedPaging = pNstGstVmcbCache->u1NestedPaging;
         pVmcbNstGstCtrl->LbrVirt.n.u1LbrVirt           = pNstGstVmcbCache->u1LbrVirt;
@@ -177 +187 @@
      * Currently, VMRUN, #VMEXIT transitions involves trips to ring-3 that would flag a full
      * CPU state change. However, if we exit to ring-3 in response to receiving a physical
-     * interrupt, we skip signaling any CPU state change as normally no change
-     * is done to the execution state (see VINF_EM_RAW_INTERRUPT handling in hmR0SvmExitToRing3).
-     * However, with nested-guests, the state can change for e.g., we might perform a
-     * SVM_EXIT_INTR #VMEXIT for the nested-guest in ring-3. Hence we signal a full CPU
-     * state change here.
+     * interrupt, we skip signaling any CPU state change as normally no change is done to the
+     * execution state (see VINF_EM_RAW_INTERRUPT handling in hmR0SvmExitToRing3).
+     *
+     * With nested-guests, the state can change on trip to ring-3 for e.g., we might perform a
+     * SVM_EXIT_INTR #VMEXIT for the nested-guest in ring-3. Hence we signal a full CPU state
+     * change here.
      */
     HMCPU_CF_SET(pVCpu, HM_CHANGED_ALL_GUEST);
@@ -439 +450 @@
 
 /**
- * Checks if the guest VMCB has the specified ctrl/instruction intercept active.
+ * Checks if the nested-guest VMCB has the specified ctrl/instruction intercept
+ * active.
  *
  * @returns @c true if in intercept is set, @c false otherwise.
@@ -456 +468 @@
 
 /**
- * Checks if the guest VMCB has the specified CR read intercept active.
+ * Checks if the nested-guest VMCB has the specified CR read intercept active.
  *
  * @returns @c true if in intercept is set, @c false otherwise.
@@ -473 +485 @@
 
 /**
- * Checks if the guest VMCB has the specified CR write intercept
- * active.
+ * Checks if the nested-guest VMCB has the specified CR write intercept active.
  *
  * @returns @c true if in intercept is set, @c false otherwise.
@@ -491 +502 @@
 
 /**
- * Checks if the guest VMCB has the specified DR read intercept
- * active.
+ * Checks if the nested-guest VMCB has the specified DR read intercept active.
  *
  * @returns @c true if in intercept is set, @c false otherwise.
@@ -509 +519 @@
 
 /**
- * Checks if the guest VMCB has the specified DR write intercept active.
+ * Checks if the nested-guest VMCB has the specified DR write intercept active.
  *
  * @returns @c true if in intercept is set, @c false otherwise.
@@ -526 +536 @@
 
 /**
- * Checks if the guest VMCB has the specified exception intercept active.
+ * Checks if the nested-guest VMCB has the specified exception intercept active.
  *
  * @returns true if in intercept is active, false otherwise.
@@ -543 +553 @@
 
 /**
- * Checks if the guest VMCB has virtual-interrupts masking enabled.
+ * Checks if the nested-guest VMCB has virtual-interrupts masking enabled.
  *
  * @returns true if virtual-interrupts are masked, @c false otherwise.
@@ -554 +564 @@
     PCSVMNESTEDVMCBCACHE pVmcbNstGstCache = &pVCpu->hm.s.svm.NstGstVmcbCache;
     return pVmcbNstGstCache->fVIntrMasking;
+}
+
+
+/**
+ * Checks if the nested-guest VMCB has nested-paging enabled.
+ *
+ * @returns true if nested-paging is enabled, @c false otherwise.
+ * @param   pVCpu   The cross context virtual CPU structure of the calling EMT.
+ * @param   pCtx    Pointer to the context.
+ */
+VMM_INT_DECL(bool) HMIsGuestSvmNestedPagingEnabled(PVMCPU pVCpu, PCCPUMCTX pCtx)
+{
+    Assert(pCtx->hwvirt.svm.fHMCachedVmcb); NOREF(pCtx);
+    PCSVMNESTEDVMCBCACHE pVmcbNstGstCache = &pVCpu->hm.s.svm.NstGstVmcbCache;
+    return RT_BOOL(pVmcbNstGstCache->u1NestedPaging);
 }
 

trunk/src/VBox/VMM/VMMAll/IEMAllCImplSvmInstr.cpp.h

@@ -152 +152 @@
         pVmcbNstGstState->u8CPL         = pCtx->ss.Attr.n.u2Dpl;   /* See comment in CPUMGetGuestCPL(). */
         Assert(CPUMGetGuestCPL(pVCpu) == pCtx->ss.Attr.n.u2Dpl);
+        if (CPUMIsGuestSvmNestedPagingEnabled(pVCpu, pCtx))
+            pVmcbNstGstState->u64PAT = pCtx->msrPAT;
 
         PSVMVMCBCTRL pVmcbCtrl = &pCtx->hwvirt.svm.CTX_SUFF(pVmcb)->ctrl;
@@ -180 +182 @@
         else
             pVmcbCtrl->IntCtrl.n.u1VIrqPending = 0;
-
-        /** @todo NRIP. */
 
         /* Save exit information. */
@@ -459 +459 @@
         }
 
-        /** @todo gPAT MSR validation? */
+        /*
+         * PAT (Page Attribute Table) MSR.
+         *
+         * The CPU only validates and loads it when nested-paging is enabled.
+         * See AMD spec. "15.25.4 Nested Paging and VMRUN/#VMEXIT".
+         */
+        if (   pVmcbCtrl->NestedPaging.n.u1NestedPaging
+            && !CPUMIsPatMsrValid(pVmcbNstGst->u64PAT))
+        {
+            Log(("iemSvmVmrun: PAT invalid. u64PAT=%#RX64 -> #VMEXIT\n", pVmcbNstGst->u64PAT));
+            return iemSvmVmexit(pVCpu, pCtx, SVM_EXIT_INVALID, 0 /* uExitInfo1 */, 0 /* uExitInfo2 */);
+        }
 
         /*
@@ -614 +625 @@
         pCtx->rip        = pVmcbNstGst->u64RIP;
         CPUMSetGuestMsrEferNoCheck(pVCpu, pCtx->msrEFER, uValidEfer);
+        if (pVmcbCtrl->NestedPaging.n.u1NestedPaging)
+            pCtx->msrPAT = pVmcbNstGst->u64PAT;
 
         /* Mask DR6, DR7 bits mandatory set/clear bits. */