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

Timestamp:

Jun 14, 2019 4:54:41 AM (6 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

131309

Message:

VMM/HM: Nested VMX: bugref:9180 Reapply CR0/CR4 fixed bits while importing nested-guest CR0/CR4 after hardware-assisted VMX execution of the nested-guest.
Flag reloading all the HM state on the HM VM-exit notification callback.

Location:

trunk/src/VBox/VMM

Files:

• VMMAll/HMVMXAll.cpp (modified) (1 diff)
• VMMR0/HMVMXR0.cpp (modified) (12 diffs)

trunk/src/VBox/VMM/VMMAll/HMVMXAll.cpp

@@ -1238 +1238 @@
     NOREF(pCtx);
 
-    /* There shouldn't be any externally kept state at this point. */
-    AssertMsg(!(pVCpu->cpum.GstCtx.fExtrn & CPUMCTX_EXTRN_ALL),
-              ("fExtrn=%#RX64 fExtrnMbz=%#RX64\n", pVCpu->cpum.GstCtx.fExtrn, CPUMCTX_EXTRN_ALL));
+    /*
+     * Transitions to ring-3 flag a full CPU-state change except if we transition to ring-3
+     * in response to a physical CPU interrupt as no changes to the guest-CPU state are
+     * expected (see VINF_EM_RAW_INTERRUPT handling in hmR0VmxExitToRing3).
+     *
+     * However, with nested-guests, the state -can- change on trips to ring-3 for we might
+     * try to inject a nested-guest physical interrupt and cause a VMX_EXIT_EXT_INT VM-exit
+     * for the nested-guest from ring-3.
+     *
+     * Flag reloading of just the guest-CPU state is -not- sufficient since HM also needs
+     * to reload related state with VM-entry/VM-exit controls and so on. Flag reloading
+     * the entire state.
+     */
+    CPUM_ASSERT_NOT_EXTRN(pVCpu, CPUMCTX_EXTRN_ALL);
+    ASMAtomicUoOrU64(&pVCpu->hm.s.fCtxChanged, HM_CHANGED_ALL_GUEST);
 
     /*

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

@@ -4464 +4464 @@
              * For nested-guests, the "IA-32e mode guest" control we initialize with what is
              * required to get the nested-guest working with hardware-assisted VMX execution.
-             * It depends on the nested-guest's IA32_EFER.LMA bit. Remember, a nested-hypervisor
+             * It depends on the nested-guest's IA32_EFER.LMA bit. Remember, a guest hypervisor
              * can skip intercepting changes to the EFER MSR. This is why it it needs to be done
              * here rather than while merging the guest VMCS controls.
@@ -4912 +4912 @@
          * Figure out fixed CR0 bits in VMX operation.
          */
+        /** @todo Why do we need to OR and AND the fixed-0 and fixed-1 bits below? */
         uint64_t       fSetCr0 = pVM->hm.s.vmx.Msrs.u64Cr0Fixed0 & pVM->hm.s.vmx.Msrs.u64Cr0Fixed1;
         uint64_t const fZapCr0 = pVM->hm.s.vmx.Msrs.u64Cr0Fixed0 | pVM->hm.s.vmx.Msrs.u64Cr0Fixed1;
@@ -5036 +5037 @@
              * originally supplied. We must copy those bits from the nested-guest CR0 into
              * the nested-guest CR0 read-shadow.
+             *
+             * Note! We are zapping away any CR0 fixed bits of our VMX emulation and applying
+             *       the hardware's VMX CR0 fixed bits here.
              */
             HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_CR0);
@@ -5199 +5203 @@
          * Figure out fixed CR4 bits in VMX operation.
          */
+        /** @todo Why do we need to OR and AND the fixed-0 and fixed-1 bits below? */
         uint64_t const fSetCr4 = pVM->hm.s.vmx.Msrs.u64Cr4Fixed0 & pVM->hm.s.vmx.Msrs.u64Cr4Fixed1;
         uint64_t const fZapCr4 = pVM->hm.s.vmx.Msrs.u64Cr4Fixed0 | pVM->hm.s.vmx.Msrs.u64Cr4Fixed1;
@@ -7759 +7764 @@
                     u64Val = (u64Val    & ~pVmcsInfo->u64Cr0Mask)
                            | (u64Shadow &  pVmcsInfo->u64Cr0Mask);
