Changeset 99132 in vbox

Timestamp:

Mar 23, 2023 9:00:20 AM (22 months ago)

Author:

vboxsync

Message:

VMM: Nested VMX: bugref:10318 PGM fixes for supporting Hyper-V in a VM using hardware-assisted execution.

Location:

trunk/src/VBox/VMM/VMMAll

Files:

• PGMAllBth.h (modified) (5 diffs)
• PGMAllPool.cpp (modified) (2 diffs)
• PGMAllShw.h (modified) (3 diffs)

trunk/src/VBox/VMM/VMMAll/PGMAllBth.h

@@ -1185 +1185 @@
 #  endif
 
+    Log7Func(("SLAT: GCPhysNestedFault=%RGp -> GCPhys=%#RGp\n", GCPhysNestedFault, pWalk->GCPhys));
+
     /*
      * Check page-access permissions.
@@ -1253 +1255 @@
     PPGMPAGE pPage;
     rc = pgmPhysGetPageEx(pVM, GCPhysPage, &pPage);
-    AssertRCReturn(rc, rc);
+    if (RT_FAILURE(rc))
+    {
+        /*
+         * We failed to get the physical page which means it's a reserved/invalid
+         * page address (not MMIO even). This can typically be observed with
+         * Microsoft Hyper-V enabled Windows guests. We must fall back to emulating
+         * the instruction, see @bugref{10318#c7}.
+         */
+        return VINF_EM_RAW_EMULATE_INSTR;
+    }
+    /* Check if this is an MMIO page and NOT the VMX APIC-access page. */
     if (PGM_PAGE_HAS_ACTIVE_HANDLERS(pPage) && !PGM_PAGE_IS_HNDL_PHYS_NOT_IN_HM(pPage))
     {
@@ -1296 +1308 @@
         {
             /* This is a read-only page. */
-            AssertMsgFailed(("Failed\n"));
-
-            Assert(!PGM_PAGE_IS_ZERO(pPage));
             AssertFatalMsg(!PGM_PAGE_IS_BALLOONED(pPage), ("Unexpected ballooned page at %RGp\n", GCPhysPage));
             STAM_STATS({ pVCpu->pgmr0.s.pStatTrap0eAttributionR0 = &pVCpu->pgm.s.Stats.StatRZTrap0eTime2MakeWritable; });
@@ -2544 +2553 @@
     PPGMPAGE pPage;
     int rc = pgmPhysGetPageEx(pVCpu->CTX_SUFF(pVM), GCPhysPage, &pPage);
-    AssertRCReturnVoid(rc);
+    if (RT_SUCCESS(rc))
+    { /* likely */ }
+    else
+    {
+        /*
+         * This is a RAM hole/invalid/reserved address (not MMIO).
+         * Nested Microsoft Hyper-V maps addresses like 0xf0220000 as RW WB memory.
+         * Shadow a not-present page similar to MMIO, see @bugref{10318#c7}.
+         */
+        Assert(rc == VERR_PGM_INVALID_GC_PHYSICAL_ADDRESS);
+        if (SHW_PTE_IS_P(*pPte))
+        {
+            Log2(("SyncPageWorker: deref! *pPte=%RX64\n", SHW_PTE_LOG64(*pPte)));
+            PGM_BTH_NAME(SyncPageWorkerTrackDeref)(pVCpu, pShwPage, SHW_PTE_GET_HCPHYS(*pPte), iPte, NIL_RTGCPHYS);
+        }
+        Log7Func(("RAM hole/reserved %RGp -> ShwPte=0\n", GCPhysPage));
+        SHW_PTE_ATOMIC_SET(*pPte, 0);
+        return;
+    }
 
     Assert(!PGM_PAGE_IS_BALLOONED(pPage));
-
-# ifndef VBOX_WITH_NEW_LAZY_PAGE_ALLOC
-    /* Make the page writable if necessary. */
-    /** @todo This needs to be applied to the regular case below, not here. And,
-     *        no we should *NOT* make the page writble, instead we need to write
-     *        protect them if necessary. */
-    if (    PGM_PAGE_GET_TYPE(pPage)  == PGMPAGETYPE_RAM
-        &&  (   PGM_PAGE_IS_ZERO(pPage)
-             || (   (pGstWalkAll->u.Ept.Pte.u & EPT_E_WRITE)
-                 && PGM_PAGE_GET_STATE(pPage) != PGM_PAGE_STATE_ALLOCATED
-#  ifdef VBOX_WITH_REAL_WRITE_MONITORED_PAGES
-                 && PGM_PAGE_GET_STATE(pPage) != PGM_PAGE_STATE_WRITE_MONITORED
-#  endif
-#  ifdef VBOX_WITH_PAGE_SHARING
-                 && PGM_PAGE_GET_STATE(pPage) != PGM_PAGE_STATE_SHARED
-#  endif
-                 && PGM_PAGE_GET_STATE(pPage) != PGM_PAGE_STATE_BALLOONED
-             )
-          )
-       )
-    {
-        AssertMsgFailed(("GCPhysPage=%RGp\n", GCPhysPage)); /** @todo Shouldn't happen but if it does deal with it later. */
-    }
-# endif
 
     /*
@@ -2579 +2582 @@
     if (!PGM_PAGE_HAS_ACTIVE_HANDLERS(pPage) || PGM_PAGE_IS_HNDL_PHYS_NOT_IN_HM(pPage))
     {
-        /** @todo access bit. */
-        Pte.u = PGM_PAGE_GET_HCPHYS(pPage) | fGstShwPteFlags;
-        Log7Func(("regular page (%R[pgmpage]) at %RGp -> %RX64\n", pPage, GCPhysPage, Pte.u));
+# ifndef VBOX_WITH_NEW_LAZY_PAGE_ALLOC
+        /* Page wasn't allocated, write protect it. */
+        if (    PGM_PAGE_GET_TYPE(pPage)  == PGMPAGETYPE_RAM
+            &&  (   PGM_PAGE_IS_ZERO(pPage)
+                 || (   (pGstWalkAll->u.Ept.Pte.u & EPT_E_WRITE)
+                     && PGM_PAGE_GET_STATE(pPage) != PGM_PAGE_STATE_ALLOCATED
+#  ifdef VBOX_WITH_REAL_WRITE_MONITORED_PAGES
+                     && PGM_PAGE_GET_STATE(pPage) != PGM_PAGE_STATE_WRITE_MONITORED
+#  endif
+#  ifdef VBOX_WITH_PAGE_SHARING
+                     && PGM_PAGE_GET_STATE(pPage) != PGM_PAGE_STATE_SHARED
+#  endif
+                     && PGM_PAGE_GET_STATE(pPage) != PGM_PAGE_STATE_BALLOONED
+                 )
+              )
+           )
+        {
+            Pte.u = PGM_PAGE_GET_HCPHYS(pPage) | (fGstShwPteFlags & ~EPT_E_WRITE);
+            Log7Func(("zero page (%R[pgmpage]) at %RGp -> %RX64\n", pPage, GCPhysPage, Pte.u));
+        }
+        else
+# endif
+        {
+            /** @todo access bit. */
+            Pte.u = PGM_PAGE_GET_HCPHYS(pPage) | fGstShwPteFlags;
+            Log7Func(("regular page (%R[pgmpage]) at %RGp -> %RX64\n", pPage, GCPhysPage, Pte.u));
+        }
     }
     else if (!PGM_PAGE_HAS_ACTIVE_ALL_HANDLERS(pPage))

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

@@ -3441 +3441 @@
 #ifdef VBOX_WITH_NESTED_HWVIRT_VMX_EPT
         case PGMPOOLKIND_EPT_PT_FOR_EPT_PT:
+# ifdef PGM_WITH_LARGE_PAGES
+        case PGMPOOLKIND_EPT_PT_FOR_EPT_2MB:
+# endif
 #endif
         {
@@ -3513 +3516 @@
         /* Large page case only. */
         case PGMPOOLKIND_EPT_PD_FOR_PHYS:
-#ifdef VBOX_WITH_NESTED_HWVIRT_VMX_EPT
-        case PGMPOOLKIND_EPT_PT_FOR_EPT_2MB:    /* X86_PDE4M_PS is same as leaf bit in EPT; be careful! */
-#endif
         {
             Assert(pVM->pgm.s.fNestedPaging);

trunk/src/VBox/VMM/VMMAll/PGMAllShw.h

@@ -319 +319 @@
 
 
+#if 0
+PGM_SHW_DECL(int, NestedGetPage)(PVMCPUCC pVCpu, PEPTPD pEptPd, PPGMPTWALK pWalk, uint64_t *pfFlags, PRTHCPHYS pHCPhys)
+{
+#if PGM_SHW_TYPE == PGM_TYPE_EPT
+    RTGCPHYS const GCPhysNested = pWalk->GCPhysNested;
+    unsigned const iEptPd = ((GCPhysNested >> SHW_PD_SHIFT) & SHW_PD_MASK);
+    Assert(iEptPd < EPT_PG_ENTRIES);
+    SHWPDE EptPde = pEptPd->a[iEptPd];
+    if (!SHW_PDE_IS_P(EptPde))
+    {
+        *pfFlags = 0;
+        *pHCPhys = NIL_RTHCPHYS;
+        return VERR_PAGE_TABLE_NOT_PRESENT;
+    }
+
+    if (SHW_PDE_IS_BIG(EptPde))
+    {
+        Assert(pWalk->fBigPage);
+        if (pfFlags)
+            *pfFlags = (EptPde.u & ~SHW_PDE_PG_MASK);
+        if (pHCPhys)
+            *pHCPhys = (EptPde.u & EPT_PDE2M_PG_MASK) + (pWalk->GCPhys & (RT_BIT(EPT_PD_SHIFT) - 1) & X86_PAGE_4K_BASE_MASK);
+        return VINF_SUCCESS;
+    }
+
+    PSHWPT pEptPt;
+    int const rc = PGM_HCPHYS_2_PTR(pVCpu->CTX_SUFF(pVM), pVCpu, EptPde.u & EPT_PDE_PG_MASK, &pEptPt);
+    if (RT_FAILURE(rc))
+    {
+        *pfFlags = 0;
+        *pHCPhys = NIL_RTHCPHYS;
+        return rc;
+    }
+
+    unsigned const iEptPt = (GCPhysNested >> SHW_PT_SHIFT) & SHW_PT_MASK;
+    Assert(iEptPt < EPT_PG_ENTRIES);
+    SHWPTE         EptPte = pEptPt->a[iEptPt];
+    if (!SHW_PTE_IS_P(EptPte))
+    {
+        *pfFlags = 0;
+        *pHCPhys = NIL_RTHCPHYS;
+        return VERR_PAGE_NOT_PRESENT;
+    }
+
+    if (pfFlags)
+    {
+        /* Read, Write and Execute bits (Present mask) are cumulative. */
+        *pfFlags = (SHW_PTE_GET_U(EptPte) & ~SHW_PTE_PG_MASK)
+                 & ((EptPde.u & EPT_PRESENT_MASK) | ~(uint64_t)EPT_PRESENT_MASK);
+    }
+    if (pHCPhys)
+        *pHCPhys = SHW_PTE_GET_HCPHYS(EptPte);
+    return VINF_SUCCESS;
+
+#else   /* PGM_SHW_TYPE != PGM_TYPE_EPT */
+    RT_NOREF(pVCpu, pEptPd, pWalk, *pfFlags, pHCPhys);
+    AssertFailed();
+    return VERR_PGM_SHW_NONE_IPE;
+#endif  /* PGM_SHW_TYPE != PGM_TYPE_EPT */
+}
+#endif
+
+
 /**
  * Gets effective page information (from the VMM page directory).
@@ -385 +448 @@
 
 # elif PGM_SHW_TYPE == PGM_TYPE_EPT
+    /*
+     * We're currently ASSUMING that the SLAT mode here is always "direct".
+     * If a guest (e.g., nested Hyper-V) turns out to require this
+     * (probably while modifying shadow non-MMIO2 pages) then handle this
+     * by calling (NestedGetPage). Asserting for now.
+     */
     Assert(pVCpu->pgm.s.enmGuestSlatMode == PGMSLAT_DIRECT);
     PEPTPD          pPDDst;
@@ -541 +610 @@
 
 # elif PGM_SHW_TYPE == PGM_TYPE_EPT
-        Assert(pVCpu->pgm.s.enmGuestSlatMode == PGMSLAT_DIRECT);
-        const unsigned  iPd = ((GCPtr >> SHW_PD_SHIFT) & SHW_PD_MASK);
-        PEPTPD          pPDDst;
-        EPTPDE          Pde;
-
-        rc = pgmShwGetEPTPDPtr(pVCpu, GCPtr, NULL, &pPDDst);
-        if (rc != VINF_SUCCESS)
+        EPTPDE Pde;
+        const unsigned iPd = ((GCPtr >> SHW_PD_SHIFT) & SHW_PD_MASK);
+        if (pVCpu->pgm.s.enmGuestSlatMode == PGMSLAT_DIRECT)
         {
-            AssertRC(rc);
-            return rc;
+            PEPTPD pPDDst;
+            rc = pgmShwGetEPTPDPtr(pVCpu, GCPtr, NULL, &pPDDst);
+            if (rc != VINF_SUCCESS)
+            {
+                AssertRC(rc);
+                return rc;
+            }
+            Assert(pPDDst);
+            Pde = pPDDst->a[iPd];
         }
-        Assert(pPDDst);
-        Pde = pPDDst->a[iPd];
+        else
+        {
+#  ifdef VBOX_WITH_NESTED_HWVIRT_VMX_EPT
+            Assert(pVCpu->pgm.s.enmGuestSlatMode == PGMSLAT_EPT);
+            Assert(!(GCPtr & GUEST_PAGE_OFFSET_MASK));
+            PGMPTWALK      Walk;
+            PGMPTWALKGST   GstWalkAll;
+            RTGCPHYS const GCPhysNestedPage = GCPtr;
+            rc = pgmGstSlatWalk(pVCpu, GCPhysNestedPage, false /*fIsLinearAddrValid*/, 0 /*GCPtrNestedFault*/, &Walk,
+                                &GstWalkAll);
+            if (RT_SUCCESS(rc))
+            {
+#   ifdef DEBUG_ramshankar
+                /* Paranoia. */
+                Assert(GstWalkAll.enmType == PGMPTWALKGSTTYPE_EPT);
+                Assert(Walk.fSucceeded);
+                Assert(Walk.fEffective & (PGM_PTATTRS_EPT_R_MASK | PGM_PTATTRS_EPT_W_MASK | PGM_PTATTRS_EPT_X_SUPER_MASK));
+                Assert(Walk.fIsSlat);
+                Assert(RT_BOOL(Walk.fEffective & PGM_PTATTRS_R_MASK)  ==  RT_BOOL(Walk.fEffective & PGM_PTATTRS_EPT_R_MASK));
+                Assert(RT_BOOL(Walk.fEffective & PGM_PTATTRS_W_MASK)  ==  RT_BOOL(Walk.fEffective & PGM_PTATTRS_EPT_W_MASK));
+                Assert(RT_BOOL(Walk.fEffective & PGM_PTATTRS_NX_MASK) == !RT_BOOL(Walk.fEffective & PGM_PTATTRS_EPT_X_SUPER_MASK));
+#   endif
+                PGM_A20_ASSERT_MASKED(pVCpu, Walk.GCPhys);
+                Assert(!(fFlags & X86_PTE_RW) || (Walk.fEffective & PGM_PTATTRS_W_MASK));
+
+                /* Update the nested-guest physical address with the translated guest-physical address. */
+                GCPtr = Walk.GCPhys;
+
+                /* Get the PD. */
+                PSHWPD pEptPd;
+                rc = pgmShwGetNestedEPTPDPtr(pVCpu, GCPhysNestedPage, NULL /*ppPdpt*/, &pEptPd, &GstWalkAll);
+                AssertRCReturn(rc, rc);
+                Assert(pEptPd);
+                Assert(iPd < EPT_PG_ENTRIES);
+                Pde = pEptPd->a[iPd];
+            }
+            else
+            {
+                Log(("Failed to translate nested-guest physical address %#RGp rc=%Rrc\n", GCPhysNestedPage, rc));
+                return rc;
+            }
+
+#  else  /* !VBOX_WITH_NESTED_HWVIRT_VMX_EPT */
+            AssertFailed();
+            return VERR_PGM_SHW_NONE_IPE;
+#  endif /* !VBOX_WITH_NESTED_HWVIRT_VMX_EPT */
+        }
 
 # else /* PGM_TYPE_32BIT || PGM_SHW_TYPE == PGM_TYPE_NESTED_32BIT */