Changeset 58781 in vbox

Timestamp:

Nov 19, 2015 10:33:00 PM (9 years ago)

Author:

vboxsync

Message:

PGM: Must always make a copy of the ROM pages, no just in RT_STRICT builds. Added pgmR3LoadDone to set the flag so that it's set for all saved state versions (and won't interfer during loading). Renamed fRestoreVirginRomPagesDuringReset to fRestoreRomPagesAtReset since 'virgin ROM pages' turned out to be a little bit confusing, see PGMROMPAGE (we always switch a ROM page back to the 'virgin' version, resetting any shadowing).

Location:

trunk/src/VBox/VMM

Files:

• VMMR3/PGM.cpp (modified) (2 diffs)
• VMMR3/PGMPhys.cpp (modified) (6 diffs)
• VMMR3/PGMSavedState.cpp (modified) (3 diffs)
• include/PGMInternal.h (modified) (1 diff)
• testcase/tstVMStruct.h (modified) (1 diff)

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

@@ -1253 +1253 @@
     pVM->pgm.s.offVM       = RT_OFFSETOF(VM, pgm.s);
     pVM->pgm.s.offVCpuPGM  = RT_OFFSETOF(VMCPU, pgm.s);
+    /*pVM->pgm.s.fRestoreRomPagesAtReset = false;*/
 
     for (unsigned i = 0; i < RT_ELEMENTS(pVM->pgm.s.aHandyPages); i++)
@@ -1312 +1313 @@
     pVM->pgm.s.GCPhys4MBPSEMask = RT_BIT_64(32) - 1; /* default; checked later */
     pVM->pgm.s.GCPtrPrevRamRangeMapping = MM_HYPER_AREA_ADDRESS;
