Changeset 38953 in vbox for trunk/src

Timestamp:

Oct 6, 2011 8:49:36 AM (14 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

74324

Message:

PGM: Attempt at fixing the VERR_MAP_FAILED during state save problem on 32-bit hosts when assigning lots of memory to the guest. PGM should lock down guest RAM pages before use and release them afterwards like everyone else. Still quite some stuff left to do there, so I've deviced a little hack for tracking unlocked mappings and using this as input when deciding to do async or sync chunk unmapping at save/load time. See xtracker #5912 and public ticket 7929.

Location:

trunk/src/VBox/VMM

Files:

• VMMAll/PGMAll.cpp (modified) (7 diffs)
• VMMAll/PGMAllBth.h (modified) (2 diffs)
• VMMAll/PGMAllPhys.cpp (modified) (46 diffs)
• VMMR0/PGMR0.cpp (modified) (2 diffs)
• VMMR3/PGM.cpp (modified) (7 diffs)
• VMMR3/PGMPhys.cpp (modified) (3 diffs)
• VMMR3/PGMSavedState.cpp (modified) (9 diffs)
• VMMR3/PGMSharedPage.cpp (modified) (1 diff)
• include/PGMInternal.h (modified) (7 diffs)
• include/internal/pgm.h (modified) (1 diff)
• testcase/tstVMStructRC.cpp (modified) (1 diff)
• testcase/tstVMStructSize.cpp (modified) (2 diffs)

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

@@ -1452 +1452 @@
     {
         RTHCPTR HCPtrGuestCR3;
-        rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhysCR3, (void **)&HCPtrGuestCR3);
+        rc = pgmPhysGCPhys2CCPtrInternalDepr(pVM, pPage, GCPhysCR3, (void **)&HCPtrGuestCR3);
         if (RT_SUCCESS(rc))
         {
@@ -1494 +1494 @@
     {
         RTHCPTR HCPtrGuestCR3;
-        rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhysCR3, (void **)&HCPtrGuestCR3);
+        rc = pgmPhysGCPhys2CCPtrInternalDepr(pVM, pPage, GCPhysCR3, (void **)&HCPtrGuestCR3);
         if (RT_SUCCESS(rc))
         {
@@ -1544 +1544 @@
         RTHCPTR     HCPtr       = NIL_RTHCPTR;
 #if !defined(IN_RC) && !defined(VBOX_WITH_2X_4GB_ADDR_SPACE_IN_R0)
-        rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &HCPtr);
+        rc = pgmPhysGCPhys2CCPtrInternalDepr(pVM, pPage, GCPhys, &HCPtr);
         AssertRC(rc);
 #endif
@@ -1604 +1604 @@
     {
         RTHCPTR HCPtrGuestCR3;
-        rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhysCR3, (void **)&HCPtrGuestCR3);
+        rc = pgmPhysGCPhys2CCPtrInternalDepr(pVM, pPage, GCPhysCR3, (void **)&HCPtrGuestCR3);
         if (RT_SUCCESS(rc))
         {
@@ -2337 +2337 @@
 VMMDECL(bool) PGMIsLockOwner(PVM pVM)
 {
-    return PDMCritSectIsOwner(&pVM->pgm.s.CritSect);
+    return PDMCritSectIsOwner(&pVM->pgm.s.CritSectX);
 }
 
@@ -2365 +2365 @@
 int pgmLock(PVM pVM)
 {
-    int rc = PDMCritSectEnter(&pVM->pgm.s.CritSect, VERR_SEM_BUSY);
+    int rc = PDMCritSectEnter(&pVM->pgm.s.CritSectX, VERR_SEM_BUSY);
 #if defined(IN_RC) || defined(IN_RING0)
     if (rc == VERR_SEM_BUSY)
@@ -2383 +2383 @@
 void pgmUnlock(PVM pVM)
 {
-    PDMCritSectLeave(&pVM->pgm.s.CritSect);
+    uint32_t cDeprecatedPageLocks = pVM->pgm.s.cDeprecatedPageLocks;
+    pVM->pgm.s.cDeprecatedPageLocks = 0;
+    int rc = PDMCritSectLeave(&pVM->pgm.s.CritSectX);
+    if (rc == VINF_SEM_NESTED)
+        pVM->pgm.s.cDeprecatedPageLocks = cDeprecatedPageLocks;
 }
 

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

@@ -4425 +4425 @@
     int rc = VINF_SUCCESS;
 # else
-    int rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPageCR3, GCPhysCR3 & GST_CR3_PAGE_MASK, (void **)&HCPtrGuestCR3); /** @todo r=bird: This GCPhysCR3 masking isn't necessary. */
+    int rc = pgmPhysGCPhys2CCPtrInternalDepr(pVM, pPageCR3, GCPhysCR3 & GST_CR3_PAGE_MASK, (void **)&HCPtrGuestCR3); /** @todo r=bird: This GCPhysCR3 masking isn't necessary. */
 # endif
     pgmUnlock(pVM);
@@ -4473 +4473 @@
                     int rc2 = VINF_SUCCESS;
 #  else
-                    int rc2 = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, (void **)&HCPtr);
+                    int rc2 = pgmPhysGCPhys2CCPtrInternalDepr(pVM, pPage, GCPhys, (void **)&HCPtr);
 #  endif
                     pgmUnlock(pVM);

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

@@ -632 +632 @@
     pVM->pgm.s.aHandyPages[iHandyPage].HCPhysGCPhys = GCPhys & ~(RTGCPHYS)PAGE_OFFSET_MASK;
 
-    const void *pvSharedPage = NULL;
-
+    void *pvSharedPage = NULL;
     if (PGM_PAGE_IS_SHARED(pPage))
     {
@@ -645 +644 @@
         pVM->pgm.s.cSharedPages--;
 
-        /* Grab the address of the page so we can make a copy later on. */
-        rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, pPage, GCPhys, &pvSharedPage);
+        /* Grab the address of the page so we can make a copy later on. (safe) */
+        rc = pgmPhysPageMap(pVM, pPage, GCPhys, &pvSharedPage);
         AssertRC(rc);
     }
@@ -670 +669 @@
     {
         /* Get the virtual address of the new page. */
-        void *pvNewPage;
-        rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvNewPage);
-        AssertRC(rc);
-        if (rc == VINF_SUCCESS)
-        {
-            /** @todo todo write ASMMemCopyPage */
-            memcpy(pvNewPage, pvSharedPage, PAGE_SIZE);
+        PGMPAGEMAPLOCK  PgMpLck;
+        void           *pvNewPage;
+        rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvNewPage, &PgMpLck); AssertRC(rc);
+        if (RT_SUCCESS(rc))
+        {
+            memcpy(pvNewPage, pvSharedPage, PAGE_SIZE); /** @todo todo write ASMMemCopyPage */
+            pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
         }
     }
@@ -922 +921 @@
  *
  * @remarks Called from within the PGM critical section.  The mapping is only
- *          valid while your inside this section.
+ *          valid while you are inside this section.
  */
 int pgmPhysPageMapByPageID(PVM pVM, uint32_t idPage, RTHCPHYS HCPhys, void **ppv)
@@ -1120 +1119 @@
  *
  * @remarks Called from within the PGM critical section.  The mapping is only
- *          valid while your inside this section.
+ *          valid while you are inside section.
  */
 int pgmPhysPageMakeWritableAndMap(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, void **ppv)
@@ -1156 +1155 @@
  *
  * @remarks Called from within the PGM critical section.  The mapping is only
