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

Timestamp:

Feb 19, 2014 3:13:19 PM (11 years ago)

Author:

vboxsync

Message:

VMM/HMVMXR0: Fix for workaround done in r92215.

File:

• trunk/src/VBox/VMM/VMMR0/HMVMXR0.cpp (modified) (15 diffs)

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

@@ -1236 +1236 @@
  * auto-load/store MSR area in the VMCS.
  *
- * @returns VBox status code.
+ * @returns true if the MSR was added -and- its value was updated, false
+ *          otherwise.
  * @param   pVCpu           Pointer to the VMCPU.
  * @param   uMsr            The MSR.
@@ -1243 +1244 @@
  *                          necessary.
  */
-static int hmR0VmxAddAutoLoadStoreMsr(PVMCPU pVCpu, uint32_t uMsr, uint64_t uGuestMsrValue, bool fUpdateHostMsr)
+static bool hmR0VmxAddAutoLoadStoreMsr(PVMCPU pVCpu, uint32_t uMsr, uint64_t uGuestMsrValue, bool fUpdateHostMsr)
 {
     PVMXAUTOMSR pGuestMsr = (PVMXAUTOMSR)pVCpu->hm.s.vmx.pvGuestMsr;
@@ -1283 +1284 @@
      * updated by hmR0VmxSaveHostMsrs(). We do this for performance reasons.
      */
+    bool fUpdatedMsrValue = false;
     if (   fAdded
         && fUpdateHostMsr)
@@ -1289 +1291 @@
         Assert(!RTThreadPreemptIsEnabled(NIL_RTTHREAD));
         pHostMsr->u64Value = ASMRdMsr(pHostMsr->u32Msr);
-    }
-
-    return VINF_SUCCESS;
+        fUpdatedMsrValue = true;
+    }
+
+    return fUpdatedMsrValue;
 }
 
@@ -2775 +2778 @@
     int rc = VERR_INTERNAL_ERROR_5;
 
+#if HC_ARCH_BITS == 64
     /*
-     * Quick fix for regression #7240.  Restore the host state if we've messed
-     * it up already, otherwise all we'll get it all wrong below!
+     * If we've executed guest code using VT-x, the host-state bits will be messed up. We
+     * should -not- save the messed up state without restoring the original host-state. See @bugref{7240}.
      */
-    if (   (pVCpu->hm.s.vmx.fRestoreHostFlags & VMX_RESTORE_HOST_REQUIRED)
-        && (pVCpu->hm.s.vmx.fRestoreHostFlags & ~VMX_RESTORE_HOST_REQUIRED))
-        VMXRestoreHostState(pVCpu->hm.s.vmx.fRestoreHostFlags, &pVCpu->hm.s.vmx.RestoreHost);
+    AssertMsgReturn(!(pVCpu->hm.s.vmx.fRestoreHostFlags & VMX_RESTORE_HOST_REQUIRED),
+                    ("Re-saving host-state after executing guest code without leaving VT-x!\n"), VERR_WRONG_ORDER);
+#endif
 
     /*
@@ -3025 +3029 @@
         hmR0VmxLazySaveHostMsrs(pVCpu);
 #endif
-
-    if (pVCpu->hm.s.vmx.cMsrs > 0)
-        hmR0VmxUpdateAutoLoadStoreHostMsrs(pVCpu);
 
     /*
@@ -4563 +4564 @@
         {
             pVCpu->hm.s.vmx.pfnStartVM = VMXR0SwitcherStartVM64;
-            HMCPU_CF_SET(pVCpu, HM_CHANGED_HOST_CONTEXT | HM_CHANGED_VMX_EXIT_CTLS | HM_CHANGED_VMX_ENTRY_CTLS);
+            /* Currently, all mode changes sends us back to ring-3, so these should be set. See @bugref{6944}. */
+            Assert(HMCPU_CF_IS_SET(pVCpu, HM_CHANGED_HOST_CONTEXT | HM_CHANGED_VMX_EXIT_CTLS | HM_CHANGED_VMX_ENTRY_CTLS));
         }
 #else
@@ -4577 +4579 @@
         {
             pVCpu->hm.s.vmx.pfnStartVM = VMXR0StartVM32;
-            HMCPU_CF_SET(pVCpu, HM_CHANGED_HOST_CONTEXT | HM_CHANGED_VMX_EXIT_CTLS | HM_CHANGED_VMX_ENTRY_CTLS);
+            /* Currently, all mode changes sends us back to ring-3, so these should be set. See @bugref{6944}. */
+            Assert(HMCPU_CF_IS_SET(pVCpu, HM_CHANGED_HOST_CONTEXT | HM_CHANGED_VMX_EXIT_CTLS | HM_CHANGED_VMX_ENTRY_CTLS));
         }
 #else
@@ -6674 +6677 @@
 #endif
 
