Changeset 15410 in vbox

Timestamp:

Dec 13, 2008 1:04:17 AM (16 years ago)

Author:

vboxsync

Message:

PGM: Deal with pgmPoolAlloc failure in MapCR3 without relying on having to clear the pool. The MapCR3 action will be postponed to SyncCR3.

Location:

trunk/src/VBox/VMM

Files:

• PGMInternal.h (modified) (1 diff)
• PGMMap.cpp (modified) (1 diff)
• VMMAll/PGMAll.cpp (modified) (7 diffs)
• VMMAll/PGMAllBth.h (modified) (1 diff)
• VMMAll/PGMAllGst.h (modified) (3 diffs)
• VMMAll/PGMAllPool.cpp (modified) (6 diffs)
• VMMR0/HWSVMR0.cpp (modified) (1 diff)
• VMMR0/HWVMXR0.cpp (modified) (2 diffs)

trunk/src/VBox/VMM/PGMInternal.h

@@ -2871 +2871 @@
 /** Check monitoring on next CR3 (re)load and invalidate page. */
 #define PGM_SYNC_MONITOR_CR3                    RT_BIT(2)
+/** Check guest mapping in SyncCR3. */
+#define PGM_SYNC_MAP_CR3                        RT_BIT(3)
 /** Clear the page pool (a light weight flush). */
 #define PGM_SYNC_CLEAR_PGM_POOL                 RT_BIT(8)

trunk/src/VBox/VMM/PGMMap.cpp

@@ -482 +482 @@
     /* Remap CR3 as we have just flushed the CR3 shadow PML4 in case we're in long mode. */
     int rc = PGM_GST_PFN(MapCR3, pVM)(pVM, pVM->pgm.s.GCPhysCR3);
-    AssertRC(rc);
+    AssertRCSuccess(rc);
 
 #ifndef VBOX_WITH_PGMPOOL_PAGING_ONLY
     rc = PGM_GST_PFN(MonitorCR3, pVM)(pVM, pVM->pgm.s.GCPhysCR3);
-    AssertRC(rc);
+    AssertRCSuccess(rc);
 #endif
     return VINF_SUCCESS;

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

@@ -1466 +1466 @@
      * Always flag the necessary updates; necessary for hardware acceleration
      */
+    /** @todo optimize this, it shouldn't always be necessary. */
     VM_FF_SET(pVM, VM_FF_PGM_SYNC_CR3_NON_GLOBAL);
     if (fGlobal)
@@ -1485 +1486 @@
     if (pVM->pgm.s.GCPhysCR3 != GCPhysCR3)
     {
-        pVM->pgm.s.GCPhysCR3 = GCPhysCR3;
+        RTGCPHYS GCPhysOldCR3 = pVM->pgm.s.GCPhysCR3;
+        pVM->pgm.s.GCPhysCR3  = GCPhysCR3;
         rc = PGM_GST_PFN(MapCR3, pVM)(pVM, GCPhysCR3);
-        if (RT_SUCCESS(rc) && !pVM->pgm.s.fMappingsFixed)
+        if (RT_LIKELY(rc == VINF_SUCCESS))
         {
-            pVM->pgm.s.fSyncFlags &= ~PGM_SYNC_MONITOR_CR3;
-            rc = PGM_GST_PFN(MonitorCR3, pVM)(pVM, GCPhysCR3);
+            if (!pVM->pgm.s.fMappingsFixed)
+            {
+                pVM->pgm.s.fSyncFlags &= ~PGM_SYNC_MONITOR_CR3;
+                rc = PGM_GST_PFN(MonitorCR3, pVM)(pVM, GCPhysCR3);
+            }
         }
+        else
+        {
+            AssertMsg(rc == VINF_PGM_SYNC_CR3, ("%Rrc\n", rc));
+            Assert(VM_FF_ISPENDING(pVM, VM_FF_PGM_SYNC_CR3_NON_GLOBAL | VM_FF_PGM_SYNC_CR3));
+            pVM->pgm.s.GCPhysCR3 = GCPhysOldCR3;
+            pVM->pgm.s.fSyncFlags |= PGM_SYNC_MAP_CR3;
+            if (!pVM->pgm.s.fMappingsFixed)
+                pVM->pgm.s.fSyncFlags |= PGM_SYNC_MONITOR_CR3;
+        }
+
         if (fGlobal)
             STAM_COUNTER_INC(&pVM->pgm.s.CTX_MID_Z(Stat,FlushTLBNewCR3Global));
@@ -1529 +1544 @@
  *
  * @returns VBox status code.
- * @retval  VINF_PGM_SYNC_CR3 if monitoring requires a CR3 sync. This can
- *          safely be ignored and overridden since the FF will be set too then.
+ * @retval  VINF_SUCCESS.
+ * @retval  (If applied when not in nested mode: VINF_PGM_SYNC_CR3 if monitoring
+ *          requires a CR3 sync. This can safely be ignored and overridden since
+ *          the FF will be set too then.)
  * @param   pVM         VM handle.
  * @param   cr3         The new cr3.
@@ -1559 +1576 @@
         pVM->pgm.s.GCPhysCR3 = GCPhysCR3;
         rc = PGM_GST_PFN(MapCR3, pVM)(pVM, GCPhysCR3);
-    }
-    AssertRC(rc);
+        AssertRCSuccess(rc); /* Assumes VINF_PGM_SYNC_CR3 doesn't apply to nested paging. */
+    }
     return rc;
 }