-
+#ifdef VBOX_WITH_NESTED_HWVIRT_VMX
+                    /*
+                     * Reapply the nested-guest's CR0 fixed bits that might have been altered while
+                     * exporting the nested-guest CR0 for executing using hardware-assisted VMX.
+                     */
+                    if (CPUMIsGuestInVmxNonRootMode(pCtx))
+                    {
+                        u64Val |= pCtx->hwvirt.vmx.Msrs.u64Cr0Fixed0;
+                        u64Val &= pCtx->hwvirt.vmx.Msrs.u64Cr0Fixed1;
+                    }
+#endif
                     VMMRZCallRing3Disable(pVCpu);   /* May call into PGM which has Log statements. */
                     CPUMSetGuestCR0(pVCpu, u64Val);
@@ -7782 +7797 @@
                     u64Val = (u64Val    & ~pVmcsInfo->u64Cr4Mask)
                            | (u64Shadow &  pVmcsInfo->u64Cr4Mask);
+#ifdef VBOX_WITH_NESTED_HWVIRT_VMX
+                    /*
+                     * Reapply the nested-guest's CR4 fixed bits that might have been altered while
+                     * exporting the nested-guest CR4 for executing using hardware-assisted VMX.
+                     */
+                    if (CPUMIsGuestInVmxNonRootMode(pCtx))
+                    {
+                        u64Val |= pCtx->hwvirt.vmx.Msrs.u64Cr4Fixed0;
+                        u64Val &= pCtx->hwvirt.vmx.Msrs.u64Cr4Fixed1;
+                    }
+#endif
                     pCtx->cr4 = u64Val;
                 }
@@ -9511 +9537 @@
  * and update error record fields accordingly.
  *
- * @return VMX_IGS_* return codes.
+ * @returns VMX_IGS_* error codes.
  * @retval VMX_IGS_REASON_NOT_FOUND if this function could not find anything
  *         wrong with the guest state.
@@ -9541 +9567 @@
          * CR0.
          */
+        /** @todo Why do we need to OR and AND the fixed-0 and fixed-1 bits below? */
         uint32_t       fSetCr0 = (uint32_t)(pVM->hm.s.vmx.Msrs.u64Cr0Fixed0 & pVM->hm.s.vmx.Msrs.u64Cr0Fixed1);
         uint32_t const fZapCr0 = (uint32_t)(pVM->hm.s.vmx.Msrs.u64Cr0Fixed0 | pVM->hm.s.vmx.Msrs.u64Cr0Fixed1);
@@ -9563 +9590 @@
          * CR4.
          */
+        /** @todo Why do we need to OR and AND the fixed-0 and fixed-1 bits below? */
         uint64_t const fSetCr4 = (pVM->hm.s.vmx.Msrs.u64Cr4Fixed0 & pVM->hm.s.vmx.Msrs.u64Cr4Fixed1);
         uint64_t const fZapCr4 = (pVM->hm.s.vmx.Msrs.u64Cr4Fixed0 | pVM->hm.s.vmx.Msrs.u64Cr4Fixed1);
@@ -10246 +10274 @@
      * These controls contains state that depends on the nested-guest state (primarily
      * EFER MSR) and is thus not constant between VMLAUNCH/VMRESUME and the nested-guest
-     * VM-exit. Although the nested-hypervisor cannot change it, we need to in order to
+     * VM-exit. Although the guest hypervisor cannot change it, we need to in order to
      * properly continue executing the nested-guest if the EFER MSR changes but does not
      * cause a nested-guest VM-exits.
@@ -10252 +10280 @@
      * VM-exit controls:
      * These controls specify the host state on return. We cannot use the controls from
-     * the guest-hypervisor state as is as it would contain the guest state rather than
+     * the guest hypervisor state as is as it would contain the guest state rather than
      * the host state. Since the host state is subject to change (e.g. preemption, trips
      * to ring-3, longjmp and rescheduling to a different host CPU) they are not constant
@@ -10267 +10295 @@
      * VM-exit MSR-load areas:
      * This must contain the real host MSRs with hardware-assisted VMX execution. Hence,
-     * we can entirely ignore what the nested-hypervisor wants to load here.
+     * we can entirely ignore what the guest hypervisor wants to load here.
      */