Changeset 92480 in vbox

Timestamp:

Nov 17, 2021 2:01:29 PM (4 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

148339

Message:

VMM: Nested VMX: bugref:10092 EPT misconfiguration and some fixes and adjustments to EPT SLAT code.

Location:

trunk/src/VBox/VMM

Files:

• VMMAll/PGMAllGstSlatEpt.cpp.h (modified) (11 diffs)
• VMMR3/PGM.cpp (modified) (2 diffs)
• include/PGMInternal.h (modified) (2 diffs)

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

@@ -17 +17 @@
 
 #if PGM_GST_TYPE == PGM_TYPE_EPT
+DECLINLINE(bool) PGM_GST_SLAT_NAME_EPT(WalkIsPermValid)(PCVMCPUCC pVCpu, uint64_t uEntry)
+{
+    if (!(uEntry & VMX_BF_EPT_PT_READ_MASK))
+    {
+        if (uEntry & VMX_BF_EPT_PT_WRITE_MASK)
+            return false;
+
+        Assert(!pVCpu->CTX_SUFF(pVM)->cpum.ro.GuestFeatures.fVmxModeBasedExecuteEpt);
+        if (   !RT_BF_GET(pVCpu->pgm.s.uEptVpidCapMsr, VMX_BF_EPT_VPID_CAP_EXEC_ONLY)
+            && (uEntry & VMX_BF_EPT_PT_EXECUTE_MASK))
+            return false;
+    }
+    return true;
+}
+
+
+DECLINLINE(bool) PGM_GST_SLAT_NAME_EPT(WalkIsMemTypeValid)(uint64_t uEntry, uint8_t uLevel)
+{
+    Assert(uLevel >= 3 && uLevel <= 1); NOREF(uLevel);
+    uint64_t const fEptMemTypeMask = uEntry & VMX_BF_EPT_PT_MEMTYPE_MASK;
+    if (   fEptMemTypeMask == EPT_E_MEMTYPE_INVALID_2
+        || fEptMemTypeMask == EPT_E_MEMTYPE_INVALID_3
+        || fEptMemTypeMask == EPT_E_MEMTYPE_INVALID_7)
+        return false;
+    return true;
+}
+
+
 DECLINLINE(int) PGM_GST_SLAT_NAME_EPT(WalkReturnNotPresent)(PCVMCPUCC pVCpu, PPGMPTWALK pWalk, uint64_t uEntry, uint8_t uLevel)
 {
@@ -43 +71 @@
 {
     NOREF(pVCpu);
-    pWalk->fRsvdError = true;
-    pWalk->uLevel     = uLevel;
+    pWalk->fRsvdError  = true;
+    pWalk->uLevel      = uLevel;
+    pWalk->enmSlatFail = PGMSLATFAIL_EPT_MISCONFIG;
     return VERR_PAGE_TABLE_NOT_PRESENT;
 }
@@ -52 +81 @@
                                             PPGMPTWALK pWalk, PGSTPTWALK pGstWalk)
 {
-    /** @todo implement figuring out fEptMisconfig. */
     /*
      * Init walk structures.
@@ -87 +115 @@
      * Do the walk.
      */
+    int const rc2 = pgmGstGetEptPML4PtrEx(pVCpu, &pGstWalk->pPml4);
+    if (RT_SUCCESS(rc2))
+    { /* likely */ }
+    else
+        return PGM_GST_SLAT_NAME_EPT(WalkReturnBadPhysAddr)(pVCpu, pWalk, 4, rc2);
+
     uint64_t fEffective;
     {
-        int rc = pgmGstGetEptPML4PtrEx(pVCpu, &pGstWalk->pPml4);
-        if (RT_SUCCESS(rc)) { /* probable */ }
-        else return PGM_GST_SLAT_NAME_EPT(WalkReturnBadPhysAddr)(pVCpu, pWalk, 4, rc);
-
+        /*
+         * PML4E.
+         */
         PEPTPML4E pPml4e;
         pGstWalk->pPml4e = pPml4e = &pGstWalk->pPml4->a[(GCPhysNested >> EPT_PML4_SHIFT) & EPT_PML4_MASK];
@@ -116 +149 @@
         pWalk->fEffective = fEffective;
 
-        rc = PGM_GCPHYS_2_PTR_BY_VMCPU(pVCpu, Pml4e.u & EPT_PML4E_PG_MASK, &pGstWalk->pPdpt);
+        int const rc = PGM_GCPHYS_2_PTR_BY_VMCPU(pVCpu, Pml4e.u & EPT_PML4E_PG_MASK, &pGstWalk->pPdpt);
         if (RT_SUCCESS(rc)) { /* probable */ }
         else return PGM_GST_SLAT_NAME_EPT(WalkReturnBadPhysAddr)(pVCpu, pWalk, 3, rc);
     }
     {
+        /*
+         * PDPTE.
+         */
         PEPTPDPTE pPdpte;
         pGstWalk->pPdpte = pPdpte = &pGstWalk->pPdpt->a[(GCPhysNested >> GST_PDPT_SHIFT) & GST_PDPT_MASK];
@@ -143 +179 @@
             pWalk->fEffective = fEffective;
         }
-        else if (GST_IS_BIG_PDPE_VALID(pVCpu, Pdpte))
+        else if (   GST_IS_BIG_PDPE_VALID(pVCpu, Pdpte)
+                 && PGM_GST_SLAT_NAME_EPT(WalkIsMemTypeValid)(Pdpte.u, 3))
         {
             uint64_t const fEptAttrs     = Pdpte.u & EPT_PDPTE1G_ATTR_MASK;
@@ -168 +205 @@
         }
         else return PGM_GST_SLAT_NAME_EPT(WalkReturnRsvdError)(pVCpu, pWalk, 3);
-    }
-    {
+
+        int const rc = PGM_GCPHYS_2_PTR_BY_VMCPU(pVCpu, Pdpte.u & EPT_PDPTE_PG_MASK, &pGstWalk->pPd);
+        if (RT_SUCCESS(rc)) { /* probable */ }
+        else return PGM_GST_SLAT_NAME_EPT(WalkReturnBadPhysAddr)(pVCpu, pWalk, 3, rc);
+    }
+    {
+        /*
+         * PDE.
+         */
         PGSTPDE pPde;
         pGstWalk->pPde  = pPde  = &pGstWalk->pPd->a[(GCPhysNested >> GST_PD_SHIFT) & GST_PD_MASK];
@@ -178 +222 @@
         else return PGM_GST_SLAT_NAME_EPT(WalkReturnNotPresent)(pVCpu, pWalk, Pde.u, 2);
 
-        if ((Pde.u & X86_PDE_PS) && GST_IS_PSE_ACTIVE(pVCpu))
+        /* The order of the following 2 "if" statements matter. */
+        if (GST_IS_PDE_VALID(pVCpu, Pde))
         {
-            if (RT_LIKELY(GST_IS_BIG_PDE_VALID(pVCpu, Pde))) { /* likely */ }
-            else return PGM_GST_SLAT_NAME_EPT(WalkReturnRsvdError)(pVCpu, pWalk, 2);
-
+            uint64_t const fEptAttrs     = Pde.u & EPT_PDE_ATTR_MASK;
+            uint8_t const fRead          = RT_BF_GET(fEptAttrs, VMX_BF_EPT_PT_READ);
+            uint8_t const fWrite         = RT_BF_GET(fEptAttrs, VMX_BF_EPT_PT_WRITE);
+            uint8_t const fAccessed      = RT_BF_GET(fEptAttrs, VMX_BF_EPT_PT_ACCESSED);
+            uint64_t const fEffectiveEpt = (fEptAttrs << PGM_PTATTRS_EPT_SHIFT) & PGM_PTATTRS_EPT_MASK;
+            fEffective &= RT_BF_MAKE(PGM_PTATTRS_R, fRead)
+                        | RT_BF_MAKE(PGM_PTATTRS_W, fWrite)
+                        | RT_BF_MAKE(PGM_PTATTRS_A, fAccessed)
+                        | (fEffectiveEpt & fCumulativeEpt);
+            pWalk->fEffective = fEffective;
+
+        }
+        else if (   GST_IS_BIG_PDE_VALID(pVCpu, Pde)
+                 && PGM_GST_SLAT_NAME_EPT(WalkIsMemTypeValid)(Pde.u, 2))
+        {
             uint64_t const fEptAttrs     = Pde.u & EPT_PDE2M_ATTR_MASK;
             uint8_t const fRead          = RT_BF_GET(fEptAttrs, VMX_BF_EPT_PT_READ);
@@ -205 +262 @@
             return VINF_SUCCESS;
         }
-
-        if (RT_UNLIKELY(!GST_IS_PDE_VALID(pVCpu, Pde)))
-            return PGM_GST_SLAT_NAME_EPT(WalkReturnRsvdError)(pVCpu, pWalk, 2);
-
-        uint64_t const fEptAttrs     = Pde.u & EPT_PDE_ATTR_MASK;
-        uint8_t const fRead          = RT_BF_GET(fEptAttrs, VMX_BF_EPT_PT_READ);
-        uint8_t const fWrite         = RT_BF_GET(fEptAttrs, VMX_BF_EPT_PT_WRITE);
-        uint8_t const fAccessed      = RT_BF_GET(fEptAttrs, VMX_BF_EPT_PT_ACCESSED);
-        uint64_t const fEffectiveEpt = (fEptAttrs << PGM_PTATTRS_EPT_SHIFT) & PGM_PTATTRS_EPT_MASK;
-        fEffective &= RT_BF_MAKE(PGM_PTATTRS_R, fRead)
-                    | RT_BF_MAKE(PGM_PTATTRS_W, fWrite)
-                    | RT_BF_MAKE(PGM_PTATTRS_A, fAccessed)
-                    | (fEffectiveEpt & fCumulativeEpt);
-        pWalk->fEffective = fEffective;
+        else return PGM_GST_SLAT_NAME_EPT(WalkReturnRsvdError)(pVCpu, pWalk, 2);
 
         int const rc = PGM_GCPHYS_2_PTR_BY_VMCPU(pVCpu, GST_GET_PDE_GCPHYS(Pde), &pGstWalk->pPt);
@@ -225 +269 @@
     }
     {
+        /*
+         * PTE.
+         */
         PGSTPTE pPte;
         pGstWalk->pPte  = pPte  = &pGstWalk->pPt->a[(GCPhysNested >> GST_PT_SHIFT) & GST_PT_MASK];
@@ -233 +280 @@
         else return PGM_GST_SLAT_NAME_EPT(WalkReturnNotPresent)(pVCpu, pWalk, Pte.u, 1);
 
-        if (RT_LIKELY(GST_IS_PTE_VALID(pVCpu, Pte))) { /* likely */ }
-        else return PGM_GST_SLAT_NAME_EPT(WalkReturnRsvdError)(pVCpu, pWalk, 1);
+        if (   GST_IS_PTE_VALID(pVCpu, Pte)
+            && PGM_GST_SLAT_NAME_EPT(WalkIsMemTypeValid)(Pte.u, 1))
+        { /* likely*/  }
+        else
+            return PGM_GST_SLAT_NAME_EPT(WalkReturnRsvdError)(pVCpu, pWalk, 1);
 
         uint64_t const fEptAttrs     = Pte.u & EPT_PTE_ATTR_MASK;

trunk/src/VBox/VMM/VMMR3/PGM.cpp

@@ -1621 +1621 @@
 
 #ifdef VBOX_WITH_NESTED_HWVIRT_VMX_EPT
+        pVCpu->pgm.s.uEptVpidCapMsr           = fEptVpidCap;
         pVCpu->pgm.s.fGstEptMbzPteMask        = fMbzPageFrameMask | EPT_PTE_MBZ_MASK;
         pVCpu->pgm.s.fGstEptMbzPdeMask        = fMbzPageFrameMask | EPT_PDE_MBZ_MASK;
@@ -1632 +1633 @@
                && !pVM->cpum.ro.GuestFeatures.fVmxSppEpt
                && !pVM->cpum.ro.GuestFeatures.fVmxEptXcptVe);
-        pVCpu->pgm.s.fGstEptPresentMask         = EPT_E_READ | EPT_E_WRITE | EPT_E_EXECUTE;
-        pVCpu->pgm.s.fGstEptShadowedPml4eMask   = EPT_E_READ | EPT_E_WRITE | EPT_E_EXECUTE | EPT_E_ACCESSED;
-        pVCpu->pgm.s.fGstEptShadowedPdpeMask    = EPT_E_READ | EPT_E_WRITE | EPT_E_EXECUTE | EPT_E_ACCESSED;
-        pVCpu->pgm.s.fGstEptShadowedBigPdpeMask = EPT_E_READ | EPT_E_WRITE | EPT_E_EXECUTE | EPT_E_ACCESSED | EPT_E_DIRTY;
-        pVCpu->pgm.s.fGstEptShadowedPdeMask     = EPT_E_READ | EPT_E_WRITE | EPT_E_EXECUTE | EPT_E_ACCESSED;
-        pVCpu->pgm.s.fGstEptShadowedBigPdeMask  = EPT_E_READ | EPT_E_WRITE | EPT_E_EXECUTE | EPT_E_ACCESSED | EPT_E_DIRTY;
-        pVCpu->pgm.s.fGstEptShadowedPteMask     = EPT_E_READ | EPT_E_WRITE | EPT_E_EXECUTE | EPT_E_ACCESSED | EPT_E_DIRTY;
+        pVCpu->pgm.s.fGstEptPresentMask       = EPT_E_READ | EPT_E_WRITE | EPT_E_EXECUTE;
 #endif
     }