- *          valid while your inside this section.
+ *          valid while you are inside section.
  */
 int pgmPhysPageMap(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, void **ppv)
@@ -1184 +1183 @@
  *
  * @remarks Called from within the PGM critical section.  The mapping is only
- *          valid while your inside this section.
+ *          valid while you are inside this section.
  */
 int pgmPhysPageMapReadOnly(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, void const **ppv)
@@ -1292 +1291 @@
  *
  * @internal
- */
-int pgmPhysGCPhys2CCPtrInternal(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, void **ppv)
+ * @deprecated Use pgmPhysGCPhys2CCPtrInternalEx.
+ */
+int pgmPhysGCPhys2CCPtrInternalDepr(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, void **ppv)
 {
     int rc;
     AssertReturn(pPage, VERR_INTERNAL_ERROR);
     PGM_LOCK_ASSERT_OWNER(pVM);
+    pVM->pgm.s.cDeprecatedPageLocks++;
 
     /*
@@ -1333 +1334 @@
 }
 
-
-/**
- * Internal version of PGMPhysGCPhys2CCPtrReadOnly that expects the caller to
- * own the PGM lock and therefore not need to lock the mapped page.
+#if !defined(IN_RC) && !defined(VBOX_WITH_2X_4GB_ADDR_SPACE_IN_R0)
+
+/**
+ * Locks a page mapping for writing.
+ *
+ * @param   pVM                 The VM handle.
+ * @param   pPage               The page.
+ * @param   pTlbe               The mapping TLB entry for the page.
+ * @param   pLock               The lock structure (output).
+ */
+DECLINLINE(void) pgmPhysPageMapLockForWriting(PVM pVM, PPGMPAGE pPage, PPGMPAGEMAPTLBE pTlbe, PPGMPAGEMAPLOCK pLock)
+{
+    PPGMPAGEMAP pMap = pTlbe->pMap;
+    if (pMap)
+        pMap->cRefs++;
+
+    unsigned cLocks = PGM_PAGE_GET_WRITE_LOCKS(pPage);
+    if (RT_LIKELY(cLocks < PGM_PAGE_MAX_LOCKS - 1))
+    {
+        if (cLocks == 0)
+            pVM->pgm.s.cWriteLockedPages++;
+        PGM_PAGE_INC_WRITE_LOCKS(pPage);
+    }
+    else if (cLocks != PGM_PAGE_MAX_LOCKS)
+    {
+        PGM_PAGE_INC_WRITE_LOCKS(pPage);
+        AssertMsgFailed(("%R[pgmpage] is entering permanent write locked state!\n", pPage));
+        if (pMap)
+            pMap->cRefs++; /* Extra ref to prevent it from going away. */
+    }
+
+    pLock->uPageAndType = (uintptr_t)pPage | PGMPAGEMAPLOCK_TYPE_WRITE;
+    pLock->pvMap = pMap;
+}
+
+/**
+ * Locks a page mapping for reading.
+ *
+ * @param   pVM                 The VM handle.
+ * @param   pPage               The page.
+ * @param   pTlbe               The mapping TLB entry for the page.
+ * @param   pLock               The lock structure (output).
+ */
+DECLINLINE(void) pgmPhysPageMapLockForReading(PVM pVM, PPGMPAGE pPage, PPGMPAGEMAPTLBE pTlbe, PPGMPAGEMAPLOCK pLock)
+{
+    PPGMPAGEMAP pMap = pTlbe->pMap;
+    if (pMap)
+        pMap->cRefs++;
+
+    unsigned cLocks = PGM_PAGE_GET_READ_LOCKS(pPage);
+    if (RT_LIKELY(cLocks < PGM_PAGE_MAX_LOCKS - 1))
+    {
+        if (cLocks == 0)
+            pVM->pgm.s.cReadLockedPages++;
+        PGM_PAGE_INC_READ_LOCKS(pPage);
+    }
+    else if (cLocks != PGM_PAGE_MAX_LOCKS)
+    {
+        PGM_PAGE_INC_READ_LOCKS(pPage);
+        AssertMsgFailed(("%R[pgmpage] is entering permanent read locked state!\n", pPage));
+        if (pMap)
+            pMap->cRefs++; /* Extra ref to prevent it from going away. */
+    }
+
+    pLock->uPageAndType = (uintptr_t)pPage | PGMPAGEMAPLOCK_TYPE_READ;
+    pLock->pvMap = pMap;
+}
+
+#endif /* !IN_RC && !VBOX_WITH_2X_4GB_ADDR_SPACE_IN_R0 */
+
+
+/**
+ * Internal version of PGMPhysGCPhys2CCPtr that expects the caller to
+ * own the PGM lock and have access to the page structure.
  *
  * @returns VBox status code.
@@ -1347 +1418 @@
  * @param   pPage       Pointer to the PGMPAGE structure for the page.
  * @param   ppv         Where to store the address corresponding to GCPhys.
+ * @param   pLock       Where to store the lock information that
+ *                      pgmPhysReleaseInternalPageMappingLock needs.
  *
  * @internal
  */
-int pgmPhysGCPhys2CCPtrInternalReadOnly(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, const void **ppv)
+int pgmPhysGCPhys2CCPtrInternal(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, void **ppv, PPGMPAGEMAPLOCK pLock)
+{
+    int rc;
+    AssertReturn(pPage, VERR_INTERNAL_ERROR);
+    PGM_LOCK_ASSERT_OWNER(pVM);
+
+    /*
+     * Make sure the page is writable.
+     */
+    if (RT_UNLIKELY(PGM_PAGE_GET_STATE(pPage) != PGM_PAGE_STATE_ALLOCATED))
+    {
+        rc = pgmPhysPageMakeWritable(pVM, pPage, GCPhys);
+        if (RT_FAILURE(rc))
+            return rc;
+        AssertMsg(rc == VINF_SUCCESS || rc == VINF_PGM_SYNC_CR3 /* not returned */, ("%Rrc\n", rc));
+    }
+    Assert(PGM_PAGE_GET_HCPHYS(pPage) != 0);
+
+    /*
+     * Do the job.
+     */
+#if defined(IN_RC) || defined(VBOX_WITH_2X_4GB_ADDR_SPACE_IN_R0)
+    void *pv;
+    PVMCPU pVCpu = VMMGetCpu(pVM);
+    rc = pgmRZDynMapHCPageInlined(pVCpu,
+                                  PGM_PAGE_GET_HCPHYS(pPage),
+                                  &pv
+                                  RTLOG_COMMA_SRC_POS);
+    if (RT_FAILURE(rc))
+        return rc;
+    *ppv = (void *)((uintptr_t)pv | (uintptr_t)(GCPhys & PAGE_OFFSET_MASK));
+    pLock->pvPage = pv;
+    pLock->pVCpu  = pVCpu;
+
+#else
+    PPGMPAGEMAPTLBE pTlbe;
+    rc = pgmPhysPageQueryTlbeWithPage(pVM, pPage, GCPhys, &pTlbe);
+    if (RT_FAILURE(rc))
+        return rc;
+    pgmPhysPageMapLockForWriting(pVM, pPage, pTlbe, pLock);
+    *ppv = (void *)((uintptr_t)pTlbe->pv | (uintptr_t)(GCPhys & PAGE_OFFSET_MASK));
+#endif
+    return VINF_SUCCESS;
+}
+
+
+/**
+ * Internal version of PGMPhysGCPhys2CCPtrReadOnly that expects the caller to
+ * own the PGM lock and have access to the page structure.
+ *
+ * @returns VBox status code.
+ * @retval  VINF_SUCCESS on success.
+ * @retval  VERR_PGM_PHYS_PAGE_RESERVED it it's a valid page but has no physical backing.
+ * @retval  VERR_PGM_INVALID_GC_PHYSICAL_ADDRESS if it's not a valid physical address.
+ *
+ * @param   pVM         The VM handle.
+ * @param   GCPhys      The guest physical address of the page that should be mapped.
+ * @param   pPage       Pointer to the PGMPAGE structure for the page.
+ * @param   ppv         Where to store the address corresponding to GCPhys.
+ * @param   pLock       Where to store the lock information that
+ *                      pgmPhysReleaseInternalPageMappingLock needs.
+ *
+ * @internal
+ */
+int pgmPhysGCPhys2CCPtrInternalReadOnly(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, const void **ppv, PPGMPAGEMAPLOCK pLock)
 {
     AssertReturn(pPage, VERR_INTERNAL_ERROR);
@@ -1357 +1494 @@
 
     /*
-     * Get the mapping address.
+     * Do the job.
      */
 #if defined(IN_RC) || defined(VBOX_WITH_2X_4GB_ADDR_SPACE_IN_R0)
     void *pv;
-    int rc = pgmRZDynMapHCPageInlined(VMMGetCpu(pVM),
+    PVMCPU pVCpu = VMMGetCpu(pVM);
+    int rc = pgmRZDynMapHCPageInlined(pVCpu,
                                       PGM_PAGE_GET_HCPHYS(pPage),
                                       &pv
@@ -1368 +1506 @@
         return rc;
     *ppv = (void *)((uintptr_t)pv | (uintptr_t)(GCPhys & PAGE_OFFSET_MASK));
+    pLock->pvPage = pv;
+    pLock->pVCpu  = pVCpu;
+
 #else
     PPGMPAGEMAPTLBE pTlbe;
@@ -1373 +1514 @@
     if (RT_FAILURE(rc))
         return rc;
+    pgmPhysPageMapLockForReading(pVM, pPage, pTlbe, pLock);
     *ppv = (void *)((uintptr_t)pTlbe->pv | (uintptr_t)(GCPhys & PAGE_OFFSET_MASK));
 #endif
@@ -1382 +1524 @@
  * Requests the mapping of a guest page into the current context.
  *
- * This API should only be used for very short term, as it will consume
- * scarse resources (R0 and GC) in the mapping cache. When you're done
- * with the page, call PGMPhysReleasePageMappingLock() ASAP to release it.
+ * This API should only be used for very short term, as it will consume scarse
+ * resources (R0 and GC) in the mapping cache. When you're done with the page,
+ * call PGMPhysReleasePageMappingLock() ASAP to release it.
  *
  * This API will assume your intention is to write to the page, and will
@@ -1396 +1538 @@
  *
  * @param   pVM         The VM handle.
- * @param   GCPhys      The guest physical address of the page that should be mapped.
+ * @param   GCPhys      The guest physical address of the page that should be
+ *                      mapped.
  * @param   ppv         Where to store the address corresponding to GCPhys.
- * @param   pLock       Where to store the lock information that PGMPhysReleasePageMappingLock needs.
+ * @param   pLock       Where to store the lock information that
+ *                      PGMPhysReleasePageMappingLock needs.
  *
  * @remarks The caller is responsible for dealing with access handlers.
  * @todo    Add an informational return code for pages with access handlers?
  *
- * @remark  Avoid calling this API from within critical sections (other than the
- *          PGM one) because of the deadlock risk. External threads may need to
- *          delegate jobs to the EMTs.
+ * @remark  Avoid calling this API from within critical sections (other than
+ *          the PGM one) because of the deadlock risk. External threads may
+ *          need to delegate jobs to the EMTs.
+ * @remarks Only one page is mapped!  Make no assumption about what's after or
+ *          before the returned page!
  * @thread  Any thread.
  */
@@ -1446 +1592 @@
 
 #else  /* IN_RING3 || IN_RING0 */
-    /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */
-    /** @todo : This can be dangerous if abused for more than one page; the ring-3 mapping is only valid for ranges that do NOT cross a chunk boundary.   */
-    /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */
-
     /*
      * Query the Physical TLB entry for the page (may fail).
@@ -1476 +1618 @@
              * Now, just perform the locking and calculate the return address.
              */
-            PPGMPAGEMAP pMap = pTlbe->pMap;
-            if (pMap)
-                pMap->cRefs++;
-
-            unsigned cLocks = PGM_PAGE_GET_WRITE_LOCKS(pPage);
-            if (RT_LIKELY(cLocks < PGM_PAGE_MAX_LOCKS - 1))
-            {
-                if (cLocks == 0)
-                    pVM->pgm.s.cWriteLockedPages++;
-                PGM_PAGE_INC_WRITE_LOCKS(pPage);
-            }
-            else if (cLocks != PGM_PAGE_GET_WRITE_LOCKS(pPage))
-            {
-                PGM_PAGE_INC_WRITE_LOCKS(pPage);
-                AssertMsgFailed(("%RGp / %R[pgmpage] is entering permanent write locked state!\n", GCPhys, pPage));
-                if (pMap)
-                    pMap->cRefs++; /* Extra ref to prevent it from going away. */
-            }
-
+            pgmPhysPageMapLockForWriting(pVM, pPage, pTlbe, pLock);
             *ppv = (void *)((uintptr_t)pTlbe->pv | (uintptr_t)(GCPhys & PAGE_OFFSET_MASK));
-            pLock->uPageAndType = (uintptr_t)pPage | PGMPAGEMAPLOCK_TYPE_WRITE;
-            pLock->pvMap = pMap;
         }
     }
@@ -1510 +1632 @@
  * Requests the mapping of a guest page into the current context.
  *
- * This API should only be used for very short term, as it will consume
- * scarse resources (R0 and GC) in the mapping cache. When you're done
- * with the page, call PGMPhysReleasePageMappingLock() ASAP to release it.
+ * This API should only be used for very short term, as it will consume scarse
+ * resources (R0 and GC) in the mapping cache.  When you're done with the page,
+ * call PGMPhysReleasePageMappingLock() ASAP to release it.
  *
  * @returns VBox status code.
@@ -1520 +1642 @@
  *
  * @param   pVM         The VM handle.
- * @param   GCPhys      The guest physical address of the page that should be mapped.
+ * @param   GCPhys      The guest physical address of the page that should be
+ *                      mapped.
  * @param   ppv         Where to store the address corresponding to GCPhys.
- * @param   pLock       Where to store the lock information that PGMPhysReleasePageMappingLock needs.
+ * @param   pLock       Where to store the lock information that
+ *                      PGMPhysReleasePageMappingLock needs.
  *
  * @remarks The caller is responsible for dealing with access handlers.
  * @todo    Add an informational return code for pages with access handlers?
  *
- * @remark  Avoid calling this API from within critical sections (other than
+ * @remarks Avoid calling this API from within critical sections (other than
  *          the PGM one) because of the deadlock risk.
+ * @remarks Only one page is mapped!  Make no assumption about what's after or
+ *          before the returned page!
  * @thread  Any thread.
  */
@@ -1567 +1693 @@
 
 #else  /* IN_RING3 || IN_RING0 */
-
-    /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */
-    /** @todo : This can be dangerous if abused for more than one page; the ring-3 mapping is only valid for ranges that do NOT cross a chunk boundary.   */
-    /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */
-
     /*
      * Query the Physical TLB entry for the page (may fail).
@@ -1588 +1709 @@
              * Now, just perform the locking and calculate the return address.
              */
-            PPGMPAGEMAP pMap = pTlbe->pMap;
-            if (pMap)
-                pMap->cRefs++;
-
-            unsigned cLocks = PGM_PAGE_GET_READ_LOCKS(pPage);
-            if (RT_LIKELY(cLocks < PGM_PAGE_MAX_LOCKS - 1))
-            {
-                if (cLocks == 0)
-                    pVM->pgm.s.cReadLockedPages++;
-                PGM_PAGE_INC_READ_LOCKS(pPage);
-            }
-            else if (cLocks != PGM_PAGE_GET_READ_LOCKS(pPage))
-            {
-                PGM_PAGE_INC_READ_LOCKS(pPage);
-                AssertMsgFailed(("%RGp / %R[pgmpage] is entering permanent readonly locked state!\n", GCPhys, pPage));
-                if (pMap)
-                    pMap->cRefs++; /* Extra ref to prevent it from going away. */
-            }
-
+            pgmPhysPageMapLockForReading(pVM, pPage, pTlbe, pLock);
             *ppv = (void *)((uintptr_t)pTlbe->pv | (uintptr_t)(GCPhys & PAGE_OFFSET_MASK));
-            pLock->uPageAndType = (uintptr_t)pPage | PGMPAGEMAPLOCK_TYPE_READ;
-            pLock->pvMap = pMap;
         }
     }
@@ -1764 +1865 @@
     pgmUnlock(pVM);
 #endif /* IN_RING3 */
+}
+
+
+/**
+ * Release the internal mapping of a guest page.
+ *
+ * This is the counter part of pgmPhysGCPhys2CCPtrInternalEx and
+ * pgmPhysGCPhys2CCPtrInternalReadOnly.
+ *
+ * @param   pVM         The VM handle.
+ * @param   pLock       The lock structure initialized by the mapping function.
+ *
+ * @remarks Caller must hold the PGM lock.
+ */
+void pgmPhysReleaseInternalPageMappingLock(PVM pVM, PPGMPAGEMAPLOCK pLock)
+{
+    PGM_LOCK_ASSERT_OWNER(pVM);
+    PGMPhysReleasePageMappingLock(pVM, pLock); /* lazy for now */
 }
 
