Changeset 29209 in vbox for trunk/src

Timestamp:

May 7, 2010 1:16:53 PM (15 years ago)

Author:

vboxsync

Message:

More shared paging updates

Location:

trunk/src/VBox/VMM/VMMR0

Files:

• GMMR0.cpp (modified) (6 diffs)
• PGMR0.cpp (modified) (6 diffs)

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

@@ -3502 +3502 @@
                 &&  !strcmp(pGlobalModule->szVersion, pszVersion))
             {
-                Assert(!pRecVM->fCollision);
-
                 /* Save reference. */
                 pRecVM->pGlobalModule = pGlobalModule;
+                if (    fNewModule
+                    ||  pRecVM->fCollision == true) /* colliding module unregistered and new one registerd since the last check */
+                    pGlobalModule->cUsers++;
+
                 pRecVM->fCollision    = false;
-                if (fNewModule)
-                    pGlobalModule->cUsers++;
                 rc = VINF_SUCCESS;
             }
@@ -3590 +3590 @@
             goto end;
         }
-        /* Force pRec to go out of scope after freeing it. */
+        /* Remove reference to global shared module. */
+        if (!pRecVM->fCollision)
         {
             PGMMSHAREDMODULE pRec = pRecVM->pGlobalModule;
             Assert(pRec);
-            Assert(pRec->cUsers);
-
-            pRec->cUsers--;
-            if (pRec->cUsers == 0)
+
+            if (pRec)   /* paranoia */
             {
-                for (unsigned i = 0; i < pRec->cRegions; i++)
-                    if (pRec->aRegions[i].paHCPhysPageID)
-                        RTMemFree(pRec->aRegions[i].paHCPhysPageID);
-
-                RTMemFree(pRec);
+                Assert(pRec->cUsers);
+                pRec->cUsers--;
+                if (pRec->cUsers == 0)
+                {
+                    /* Free the ranges, but leave the pages intact as there might still be references; they will be cleared by the COW mechanism. */
+                    for (unsigned i = 0; i < pRec->cRegions; i++)
+                        if (pRec->aRegions[i].paHCPhysPageID)
+                            RTMemFree(pRec->aRegions[i].paHCPhysPageID);
+
+                    RTMemFree(pRec);
+                }
             }
-        }
+            else
+                rc = VERR_PGM_SHARED_MODULE_REGISTRATION_INCONSISTENCY;
+        }
+        else
+            Assert(!pRecVM->pGlobalModule);
+
         RTMemFree(pRecVM);
 
