Changeset 38955 in vbox for trunk/src/VBox

Timestamp:

Oct 6, 2011 12:23:39 PM (13 years ago)

Author:

vboxsync

Message:

pgmR3PhysChunkMap: Make sure we don't unmap the chunk we just added.

Location:

trunk/src/VBox/VMM

Files:

• VMMAll/PGMAllPhys.cpp (modified) (3 diffs)
• VMMR3/PGMPhys.cpp (modified) (10 diffs)
• include/PGMInline.h (modified) (1 diff)

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

@@ -1063 +1063 @@
         STAM_COUNTER_INC(&pVM->pgm.s.CTX_SUFF(pStats)->CTX_MID_Z(Stat,ChunkR3MapTlbHits));
         pMap = pTlbe->pChunk;
+        AssertPtr(pMap->pv);
     }
     else
@@ -1072 +1073 @@
          */
         pMap = (PPGMCHUNKR3MAP)RTAvlU32Get(&pVM->pgm.s.ChunkR3Map.pTree, idChunk);
-        if (!pMap)
+        if (pMap)
+            AssertPtr(pMap->pv);
+        else
         {
 #ifdef IN_RING0
@@ -1084 +1087 @@
                 return rc;
 #endif
+            AssertPtr(pMap->pv);
         }
 

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

@@ -3769 +3769 @@
 static DECLCALLBACK(int) pgmR3PhysChunkUnmapCandidateCallback(PAVLU32NODECORE pNode, void *pvUser)
 {
-    PPGMCHUNKR3MAP pChunk = (PPGMCHUNKR3MAP)pNode;
-    PPGMR3PHYSCHUNKUNMAPCB pArg = (PPGMR3PHYSCHUNKUNMAPCB)pvUser;
-
-    if (    pChunk->iAge
-        &&  !pChunk->cRefs
-        &&  pArg->iLastAge < pChunk->iAge)
-    {
-        /*
-         * Check that it's not in any of the TLBs.
-         */
-        PVM pVM = pArg->pVM;
-        for (unsigned i = 0; i < RT_ELEMENTS(pVM->pgm.s.ChunkR3Map.Tlb.aEntries); i++)
-            if (pVM->pgm.s.ChunkR3Map.Tlb.aEntries[i].pChunk == pChunk)
-            {
-                pChunk = NULL;
-                break;
-            }
-        if (pChunk)
-            for (unsigned i = 0; i < RT_ELEMENTS(pVM->pgm.s.PhysTlbHC.aEntries); i++)
-                if (pVM->pgm.s.PhysTlbHC.aEntries[i].pMap == pChunk)
-                {
-                    pChunk = NULL;
-                    break;
-                }
-        if (pChunk)
-        {
-            pArg->pChunk = pChunk;
-            pArg->iLastAge = pChunk->iAge;
-        }
-    }
+    PPGMCHUNKR3MAP          pChunk = (PPGMCHUNKR3MAP)pNode;
+    PPGMR3PHYSCHUNKUNMAPCB  pArg   = (PPGMR3PHYSCHUNKUNMAPCB)pvUser;
+
+    /*
+     * Check for locks and age.
+     */
+    if (pChunk->cRefs)
+        return 0;
+    if (!pChunk->iAge)
+        return 0;
+    if (pArg->iLastAge >= pChunk->iAge)
+        return 0;
+
+    /*
+     * Check that it's not in any of the TLBs.
+     */
+    PVM pVM = pArg->pVM;
+    if (   pVM->pgm.s.ChunkR3Map.Tlb.aEntries[PGM_CHUNKR3MAPTLB_IDX(pChunk->Core.Key)].idChunk
+        == pChunk->Core.Key)
+    {
+        pChunk = NULL;
+        return 0;
+    }
+#ifdef VBOX_STRICT
+    for (unsigned i = 0; i < RT_ELEMENTS(pVM->pgm.s.ChunkR3Map.Tlb.aEntries); i++)
+    {
+        Assert(pVM->pgm.s.ChunkR3Map.Tlb.aEntries[i].pChunk != pChunk);
+        Assert(pVM->pgm.s.ChunkR3Map.Tlb.aEntries[i].idChunk != pChunk->Core.Key);
+    }
+#endif
+
+    for (unsigned i = 0; i < RT_ELEMENTS(pVM->pgm.s.PhysTlbHC.aEntries); i++)
+        if (pVM->pgm.s.PhysTlbHC.aEntries[i].pMap == pChunk)
+            return 0;
+
+    pArg->pChunk   = pChunk;
+    pArg->iLastAge = pChunk->iAge;
     return 0;
 }
@@ -3838 +3845 @@
     if (Args.pChunk)
     {
+        Assert(Args.pChunk->cRefs == 0);
+        Assert(Args.pChunk->cPermRefs == 0);
         STAM_PROFILE_STOP(&pVM->pgm.s.CTX_SUFF(pStats)->StatChunkFindCandidate, a);
         return Args.pChunk->Core.Key;
@@ -3845 +3854 @@
     return INT32_MAX;
 }
