Changeset 20135 in vbox

Timestamp:

May 29, 2009 7:44:12 AM (16 years ago)

Author:

vboxsync

Message:

Prevent reuse of cached larged pages with different access attributes.

Location:

trunk/src/VBox/VMM

Files:

• PGMInternal.h (modified) (3 diffs)
• PGMPhys.cpp (modified) (2 diffs)
• VMMAll/PGMAllBth.h (modified) (5 diffs)
• VMMAll/PGMAllPool.cpp (modified) (10 diffs)
• testcase/tstVMStructGC.cpp (modified) (1 diff)

trunk/src/VBox/VMM/PGMInternal.h

@@ -1581 +1581 @@
 
 
+typedef enum
+{
+    PGMPOOLACCESS_DONTCARE = 0,
+    PGMPOOLACCESS_USER_RW,
+    PGMPOOLACCESS_USER_R,
+    PGMPOOLACCESS_SUPERVISOR_RW,
+    PGMPOOLACCESS_SUPERVISOR_R
+} PGMPOOLACCESS;
+
 /**
  * The tracking data for a page in the pool.
@@ -1601 +1610 @@
     /** The kind of page we're shadowing. (This is really a PGMPOOLKIND enum.) */
     uint8_t             enmKind;
-    uint8_t             bPadding;
+    /** The subkind of page we're shadowing. (This is really a PGMPOOLACCESS enum.) */
+    uint8_t             enmAccess;
     /** The index of this page. */
     uint16_t            idx;
@@ -2959 +2969 @@
 int             pgmR0DynMapHCPageCommon(PVM pVM, PPGMMAPSET pSet, RTHCPHYS HCPhys, void **ppv);
 #endif
-int             pgmPoolAlloc(PVM pVM, RTGCPHYS GCPhys, PGMPOOLKIND enmKind, uint16_t iUser, uint32_t iUserTable, PPPGMPOOLPAGE ppPage);
+int             pgmPoolAllocEx(PVM pVM, RTGCPHYS GCPhys, PGMPOOLKIND enmKind, PGMPOOLACCESS enmAccess, uint16_t iUser, uint32_t iUserTable, PPPGMPOOLPAGE ppPage);
+
+DECLINLINE(int) pgmPoolAlloc(PVM pVM, RTGCPHYS GCPhys, PGMPOOLKIND enmKind, uint16_t iUser, uint32_t iUserTable, PPPGMPOOLPAGE ppPage)
+{
+    return pgmPoolAllocEx(pVM, GCPhys, enmKind, PGMPOOLACCESS_DONTCARE, iUser, iUserTable, ppPage);
+}
+
 void            pgmPoolFree(PVM pVM, RTHCPHYS HCPhys, uint16_t iUser, uint32_t iUserTable);
 void            pgmPoolFreeByPage(PPGMPOOL pPool, PPGMPOOLPAGE pPage, uint16_t iUser, uint32_t iUserTable);

trunk/src/VBox/VMM/PGMPhys.cpp