@@ -3738 +3748 @@
                 else
                 {
+                    uint8_t  *pbLocalPage, *pbSharedPage;
+                    uint8_t  *pbChunk;
+                    PGMMCHUNK pChunk;
+
                     Assert(paPageDesc[i].uHCPhysPageId != pGlobalRegion->paHCPhysPageID[i]);
 
-                    PGMMPAGE pPage = gmmR0GetPage(pGMM, paPageDesc[i].uHCPhysPageId);
+                    /* Get the shared page source. */
+                    PGMMPAGE pPage = gmmR0GetPage(pGMM, pGlobalRegion->paHCPhysPageID[i]);
                     if (!pPage)
                     {
@@ -3751 +3766 @@
                     Log(("Replace existing page guest %RGp host %RHp -> %RHp\n", paPageDesc[i].GCPhys, paPageDesc[i].HCPhys, (pPage->Private.pfn << 12)));
 
-                    PGMMCHUNK pChunk = gmmR0GetChunk(pGMM, paPageDesc[i].uHCPhysPageId >> GMM_CHUNKID_SHIFT);
+                    /* Calculate the virtual address of the local page. */
+                    pChunk = gmmR0GetChunk(pGMM, paPageDesc[i].uHCPhysPageId >> GMM_CHUNKID_SHIFT);
+                    if (pChunk)
+                    {
+                        if (!gmmR0IsChunkMapped(pGVM, pChunk, (PRTR3PTR)&pbChunk))
+                        {
+                            AssertFailed();
+                            rc = VERR_PGM_PHYS_INVALID_PAGE_ID;
+                            goto end;
+                        }
+                        pbLocalPage = pbChunk + ((paPageDesc[i].uHCPhysPageId & GMM_PAGEID_IDX_MASK) << PAGE_SHIFT);
+                    }
+                    else
+                    {
+                        AssertFailed();
+                        rc = VERR_PGM_PHYS_INVALID_PAGE_ID;
+                        goto end;
+                    }
+                    
+                    /* Calculate the virtual address of the shared page. */
+                    pChunk = gmmR0GetChunk(pGMM, pGlobalRegion->paHCPhysPageID[i] >> GMM_CHUNKID_SHIFT);
                     Assert(pChunk); /* can't fail as gmmR0GetPage succeeded. */
-                    uint8_t *pbR3Page, *pbR3Chunk;
 
                     /* Get the virtual address of the physical page; map the chunk into the VM process if not already done. */
-                    if (!gmmR0IsChunkMapped(pGVM, pChunk, (PRTR3PTR)&pbR3Chunk))
+                    if (!gmmR0IsChunkMapped(pGVM, pChunk, (PRTR3PTR)&pbChunk))
                     {
-                        rc = gmmR0MapChunk(pGMM, pGVM, pChunk, (PRTR3PTR)&pbR3Chunk);
+                        rc = gmmR0MapChunk(pGMM, pGVM, pChunk, (PRTR3PTR)&pbChunk);
                         if (rc != VINF_SUCCESS)
                         {
@@ -3765 +3799 @@
                         }
                     }
-                    pbR3Page = pbR3Chunk + ((paPageDesc[i].uHCPhysPageId & GMM_PAGEID_IDX_MASK) << PAGE_SHIFT);
+                    pbSharedPage = pbChunk + ((pGlobalRegion->paHCPhysPageID[i] & GMM_PAGEID_IDX_MASK) << PAGE_SHIFT);
+
+                    /** todo write ASMMemComparePage. */
+                    if (memcmp(pbSharedPage, pbLocalPage, PAGE_SIZE))
+                    {
+                        Log(("Unexpected differences found between local and shared page; skip\n"));
+                        continue;
+                    }
 
                     /* Free the old local page. */
@@ -3773 +3814 @@
                     rc = gmmR0FreePages(pGMM, pGVM, 1, &PageDesc, GMMACCOUNT_BASE);
                     AssertRC(rc);
+
+                    /* Increase reference count. */
+                    pPage->Shared.cRefs++;
 
                     /* Pass along the new physical address & page id. */

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

@@ -342 +342 @@
                 RTMemFree(paPageDesc);
 
-            paPageDesc = (PGMMSHAREDPAGEDESC)RTMemAlloc((cbRegion << PAGE_SHIFT) * sizeof(*paPageDesc));
+            paPageDesc = (PGMMSHAREDPAGEDESC)RTMemAlloc((cbRegion >> PAGE_SHIFT) * sizeof(*paPageDesc));
             if (!paPageDesc)
             {
@@ -359 +359 @@
             rc = PGMGstGetPage(pVCpu, GCRegion, &GCPhys, &fFlags);
             if (    rc == VINF_SUCCESS
-                &&  !(fFlags & X86_PTE_RW))
+                &&  !(fFlags & X86_PTE_RW)) /* important as we make assumptions about this below! */
             {
                 PPGMPAGE pPage = pgmPhysGetPage(&pVM->pgm.s, GCPhys);
@@ -387 +387 @@
             if (RT_FAILURE(rc))
                 break;
-
+                
             for (unsigned i = 0; i < idxPage; i++)
             {
@@ -394 +394 @@
                 {
                     /** todo: maybe cache these to prevent the nth lookup. */
-                    PPGMPAGE pPage = pgmPhysGetPage(&pVM->pgm.s, paPageDesc[idxPage].GCPhys);
+                    PPGMPAGE pPage = pgmPhysGetPage(&pVM->pgm.s, paPageDesc[i].GCPhys);
                     if (!pPage)
                     {
@@ -404 +404 @@
                     Assert(!PGM_PAGE_IS_SHARED(pPage));
 
-                    if (paPageDesc[idxPage].HCPhys != PGM_PAGE_GET_HCPHYS(pPage))
+                    if (paPageDesc[i].HCPhys != PGM_PAGE_GET_HCPHYS(pPage))
                     {
                         bool fFlush = false;
 
-                        /* Page was replaced by an existing shared version of it; dereference the page first. */
-                        rc = pgmPoolTrackUpdateGCPhys(pVM, paPageDesc[idxPage].GCPhys, pPage, true /* clear the entries */, &fFlush);
+                        /* Page was replaced by an existing shared version of it; clear all references first. */
+                        rc = pgmPoolTrackUpdateGCPhys(pVM, paPageDesc[i].GCPhys, pPage, true /* clear the entries */, &fFlush);
                         if (RT_FAILURE(rc))
                         {
@@ -420 +420 @@
 
                         /* Update the physical address and page id now. */
-                        PGM_PAGE_SET_HCPHYS(pPage, paPageDesc[idxPage].HCPhys);
-                        PGM_PAGE_SET_PAGEID(pPage, paPageDesc[idxPage].uHCPhysPageId);
+                        PGM_PAGE_SET_HCPHYS(pPage, paPageDesc[i].HCPhys);
+                        PGM_PAGE_SET_PAGEID(pPage, paPageDesc[i].uHCPhysPageId);
                     }
                     /* else nothing changed (== this page is now a shared page), so no need to flush anything. */