-    pVM->pgm.s.fRestoreVirginRomPagesDuringReset = false;
 
     rc = CFGMR3QueryBoolDef(CFGMR3GetRoot(pVM), "RamPreAlloc", &pVM->pgm.s.fRamPreAlloc,

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

@@ -3387 +3387 @@
                     pRomNew->idSavedState = UINT8_MAX;
                     pRomNew->cbOriginal = cbBinary;
-#ifdef VBOX_STRICT
+                    pRomNew->pszDesc    = pszDesc;
                     pRomNew->pvOriginal = fFlags & PGMPHYS_ROM_FLAGS_PERMANENT_BINARY
                                         ? pvBinary : RTMemDup(pvBinary, cbBinary);
-#else
-                    pRomNew->pvOriginal = fFlags & PGMPHYS_ROM_FLAGS_PERMANENT_BINARY ? pvBinary : NULL;
-#endif
-                    pRomNew->pszDesc    = pszDesc;
-
-                    for (unsigned iPage = 0; iPage < cPages; iPage++)
+                    if (pRomNew->pvOriginal)
                     {
-                        PPGMROMPAGE pPage = &pRomNew->aPages[iPage];
-                        pPage->enmProt = PGMROMPROT_READ_ROM_WRITE_IGNORE;
-                        PGM_PAGE_INIT_ZERO(&pPage->Shadow, pVM, PGMPAGETYPE_ROM_SHADOW);
+                        for (unsigned iPage = 0; iPage < cPages; iPage++)
+                        {
+                            PPGMROMPAGE pPage = &pRomNew->aPages[iPage];
+                            pPage->enmProt = PGMROMPROT_READ_ROM_WRITE_IGNORE;
+                            PGM_PAGE_INIT_ZERO(&pPage->Shadow, pVM, PGMPAGETYPE_ROM_SHADOW);
+                        }
+
+                        /* update the page count stats for the shadow pages. */
+                        if (fFlags & PGMPHYS_ROM_FLAGS_SHADOWED)
+                        {
+                            pVM->pgm.s.cZeroPages += cPages;
+                            pVM->pgm.s.cAllPages  += cPages;
+                        }
+
+                        /*
+                         * Insert the ROM range, tell REM and return successfully.
+                         */
+                        pRomNew->pNextR3 = pRom;
+                        pRomNew->pNextR0 = pRom ? MMHyperCCToR0(pVM, pRom) : NIL_RTR0PTR;
+                        pRomNew->pNextRC = pRom ? MMHyperCCToRC(pVM, pRom) : NIL_RTRCPTR;
+
+                        if (pRomPrev)
+                        {
+                            pRomPrev->pNextR3 = pRomNew;
+                            pRomPrev->pNextR0 = MMHyperCCToR0(pVM, pRomNew);
+                            pRomPrev->pNextRC = MMHyperCCToRC(pVM, pRomNew);
+                        }
+                        else
+                        {
+                            pVM->pgm.s.pRomRangesR3 = pRomNew;
+                            pVM->pgm.s.pRomRangesR0 = MMHyperCCToR0(pVM, pRomNew);
+                            pVM->pgm.s.pRomRangesRC = MMHyperCCToRC(pVM, pRomNew);
+                        }
+
+                        pgmPhysInvalidatePageMapTLB(pVM);
+                        GMMR3AllocatePagesCleanup(pReq);
+                        return VINF_SUCCESS;
                     }
 
-                    /* update the page count stats for the shadow pages. */
-                    if (fFlags & PGMPHYS_ROM_FLAGS_SHADOWED)
-                    {
-                        pVM->pgm.s.cZeroPages += cPages;
-                        pVM->pgm.s.cAllPages  += cPages;
-                    }
-
-                    /*
-                     * Insert the ROM range, tell REM and return successfully.
-                     */
-                    pRomNew->pNextR3 = pRom;
-                    pRomNew->pNextR0 = pRom ? MMHyperCCToR0(pVM, pRom) : NIL_RTR0PTR;
-                    pRomNew->pNextRC = pRom ? MMHyperCCToRC(pVM, pRom) : NIL_RTRCPTR;
-
-                    if (pRomPrev)
-                    {
-                        pRomPrev->pNextR3 = pRomNew;
-                        pRomPrev->pNextR0 = MMHyperCCToR0(pVM, pRomNew);
-                        pRomPrev->pNextRC = MMHyperCCToRC(pVM, pRomNew);
-                    }
-                    else
-                    {
-                        pVM->pgm.s.pRomRangesR3 = pRomNew;
-                        pVM->pgm.s.pRomRangesR0 = MMHyperCCToR0(pVM, pRomNew);
-                        pVM->pgm.s.pRomRangesRC = MMHyperCCToRC(pVM, pRomNew);
-                    }
-
-                    pgmPhysInvalidatePageMapTLB(pVM);
-                    GMMR3AllocatePagesCleanup(pReq);
-                    return VINF_SUCCESS;
+                    /* bail out */
+                    rc = VERR_NO_MEMORY;
                 }
-
-                /* bail out */
 
                 int rc2 = PGMHandlerPhysicalDeregister(pVM, GCPhys);
@@ -3574 +3573 @@
 
         /*
-         * Restore virgin ROM pages after a saved state load or check that the
-         * virgin pages are unchanged if possible.
+         * Restore the original ROM pages after a saved state load.
+         * Also, in strict builds check that ROM pages remain unmodified.
          */