@@ -1780 +1899 @@
  * @param   pVM         The VM handle.
  * @param   GCPhys      The GC physical address to convert.
- * @param   cbRange     Physical range
  * @param   pR3Ptr      Where to store the R3 pointer on success.
  *
  * @deprecated  Avoid when possible!
  */
-VMMDECL(int) PGMPhysGCPhys2R3Ptr(PVM pVM, RTGCPHYS GCPhys, RTUINT cbRange, PRTR3PTR pR3Ptr)
+int pgmPhysGCPhys2R3Ptr(PVM pVM, RTGCPHYS GCPhys, PRTR3PTR pR3Ptr)
 {
 /** @todo this is kind of hacky and needs some more work. */
@@ -1792 +1910 @@
 #endif
 
-    Log(("PGMPhysGCPhys2R3Ptr(,%RGp,%#x,): dont use this API!\n", GCPhys, cbRange)); /** @todo eliminate this API! */
+    Log(("pgmPhysGCPhys2R3Ptr(,%RGp,): dont use this API!\n", GCPhys)); /** @todo eliminate this API! */
 #if defined(IN_RC) || defined(VBOX_WITH_2X_4GB_ADDR_SPACE_IN_R0)
     AssertFailedReturn(VERR_NOT_IMPLEMENTED);
@@ -1802 +1920 @@
     int rc = pgmPhysGetPageAndRangeEx(pVM, GCPhys, &pPage, &pRam);
     if (RT_SUCCESS(rc))
-        rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, (void **)pR3Ptr);
+        rc = pgmPhysGCPhys2CCPtrInternalDepr(pVM, pPage, GCPhys, (void **)pR3Ptr);
 
     pgmUnlock(pVM);