+    /* Update auto-load/store host MSRs values when we re-enter VT-x (as we could be on a different CPU). */
+    pVCpu->hm.s.vmx.fUpdatedHostMsrs = false;
+
     STAM_PROFILE_ADV_SET_STOPPED(&pVCpu->hm.s.StatEntry);
     STAM_PROFILE_ADV_SET_STOPPED(&pVCpu->hm.s.StatLoadGuestState);
@@ -6894 +6900 @@
         }
 #endif
+        pVCpu->hm.s.vmx.fUpdatedHostMsrs = false;
         VMCPU_CMPXCHG_STATE(pVCpu, VMCPUSTATE_STARTED_HM, VMCPUSTATE_STARTED_EXEC);
         if (pVCpu->hm.s.vmx.uVmcsState & HMVMX_VMCS_STATE_ACTIVE)
@@ -7077 +7084 @@
     if (pVCpu->hm.s.Event.fPending)
     {
+#if defined(DEBUG) || defined(VBOX_STRICT) || defined(VBOX_WITH_STATISTICS)
+        uint32_t uIntType = VMX_EXIT_INTERRUPTION_INFO_TYPE(pVCpu->hm.s.Event.u64IntInfo);
+#endif
 #if defined(VBOX_STRICT) || defined(VBOX_WITH_STATISTICS)
-        uint32_t uIntType = VMX_EXIT_INTERRUPTION_INFO_TYPE(pVCpu->hm.s.Event.u64IntInfo);
         if (uIntType == VMX_EXIT_INTERRUPTION_INFO_TYPE_EXT_INT)
         {
@@ -7094 +7103 @@
         }
 #endif
-        Log4(("Injecting pending event vcpu[%RU32] u64IntInfo=%#RX64\n", pVCpu->idCpu, pVCpu->hm.s.Event.u64IntInfo));
+        Log4(("Injecting pending event vcpu[%RU32] u64IntInfo=%#RX64 Type=%#x\n", pVCpu->idCpu, pVCpu->hm.s.Event.u64IntInfo,
+              (uint8_t)uIntType));
         rc = hmR0VmxInjectEventVmcs(pVCpu, pMixedCtx, pVCpu->hm.s.Event.u64IntInfo, pVCpu->hm.s.Event.cbInstr,
                                     pVCpu->hm.s.Event.u32ErrCode, pVCpu->hm.s.Event.GCPtrFaultAddress, &uIntrState);
@@ -8102 +8112 @@
 
     /*
-     * The host MSR values the very first time around won't be updated, so we need to
-     * fill those values in. Subsequently, it's updated as part of the host state.
+     * Lazy-update of the host MSRs values in the auto-load/store MSR area.
      */
     if (   !pVCpu->hm.s.vmx.fUpdatedHostMsrs
         && pVCpu->hm.s.vmx.cMsrs > 0)
     {
-        HMCPU_CF_SET(pVCpu, HM_CHANGED_HOST_CONTEXT);
+        hmR0VmxUpdateAutoLoadStoreHostMsrs(pVCpu);
     }
 
@@ -8181 +8190 @@
             AssertRC(rc2);
             Assert(HMVMXCPU_GST_IS_UPDATED(pVCpu, HMVMX_UPDATED_GUEST_AUTO_LOAD_STORE_MSRS));
-            hmR0VmxAddAutoLoadStoreMsr(pVCpu, MSR_K8_TSC_AUX, CPUMR0GetGuestTscAux(pVCpu), true /* fUpdateHostMsr */);
+            bool fMsrUpdated = hmR0VmxAddAutoLoadStoreMsr(pVCpu, MSR_K8_TSC_AUX, CPUMR0GetGuestTscAux(pVCpu),
+                                                          true /* fUpdateHostMsr */);
+            Assert(fMsrUpdated || pVCpu->hm.s.vmx.fUpdatedHostMsrs);
+            /* Finally, mark that all host MSR values are updated so we don't redo it without leaving VT-x. See @bugref{6956}. */
+            pVCpu->hm.s.vmx.fUpdatedHostMsrs = true;
         }
         else
+        {
             hmR0VmxRemoveAutoLoadStoreMsr(pVCpu, MSR_K8_TSC_AUX);
+            Assert(!pVCpu->hm.s.vmx.cMsrs || pVCpu->hm.s.vmx.fUpdatedHostMsrs);
+        }
     }
 #ifdef VBOX_STRICT
@@ -8242 +8258 @@
 #endif
 
+#if HC_ARCH_BITS == 64
     pVCpu->hm.s.vmx.fRestoreHostFlags |= VMX_RESTORE_HOST_REQUIRED;   /* Host state messed up by VT-x, we must restore. */
+#endif
     pVCpu->hm.s.vmx.uVmcsState |= HMVMX_VMCS_STATE_LAUNCHED;          /* Use VMRESUME instead of VMLAUNCH in the next run. */
     ASMSetFlags(pVmxTransient->uEflags);                              /* Enable interrupts. */