@@ -1581 +1598 @@
 VMMDECL(int) PGMSyncCR3(PVM pVM, uint64_t cr0, uint64_t cr3, uint64_t cr4, bool fGlobal)
 {
+    int rc;
+
     /*
      * We might be called when we shouldn't.
@@ -1597 +1616 @@
     }
 
-    /* If global pages are not supported, then all flushes are global */
+    /* If global pages are not supported, then all flushes are global. */
     if (!(cr4 & X86_CR4_PGE))
         fGlobal = true;
@@ -1603 +1622 @@
              VM_FF_ISSET(pVM, VM_FF_PGM_SYNC_CR3), VM_FF_ISSET(pVM, VM_FF_PGM_SYNC_CR3_NON_GLOBAL)));
 
+#ifdef PGMPOOL_WITH_MONITORING
+    /*
+     * The pool may have pending stuff and even require a return to ring-3 to
+     * clear the whole thing.
+     */
+    rc = pgmPoolSyncCR3(pVM);
+    if (rc != VINF_SUCCESS)
+        return rc;
+#endif
+
+    /*
+     * Check if we need to finish an aborted MapCR3 call (see PGMFlushTLB).
+     * This should be done before SyncCR3.
+     */
+    if (pVM->pgm.s.fSyncFlags & PGM_SYNC_MAP_CR3)
+    {
+        pVM->pgm.s.fSyncFlags &= ~PGM_SYNC_MAP_CR3;
+
+        RTGCPHYS GCPhysCR3Old = pVM->pgm.s.GCPhysCR3;
+        RTGCPHYS GCPhysCR3;
+        if (    pVM->pgm.s.enmGuestMode == PGMMODE_PAE
+            ||  pVM->pgm.s.enmGuestMode == PGMMODE_PAE_NX
+            ||  pVM->pgm.s.enmGuestMode == PGMMODE_AMD64
+            ||  pVM->pgm.s.enmGuestMode == PGMMODE_AMD64_NX)
+            GCPhysCR3 = (RTGCPHYS)(cr3 & X86_CR3_PAE_PAGE_MASK);
+        else
+            GCPhysCR3 = (RTGCPHYS)(cr3 & X86_CR3_PAGE_MASK);
+        pVM->pgm.s.GCPhysCR3 = GCPhysCR3;
+        rc = PGM_GST_PFN(MapCR3, pVM)(pVM, GCPhysCR3);
+#ifdef IN_RING3
+        if (rc == VINF_PGM_SYNC_CR3)
+            rc = pgmPoolSyncCR3(pVM);
+#else
+        if (rc == VINF_PGM_SYNC_CR3)
+        {
+            pVM->pgm.s.GCPhysCR3 = GCPhysCR3Old;
+            return rc;
+        }
+#endif
+        AssertRCReturn(rc, rc);
+        AssertRCSuccessReturn(rc, VERR_INTERNAL_ERROR);
+    }
+
     /*
      * Let the 'Bth' function do the work and we'll just keep track of the flags.
      */
     STAM_PROFILE_START(&pVM->pgm.s.CTX_MID_Z(Stat,SyncCR3), a);