@@ -1809 +1927 @@
 #endif
 }
-
-
-#ifdef VBOX_STRICT
-/**
- * PGMPhysGCPhys2R3Ptr convenience for use with assertions.
- *
- * @returns The R3Ptr, NIL_RTR3PTR on failure.
- * @param   pVM         The VM handle.
- * @param   GCPhys      The GC Physical address.
- * @param   cbRange     Physical range.
- *
- * @deprecated  Avoid when possible.
- */
-VMMDECL(RTR3PTR) PGMPhysGCPhys2R3PtrAssert(PVM pVM, RTGCPHYS GCPhys, RTUINT cbRange)
-{
-    RTR3PTR R3Ptr;
-    int rc = PGMPhysGCPhys2R3Ptr(pVM, GCPhys, cbRange, &R3Ptr);
-    if (RT_SUCCESS(rc))
-        return R3Ptr;
-    return NIL_RTR3PTR;
-}
-#endif /* VBOX_STRICT */
 
 
@@ -1931 +2027 @@
      * Whatever we do we need the source page, map it first.
      */
-    const void *pvSrc = NULL;
-    int         rc    = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, pPage, GCPhys, &pvSrc);
+    PGMPAGEMAPLOCK PgMpLck;
+    const void    *pvSrc = NULL;
+    int rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, pPage, GCPhys, &pvSrc, &PgMpLck);
     if (RT_FAILURE(rc))
     {
@@ -1977 +2074 @@
         /* In R0 and RC the callbacks cannot handle this context, so we'll fail. */
         //AssertReleaseMsgFailed(("Wrong API! GCPhys=%RGp cb=%#x\n", GCPhys, cb));
+        pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
         return VERR_PGM_PHYS_WR_HIT_HANDLER;
 #endif
@@ -2018 +2116 @@
         /* In R0 and RC the callbacks cannot handle this context, so we'll fail. */
         //AssertReleaseMsgFailed(("Wrong API! GCPhys=%RGp cb=%#x\n", GCPhys, cb));
+        pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
         return VERR_PGM_PHYS_WR_HIT_HANDLER;
 #endif
@@ -2027 +2126 @@
     if (rc == VINF_PGM_HANDLER_DO_DEFAULT)
         memcpy(pvBuf, pvSrc, cb);
+    pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
     return rc;
 }
@@ -2094 +2194 @@
                      * Get the pointer to the page.
                      */
-                    const void *pvSrc;
-                    int rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, pPage, pRam->GCPhys + off, &pvSrc);
+                    PGMPAGEMAPLOCK PgMpLck;
+                    const void    *pvSrc;
+                    int rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, pPage, pRam->GCPhys + off, &pvSrc, &PgMpLck);
                     if (RT_SUCCESS(rc))
