Changeset 54799 in vbox for trunk/src/VBox/VMM

Timestamp:

Mar 16, 2015 9:23:03 PM (10 years ago)

Author:

vboxsync

Message:

CPUM: Fixed two problems with the CPUID code. cLeaves wasn't updated in cpumR3CpuIdInstallAndExplodeLeaves leading to garbage entries at the end of the array. And when the CPUID insertion code had a bug when inserting subleafs that triggered when doing the cache leaf (4) fake up during old saved state loading.

Location:

trunk/src/VBox/VMM

Files:

• Makefile.kmk (modified) (1 diff)
• VMMR3/CPUMR3CpuId.cpp (modified) (11 diffs)
• include/CPUMInternal.h (modified) (1 diff)

trunk/src/VBox/VMM/Makefile.kmk

@@ -109 +109 @@
 ifdef VBOX_WITH_VMM_R0_SWITCH_STACK
  VBoxVMM_DEFS   += VMM_R0_SWITCH_STACK
+endif
+if "$(KBUILD_TYPE)" == "debug" && "$(USERNAME)" == "bird"
+ VBoxVMM_DEFS   += RTMEM_WRAP_TO_EF_APIS
 endif
 VBoxVMM_DEFS.darwin = VMM_R0_SWITCH_STACK

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

@@ -679 +679 @@
 
 /**
+ * Checks that we've updated the CPUID leaves array correctly.
+ *
+ * This is a no-op in non-strict builds.
+ *
+ * @param   paLeaves            The leaves array.
+ * @param   cLeaves             The number of leaves.
+ */
+static void cpumR3CpuIdAssertOrder(PCPUMCPUIDLEAF paLeaves, uint32_t cLeaves)
+{
+#ifdef VBOX_STRICT
+    for (uint32_t i = 1; i < cLeaves; i++)
+        if (paLeaves[i].uLeaf != paLeaves[i - 1].uLeaf)
+            AssertMsg(paLeaves[i].uLeaf > paLeaves[i - 1].uLeaf, ("%#x vs %#x\n", paLeaves[i].uLeaf, paLeaves[i - 1].uLeaf));
+        else
+        {
+            AssertMsg(paLeaves[i].uSubLeaf > paLeaves[i - 1].uSubLeaf,
+                      ("%#x: %#x vs %#x\n", paLeaves[i].uLeaf, paLeaves[i].uSubLeaf, paLeaves[i - 1].uSubLeaf));
+            AssertMsg(paLeaves[i].fSubLeafMask == paLeaves[i - 1].fSubLeafMask,
+                      ("%#x/%#x: %#x vs %#x\n", paLeaves[i].uLeaf, paLeaves[i].uSubLeaf, paLeaves[i].fSubLeafMask, paLeaves[i - 1].fSubLeafMask));
+            AssertMsg(paLeaves[i].fFlags == paLeaves[i - 1].fFlags,
+                      ("%#x/%#x: %#x vs %#x\n", paLeaves[i].uLeaf, paLeaves[i].uSubLeaf, paLeaves[i].fFlags, paLeaves[i - 1].fFlags));
+        }
+#else
+    NOREF(paLeaves);
+    NOREF(cLeaves);
+#endif
+}
+
+
+/**
  * Inserts a CPU ID leaf, replacing any existing ones.
  *
@@ -782 +812 @@
 
             paLeaves[i] = *pNewLeaf;
+            cpumR3CpuIdAssertOrder(*ppaLeaves, *pcLeaves);
             return VINF_SUCCESS;
         }
@@ -787 +818 @@
         /* Find sub-leaf insertion point. */
         while (   i < cLeaves
-               && paLeaves[i].uSubLeaf < pNewLeaf->uSubLeaf)
+               && paLeaves[i].uSubLeaf < pNewLeaf->uSubLeaf
+               && paLeaves[i].uLeaf == pNewLeaf->uLeaf)
             i++;
 
@@ -793 +825 @@
          * If we've got an exactly matching leaf, replace it.
          */
