Changeset 14748

Timestamp:

11/28/08 01:34:24 (1 month ago)

Author:

vboxsync

Message:

PGMR0DynMap: a couple of bugs, guard pages and PGMR0DynMapAssertIntegrity.

Files:

• trunk/include/VBox/pgm.h (modified) (1 diff)
• trunk/src/VBox/VMM/VMMR0/PGMR0DynMap.cpp (modified) (37 diffs)

trunk/include/VBox/pgm.h

@@ -470 +470 @@
-VMMR0DECL(int)      PGMR0DynMapInitVM(PVM pVM);
-VMMR0DECL(void)     PGMR0DynMapTermVM(PVM pVM);
+VMMR0DECL(int)      PGMR0DynMapAssertIntegrity(void);
-# endif
-/** @} */

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

@@ -50 +50 @@
-/** The number of pages we reserve per CPU. */
-#define PGMR0DYNMAP_PAGES_PER_CPU           64
+/** The number of guard pages. */
+#if defined(VBOX_STRICT)
+# define PGMR0DYNMAP_GUARD_PAGES            7
+#else
+# define PGMR0DYNMAP_GUARD_PAGES            0
+#endif
+/** The dummy physical address of guard pages. */
+#define PGMR0DYNMAP_GUARD_PAGE_HCPHYS       UINT32_C(0x7777feed)
+/** The dummy reference count of guard pages. (Must be non-zero.) */
+#define PGMR0DYNMAP_GUARD_PAGE_REF_COUNT    INT32_C(0x7777feed)
+#if 0
+/** Define this to just clear the present bit on guard pages.
+ * The alternative is to replace the entire PTE with an bad not-present
+ * PTE. Either way, XNU will screw us. :-/   */
+#define PGMR0DYNMAP_GUARD_NP
+#endif
+/** The dummy PTE value for a page. */
+#define PGMR0DYNMAP_GUARD_PAGE_LEGACY_PTE   X86_PTE_PG_MASK
+/** The dummy PTE value for a page. */
+#define PGMR0DYNMAP_GUARD_PAGE_PAE_PTE      UINT64_MAX /*X86_PTE_PAE_PG_MASK*/
-/** Calcs the overload threshold. Current set at 50%. */
-#define PGMR0DYNMAP_CALC_OVERLOAD(cPages)   ((cPages) / 2)
-
+#if 0
-/* Assertions causes panics if preemption is disabled, this can be used to work aroudn that. */
-/*
- */
-
+
+
+
-
-
@@ -145 +166 @@
-    /** Whether it's 32-bit legacy or PAE/AMD64 paging mode. */
-    bool                        fLegacyMode;
-
+
+
-    uint32_t                    cLoad;
-    /** The max load ever.
@@ -152 +174 @@
-    /** Initialization / termination lock. */
-    RTSEMFASTMUTEX              hInitLock;
+    /** The number of guard pages. */
+    uint32_t                    cGuardPages;
-    /** The number of users (protected by hInitLock). */
-    uint32_t                    cUsers;
@@ -365 +389 @@
-#endif
-    }
-
+ - pThis->cGuardPages
-        rc = pgmR0DynMapExpand(pThis);
-    if (RT_SUCCESS(rc))
@@ -399 +423 @@
-    {
-        pVM->pgm.s.pvR0DynMapUsed = NULL;
+
+#ifdef VBOX_STRICT
+        PGMR0DynMapAssertIntegrity();
+#endif
-
-        /*
@@ -455 +483 @@
-
-/**
+ * Shoots down the TLBs for all the cache pages, pgmR0DynMapTearDown helper.
+ *
+ * @param   idCpu           The current CPU.
+ * @param   pvUser1         The dynamic mapping cache instance.
+ * @param   pvUser2         Unused, NULL.
+ */
+static DECLCALLBACK(void) pgmR0DynMapShootDownTlbs(RTCPUID idCpu, void *pvUser1, void *pvUser2)
+{
+    Assert(!pvUser2);
+    PPGMR0DYNMAP        pThis   = (PPGMR0DYNMAP)pvUser1;
+    Assert(pThis == g_pPGMR0DynMap);
+    PPGMR0DYNMAPENTRY   paPages = pThis->paPages;
+    uint32_t            iPage   = pThis->cPages;
+    while (iPage-- > 0)
+        ASMInvalidatePage(paPages[iPage].pvPage);
+}
+
+
+/**
+ * Shoot down the TLBs for every single cache entry on all CPUs.
+ *
+ * @returns IPRT status code (RTMpOnAll).
+ * @param   pThis       The dynamic mapping cache instance.
+ */
+static int pgmR0DynMapTlbShootDown(PPGMR0DYNMAP pThis)
+{
+    int rc = RTMpOnAll(pgmR0DynMapShootDownTlbs, pThis, NULL);
+    AssertRC(rc);
+    if (RT_FAILURE(rc))
+    {
+        uint32_t iPage = pThis->cPages;
+        while (iPage-- > 0)
+            ASMInvalidatePage(pThis->paPages[iPage].pvPage);
+    }
+    return rc;
+}
+
+
+/**
- * Calculate the new cache size based on cMaxLoad statistics.
- *
@@ -468 +535 @@
-    RTCPUID     cCpus     = RTMpGetCount();
-    AssertReturn(cCpus > 0 && cCpus <= RTCPUSET_MAX_CPUS, 0);
-
+ 
-    uint32_t    cMinPages = cCpus * (PGMR0DYNMAP_PAGES_PER_CPU / 2);
-
@@ -485 +552 @@
-    if (cPages < pThis->cPages)
-        cPages = pThis->cPages;
+    cPages *= PGMR0DYNMAP_GUARD_PAGES + 1;
-    if (cPages > PGMR0DYNMAP_MAX_PAGES)
-        cPages = PGMR0DYNMAP_MAX_PAGES;
@@ -490 +558 @@
-    if (cMinPages < pThis->cPages)
-        cMinPages = pThis->cPages;
+    cMinPages *= PGMR0DYNMAP_GUARD_PAGES + 1;
-    if (cMinPages > PGMR0DYNMAP_MAX_PAGES)
-        cMinPages = PGMR0DYNMAP_MAX_PAGES;
@@ -687 +756 @@
-            return VERR_INTERNAL_ERROR;
-        }
-Log(("#%d: iEntry=%4d uEntry=%#llx pvEntry=%p HCPhys=%RHp \n", i, iEntry, uEntry, pvEntry, pPgLvl->a[i].HCPhys));
+/*Log(("#%d: iEntry=%4d uEntry=%#llx pvEntry=%p HCPhys=%RHp \n", i, iEntry, uEntry, pvEntry, pPgLvl->a[i].HCPhys));*/
-    }
-
@@ -697 +766 @@
-
-/**
+ * Sets up a guard page.
+ *
+ * @param   pThis       The dynamic mapping cache instance.
+ * @param   pPage       The page.
+ */
+DECLINLINE(void) pgmR0DynMapSetupGuardPage(PPGMR0DYNMAP pThis, PPGMR0DYNMAPENTRY pPage)
+{
+    memset(pPage->pvPage, 0xfd, PAGE_SIZE);
+    pPage->cRefs  = PGMR0DYNMAP_GUARD_PAGE_REF_COUNT;
+    pPage->HCPhys = PGMR0DYNMAP_GUARD_PAGE_HCPHYS;
+#ifdef PGMR0DYNMAP_GUARD_NP
+    ASMAtomicBitClear(pPage->uPte.pv, X86_PTE_BIT_P);
+#else
+    if (pThis->fLegacyMode)
+        ASMAtomicWriteU32(&pPage->uPte.pLegacy->u, PGMR0DYNMAP_GUARD_PAGE_LEGACY_PTE);
+    else
+        ASMAtomicWriteU64(&pPage->uPte.pPae->u,    PGMR0DYNMAP_GUARD_PAGE_PAE_PTE);
+#endif
+    pThis->cGuardPages++;
+}
+
+
+/**
- * Adds a new segment of the specified size.
- *
@@ -709 +801 @@
-
-    /*
-llocation
+allocations
-     * (The pages array has to be replaced behind the spinlock of course.)
-     */
@@ -760 +852 @@
-        PGMR0DYNMAPPGLVL    PgLvl;
-        pgmR0DynMapPagingArrayInit(pThis, &PgLvl);
-     
+const
-        for (uint32_t iPage = pThis->cPages;
-             iPage < iEndPage;
@@ -809 +901 @@
-        if (RT_SUCCESS(rc))
-        {
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-
-            /*
@@ -882 +990 @@
-    }
-    Assert(ASMGetFlags() & X86_EFL_IF);
+
+#if PGMR0DYNMAP_GUARD_PAGES > 0
+    /* paranoia */
+    if (RT_SUCCESS(rc))
+        pgmR0DynMapTlbShootDown(pThis);
+#endif
-    return rc;
-}
@@ -919 +1033 @@
-    }
-    Assert(ASMGetFlags() & X86_EFL_IF);
+
+#if PGMR0DYNMAP_GUARD_PAGES > 0
+    /* paranoia */
+    if (RT_SUCCESS(rc))
+        pgmR0DynMapTlbShootDown(pThis);
+#endif
-    return rc;
-}
-
-
-/**
- * Shoots down the TLBs for all the cache pages, pgmR0DynMapTearDown helper.
- *
- * @param   idCpu           The current CPU.
- * @param   pvUser1         The dynamic mapping cache instance.
- * @param   pvUser2         Unused, NULL.
- */
-static DECLCALLBACK(void) pgmR0DynMapShootDownTlbs(RTCPUID idCpu, void *pvUser1, void *pvUser2)
-{
-    Assert(!pvUser2);
-    PPGMR0DYNMAP        pThis   = (PPGMR0DYNMAP)pvUser1;
-    Assert(pThis == g_pPGMR0DynMap);
-    PPGMR0DYNMAPENTRY   paPages = pThis->paPages;
-    uint32_t            iPage   = pThis->cPages;
-    while (iPage-- > 0)
-        ASMInvalidatePage(paPages[iPage].pvPage);
-}
-
@@ -982 +1083 @@
-    /*
-     * Shoot down the TLBs on all CPUs before freeing them.
-
-
-
-
-
-
-
-
-
-
+
+
-
-    /*
@@ -998 +1091 @@
-    while (pThis->pSegHead)
-    {
+        int             rc;
-        PPGMR0DYNMAPSEG pSeg = pThis->pSegHead;
-        pThis->pSegHead = pSeg->pNext;
@@ -1081 +1175 @@
-        iFreePage = iPage;
-    else if (!paPages[(iPage + 1) % cPages].cRefs)
-
+ + 1
-    else if (!paPages[(iPage + 2) % cPages].cRefs)
-
+ + 2
-    else if (!paPages[(iPage + 3) % cPages].cRefs)
-
+ + 3
-    else if (!paPages[(iPage + 4) % cPages].cRefs)
-
+ + 4
-    else
-    {
@@ -1093 +1187 @@
-         * Search for an unused or matching entry.
-         */
-pThis->
+
-        for (;;)
-        {
@@ -1103 +1197 @@
-            /* advance */
-            iFreePage = (iFreePage + 1) % cPages;
-!
+=
-                return UINT32_MAX;
-        }
@@ -1111 +1205 @@
-     * Setup the new entry.
-     */
+    /*Log6(("pgmR0DynMapPageSlow: old - %RHp %#x %#llx\n", paPages[iFreePage].HCPhys, paPages[iFreePage].cRefs, paPages[iFreePage].uPte.pPae->u));*/
-    paPages[iFreePage].HCPhys = HCPhys;
-    RTCpuSetFill(&paPages[iFreePage].PendingSet);
@@ -1141 +1236 @@
- * Maps a page into the pool.
- *
-ointer to the mapping
+age index on success, UINT32_MAX on failure
- * @param   pThis       The dynamic mapping cache instance.
- * @param   HCPhys      The address of the page to be mapped.
-iPage      Where to store the page index
-
-void *) pgmR0DynMapPage(PPGMR0DYNMAP pThis, RTHCPHYS HCPhys, uint32_t *pi
+pvPage      Where to the page address
+
+uint32_t) pgmR0DynMapPage(PPGMR0DYNMAP pThis, RTHCPHYS HCPhys, void **ppv
-{
-    RTSPINLOCKTMP   Tmp       = RTSPINLOCKTMP_INITIALIZER;
@@ -1181 +1276 @@
-                        {
-                            RTSpinlockRelease(pThis->hSpinlock, &Tmp);
-NULL
+iPage
-                        }
-                    }
@@ -1206 +1301 @@
-        if (pThis->cLoad > pThis->cMaxLoad)
-            pThis->cMaxLoad = pThis->cLoad;
-, ("%d/%d\n", pThis->cLoad, pThis->c
+ - pThis->cGuardPages, ("%d/%d\n", pThis->cLoad, pThis->cPages - pThis->cGuard
-    }
-    else if (RT_UNLIKELY(cRefs <= 0))
@@ -1212 +1307 @@
-        ASMAtomicDecS32(&paPages[iPage].cRefs);
-        RTSpinlockRelease(pThis->hSpinlock, &Tmp);
-NULL
+UINT32_MAX
-    }
-    void *pvPage = paPages[iPage].pvPage;
@@ -1231 +1326 @@
-    ASMInvalidatePage(pvPage);
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-}
-
@@ -1285 +1498 @@
-        }
-
-
+ - pThis->cGuardPages
-        RTSpinlockRelease(pThis->hSpinlock, &Tmp);
-    }
@@ -1399 +1612 @@
-     * Map it.
-     */
-
-
-
+
+
-    {
-        static uint32_t s_cBitched = 0;
-        if (++s_cBitched < 10)
-
-
+ cGuardPages=%u
+, g_pPGMR0DynMap->cGuardPages
-        return VERR_PGM_DYNMAP_FAILED;
-    }
-    *ppv = pvPage;
-
-    /*
@@ -1497 +1708 @@
-    PPGMMAPSET      pSet  = &pVM->aCpus[0].pgm.s.AutoSet;
-    uint32_t        i;
+
+    /*
+     * Assert internal integrity first.
+     */
+    LogRel(("Test #0\n"));
+    int rc = PGMR0DynMapAssertIntegrity();
+    if (RT_FAILURE(rc))
+        return rc;
+
-    void           *pvR0DynMapUsedSaved = pVM->pgm.s.pvR0DynMapUsed;
-    pVM->pgm.s.pvR0DynMapUsed = pThis;
@@ -1511 +1731 @@
-    void    *pv  = (void *)(intptr_t)-1;
-    void    *pv2 = (void *)(intptr_t)-2;
-int      rc
+rc         
-    int      rc2 = PGMDynMapHCPage(pVM, cr3, &pv2);
-    ASMIntEnable();
@@ -1518 +1738 @@
-        &&  pv == pv2)
-    {
-
+
+
-
-        /*
@@ -1554 +1775 @@
-        }
-        if (RT_SUCCESS(rc))
+            rc = PGMR0DynMapAssertIntegrity();
+        if (RT_SUCCESS(rc))
-        {
-            /*
@@ -1577 +1800 @@
-                rc = VERR_INTERNAL_ERROR;
-            }
-
+
+
+
-            if (RT_SUCCESS(rc))
-            {
@@ -1585 +1810 @@
-                LogRel(("Test #4\n"));
-                ASMIntDisable();
-
-
+
+
+
+
+
+
+
+
-                ASMIntEnable();
-                if (rc == VERR_PGM_DYNMAP_FULL_SET)
-                {
-                    rc = VINF_SUCCESS;
-
-                    /* flush the set. */
+                    LogRel(("Test #5\n"));
-                    ASMIntDisable();
-                    PGMDynMapMigrateAutoSet(&pVM->aCpus[0]);
@@ -1598 +1828 @@
-                    PGMDynMapStartAutoSet(&pVM->aCpus[0]);
-                    ASMIntEnable();
+
+                    rc = PGMR0DynMapAssertIntegrity();
-                }
-                else
-                {
-
-
+; i=%d
+, i
-                    if (RT_SUCCESS(rc)) rc = VERR_INTERNAL_ERROR;
-                }
@@ -1662 +1894 @@
-    ASMIntEnable();
-
+    if (RT_SUCCESS(rc))
+        rc = PGMR0DynMapAssertIntegrity();
+    else
+        PGMR0DynMapAssertIntegrity();
+
-    LogRel(("Result: rc=%Rrc Load=%u/%u/%u Set=%#x/%u\n", rc,
-
+ - pThis->cPages
-    pVM->pgm.s.pvR0DynMapUsed = pvR0DynMapUsedSaved;
-    LogRel(("pgmR0DynMapTest: ****** END ******\n"));