Changeset 45943 in vbox

Timestamp:

May 8, 2013 9:30:28 AM (12 years ago)

Author:

vboxsync

Message:

VMM/HMVMXR0: Fix overly-delayed update of PGM's view of CR3. More longjmp fun.

File:

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

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

@@ -5124 +5124 @@
             {
                 CPUMSetGuestCR3(pVCpu, uVal);
-                /* Set the force flag to inform PGM about it when necessary. It is cleared by PGMUpdateCR3().*/
-                VMCPU_FF_SET(pVCpu, VMCPU_FF_HM_UPDATE_CR3);
+                if (VMMRZCallRing3IsEnabled(pVCpu))
+                    PGMUpdateCR3(pVCpu, uVal);
+                else
+                {
+                    /* Set the force flag to inform PGM about it when necessary. It is cleared by PGMUpdateCR3().*/
+                    VMCPU_FF_SET(pVCpu, VMCPU_FF_HM_UPDATE_CR3);
+                }
             }
 
@@ -5135 +5140 @@
                 rc = VMXReadVmcs64(VMX_VMCS64_GUEST_PDPTE2_FULL, &pVCpu->hm.s.aPdpes[2].u);        AssertRCReturn(rc, rc);
                 rc = VMXReadVmcs64(VMX_VMCS64_GUEST_PDPTE3_FULL, &pVCpu->hm.s.aPdpes[3].u);        AssertRCReturn(rc, rc);
-                /* Set the force flag to inform PGM about it when necessary. It is cleared by PGMGstUpdatePaePdpes(). */
-                VMCPU_FF_SET(pVCpu, VMCPU_FF_HM_UPDATE_PAE_PDPES);
+
+                if (VMMRZCallRing3IsEnabled(pVCpu))
+                    PGMGstUpdatePaePdpes(pVCpu, &pVCpu->hm.s.aPdpes[0]);
+                else
+                {
+                    /* Set the force flag to inform PGM about it when necessary. It is cleared by PGMGstUpdatePaePdpes(). */
+                    VMCPU_FF_SET(pVCpu, VMCPU_FF_HM_UPDATE_PAE_PDPES);
+                }
             }
         }
@@ -5142 +5153 @@
         pVCpu->hm.s.vmx.fUpdatedGuestState |= HMVMX_UPDATED_GUEST_CR3;
     }
+
+    /*
+     * Consider this scenario: VM-exit -> VMMRZCallRing3Enable() -> do stuff that causes a longjmp -> hmR0VmxCallRing3Callback()
+     * -> VMMRZCallRing3Disable() -> hmR0VmxSaveGuestState() -> Set VMCPU_FF_HM_UPDATE_CR3 pending -> return from the longjmp
+     * -> continue with VM-exit handling -> hmR0VmxSaveGuestControlRegs() and here we are.
+     *
+     * The longjmp exit path can't check these CR3 force-flags and call code that takes a lock again.
+     * We cover for it here.
+     */
+    if (VMMRZCallRing3IsEnabled(pVCpu))
+    {
+        if (VMCPU_FF_IS_PENDING(pVCpu, VMCPU_FF_HM_UPDATE_CR3))
+            PGMUpdateCR3(pVCpu, CPUMGetGuestCR3(pVCpu));
+
+        if (VMCPU_FF_IS_PENDING(pVCpu, VMCPU_FF_HM_UPDATE_PAE_PDPES))
+            PGMGstUpdatePaePdpes(pVCpu, &pVCpu->hm.s.aPdpes[0]);
+
+        Assert(!VMCPU_FF_IS_PENDING(pVCpu, VMCPU_FF_HM_UPDATE_CR3));
+        Assert(!VMCPU_FF_IS_PENDING(pVCpu, VMCPU_FF_HM_UPDATE_PAE_PDPES));
+    }
+
     return rc;
 }
@@ -5389 +5421 @@
         return VINF_SUCCESS;
 
-    VMMRZCallRing3Disable(pVCpu);
-    Assert(VMMR0IsLogFlushDisabled(pVCpu));
+    /* Though we can longjmp to ring-3 due to log-flushes here and get recalled again on the ring-3 callback path,
+       there is no real need to. */
+    if (VMMRZCallRing3IsEnabled(pVCpu))
+        VMMR0LogFlushDisable(pVCpu);
+    else
+        Assert(VMMR0IsLogFlushDisabled(pVCpu));
     LogFunc(("\n"));
 
@@ -5429 +5465 @@
               ("Missed guest state bits while saving state; residue %RX32\n", pVCpu->hm.s.vmx.fUpdatedGuestState));
 
-    VMMRZCallRing3Enable(pVCpu);
+    if (VMMRZCallRing3IsEnabled(pVCpu))
+        VMMR0LogFlushEnable(pVCpu);
+
     return rc;
 }
@@ -5474 +5512 @@
             rc = PGMUpdateCR3(pVCpu, pMixedCtx->cr3);
             Assert(rc == VINF_SUCCESS || rc == VINF_PGM_SYNC_CR3);
+            Assert(!VMCPU_FF_IS_SET(pVCpu, VMCPU_FF_HM_UPDATE_CR3));
         }
         if (VMCPU_FF_IS_SET(pVCpu, VMCPU_FF_HM_UPDATE_PAE_PDPES))
@@ -5479 +5518 @@
             rc = PGMGstUpdatePaePdpes(pVCpu, &pVCpu->hm.s.aPdpes[0]);
             AssertRC(rc);
+            Assert(!VMCPU_FF_IS_SET(pVCpu, VMCPU_FF_HM_UPDATE_PAE_PDPES));
         }
 
@@ -6762 +6802 @@
         /*
          * If the TPR was raised by the guest, it wouldn't cause a VM-exit immediately. Instead we sync the TPR lazily whenever
-         * we eventually get a VM-exit for any reason. This maybe expensive as PDMApicSetTPR() can longjmp to ring-3; also why
-         * we do it outside of hmR0VmxSaveGuestState() which must never cause longjmps.
+         * we eventually get a VM-exit for any reason. This maybe expensive as PDMApicSetTPR() can longjmp to ring-3 and which is
+         * why it's done here as it's easier and no less efficient to deal with it here than making hmR0VmxSaveGuestState()
+         * cope with longjmps safely (see VMCPU_FF_HM_UPDATE_CR3 handling).
          */
         if (   (pVCpu->hm.s.vmx.u32ProcCtls & VMX_VMCS_CTRL_PROC_EXEC_CONTROLS_USE_TPR_SHADOW)