+                    {
                         memcpy(pvBuf, pvSrc, cb);
+                        pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
+                    }
                     else
                     {
@@ -2164 +2268 @@
 static int pgmPhysWriteHandler(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, void const *pvBuf, size_t cbWrite)
 {
-    void *pvDst = NULL;
-    int rc;
+    PGMPAGEMAPLOCK  PgMpLck;
+    void           *pvDst = NULL;
+    int             rc;
 
     /*
@@ -2196 +2301 @@
             Log5(("pgmPhysWriteHandler: GCPhys=%RGp cbRange=%#x pPage=%R[pgmpage] phys %s\n", GCPhys, cbRange, pPage, R3STRING(pCur->pszDesc) ));
             if (!PGM_PAGE_IS_MMIO(pPage))
-                rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvDst);
+                rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvDst, &PgMpLck);
             else
                 rc = VINF_SUCCESS;
@@ -2217 +2322 @@
                 pCur = NULL; /* might not be valid anymore. */
 # endif
-                if (rc == VINF_PGM_HANDLER_DO_DEFAULT)
-                    memcpy(pvDst, pvBuf, cbRange);
+                if (rc == VINF_PGM_HANDLER_DO_DEFAULT && pvDst)
+                {
+                    if (pvDst)
+                        memcpy(pvDst, pvBuf, cbRange);
+                }
                 else
                     AssertLogRelMsg(rc == VINF_SUCCESS || rc == VINF_PGM_HANDLER_DO_DEFAULT, ("rc=%Rrc GCPhys=%RGp pPage=%R[pgmpage] %s\n", rc, GCPhys, pPage, (pCur) ? pCur->pszDesc : ""));
@@ -2226 +2334 @@
                                              GCPhys, pPage, rc), rc);
             if (RT_LIKELY(cbRange == cbWrite))
+            {
+                if (pvBuf)
+                    pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
                 return VINF_SUCCESS;
+            }
 
             /* more fun to be had below */
@@ -2262 +2374 @@
 
             Log5(("pgmPhysWriteHandler: GCPhys=%RGp cbRange=%#x pPage=%R[pgmpage] virt %s\n", GCPhys, cbRange, pPage, R3STRING(pCur->pszDesc) ));
-            rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvDst);
+            rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvDst, &PgMpLck);
             if (RT_SUCCESS(rc))
             {
@@ -2285 +2397 @@
                                              GCPhys, pPage, rc), rc);
             if (RT_LIKELY(cbRange == cbWrite))
+            {
+                if (pvBuf)
+                    pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
                 return VINF_SUCCESS;
+            }
 
             /* more fun to be had below */
@@ -2304 +2420 @@
     if (!pvDst)
     {
-        rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvDst);
+        rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvDst, &PgMpLck);
         AssertLogRelMsgReturn(RT_SUCCESS(rc),
                               ("pgmPhysGCPhys2CCPtrInternal failed on %RGp / %R[pgmpage] -> %Rrc\n",
@@ -2434 +2550 @@
             NOREF(cbRange);
             //AssertReleaseMsgFailed(("Wrong API! GCPhys=%RGp cbRange=%#x\n", GCPhys, cbRange));
+            pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
             return VERR_PGM_PHYS_WR_HIT_HANDLER;
 #endif
@@ -2463 +2580 @@
             NOREF(cbRange);
             //AssertReleaseMsgFailed(("Wrong API! GCPhys=%RGp cbRange=%#x\n", GCPhys, cbRange));
+            pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
             return VERR_PGM_PHYS_WR_HIT_HANDLER;
 #endif
@@ -2519 +2637 @@
             NOREF(cbRange);
             //AssertReleaseMsgFailed(("Wrong API! GCPhys=%RGp cbRange=%#x\n", GCPhys, cbRange));
+            pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
             return VERR_PGM_PHYS_WR_HIT_HANDLER;
 #endif
@@ -2529 +2648 @@
          */
         if (cbRange >= cbWrite)
+        {
+            pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
             return VINF_SUCCESS;
+        }
 
         cbWrite         -= cbRange;
@@ -2608 +2730 @@
                      * Get the pointer to the page.
                      */
-                    void *pvDst;
-                    int rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, pRam->GCPhys + off, &pvDst);
+                    PGMPAGEMAPLOCK PgMpLck;
+                    void          *pvDst;
+                    int rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, pRam->GCPhys + off, &pvDst, &PgMpLck);
                     if (RT_SUCCESS(rc))
                     {
                         Assert(!PGM_PAGE_IS_BALLOONED(pPage));
                         memcpy(pvDst, pvBuf, cb);
+                        pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
                     }
-                    else
                     /* Ignore writes to ballooned pages. */
-                    if (!PGM_PAGE_IS_BALLOONED(pPage))
+                    else if (!PGM_PAGE_IS_BALLOONED(pPage))
                         AssertLogRelMsgFailed(("pgmPhysGCPhys2CCPtrInternal failed on %RGp / %R[pgmpage] -> %Rrc\n",
                                                 pRam->GCPhys + off, pPage, rc));