@@ -3233 +3233 @@
             else
             {
-                /* Temporariliy disabled phycial handler(s), since the recompiler
-                   doesn't get notified when it's reset we'll have to pretend its
+                /* Temporarily disabled physical handler(s), since the recompiler
+                   doesn't get notified when it's reset we'll have to pretend it's
                    operating normally. */
                 if (pgmHandlerPhysicalIsAll(pVM, GCPhys))
@@ -3266 +3266 @@
             *ppv = (void *)((uintptr_t)pTlbe->pv | (GCPhys & PAGE_OFFSET_MASK));
             /** @todo mapping/locking hell; this isn't horribly efficient since
-             *        pgmPhysPageLoadIntoTlb will repeate the lookup we've done here. */
+             *        pgmPhysPageLoadIntoTlb will repeat the lookup we've done here. */
 
             Log6(("PGMR3PhysTlbGCPhys2Ptr: GCPhys=%RGp rc=%Rrc pPage=%R[pgmpage] *ppv=%p\n", GCPhys, rc, pPage, *ppv));

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

@@ -1188 +1188 @@
     if (PdeSrc.n.u1Present)
     {
+# ifndef PGM_WITHOUT_MAPPING
         if (PdeDst.u & PGM_PDFLAGS_MAPPING)
         {
@@ -1199 +1200 @@
             pgmUnlock(pVM);
         }
-        else if (   PdeSrc.n.u1User != PdeDst.n.u1User
-                 || (!PdeSrc.n.u1Write && PdeDst.n.u1Write))
+        else 
+# endif /* !PGM_WITHOUT_MAPPING */
+        if (   PdeSrc.n.u1User != PdeDst.n.u1User
+            || (!PdeSrc.n.u1Write && PdeDst.n.u1Write))
         {
             /*
@@ -2589 +2592 @@
         else
         {
+            PGMPOOLACCESS enmAccess;
+
             GCPhys = GST_GET_PDE_BIG_PG_GCPHYS(PdeSrc);
 # if PGM_SHW_TYPE == PGM_TYPE_PAE && PGM_GST_TYPE == PGM_TYPE_32BIT
@@ -2594 +2599 @@
             GCPhys |= GCPtrPage & (1 << X86_PD_PAE_SHIFT);
 # endif
-            rc = pgmPoolAlloc(pVM, GCPhys, BTH_PGMPOOLKIND_PT_FOR_BIG, pShwPde->idx,      iPDDst, &pShwPage);
+            /* Determine the right kind of large page to avoid incorrect cached entry reuse. */
+            if (PdeSrc.n.u1User)
+            {
+                if (PdeSrc.n.u1Write)
+                    enmAccess = PGMPOOLACCESS_USER_RW;
+                else
+                    enmAccess = PGMPOOLACCESS_USER_R;
+            }
+            else
+            {
+                if (PdeSrc.n.u1Write)
+                    enmAccess = PGMPOOLACCESS_SUPERVISOR_RW;
+                else
+                    enmAccess = PGMPOOLACCESS_SUPERVISOR_R;
+            }
+            rc = pgmPoolAllocEx(pVM, GCPhys, BTH_PGMPOOLKIND_PT_FOR_BIG, enmAccess, pShwPde->idx, iPDDst, &pShwPage);
         }
         if (rc == VINF_SUCCESS)
@@ -3331 +3351 @@
 #endif /* PGM_GST_TYPE != PGM_TYPE_32BIT */
 }
-
-
-#if PGM_GST_TYPE == PGM_TYPE_32BIT || PGM_GST_TYPE == PGM_TYPE_PAE || PGM_GST_TYPE == PGM_TYPE_AMD64
-# if PGM_SHW_TYPE == PGM_TYPE_32BIT || PGM_SHW_TYPE == PGM_TYPE_PAE || PGM_SHW_TYPE == PGM_TYPE_AMD64
-/**
- * Figures out which kind of shadow page this guest PDE warrants.
- *
- * @returns Shadow page kind.
- * @param   pPdeSrc     The guest PDE in question.
- * @param   cr4         The current guest cr4 value.
- */
-DECLINLINE(PGMPOOLKIND) PGM_BTH_NAME(CalcPageKind)(const GSTPDE *pPdeSrc, uint32_t cr4)
-{
-#  if PMG_GST_TYPE == PGM_TYPE_AMD64
-    if (!pPdeSrc->n.u1Size)
-#  else
-    if (!pPdeSrc->n.u1Size || !(cr4 & X86_CR4_PSE))
-#  endif
-        return BTH_PGMPOOLKIND_PT_FOR_PT;
-    //switch (pPdeSrc->u & (X86_PDE4M_RW | X86_PDE4M_US /*| X86_PDE4M_PAE_NX*/))
-    //{
-    //    case 0:
-    //      return BTH_PGMPOOLKIND_PT_FOR_BIG_RO;
-    //    case X86_PDE4M_RW:
-    //      return BTH_PGMPOOLKIND_PT_FOR_BIG_RW;
-    //    case X86_PDE4M_US:
-    //      return BTH_PGMPOOLKIND_PT_FOR_BIG_US;
-    //    case X86_PDE4M_RW | X86_PDE4M_US:
-    //      return BTH_PGMPOOLKIND_PT_FOR_BIG_RW_US;
-#  if 0
-    //    case X86_PDE4M_PAE_NX:
-    //      return BTH_PGMPOOLKIND_PT_FOR_BIG_NX;
-    //    case X86_PDE4M_RW | X86_PDE4M_PAE_NX:
-    //      return BTH_PGMPOOLKIND_PT_FOR_BIG_RW_NX;
-    //    case X86_PDE4M_US | X86_PDE4M_PAE_NX:
-    //      return BTH_PGMPOOLKIND_PT_FOR_BIG_US_NX;
-    //    case X86_PDE4M_RW | X86_PDE4M_US | X86_PDE4M_PAE_NX:
-    //      return BTH_PGMPOOLKIND_PT_FOR_BIG_RW_US_NX;
-#  endif
-            return BTH_PGMPOOLKIND_PT_FOR_BIG;
-    //}
-}
-# endif
-#endif
 
 #undef MY_STAM_COUNTER_INC

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

@@ -1429 +1429 @@
  * @param   ppPage      Where to store the pointer to the page.
  */
-static int pgmPoolCacheAlloc(PPGMPOOL pPool, RTGCPHYS GCPhys, PGMPOOLKIND enmKind, uint16_t iUser, uint32_t iUserTable, PPPGMPOOLPAGE ppPage)
+static int pgmPoolCacheAlloc(PPGMPOOL pPool, RTGCPHYS GCPhys, PGMPOOLKIND enmKind, PGMPOOLACCESS enmAccess, uint16_t iUser, uint32_t iUserTable, PPPGMPOOLPAGE ppPage)
 {
 #ifndef IN_RC
@@ -1447 +1447 @@
             if (pPage->GCPhys == GCPhys)
             {
-                if ((PGMPOOLKIND)pPage->enmKind == enmKind)
+                if (    (PGMPOOLKIND)pPage->enmKind == enmKind
+                    &&  (PGMPOOLACCESS)pPage->enmAccess == enmAccess)
                 {
                     /* Put it at the start of the use list to make sure pgmPoolTrackAddUser
@@ -1465 +1466 @@
                 }
 
-                /*
-                 * The kind is different. In some cases we should now flush the page
-                 * as it has been reused, but in most cases this is normal remapping
-                 * of PDs as PT or big pages using the GCPhys field in a slightly
-                 * different way than the other kinds.
-                 */
-                if (pgmPoolCacheReusedByKind((PGMPOOLKIND)pPage->enmKind, enmKind))
+                if ((PGMPOOLKIND)pPage->enmKind != enmKind)
                 {
-                    STAM_COUNTER_INC(&pPool->StatCacheKindMismatches);
-                    pgmPoolFlushPage(pPool, pPage);
-                    PGM_INVL_VCPU_TLBS(VMMGetCpu(pVM)); /* see PT handler. */
-                    break;
+                    /*
+                     * The kind is different. In some cases we should now flush the page
+                     * as it has been reused, but in most cases this is normal remapping
+                     * of PDs as PT or big pages using the GCPhys field in a slightly
+                     * different way than the other kinds.
+                     */
+                    if (pgmPoolCacheReusedByKind((PGMPOOLKIND)pPage->enmKind, enmKind))
+                    {
+                        STAM_COUNTER_INC(&pPool->StatCacheKindMismatches);
+                        pgmPoolFlushPage(pPool, pPage);
+                        PGM_INVL_VCPU_TLBS(VMMGetCpu(pVM)); /* see PT handler. */
+                        break;
+                    }
                 }
             }
@@ -3916 +3920 @@
     pPool->iFreeHead = pPage->idx;
     pPage->enmKind = PGMPOOLKIND_FREE;
+    pPage->enmAccess = PGMPOOLACCESS_DONTCARE;
     pPage->GCPhys = NIL_RTGCPHYS;
     pPage->fReusedFlushPending = false;
@@ -4018 +4023 @@
 #endif
 }
-
 
 /**
@@ -4035 +4039 @@
  *                      shadow PT is covering.
  * @param   enmKind     The kind of mapping.
+ * @param   enmAccess   Access type for the mapping (only relevant for big pages)
  * @param   iUser       The shadow page pool index of the user table.
  * @param   iUserTable  The index into the user table (shadowed).
  * @param   ppPage      Where to store the pointer to the page. NULL is stored here on failure.
  */
-int pgmPoolAlloc(PVM pVM, RTGCPHYS GCPhys, PGMPOOLKIND enmKind, uint16_t iUser, uint32_t iUserTable, PPPGMPOOLPAGE ppPage)
+int pgmPoolAllocEx(PVM pVM, RTGCPHYS GCPhys, PGMPOOLKIND enmKind, PGMPOOLACCESS enmAccess, uint16_t iUser, uint32_t iUserTable, PPPGMPOOLPAGE ppPage)
 {
     PPGMPOOL pPool = pVM->pgm.s.CTX_SUFF(pPool);
@@ -4054 +4059 @@
     if (pPool->fCacheEnabled)
     {
-        int rc2 = pgmPoolCacheAlloc(pPool, GCPhys, enmKind, iUser, iUserTable, ppPage);
+        int rc2 = pgmPoolCacheAlloc(pPool, GCPhys, enmKind, enmAccess, iUser, iUserTable, ppPage);
         if (RT_SUCCESS(rc2))
         {
@@ -4094 +4099 @@
     pPool->cUsedPages++;                /* physical handler registration / pgmPoolTrackFlushGCPhysPTsSlow requirement. */
     pPage->enmKind = enmKind;
+    pPage->enmAccess = enmAccess;
     pPage->GCPhys = GCPhys;
     pPage->fSeenNonGlobal = false;      /* Set this to 'true' to disable this feature. */
@@ -4117 +4123 @@
     {
         pPool->cUsedPages--;
-        pPage->enmKind = PGMPOOLKIND_FREE;
-        pPage->GCPhys = NIL_RTGCPHYS;
-        pPage->iNext = pPool->iFreeHead;
-        pPool->iFreeHead = pPage->idx;
+        pPage->enmKind      = PGMPOOLKIND_FREE;
+        pPage->enmAccess    = PGMPOOLACCESS_DONTCARE;
+        pPage->GCPhys       = NIL_RTGCPHYS;
+        pPage->iNext        = pPool->iFreeHead;
+        pPool->iFreeHead    = pPage->idx;
         pgmUnlock(pVM);
         STAM_PROFILE_ADV_STOP(&pPool->StatAlloc, a);
@@ -4244 +4251 @@
         pPage->cModifications = 0;
 #endif
-        pPage->GCPhys    = NIL_RTGCPHYS;
-        pPage->enmKind   = PGMPOOLKIND_FREE;
+        pPage->GCPhys     = NIL_RTGCPHYS;
+        pPage->enmKind    = PGMPOOLKIND_FREE;
+        pPage->enmAccess  = PGMPOOLACCESS_DONTCARE;
         Assert(pPage->idx == i);
-        pPage->iNext     = i + 1;
-        pPage->fZeroed   = false;       /* This could probably be optimized, but better safe than sorry. */
+        pPage->iNext      = i + 1;
+        pPage->fZeroed    = false;       /* This could probably be optimized, but better safe than sorry. */
         pPage->fSeenNonGlobal = false;
-        pPage->fMonitored= false;
-        pPage->fCached   = false;
+        pPage->fMonitored = false;
+        pPage->fCached    = false;
         pPage->fReusedFlushPending = false;
 #ifdef PGMPOOL_WITH_USER_TRACKING
-        pPage->iUserHead = NIL_PGMPOOL_USER_INDEX;
+        pPage->iUserHead  = NIL_PGMPOOL_USER_INDEX;
 #else
-        pPage->fCR3Mix = false;
+        pPage->fCR3Mix    = false;
 #endif
 #ifdef PGMPOOL_WITH_CACHE
-        pPage->iAgeNext  = NIL_PGMPOOL_IDX;
-        pPage->iAgePrev  = NIL_PGMPOOL_IDX;
-#endif
-        pPage->cLocked   = 0;
+        pPage->iAgeNext   = NIL_PGMPOOL_IDX;
+        pPage->iAgePrev   = NIL_PGMPOOL_IDX;
+#endif
+        pPage->cLocked    = 0;
     }
     pPool->aPages[pPool->cCurPages - 1].iNext = NIL_PGMPOOL_IDX;

trunk/src/VBox/VMM/testcase/tstVMStructGC.cpp

@@ -649 +649 @@
     GEN_CHECK_OFF(PGMPOOLPAGE, pvPageR3);
     GEN_CHECK_OFF(PGMPOOLPAGE, enmKind);
-    GEN_CHECK_OFF(PGMPOOLPAGE, bPadding);
+    GEN_CHECK_OFF(PGMPOOLPAGE, enmAccess);
     GEN_CHECK_OFF(PGMPOOLPAGE, idx);
     GEN_CHECK_OFF(PGMPOOLPAGE, iNext);