Changeset 98150 in vbox for trunk/src/VBox/VMM/VMMAll

Timestamp:

Jan 20, 2023 6:46:21 AM (2 years ago)

Author:

vboxsync

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

155323

Message:

VMM: Nested VMX: bugref:10318 Fix VMX CR0/CR4 fixed bits masking.

Location:

trunk/src/VBox/VMM/VMMAll

Files:

• IEMAllCImpl.cpp (modified) (2 diffs)
• IEMAllCImplVmxInstr.cpp (modified) (5 diffs)
• VMXAllTemplate.cpp.h (modified) (7 diffs)

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

@@ -5927 +5927 @@
             }
 
+#ifdef VBOX_WITH_NESTED_HWVIRT_VMX
             /* Check for bits that must remain set or cleared in VMX operation,
                see Intel spec. 23.8 "Restrictions on VMX operation". */
             if (IEM_VMX_IS_ROOT_MODE(pVCpu))
             {
-#ifdef VBOX_WITH_NESTED_HWVIRT_VMX
-                uint64_t const uCr0Fixed0 = IEM_VMX_IS_NON_ROOT_MODE(pVCpu) ? iemVmxGetCr0Fixed0(pVCpu) : VMX_V_CR0_FIXED0;
-#else
-                uint64_t const uCr0Fixed0 = VMX_V_CR0_FIXED0;
-#endif
+                uint64_t const uCr0Fixed0 = iemVmxGetCr0Fixed0(pVCpu, IEM_VMX_IS_NON_ROOT_MODE(pVCpu));
                 if ((uNewCrX & uCr0Fixed0) != uCr0Fixed0)
                 {
@@ -5949 +5946 @@
                 }
             }
+#endif
 
             /*

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

@@ -1740 +1740 @@
     {
         /* Bits 63:32, 28:19, 17, 15:6, ET, CD, NW and CR0 fixed bits are not modified. */
-        uint64_t const uCr0Mb1       = iemVmxGetCr0Fixed0(pVCpu);
-        uint64_t const uCr0Mb0       = VMX_V_CR0_FIXED1;
-        uint64_t const fCr0IgnMask   = VMX_EXIT_HOST_CR0_IGNORE_MASK | uCr0Mb1 | ~uCr0Mb0;
+        uint64_t const fCr0IgnMask   = VMX_EXIT_HOST_CR0_IGNORE_MASK;
         uint64_t const uHostCr0      = pVmcs->u64HostCr0.u;
         uint64_t const uGuestCr0     = pVCpu->cpum.GstCtx.cr0;
         uint64_t const uValidHostCr0 = (uHostCr0 & ~fCr0IgnMask) | (uGuestCr0 & fCr0IgnMask);
 
-        /* Verify we have not modified CR0 fixed bits in VMX non-root operation. */
-        Assert((uGuestCr0 &  uCr0Mb1) == uCr0Mb1);
-        Assert((uGuestCr0 & ~uCr0Mb0) == 0);
+        /* Verify we have not modified CR0 fixed bits in VMX operation. */
+#ifdef VBOX_STRICT
+        uint64_t const uCr0Mb1 = iemVmxGetCr0Fixed0(pVCpu, true /* fVmxNonRootMode */);
+        bool const fUx         = RT_BOOL(pVmcs->u32ProcCtls2 & VMX_PROC_CTLS2_UNRESTRICTED_GUEST);
+        AssertMsg(   (uValidHostCr0 & uCr0Mb1) == uCr0Mb1
+                  && (uValidHostCr0 & ~VMX_V_CR0_FIXED1) == 0,
+                  ("host=%#RX64 guest=%#RX64 mb1=%#RX64 valid_host_cr0=%#RX64 fUx=%RTbool\n",
+                   uHostCr0, uGuestCr0, uCr0Mb1, uValidHostCr0, fUx));
+#endif
+        Assert(!(uValidHostCr0 >> 32));
         CPUMSetGuestCR0(pVCpu, uValidHostCr0);
     }
@@ -1758 +1763 @@
         uint64_t const uCr4Mb1       = pVCpu->cpum.GstCtx.hwvirt.vmx.Msrs.u64Cr4Fixed0;
         uint64_t const uCr4Mb0       = pVCpu->cpum.GstCtx.hwvirt.vmx.Msrs.u64Cr4Fixed1;