-    int rc = PGM_BTH_PFN(SyncCR3, pVM)(pVM, cr0, cr3, cr4, fGlobal);
+    rc = PGM_BTH_PFN(SyncCR3, pVM)(pVM, cr0, cr3, cr4, fGlobal);
     STAM_PROFILE_STOP(&pVM->pgm.s.CTX_MID_Z(Stat,SyncCR3), a);
     AssertMsg(rc == VINF_SUCCESS || rc == VINF_PGM_SYNC_CR3 || RT_FAILURE(rc), ("rc=%Rrc\n", rc));

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

@@ -3120 +3120 @@
 #endif
 
-#ifdef PGMPOOL_WITH_MONITORING
-    int rc = pgmPoolSyncCR3(pVM);
-    if (rc != VINF_SUCCESS)
-        return rc;
-#endif
-
 #if PGM_SHW_TYPE == PGM_TYPE_NESTED || PGM_SHW_TYPE == PGM_TYPE_EPT
     /*
      * Nested / EPT - almost no work.
      */
-    /** @todo check if this is really necessary */
+    /** @todo check if this is really necessary; the call does it as well... */
     HWACCMFlushTLB(pVM);
     return VINF_SUCCESS;

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

@@ -423 +423 @@
  * CR3 is updated we simply call MapCR3 again.
  *
- * @returns VBox status, no specials.
+ * @returns Strict VBox status code.
+ * @retval  VINF_SUCCESS.
+ * @retval  VINF_PGM_SYNC_CR3 if the shadow page pool overflowed.
+ *
  * @param   pVM             VM handle.
  * @param   GCPhysCR3       The physical address in the CR3 register.
@@ -509 +512 @@
             if (!HWACCMIsNestedPagingActive(pVM))
             {
+                /*
+                 * Update the shadow root page as well since that's not fixed.
+                 */
+                /** @todo Move this into PGMAllBth.h. */
                 PPGMPOOL pPool = pVM->pgm.s.CTX_SUFF(pPool);
                 if (pVM->pgm.s.CTX_SUFF(pShwAmd64CR3))
                 {
                     /* It might have been freed already by a pool flush (see e.g. PGMR3MappingsUnfix). */
+                    /** @todo Coordinate this better with the pool. */
                     if (pVM->pgm.s.CTX_SUFF(pShwAmd64CR3)->enmKind != PGMPOOLKIND_FREE)
                         pgmPoolFreeByPage(pPool, pVM->pgm.s.CTX_SUFF(pShwAmd64CR3), PGMPOOL_IDX_AMD64_CR3, pVM->pgm.s.CTX_SUFF(pShwAmd64CR3)->GCPhys >> PAGE_SHIFT);
-                    pVM->pgm.s.CTX_SUFF(pShwAmd64CR3) = 0;
-                    pVM->pgm.s.pShwPaePml4R3          = 0;
+                    pVM->pgm.s.pShwAmd64CR3R3   = 0;
+                    pVM->pgm.s.pShwAmd64CR3R0   = 0;
+                    pVM->pgm.s.pShwPaePml4R3    = 0;
 #  ifndef VBOX_WITH_2X_4GB_ADDR_SPACE
-                    pVM->pgm.s.pShwPaePml4R0          = 0;
+                    pVM->pgm.s.pShwPaePml4R0    = 0;
 #  endif
-                    pVM->pgm.s.HCPhysShwPaePml4       = 0;
+                    pVM->pgm.s.HCPhysShwPaePml4 = 0;
                 }
 
                 Assert(!(GCPhysCR3 >> (PAGE_SHIFT + 32)));
-l_try_again:
                 rc = pgmPoolAlloc(pVM, GCPhysCR3, PGMPOOLKIND_64BIT_PML4_FOR_64BIT_PML4, PGMPOOL_IDX_AMD64_CR3, GCPhysCR3 >> PAGE_SHIFT, &pVM->pgm.s.CTX_SUFF(pShwAmd64CR3));
                 if (rc == VERR_PGM_POOL_FLUSHED)
                 {
-                    Log(("MapCR3: Flush pool and try again\n"));
-                    Assert(pVM->pgm.s.fSyncFlags & PGM_SYNC_CLEAR_PGM_POOL);
-                    rc = pgmPoolSyncCR3(pVM);
-                    AssertRC(rc);
-                    goto l_try_again;
+                    Log(("MapCR3: PGM pool flushed -> signal sync cr3\n"));
+                    Assert(VM_FF_ISSET(pVM, VM_FF_PGM_SYNC_CR3));
+                    return VINF_PGM_SYNC_CR3;
                 }
+                AssertRCReturn(rc, rc);
 #  ifdef IN_RING0
                 pVM->pgm.s.pShwAmd64CR3R3 = MMHyperCCToR3(pVM, pVM->pgm.s.CTX_SUFF(pShwAmd64CR3));
@@ -545 +552 @@
 #  endif
                 pVM->pgm.s.HCPhysShwPaePml4 = pVM->pgm.s.CTX_SUFF(pShwAmd64CR3)->Core.Key;
+                rc = VINF_SUCCESS; /* clear it - pgmPoolAlloc returns hints. */
             }
 # endif

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

