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

Timestamp:

Sep 21, 2013 12:23:40 PM (11 years ago)

Author:

vboxsync

Message:

VMM/VMMR0: Don't disable interrupts for a long time when disabling preemption is sufficient. Other minor fixes.

Location:

trunk/src/VBox/VMM/VMMR0

Files:

• HMR0.cpp (modified) (5 diffs)
• HMSVMR0.cpp (modified) (10 diffs)
• HMVMXR0.cpp (modified) (4 diffs)

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

@@ -1284 +1284 @@
      * Call the hardware specific initialization method.
      */
-    RTCCUINTREG      fFlags = ASMIntDisableFlags();
-    PHMGLOBALCPUINFO pCpu   = HMR0GetCurrentCpu();
-    ASMSetFlags(fFlags);
-
-    int rc = g_HvmR0.pfnInitVM(pVM);
-    return rc;
+    return g_HvmR0.pfnInitVM(pVM);
 }
 
@@ -1304 +1299 @@
     AssertReturn(pVM, VERR_INVALID_PARAMETER);
 
-    /* Make sure we don't touch HM after we've disabled HM in preparation
-       of a suspend. */
-    /** @todo r=bird: This cannot be right, the termination functions are
-     *        just freeing memory and resetting pVM/pVCpu members...
-     *  ==> memory leak. */
-    AssertReturn(!ASMAtomicReadBool(&g_HvmR0.fSuspended), VERR_HM_SUSPEND_PENDING);
-
     /*
      * Call the hardware specific method.
+     *
+     * Note! We might be preparing for a suspend, so the pfnTermVM() functions should probably not
+     * mess with VT-x/AMD-V features on the CPU, currently all they do is free memory so this is safe.
      */
-    RTCCUINTREG      fFlags = ASMIntDisableFlags();
-    PHMGLOBALCPUINFO pCpu   = HMR0GetCurrentCpu();
-    ASMSetFlags(fFlags);
-
-    int rc = g_HvmR0.pfnTermVM(pVM);
-    return rc;
+    return g_HvmR0.pfnTermVM(pVM);
 }
 
@@ -1340 +1326 @@
     AssertReturn(!ASMAtomicReadBool(&g_HvmR0.fSuspended), VERR_HM_SUSPEND_PENDING);
 
-    /*
-     * Call the hardware specific setup VM method.  This requires the CPU to be
-     * enabled for AMD-V/VT-x and preemption to be prevented.
-     */
-    RTCCUINTREG      fFlags = ASMIntDisableFlags();
-    RTCPUID          idCpu  = RTMpCpuId();
-    PHMGLOBALCPUINFO pCpu   = &g_HvmR0.aCpuInfo[idCpu];
-
     /* On first entry we'll sync everything. */
     for (VMCPUID i = 0; i < pVM->cCpus; i++)
         VMCPU_HMCF_RESET_TO(&pVM->aCpus[i], HM_CHANGED_HOST_CONTEXT | HM_CHANGED_ALL_GUEST);
 
+    /*
+     * Call the hardware specific setup VM method. This requires the CPU to be
+     * enabled for AMD-V/VT-x and preemption to be prevented.
+     */
+    RTTHREADPREEMPTSTATE PreemptState = RTTHREADPREEMPTSTATE_INITIALIZER;
+    RTThreadPreemptDisable(&PreemptState);
+    RTCPUID          idCpu  = RTMpCpuId();
+    PHMGLOBALCPUINFO pCpu   = &g_HvmR0.aCpuInfo[idCpu];
+
     /* Enable VT-x or AMD-V if local init is required. */
     int rc;
@@ -1357 +1344 @@
     {
         rc = hmR0EnableCpu(pVM, idCpu);
-        AssertReturnStmt(RT_SUCCESS_NP(rc), ASMSetFlags(fFlags), rc);
+        AssertRCReturnStmt(rc, RTThreadPreemptRestore(&PreemptState), rc);
     }
 
@@ -1370 +1357 @@
     }
 
-    ASMSetFlags(fFlags);
+    RTThreadPreemptRestore(&PreemptState);
     return rc;
 }

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