@@ -3279 +3402 @@
         {
             /** @todo we should check reserved bits ... */
-            void *pvSrc;
-            rc = PGM_GCPHYS_2_PTR_V2(pVM, pVCpu, GCPhys, &pvSrc);
+            PGMPAGEMAPLOCK PgMpLck;
+            void const    *pvSrc;
+            rc = PGMPhysGCPhys2CCPtrReadOnly(pVM, GCPhys, &pvSrc, &PgMpLck);
             switch (rc)
             {
@@ -3286 +3410 @@
                     Log(("PGMPhysInterpretedRead: pvDst=%p pvSrc=%p cb=%d\n", pvDst, (uint8_t *)pvSrc + (GCPtrSrc & PAGE_OFFSET_MASK), cb));
                     memcpy(pvDst, (uint8_t *)pvSrc + (GCPtrSrc & PAGE_OFFSET_MASK), cb);
+                    PGMPhysReleasePageMappingLock(pVM, &PgMpLck);
                     break;
                 case VERR_PGM_PHYS_PAGE_RESERVED:
                 case VERR_PGM_INVALID_GC_PHYSICAL_ADDRESS:
-                    memset(pvDst, 0, cb); /** @todo this is wrong, it should be 0xff */
+                    memset(pvDst, 0xff, cb);
                     break;
                 default:
+                    Assert(RT_FAILURE_NP(rc));
                     return rc;
             }
@@ -3321 +3447 @@
             /** @todo we should check reserved bits ... */
             AssertMsgFailed(("cb=%d cb1=%d cb2=%d GCPtrSrc=%RGv\n", cb, cb1, cb2, GCPtrSrc));
-            void *pvSrc1;
-            rc = PGM_GCPHYS_2_PTR_V2(pVM, pVCpu, GCPhys1, &pvSrc1);
+            PGMPAGEMAPLOCK PgMpLck;
+            void const *pvSrc1;
+            rc = PGMPhysGCPhys2CCPtrReadOnly(pVM, GCPhys1, &pvSrc1, &PgMpLck);
             switch (rc)
             {
                 case VINF_SUCCESS:
                     memcpy(pvDst, (uint8_t *)pvSrc1 + (GCPtrSrc & PAGE_OFFSET_MASK), cb1);
+                    PGMPhysReleasePageMappingLock(pVM, &PgMpLck);
                     break;
                 case VERR_PGM_INVALID_GC_PHYSICAL_ADDRESS:
-                    memset(pvDst, 0, cb1); /** @todo this is wrong, it should be 0xff */
+                    memset(pvDst, 0xff, cb1);
                     break;
                 default:
+                    Assert(RT_FAILURE_NP(rc));
                     return rc;
             }
 
-            void *pvSrc2;
-            rc = PGM_GCPHYS_2_PTR_V2(pVM, pVCpu, GCPhys2, &pvSrc2);
+            void const *pvSrc2;
+            rc = PGMPhysGCPhys2CCPtrReadOnly(pVM, GCPhys2, &pvSrc2, &PgMpLck);
             switch (rc)
             {
                 case VINF_SUCCESS:
                     memcpy((uint8_t *)pvDst + cb1, pvSrc2, cb2);
+                    PGMPhysReleasePageMappingLock(pVM, &PgMpLck);
                     break;
                 case VERR_PGM_INVALID_GC_PHYSICAL_ADDRESS:
-                    memset((uint8_t *)pvDst + cb1, 0, cb2);  /** @todo this is wrong, it should be 0xff */
+                    memset((uint8_t *)pvDst + cb1, 0xff, cb2);
                     break;
                 default:
+                    Assert(RT_FAILURE_NP(rc));
                     return rc;
             }

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

@@ -68 +68 @@
 VMMR0DECL(int) PGMR0PhysAllocateHandyPages(PVM pVM, PVMCPU pVCpu)
 {
-    Assert(PDMCritSectIsOwnerEx(&pVM->pgm.s.CritSect, pVCpu));
+    PGM_LOCK_ASSERT_OWNER_EX(pVM, pVCpu);
 
     /*
@@ -183 +183 @@
 VMMR0DECL(int) PGMR0PhysAllocateLargeHandyPage(PVM pVM, PVMCPU pVCpu)
 {
-    Assert(PDMCritSectIsOwnerEx(&pVM->pgm.s.CritSect, pVCpu));
-
+    PGM_LOCK_ASSERT_OWNER_EX(pVM, pVCpu);
     Assert(!pVM->pgm.s.cLargeHandyPages);
-    int rc = GMMR0AllocateLargePage(pVM, pVCpu->idCpu, _2M, &pVM->pgm.s.aLargeHandyPage[0].idPage, &pVM->pgm.s.aLargeHandyPage[0].HCPhysGCPhys);
+
+    int rc = GMMR0AllocateLargePage(pVM, pVCpu->idCpu, _2M,
+                                    &pVM->pgm.s.aLargeHandyPage[0].idPage,
+                                    &pVM->pgm.s.aLargeHandyPage[0].HCPhysGCPhys);
     if (RT_SUCCESS(rc))
         pVM->pgm.s.cLargeHandyPages = 1;

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

@@ -1186 +1186 @@
     AssertCompile(sizeof(pVM->pgm.s) <= sizeof(pVM->pgm.padding));
     AssertCompile(sizeof(pVM->aCpus[0].pgm.s) <= sizeof(pVM->aCpus[0].pgm.padding));
-    AssertCompileMemberAlignment(PGM, CritSect, sizeof(uintptr_t));
+    AssertCompileMemberAlignment(PGM, CritSectX, sizeof(uintptr_t));
 
     /*
@@ -1342 +1342 @@
      * Initialize the PGM critical section and flush the phys TLBs
      */
-    rc = PDMR3CritSectInit(pVM, &pVM->pgm.s.CritSect, RT_SRC_POS, "PGM");
+    rc = PDMR3CritSectInit(pVM, &pVM->pgm.s.CritSectX, RT_SRC_POS, "PGM");
     AssertRCReturn(rc, rc);
 
@@ -1454 +1454 @@
 
     /* Almost no cleanup necessary, MM frees all memory. */
-    PDMR3CritSectDelete(&pVM->pgm.s.CritSect);
+    PDMR3CritSectDelete(&pVM->pgm.s.CritSectX);
 
     return rc;
@@ -2578 +2578 @@
 
     PGMDeregisterStringFormatTypes();
-    return PDMR3CritSectDelete(&pVM->pgm.s.CritSect);
+    return PDMR3CritSectDelete(&pVM->pgm.s.CritSectX);
 }
 
@@ -2689 +2689 @@
      * Get page directory addresses.
      */
+    pgmLock(pVM);
     PX86PD     pPDSrc = pgmGstGet32bitPDPtr(pVCpu);
     Assert(pPDSrc);
-    Assert(PGMPhysGCPhys2R3PtrAssert(pVM, (RTGCPHYS)(CPUMGetGuestCR3(pVCpu) & X86_CR3_PAGE_MASK), sizeof(*pPDSrc)) == pPDSrc);
 
     /*
@@ -2715 +2715 @@
         }
     }
+    pgmUnlock(pVM);
 }
 
@@ -2726 +2727 @@
 VMMR3DECL(int) PGMR3LockCall(PVM pVM)
 {
-    int rc = PDMR3CritSectEnterEx(&pVM->pgm.s.CritSect, true /* fHostCall */);
+    int rc = PDMR3CritSectEnterEx(&pVM->pgm.s.CritSectX, true /* fHostCall */);
     AssertRC(rc);
     return rc;

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

@@ -150 +150 @@
                  * Simple stuff, go ahead.
                  */
-                size_t   cb    = PAGE_SIZE - (off & PAGE_OFFSET_MASK);
+                size_t cb = PAGE_SIZE - (off & PAGE_OFFSET_MASK);
                 if (cb > cbRead)
                     cb = cbRead;
-                const void *pvSrc;
-                int rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, pPage, pRam->GCPhys + off, &pvSrc);
+                PGMPAGEMAPLOCK PgMpLck;
+                const void    *pvSrc;
+                int rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, pPage, pRam->GCPhys + off, &pvSrc, &PgMpLck);
                 if (RT_SUCCESS(rc))
+                {
                     memcpy(pvBuf, pvSrc, cb);
+                    pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
+                }
                 else
                 {
@@ -290 +294 @@
                  * Simple stuff, go ahead.
                  */
-                size_t      cb    = PAGE_SIZE - (off & PAGE_OFFSET_MASK);
+                size_t cb = PAGE_SIZE - (off & PAGE_OFFSET_MASK);
                 if (cb > cbWrite)
                     cb = cbWrite;
-                void *pvDst;
-                int rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, pRam->GCPhys + off, &pvDst);
+                PGMPAGEMAPLOCK PgMpLck;
+                void          *pvDst;
+                int rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, pRam->GCPhys + off, &pvDst, &PgMpLck);
                 if (RT_SUCCESS(rc))
+                {
                     memcpy(pvDst, pvBuf, cb);
+                    pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
+                }
                 else
                     AssertLogRelMsgFailed(("pgmPhysGCPhys2CCPtrInternal failed on %RGp / %R[pgmpage] -> %Rrc\n",
@@ -3996 +4004 @@
         pVM->pgm.s.cMappedChunks++;
 
-        /* If we're running out of virtual address space, then we should unmap another chunk. */
+        /*
+         * If we're running out of virtual address space, then we should
+         * unmap another chunk.
+         *
+         * Currently, an unmap operation requires that all other virtual CPUs
+         * are idling and not by chance making use of the memory we're
+         * unmapping.  So, we create an async unmap operation here.
+         *
+         * Now, when creating or restoring a saved state this wont work very
+         * well since we may want to restore all guest RAM + a little something.
+         * So, we have to do the unmap synchronously.  Fortunately for us
+         * though, during these operations the other virtual CPUs are inactive
+         * and it should be safe to do this.
+         */
+        /** @todo Eventually we should lock all memory when used and do
+         *        map+unmap as one kernel call without any rendezvous or
+         *        other precautions. */
         if (pVM->pgm.s.ChunkR3Map.c >= pVM->pgm.s.ChunkR3Map.cMax)
         {
-            /* Postpone the unmap operation (which requires a rendezvous operation) as we own the PGM lock here. */
-            rc = VMR3ReqCallNoWait(pVM, VMCPUID_ANY_QUEUE, (PFNRT)pgmR3PhysUnmapChunk, 1, pVM);
-            AssertRC(rc);
+            switch (VMR3GetState(pVM))
+            {
+                case VMSTATE_LOADING:
+                case VMSTATE_SAVING:
+                {
+                    PVMCPU pVCpu = VMMGetCpu(pVM);
+                    if (   pVCpu
+                        && pVM->pgm.s.cDeprecatedPageLocks == 0)
+                    {
+                        pgmR3PhysUnmapChunkRendezvous(pVM, pVCpu, NULL);
+                        break;
+                    }
+                    /* fall thru */
+                }
+                default:
+                    rc = VMR3ReqCallNoWait(pVM, VMCPUID_ANY_QUEUE, (PFNRT)pgmR3PhysUnmapChunk, 1, pVM);
+                    AssertRC(rc);
+                    break;
+            }
         }
     }

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

@@ -1243 +1243 @@
 static void pgmR3StateCalcCrc32ForRamPage(PVM pVM, PPGMRAMRANGE pCur, PPGMLIVESAVERAMPAGE paLSPages, uint32_t iPage)
 {
-    RTGCPHYS    GCPhys = pCur->GCPhys + ((RTGCPHYS)iPage << PAGE_SHIFT);
-    void const *pvPage;
-    int rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, &pCur->aPages[iPage], GCPhys, &pvPage);
+    RTGCPHYS        GCPhys = pCur->GCPhys + ((RTGCPHYS)iPage << PAGE_SHIFT);
+    PGMPAGEMAPLOCK  PgMpLck;
+    void const     *pvPage;
+    int rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, &pCur->aPages[iPage], GCPhys, &pvPage, &PgMpLck);
     if (RT_SUCCESS(rc))