@@ -146 +146 @@
             HCPhys = pPGM->HCPhysShwPaePdpt;
             break;
+        case PGMPOOL_IDX_NESTED_ROOT:
+            HCPhys = pPGM->HCPhysShwNestedRoot;
+            break;
         case PGMPOOL_IDX_PAE_PD:
             AssertReleaseMsgFailed(("PGMPOOL_IDX_PAE_PD is not usable in VBOX_WITH_2X_4GB_ADDR_SPACE_IN_R0 context\n"));
@@ -153 +156 @@
             return NULL;
     }
+    AssertMsg(HCPhys && HCPhys != NIL_RTHCPHYS && !(PAGE_OFFSET_MASK & HCPhys), ("%RHp\n", HCPhys));
+
     void *pv;
     int rc = pgmR0DynMapHCPageInlined(pPGM, HCPhys, &pv);
@@ -2071 +2076 @@
     else
     {
-# ifndef IN_RC //def IN_RING3 - fixing properly in a bit...
+# ifdef IN_RING3 /* Don't flush in ring-0 or raw mode, it's taking too long. */
         pVM->pgm.s.fSyncFlags &= ~PGM_SYNC_CLEAR_PGM_POOL;
         pgmPoolClearAll(pVM);
@@ -3810 +3815 @@
     }
 
+    /*
+     * Finally, assert the FF.
+     */
+    VM_FF_SET(pPool->CTX_SUFF(pVM), VM_FF_PGM_SYNC_CR3);
+
     STAM_PROFILE_STOP(&pPool->StatFlushAllInt, a);
 }
@@ -3967 +3977 @@
     /*
      * Flush the pool.
+     *
      * If we have tracking enabled, it should be possible to come up with
      * a cheap replacement strategy...
@@ -4003 +4014 @@
     LogFlow(("pgmPoolAlloc: GCPhys=%RGp enmKind=%d iUser=%#x iUserTable=%#x\n", GCPhys, enmKind, iUser, iUserTable));
     *ppPage = NULL;
+    Assert(!(pVM->pgm.s.fSyncFlags & PGM_SYNC_CLEAR_PGM_POOL));
 
 #ifdef PGMPOOL_WITH_CACHE

trunk/src/VBox/VMM/VMMR0/HWSVMR0.cpp

@@ -678 +678 @@
         {
             pVMCB->guest.u64CR3             = PGMGetHyperCR3(pVM);
-            Assert(pVMCB->guest.u64CR3);
+            Assert(pVMCB->guest.u64CR3 || VM_FF_ISPENDING(pVM, VM_FF_PGM_SYNC_CR3 | VM_FF_PGM_SYNC_CR3_NON_GLOBAL));
         }
     }

trunk/src/VBox/VMM/VMMR0/HWVMXR0.cpp

@@ -1426 +1426 @@
         if (pVM->hwaccm.s.fNestedPaging)
         {
-            AssertMsg(PGMGetEPTCR3(pVM) == PGMGetHyperCR3(pVM), ("%RHp vs %RHp\n", PGMGetEPTCR3(pVM), PGMGetHyperCR3(pVM)));
+            AssertMsg(   PGMGetEPTCR3(pVM) == PGMGetHyperCR3(pVM)
+                      || VM_FF_ISPENDING(pVM, VM_FF_PGM_SYNC_CR3 | VM_FF_PGM_SYNC_CR3_NON_GLOBAL),
+                      ("%RHp vs %RHp\n", PGMGetEPTCR3(pVM), PGMGetHyperCR3(pVM)));
             pVCpu->hwaccm.s.vmx.GCPhysEPTP = PGMGetEPTCR3(pVM);
 
@@ -1461 +1463 @@
         {
             val = PGMGetHyperCR3(pVM);
-            Assert(val);
+            Assert(val || VM_FF_ISPENDING(pVM, VM_FF_PGM_SYNC_CR3 | VM_FF_PGM_SYNC_CR3_NON_GLOBAL));
         }