+
 
 /**
@@ -3866 +3876 @@
     {
         /* Flush the pgm pool cache; call the internal rendezvous handler as we're already in a rendezvous handler here. */
-        /* todo: also not really efficient to unmap a chunk that contains PD or PT pages. */
+        /** @todo also not really efficient to unmap a chunk that contains PD
+         *  or PT pages. */
         pgmR3PoolClearAllRendezvous(pVM, &pVM->aCpus[0], NULL /* no need to flush the REM TLB as we already did that above */);
 
@@ -3874 +3885 @@
         GMMMAPUNMAPCHUNKREQ Req;
         Req.Hdr.u32Magic = SUPVMMR0REQHDR_MAGIC;
-        Req.Hdr.cbReq = sizeof(Req);
-        Req.pvR3 = NULL;
-        Req.idChunkMap = NIL_GMM_CHUNKID;
+        Req.Hdr.cbReq    = sizeof(Req);
+        Req.pvR3         = NULL;
+        Req.idChunkMap   = NIL_GMM_CHUNKID;
         Req.idChunkUnmap = pgmR3PhysChunkFindUnmapCandidate(pVM);
-
         if (Req.idChunkUnmap != INT32_MAX)
         {
@@ -3886 +3896 @@
             if (RT_SUCCESS(rc))
             {
-                /* remove the unmapped one. */
+                /*
+                 * Remove the unmapped one.
+                 */
                 PPGMCHUNKR3MAP pUnmappedChunk = (PPGMCHUNKR3MAP)RTAvlU32Remove(&pVM->pgm.s.ChunkR3Map.pTree, Req.idChunkUnmap);
                 AssertRelease(pUnmappedChunk);
-                pUnmappedChunk->pv = NULL;
+                AssertRelease(!pUnmappedChunk->cRefs);
+                AssertRelease(!pUnmappedChunk->cPermRefs);
+                pUnmappedChunk->pv       = NULL;
                 pUnmappedChunk->Core.Key = UINT32_MAX;
 #ifdef VBOX_WITH_2X_4GB_ADDR_SPACE
@@ -3899 +3913 @@
                 pVM->pgm.s.cUnmappedChunks++;
 
-                /* Flush dangling PGM pointers (R3 & R0 ptrs to GC physical addresses) */
-                /* todo: we should not flush chunks which include cr3 mappings. */
+                /*
+                 * Flush dangling PGM pointers (R3 & R0 ptrs to GC physical addresses).
+                 */
+                /** todo: we should not flush chunks which include cr3 mappings. */
                 for (VMCPUID idCpu = 0; idCpu < pVM->cCpus; idCpu++)
                 {
@@ -3993 +4009 @@
     if (RT_SUCCESS(rc))
     {
-        /*
-         * Update the tree.
-         */
-        /* insert the new one. */
-        AssertPtr(Req.pvR3);
-        pChunk->pv = Req.pvR3;
-        bool fRc = RTAvlU32Insert(&pVM->pgm.s.ChunkR3Map.pTree, &pChunk->Core);
-        AssertRelease(fRc);
-        pVM->pgm.s.ChunkR3Map.c++;
-        pVM->pgm.s.cMappedChunks++;
-
         /*
          * If we're running out of virtual address space, then we should
@@ -4021 +4026 @@
          *        map+unmap as one kernel call without any rendezvous or
          *        other precautions. */
-        if (pVM->pgm.s.ChunkR3Map.c >= pVM->pgm.s.ChunkR3Map.cMax)
+        if (pVM->pgm.s.ChunkR3Map.c + 1 >= pVM->pgm.s.ChunkR3Map.cMax)
         {
             switch (VMR3GetState(pVM))
@@ -4043 +4048 @@
             }
         }
+
+        /*
+         * Update the tree.  We must do this after any unmapping to make sure
+         * the chunk we're going to return isn't unmapped by accident.
+         */
+        /* insert the new one. */
+        AssertPtr(Req.pvR3);
+        pChunk->pv = Req.pvR3;
+        bool fRc = RTAvlU32Insert(&pVM->pgm.s.ChunkR3Map.pTree, &pChunk->Core);
+        AssertRelease(fRc);
+        pVM->pgm.s.ChunkR3Map.c++;
+        pVM->pgm.s.cMappedChunks++;
     }
     else

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

@@ -491 +491 @@
         STAM_COUNTER_INC(&pVM->pgm.s.CTX_SUFF(pStats)->CTX_MID_Z(Stat,PageMapTlbHits));
         rc = VINF_SUCCESS;
+        AssertPtr(pTlbe->pv);
+        Assert(!pTlbe->pMap || RT_VALID_PTR(pTlbe->pMap->pv));
     }
     else