-        if (pRom->pvOriginal)
+        if (   pRom->pvOriginal
+#ifndef VBOX_STRICT
+            && pVM->pgm.s.fRestoreRomPagesAtReset
+#endif
+            )
         {
             size_t         cbSrcLeft = pRom->cbOriginal;
             uint8_t const *pbSrcPage = (uint8_t const *)pRom->pvOriginal;
-            bool           fChanged = false;
+            uint32_t       cRestored = 0;
             for (uint32_t iPage = 0; iPage < cPages && cbSrcLeft > 0; iPage++, pbSrcPage += PAGE_SIZE)
             {
@@ -3592 +3595 @@
                 if (memcmp(pvDstPage, pbSrcPage, RT_MIN(cbSrcLeft, PAGE_SIZE)))
                 {
-                    if (pVM->pgm.s.fRestoreVirginRomPagesDuringReset)
+                    if (pVM->pgm.s.fRestoreRomPagesAtReset)
                     {
                         void *pvDstPageW;
@@ -3598 +3601 @@
                         AssertLogRelRCReturn(rc, rc);
                         memcpy(pvDstPageW, pbSrcPage, RT_MIN(cbSrcLeft, PAGE_SIZE));
-                        fChanged = true;
+                        cRestored++;
                     }
                     else
-                    {
-#ifdef VBOX_STRICT
-                        LogRel(("pgmR3PhysRomReset: %RGp rom page changed (%s)?\n", GCPhys, pRom->pszDesc));
-#endif
-                    }
+                        LogRel(("pgmR3PhysRomReset: %RGp: ROM page changed (%s)\n", GCPhys, pRom->pszDesc));
                 }
                 cbSrcLeft -= RT_MIN(cbSrcLeft, PAGE_SIZE);
             }
-            if (fChanged)
-                LogRel(("PGM: ROM \"%s\" changed - restored original\n", pRom->pszDesc));
-        }
-    }
-
-    pVM->pgm.s.fRestoreVirginRomPagesDuringReset = false;
+            if (cRestored > 0)
+                LogRel(("PGM: ROM \"%s\": Reloaded %u of %u pages.\n", pRom->pszDesc, cRestored, cPages));
+        }
+    }
+
+    /* Clear the ROM restore flag now as we only need to do this once after
+       loading saved state. */
+    pVM->pgm.s.fRestoreRomPagesAtReset = false;
+
     return VINF_SUCCESS;
 }
@@ -3628 +3630 @@
 void pgmR3PhysRomTerm(PVM pVM)
 {
-#ifdef RT_STRICT
     /*
      * Free the heap copy of the original bits.
@@ -3641 +3642 @@
         }
     }
-#endif
 }
 

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

@@ -336 +336 @@
                                 ("The \"%s\" ROM was not found in the saved state. Probably due to some misconfiguration\n",
                                  pRom->pszDesc));
-
-            pVM->pgm.s.fRestoreVirginRomPagesDuringReset = true;
             return VINF_SUCCESS;        /* the end */
         }
@@ -3299 +3297 @@
 
 /**
+ * @callback_method_impl{FNSSMINTLOADDONE}
+ */
+static DECLCALLBACK(int) pgmR3LoadDone(PVM pVM, PSSMHANDLE pSSM)
+{
+    pVM->pgm.s.fRestoreRomPagesAtReset = true;
+    NOREF(pSSM);
+    return VINF_SUCCESS;
+}
+
+
+/**
  * Registers the saved state callbacks with SSM.
  *
@@ -3310 +3319 @@
                                  pgmR3LivePrep, pgmR3LiveExec, pgmR3LiveVote,
                                  NULL,          pgmR3SaveExec, pgmR3SaveDone,
-                                 pgmR3LoadPrep, pgmR3Load,     NULL);
-}
-
+                                 pgmR3LoadPrep, pgmR3Load,     pgmR3LoadDone);
+}
+

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

@@ -3245 +3245 @@
     /** The number of MMIO2 regions (serves as the next MMIO2 ID). */
     uint8_t                         cMmio2Regions;
-    /** Flag indicating that ROM pages should be restored to their original
-     * during reset. Primary use is for getting firmware updates when the
-     * VM is reset after a saved state was loaded. */
-    bool                            fRestoreVirginRomPagesDuringReset;
+    /** Restore original ROM page content when resetting after loading state.
+     * The flag is set by pgmR3LoadRomRanges and cleared at reset.  This
+     * enables the VM to start using an updated ROM without requiring powering
+     * down the VM, just rebooting or resetting it. */
+    bool                            fRestoreRomPagesAtReset;
 
     /** Indicates that PGMR3FinalizeMappings has been called and that further

trunk/src/VBox/VMM/testcase/tstVMStruct.h

@@ -674 +674 @@
     GEN_CHECK_OFF(PGM, paDynPageMapPaePTEsGC);
     GEN_CHECK_OFF(PGM, enmHostMode);
-    GEN_CHECK_OFF(PGM, fRestoreVirginRomPagesDuringReset);
+    GEN_CHECK_OFF(PGM, fRestoreRomPagesAtReset);
     GEN_CHECK_OFF(PGM, GCPhys4MBPSEMask);
     GEN_CHECK_OFF(PGM, pRamRangesXR3);