@@ -321 +321 @@
                               void *pvArg)
 {
+    Assert(!RTThreadPreemptIsEnabled(NIL_RTTHREAD));
     AssertReturn(!fEnabledByHost, VERR_INVALID_PARAMETER);
     AssertReturn(   HCPhysCpuPage
@@ -328 +329 @@
     NOREF(fEnabledByHost);
 
+    /* Paranoid: Disable interrupt as, in theory, interrupt handlers might mess with EFER. */
+    RTCCUINTREG uEflags = ASMIntDisableFlags();
+
     /*
      * We must turn on AMD-V and setup the host state physical address, as those MSRs are per CPU.
@@ -342 +346 @@
 
         if (!pCpu->fIgnoreAMDVInUseError)
+        {
+            ASMSetFlags(uEflags);
             return VERR_SVM_IN_USE;
+        }
     }
 
@@ -350 +357 @@
     /* Write the physical page address where the CPU will store the host state while executing the VM. */
     ASMWrMsr(MSR_K8_VM_HSAVE_PA, HCPhysCpuPage);
+
+    /* Restore interrupts. */
+    ASMSetFlags(uEflags);
 
     /*
@@ -378 +388 @@
 VMMR0DECL(int) SVMR0DisableCpu(PHMGLOBALCPUINFO pCpu, void *pvCpuPage, RTHCPHYS HCPhysCpuPage)
 {
+    Assert(!RTThreadPreemptIsEnabled(NIL_RTTHREAD));
     AssertReturn(   HCPhysCpuPage
                  && HCPhysCpuPage != NIL_RTHCPHYS, VERR_INVALID_PARAMETER);
@@ -383 +394 @@
     NOREF(pCpu);
 
+    /* Paranoid: Disable interrupts as, in theory, interrupt handlers might mess with EFER. */
+    RTCCUINTREG uEflags = ASMIntDisableFlags();
+
     /* Turn off AMD-V in the EFER MSR. */
     uint64_t u64HostEfer = ASMRdMsr(MSR_K6_EFER);
@@ -389 +403 @@
     /* Invalidate host state physical address. */
     ASMWrMsr(MSR_K8_VM_HSAVE_PA, 0);
+
+    /* Restore interrupts. */
+    ASMSetFlags(uEflags);
 
     return VINF_SUCCESS;
@@ -641 +658 @@
 VMMR0DECL(int) SVMR0SetupVM(PVM pVM)
 {
-    int rc = VINF_SUCCESS;
-
+    Assert(!RTThreadPreemptIsEnabled(NIL_RTTHREAD));
     AssertReturn(pVM, VERR_INVALID_PARAMETER);
     Assert(pVM->hm.s.svm.fSupported);
@@ -651 +667 @@
         PSVMVMCB pVmcb = (PSVMVMCB)pVM->aCpus[i].hm.s.svm.pvVmcb;
 
-        AssertMsgReturn(pVmcb, ("Invalid pVmcb\n"), VERR_SVM_INVALID_PVMCB);
+        AssertMsgReturn(pVmcb, ("Invalid pVmcb for vcpu[%u]\n", i), VERR_SVM_INVALID_PVMCB);
 
         /* Trap exceptions unconditionally (debug purposes). */
@@ -770 +786 @@
     }
 
-    return rc;
+    return VINF_SUCCESS;
 }
 

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

@@ -740 +740 @@
     }
 
+    /* Paranoid: Disable interrupts as, in theory, interrupt handlers might mess with CR4. */
+    RTCCUINTREG uEflags = ASMIntDisableFlags();
+
     /* Enable the VMX bit in CR4 if necessary. */
     RTCCUINTREG uCr4 = ASMGetCR4();
@@ -750 +753 @@
         ASMSetCR4(uCr4);
 
+    /* Restore interrupts. */
+    ASMSetFlags(uEflags);
     return rc;
 }
@@ -762 +767 @@
 {
     Assert(!RTThreadPreemptIsEnabled(NIL_RTTHREAD));
+
+    /* Paranoid: Disable interrupts as, in theory, interrupts handlers might mess with CR4. */
+    RTCCUINTREG uEflags = ASMIntDisableFlags();
 
     /* If we're for some reason not in VMX root mode, then don't leave it. */
     RTCCUINTREG uHostCR4 = ASMGetCR4();
+
+    int rc;
     if (uHostCR4 & X86_CR4_VMXE)
     {
@@ -770 +780 @@
         VMXDisable();
         ASMSetCR4(uHostCR4 & ~X86_CR4_VMXE);
-        return VINF_SUCCESS;
-    }
-
-    return VERR_VMX_NOT_IN_VMX_ROOT_MODE;
+        rc = VINF_SUCCESS;
+    }
+    else
+        rc = VERR_VMX_NOT_IN_VMX_ROOT_MODE;
+
+    /* Restore interrupts. */
+    ASMSetFlags(uEflags);
+    return rc;
 }