-        if (   paLeaves[i].uLeaf    == pNewLeaf->uLeaf
+        if (   i < cLeaves
+            && paLeaves[i].uLeaf    == pNewLeaf->uLeaf
             && paLeaves[i].uSubLeaf == pNewLeaf->uSubLeaf)
         {
             paLeaves[i] = *pNewLeaf;
+            cpumR3CpuIdAssertOrder(*ppaLeaves, *pcLeaves);
             return VINF_SUCCESS;
         }
@@ -813 +847 @@
     *pcLeaves += 1;
     paLeaves[i] = *pNewLeaf;
+
+    cpumR3CpuIdAssertOrder(*ppaLeaves, *pcLeaves);
     return VINF_SUCCESS;
 }
@@ -858 +894 @@
         *pcLeaves = cLeaves -= (iEnd - iFirst);
     }
+
+    cpumR3CpuIdAssertOrder(paLeaves, *pcLeaves);
 }
 
@@ -1191 +1229 @@
     }
 
+    cpumR3CpuIdAssertOrder(*ppaLeaves, *pcLeaves);
     return VINF_SUCCESS;
 }
@@ -1859 +1898 @@
 static int cpumR3CpuIdInstallAndExplodeLeaves(PVM pVM, PCPUM pCpum, PCPUMCPUIDLEAF paLeaves, uint32_t cLeaves)
 {
+    cpumR3CpuIdAssertOrder(paLeaves, cLeaves);
+
     /*
      * Install the CPUID information.
@@ -1866 +1907 @@
 
     AssertLogRelRCReturn(rc, rc);
-
-
+    pCpum->GuestInfo.cCpuIdLeaves = cLeaves;
     pCpum->GuestInfo.paCpuIdLeavesR0 = MMHyperR3ToR0(pVM, pCpum->GuestInfo.paCpuIdLeavesR3);
     pCpum->GuestInfo.paCpuIdLeavesRC = MMHyperR3ToRC(pVM, pCpum->GuestInfo.paCpuIdLeavesR3);
@@ -3471 +3511 @@
                     /*
                      * Load the leaves one by one.
+                     *
+                     * The uPrev stuff is a kludge for working around a week worth of bad saved
+                     * states during the CPUID revamp in March 2015.  We saved too many leaves
+                     * due to a bug in cpumR3CpuIdInstallAndExplodeLeaves, thus ending up with
+                     * garbage entires at the end of the array when restoring.  We also had
+                     * a subleaf insertion bug that triggered with the leaf 4 stuff below,
+                     * this kludge doesn't deal correctly with that, but who cares...
                      */
+                    uint32_t uPrev = 0;
                     for (uint32_t i = 0; i < cLeaves && RT_SUCCESS(rc); i++)
                     {
@@ -3477 +3525 @@
                         rc = SSMR3GetMem(pSSM, &Leaf, sizeof(Leaf));
                         if (RT_SUCCESS(rc))
-                            rc = cpumR3CpuIdInsert(NULL /* pVM */, ppaLeaves, pcLeaves, &Leaf);
+                        {
+                            if (   uVersion != CPUM_SAVED_STATE_VERSION_BAD_CPUID_COUNT
+                                || Leaf.uLeaf >= uPrev)
+                            {
+                                rc = cpumR3CpuIdInsert(NULL /* pVM */, ppaLeaves, pcLeaves, &Leaf);
+                                uPrev = Leaf.uLeaf;
+                            }
+                            else
+                                uPrev = UINT32_MAX;
+                        }
                     }
                 }

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

@@ -123 +123 @@
  * @{ */
 /** The current saved state version. */
-#define CPUM_SAVED_STATE_VERSION                15
+#define CPUM_SAVED_STATE_VERSION                16
+/** CPUID changes with explode forgetting to update the leaf count on
+ * restore, resulting in garbage being saved restoring+saving old states). */
+#define CPUM_SAVED_STATE_VERSION_BAD_CPUID_COUNT 15
 /** The saved state version before the CPUIDs changes. */
 #define CPUM_SAVED_STATE_VERSION_PUT_STRUCT     14