-        uint64_t const fCr4IgnMask   = uCr4Mb1 | ~uCr4Mb0;
         uint64_t const uHostCr4      = pVmcs->u64HostCr4.u;
-        uint64_t const uGuestCr4     = pVCpu->cpum.GstCtx.cr4;
-        uint64_t       uValidHostCr4 = (uHostCr4 & ~fCr4IgnMask) | (uGuestCr4 & fCr4IgnMask);
+        uint64_t       uValidHostCr4 = (uHostCr4 & uCr4Mb0) | uCr4Mb1;
         if (fHostInLongMode)
             uValidHostCr4 |= X86_CR4_PAE;
@@ -1768 +1771 @@
 
         /* Verify we have not modified CR4 fixed bits in VMX non-root operation. */
-        Assert((uGuestCr4 &  uCr4Mb1) == uCr4Mb1);
-        Assert((uGuestCr4 & ~uCr4Mb0) == 0);
+        AssertMsg(   (uValidHostCr4 &  uCr4Mb1) == uCr4Mb1
+                  && (uValidHostCr4 & ~uCr4Mb0) == 0,
+                  ("host=%#RX64 guest=%#RX64, uCr4Mb1=%#RX64 uCr4Mb0=%#RX64 valid_host_cr4=%#RX64\n",
+                   uHostCr4, pVCpu->cpum.GstCtx.cr4, uCr4Mb1, uCr4Mb0, uValidHostCr4));
         CPUMSetGuestCR4(pVCpu, uValidHostCr4);
     }