trunk/src/VBox/VMM/include/PGMInternal.h

@@ -3436 +3436 @@
     /** The guest's EPT pointer (copy of virtual VMCS). */
     uint64_t                        uEptPtr;
+    /** Copy of the VM's IA32_VMX_EPT_VPID_CAP VPID MSR for faster access. Doesn't
+     *  change through the lifetime of the VM. */
+    uint64_t                        uEptVpidCapMsr;
     /** Mask containing the MBZ PTE bits. */
     uint64_t                        fGstEptMbzPteMask;
@@ -3450 +3453 @@
     /** Mask to determine whether an entry is present. */
     uint64_t                        fGstEptPresentMask;
-    /** Mask containing the PML4E bits that we shadow. */
-    uint64_t                        fGstEptShadowedPml4eMask;
-    /** Mask containing the PDPE bits that we shadow. */
-    uint64_t                        fGstEptShadowedPdpeMask;
-    /** Mask containing the big page PDPE bits that we shadow. */
-    uint64_t                        fGstEptShadowedBigPdpeMask;
-    /** Mask containing the PDE bits that we shadow. */
-    uint64_t                        fGstEptShadowedPdeMask;
-    /** Mask containing the big page PDE bits that we shadow. */
-    uint64_t                        fGstEptShadowedBigPdeMask;
-    /** Mask containing the PTE bits that we shadow. */
-    uint64_t                        fGstEptShadowedPteMask;
     /** @} */