+    {
         paLSPages[iPage].u32Crc = RTCrc32(pvPage, PAGE_SIZE);
+        pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
+    }
     else
         paLSPages[iPage].u32Crc = UINT32_MAX; /* Invalid */
@@ -1290 +1294 @@
     if (paLSPages[iPage].u32Crc != UINT32_MAX)
     {
-        RTGCPHYS    GCPhys = pCur->GCPhys + ((RTGCPHYS)iPage << PAGE_SHIFT);
-        void const *pvPage;
-        int rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, &pCur->aPages[iPage], GCPhys, &pvPage);
+        RTGCPHYS        GCPhys = pCur->GCPhys + ((RTGCPHYS)iPage << PAGE_SHIFT);
+        PGMPAGEMAPLOCK  PgMpLck;
+        void const     *pvPage;
+        int rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, &pCur->aPages[iPage], GCPhys, &pvPage, &PgMpLck);
         if (RT_SUCCESS(rc))
+        {
             pgmR3StateVerifyCrc32ForPage(pvPage, pCur, paLSPages, iPage, pszWhere);
+            pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
+        }
     }
 }
@@ -1333 +1341 @@
                         && (iPage & 0x7ff) == 0x100
 #endif
-                        && PDMR3CritSectYield(&pVM->pgm.s.CritSect)
+                        && PDMR3CritSectYield(&pVM->pgm.s.CritSectX)
                         && pVM->pgm.s.idRamRangesGen != idRamRangesGen)
                     {
@@ -1558 +1566 @@
                     if (   uPass != SSM_PASS_FINAL
                         && (iPage & 0x7ff) == 0x100
-                        && PDMR3CritSectYield(&pVM->pgm.s.CritSect)
+                        && PDMR3CritSectYield(&pVM->pgm.s.CritSectX)
                         && pVM->pgm.s.idRamRangesGen != idRamRangesGen)
                     {
@@ -1622 +1630 @@
                          * SSM call may block).
                          */
-                        uint8_t     abPage[PAGE_SIZE];
-                        void const *pvPage;
-                        rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, pCurPage, GCPhys, &pvPage);
+                        uint8_t         abPage[PAGE_SIZE];
+                        PGMPAGEMAPLOCK  PgMpLck;
+                        void const     *pvPage;
+                        rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, pCurPage, GCPhys, &pvPage, &PgMpLck);
                         if (RT_SUCCESS(rc))
                         {
@@ -1632 +1641 @@
                                 pgmR3StateVerifyCrc32ForPage(abPage, pCur, paLSPages, iPage, "save#3");
 #endif
+                            pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
                         }
                         pgmUnlock(pVM);
@@ -2231 +2241 @@
      * Load the page.
      */
-    void *pvPage;
-    int rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvPage);
+    PGMPAGEMAPLOCK PgMpLck;
+    void          *pvPage;
+    int rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvPage, &PgMpLck);
     if (RT_SUCCESS(rc))
+    {
         rc = SSMR3GetMem(pSSM, pvPage, PAGE_SIZE);
+        pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
+    }
 
     return rc;
@@ -2677 +2691 @@
                             || PGM_PAGE_GET_TYPE(pPage) == PGMPAGETYPE_ROM_SHADOW)
                         {
-                            void *pvDstPage;
-                            rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvDstPage);
+                            PGMPAGEMAPLOCK PgMpLck;
+                            void          *pvDstPage;
+                            rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvDstPage, &PgMpLck);
                             AssertLogRelMsgRCReturn(rc, ("GCPhys=%RGp %R[pgmpage] rc=%Rrc\n", GCPhys, pPage, rc), rc);
+
                             ASMMemZeroPage(pvDstPage);
+                            pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
                         }
                         /* Free it only if it's not part of a previously
@@ -2719 +2736 @@
                     case PGM_STATE_REC_RAM_RAW:
                     {
-                        void *pvDstPage;
-                        rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvDstPage);
+                        PGMPAGEMAPLOCK PgMpLck;
+                        void          *pvDstPage;
+                        rc = pgmPhysGCPhys2CCPtrInternal(pVM, pPage, GCPhys, &pvDstPage, &PgMpLck);
                         AssertLogRelMsgRCReturn(rc, ("GCPhys=%RGp %R[pgmpage] rc=%Rrc\n", GCPhys, pPage, rc), rc);
                         rc = SSMR3GetMem(pSSM, pvDstPage, PAGE_SIZE);
+                        pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
                         if (RT_FAILURE(rc))
                             return rc;

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

@@ -342 +342 @@
                     case PGM_PAGE_STATE_WRITE_MONITORED:
                     {
-                        const void *pvPage;
                         /* Check if the page was allocated, but completely zero. */
-                        int rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, pPage, GCPhys, &pvPage);
-                        if (    rc == VINF_SUCCESS
+                        PGMPAGEMAPLOCK PgMpLck;
+                        const void    *pvPage;
+                        int rc = pgmPhysGCPhys2CCPtrInternalReadOnly(pVM, pPage, GCPhys, &pvPage, &PgMpLck);
+                        if (    RT_SUCCESS(rc)
                             &&  ASMMemIsZeroPage(pvPage))
-                        {
                             cAllocZero++;
-                        }
-                        else
-                        if (GMMR3IsDuplicatePage(pVM, PGM_PAGE_GET_PAGEID(pPage)))
+                        else if (GMMR3IsDuplicatePage(pVM, PGM_PAGE_GET_PAGEID(pPage)))
                             cDuplicate++;
                         else
                             cUnique++;
-
+                        if (RT_SUCCESS(rc))
+                            pgmPhysReleaseInternalPageMappingLock(pVM, &PgMpLck);
                         break;
                     }

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

@@ -257 +257 @@
 #else
 # define PGM_GCPHYS_2_PTR_V2(pVM, pVCpu, GCPhys, ppv) \