@@ -5135 +5140 @@
     {
         /* CR0 MB1 bits. */
-        uint64_t const u64Cr0Fixed0 = iemVmxGetCr0Fixed0(pVCpu);
+        uint64_t const u64Cr0Fixed0 = iemVmxGetCr0Fixed0(pVCpu, true /* fVmxNonRootMode */);
         if ((pVmcs->u64GuestCr0.u & u64Cr0Fixed0) == u64Cr0Fixed0)
         { /* likely */ }
@@ -6079 +6084 @@
     {
         /* CR0 MB1 bits. */
-        uint64_t const u64Cr0Fixed0 = iemVmxGetCr0Fixed0(pVCpu);
+        uint64_t const u64Cr0Fixed0 = iemVmxGetCr0Fixed0(pVCpu, true /* fVmxNonRootMode */);
         if ((pVmcs->u64HostCr0.u & u64Cr0Fixed0) == u64Cr0Fixed0)
         { /* likely */ }

trunk/src/VBox/VMM/VMMAll/VMXAllTemplate.cpp.h

@@ -734 +734 @@
      *        and @bugref{6944}. */
     PCVMCC pVM = pVCpu->CTX_SUFF(pVM);
+    AssertCompile(RT_HI_U32(VMX_EXIT_HOST_CR0_IGNORE_MASK) == UINT32_C(0xffffffff));    /* Paranoia. */
     return (  X86_CR0_PE
             | X86_CR0_NE
@@ -2139 +2140 @@
             u64GuestCr0 &= ~(uint64_t)(X86_CR0_CD | X86_CR0_NW);
 
+            Assert(!RT_HI_U32(u64GuestCr0));
+            Assert(u64GuestCr0 & X86_CR0_NE);
+
             /* Commit the CR0 and related fields to the guest VMCS. */
             int rc = VMX_VMCS_WRITE_NW(pVCpu, VMX_VMCS_GUEST_CR0, u64GuestCr0);               AssertRC(rc);
@@ -2171 +2175 @@
             uint64_t       u64GuestCr0  = pVCpu->cpum.GstCtx.cr0;
             uint64_t const u64ShadowCr0 = CPUMGetGuestVmxMaskedCr0(&pVCpu->cpum.GstCtx, pVmcsInfo->u64Cr0Mask);
-            Assert(!RT_HI_U32(u64GuestCr0));
-            Assert(u64GuestCr0 & X86_CR0_NE);
 
             /* Apply the hardware specified CR0 fixed bits and enable caching. */
@@ -2179 +2181 @@
             u64GuestCr0 &= ~(uint64_t)(X86_CR0_CD | X86_CR0_NW);
 
+            Assert(!RT_HI_U32(u64GuestCr0));
+            Assert(u64GuestCr0 & X86_CR0_NE);
+
             /* Commit the CR0 and CR0 read-shadow to the nested-guest VMCS. */
             int rc = VMX_VMCS_WRITE_NW(pVCpu, VMX_VMCS_GUEST_CR0, u64GuestCr0);               AssertRC(rc);
             rc     = VMX_VMCS_WRITE_NW(pVCpu, VMX_VMCS_CTRL_CR0_READ_SHADOW, u64ShadowCr0);   AssertRC(rc);
 
-            Log4Func(("cr0=%#RX64 shadow=%#RX64 (set=%#RX64 zap=%#RX64)\n", u64GuestCr0, u64ShadowCr0, fSetCr0, fZapCr0));
+            Log4Func(("cr0=%#RX64 shadow=%#RX64 vmcs_read_shw=%#RX64 (set=%#RX64 zap=%#RX64)\n", u64GuestCr0, u64ShadowCr0,
+                      pVCpu->cpum.GstCtx.hwvirt.vmx.Vmcs.u64Cr0ReadShadow.u, fSetCr0, fZapCr0));
         }
 
@@ -2416 +2422 @@
         u64GuestCr4 |= fSetCr4;
         u64GuestCr4 &= fZapCr4;
+
+        Assert(!RT_HI_U32(u64GuestCr4));
+        Assert(u64GuestCr4 & X86_CR4_VMXE);
 
         /* Commit the CR4 and CR4 read-shadow to the guest VMCS. */
@@ -4089 +4098 @@
             PCVMXVMCSINFO const pVmcsInfoGst = &pVCpu->hmr0.s.vmx.VmcsInfo;
             PVMXVVMCS const     pVmcsNstGst  = &pVCpu->cpum.GstCtx.hwvirt.vmx.Vmcs;
-            u64Cr0 = (u64Cr0                     & ~pVmcsInfo->u64Cr0Mask)
-                   | (pVmcsNstGst->u64GuestCr0.u &  pVmcsNstGst->u64Cr0Mask.u)
-                   | (u64Shadow                  & (pVmcsInfoGst->u64Cr0Mask & ~pVmcsNstGst->u64Cr0Mask.u));
+            u64Cr0 = (u64Cr0                     & ~(pVmcsInfoGst->u64Cr0Mask & pVmcsNstGst->u64Cr0Mask.u))
+                   | (pVmcsNstGst->u64GuestCr0.u &   pVmcsNstGst->u64Cr0Mask.u)
+                   | (u64Shadow                  &  (pVmcsInfoGst->u64Cr0Mask & ~pVmcsNstGst->u64Cr0Mask.u));
+            Assert(u64Cr0 & X86_CR0_NE);
         }
 #endif
@@ -4125 +4135 @@
             PCVMXVMCSINFO const pVmcsInfoGst = &pVCpu->hmr0.s.vmx.VmcsInfo;
             PVMXVVMCS const     pVmcsNstGst  = &pVCpu->cpum.GstCtx.hwvirt.vmx.Vmcs;
-            u64Cr4 = (u64Cr4                     & ~pVmcsInfo->u64Cr4Mask)
-                   | (pVmcsNstGst->u64GuestCr4.u &  pVmcsNstGst->u64Cr4Mask.u)
-                   | (u64Shadow                  & (pVmcsInfoGst->u64Cr4Mask & ~pVmcsNstGst->u64Cr4Mask.u));
+            u64Cr4 = (u64Cr4                     & ~(pVmcsInfo->u64Cr4Mask & pVmcsNstGst->u64Cr4Mask.u))
+                   | (pVmcsNstGst->u64GuestCr4.u &   pVmcsNstGst->u64Cr4Mask.u)
+                   | (u64Shadow                  &  (pVmcsInfoGst->u64Cr4Mask & ~pVmcsNstGst->u64Cr4Mask.u));
+            Assert(u64Cr4 & X86_CR4_VMXE);
         }
 #endif