-     PGMPhysGCPhys2R3Ptr(pVM, GCPhys, 1 /* one page only */, (PRTR3PTR)(ppv)) /** @todo this isn't asserting, use PGMRamGCPhys2HCPtr! */
+     pgmPhysGCPhys2R3Ptr(pVM, GCPhys, (PRTR3PTR)(ppv)) /** @todo this isn't asserting! */
 #endif
 
@@ -305 +305 @@
 #else
 # define PGM_GCPHYS_2_PTR_EX(pVM, GCPhys, ppv) \
-     PGMPhysGCPhys2R3Ptr(pVM, GCPhys, 1 /* one page only */, (PRTR3PTR)(ppv)) /** @todo this isn't asserting, use PGMRamGCPhys2HCPtr! */
+     pgmPhysGCPhys2R3Ptr(pVM, GCPhys, (PRTR3PTR)(ppv)) /** @todo this isn't asserting! */
 #endif
 
@@ -3146 +3146 @@
     /** The address of the ring-0 mapping cache if we're making use of it.  */
     RTR0PTR                         pvR0DynMapUsed;
-#if HC_ARCH_BITS == 32
-    /** Alignment padding that makes the next member start on a 8 byte boundary. */
+
+    /** Hack: Number of deprecated page mapping locks taken by the current lock
+     *  owner via pgmPhysGCPhys2CCPtrInternalDepr. */
+    uint32_t                        cDeprecatedPageLocks;
+#if HC_ARCH_BITS == 64
+    /** Alignment padding.  */
     uint32_t                        u32Alignment2;
 #endif
+
 
     /** PGM critical section.
@@ -3155 +3160 @@
      * and the page flag updating (some of it anyway).
      */
-    PDMCRITSECT                     CritSect;
+    PDMCRITSECT                     CritSectX;
 
     /**
@@ -3341 +3346 @@
 AssertCompileMemberAlignment(PGM, GCPtrMappingFixed, sizeof(RTGCPTR));
 AssertCompileMemberAlignment(PGM, HCPhysInterPD, 8);
-AssertCompileMemberAlignment(PGM, CritSect, 8);
+AssertCompileMemberAlignment(PGM, CritSectX, 8);
 AssertCompileMemberAlignment(PGM, ChunkR3Map, 8);
 AssertCompileMemberAlignment(PGM, PhysTlbHC, 8);
@@ -3854 +3859 @@
  * @param   a_pVM           The VM handle.
  */
-#define PGM_LOCK_ASSERT_OWNER(a_pVM)    Assert(PDMCritSectIsOwner(&(a_pVM)->pgm.s.CritSect))
+#define PGM_LOCK_ASSERT_OWNER(a_pVM)    Assert(PDMCritSectIsOwner(&(a_pVM)->pgm.s.CritSectX))
+/**
+ * Asserts that the caller owns the PDM lock.
+ * This is the internal variant of PGMIsLockOwner.
+ * @param   a_pVM           The VM handle.
+ * @param   a_pVCpu         The current CPU handle.
+ */
+#define PGM_LOCK_ASSERT_OWNER_EX(a_pVM, a_pVCpu)  Assert(PDMCritSectIsOwnerEx(&(a_pVM)->pgm.s.CritSectX, pVCpu))
 
 int             pgmR3MappingsFixInternal(PVM pVM, RTGCPTR GCPtrBase, uint32_t cb);
@@ -3887 +3899 @@
 int             pgmPhysPageMapReadOnly(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, void const **ppv);
 int             pgmPhysPageMapByPageID(PVM pVM, uint32_t idPage, RTHCPHYS HCPhys, void **ppv);
-int             pgmPhysGCPhys2CCPtrInternal(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, void **ppv);
-int             pgmPhysGCPhys2CCPtrInternalReadOnly(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, const void **ppv);
+int             pgmPhysGCPhys2R3Ptr(PVM pVM, RTGCPHYS GCPhys, PRTR3PTR pR3Ptr);
+int             pgmPhysGCPhys2CCPtrInternalDepr(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, void **ppv);
+int             pgmPhysGCPhys2CCPtrInternal(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, void **ppv, PPGMPAGEMAPLOCK pLock);
+int             pgmPhysGCPhys2CCPtrInternalReadOnly(PVM pVM, PPGMPAGE pPage, RTGCPHYS GCPhys, const void **ppv, PPGMPAGEMAPLOCK pLock);
+void            pgmPhysReleaseInternalPageMappingLock(PVM pVM, PPGMPAGEMAPLOCK pLock);
 VMMDECL(int)    pgmPhysHandlerRedirectToHC(PVM pVM, RTGCUINT uErrorCode, PCPUMCTXCORE pRegFrame, RTGCPTR pvFault, RTGCPHYS GCPhysFault, void *pvUser);
 VMMDECL(int)    pgmPhysRomWriteHandler(PVM pVM, RTGCUINT uErrorCode, PCPUMCTXCORE pRegFrame, RTGCPTR pvFault, RTGCPHYS GCPhysFault, void *pvUser);

trunk/src/VBox/VMM/include/internal/pgm.h

@@ -65 +65 @@
 VMMDECL(int)        PGMPhysGCPtr2CCPtr(PVMCPU pVCpu, RTGCPTR GCPtr, void **ppv, PPGMPAGEMAPLOCK pLock);
 VMMDECL(int)        PGMPhysGCPtr2CCPtrReadOnly(PVMCPU pVCpu, RTGCPTR GCPtr, void const **ppv, PPGMPAGEMAPLOCK pLock);
-VMMDECL(int)        PGMPhysGCPhys2R3Ptr(PVM pVM, RTGCPHYS GCPhys, RTUINT cbRange, PRTR3PTR pR3Ptr);
-#ifdef VBOX_STRICT
-VMMDECL(RTR3PTR)    PGMPhysGCPhys2R3PtrAssert(PVM pVM, RTGCPHYS GCPhys, RTUINT cbRange);
-#endif
 VMMR3DECL(void)     PGMR3ResetNoMorePhysWritesFlag(PVM pVM);
 

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

@@ -599 +599 @@
     GEN_CHECK_OFF(PGMCPU, fA20Enabled);
     GEN_CHECK_OFF(PGMCPU, fSyncFlags);
-    GEN_CHECK_OFF(PGM, CritSect);
+    GEN_CHECK_OFF(PGM, CritSectX);
     GEN_CHECK_OFF(PGM, pPoolR3);
     GEN_CHECK_OFF(PGM, pPoolR0);

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

@@ -203 +203 @@
     CHECK_PADDING_VM(64, vmm);
     PRINT_OFFSET(VM, pgm);
-    PRINT_OFFSET(VM, pgm.s.CritSect);
+    PRINT_OFFSET(VM, pgm.s.CritSectX);
     CHECK_PADDING_VM(64, pgm);
     PRINT_OFFSET(VM, hwaccm);
@@ -383 +383 @@
     CHECK_MEMBER_ALIGNMENT(IOM, CritSect, sizeof(uintptr_t));
     CHECK_MEMBER_ALIGNMENT(EM, CritSectREM, sizeof(uintptr_t));
-    CHECK_MEMBER_ALIGNMENT(PGM, CritSect, sizeof(uintptr_t));
+    CHECK_MEMBER_ALIGNMENT(PGM, CritSectX, sizeof(uintptr_t));
     CHECK_MEMBER_ALIGNMENT(PDM, CritSect, sizeof(uintptr_t));
     CHECK_MEMBER_ALIGNMENT(MMHYPERHEAP, Lock, sizeof(uintptr_t));