Changeset 72849 in vbox

Timestamp:

Jul 4, 2018 5:27:56 AM (6 years ago)

Author:

vboxsync

Message:

VMM/HMVMXR0: bugref:9193 Add assertions where possible to verify we don't overwrite stale values from CPUMCTX into the VMCS.

File:

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

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

@@ -153 +153 @@
 
 /** Assert that preemption is disabled or covered by thread-context hooks. */
-#define HMVMX_ASSERT_PREEMPT_SAFE()       Assert(   VMMR0ThreadCtxHookIsEnabled(pVCpu)   \
-                                                 || !RTThreadPreemptIsEnabled(NIL_RTTHREAD));
+#define HMVMX_ASSERT_PREEMPT_SAFE()             Assert(   VMMR0ThreadCtxHookIsEnabled(pVCpu)   \
+                                                       || !RTThreadPreemptIsEnabled(NIL_RTTHREAD))
 
 /** Assert that we haven't migrated CPUs when thread-context hooks are not
  *  used. */
-#define HMVMX_ASSERT_CPU_SAFE()           AssertMsg(   VMMR0ThreadCtxHookIsEnabled(pVCpu) \
-                                                    || pVCpu->hm.s.idEnteredCpu == RTMpCpuId(), \
-                                                    ("Illegal migration! Entered on CPU %u Current %u\n", \
-                                                    pVCpu->hm.s.idEnteredCpu, RTMpCpuId())); \
+#define HMVMX_ASSERT_CPU_SAFE()                 AssertMsg(   VMMR0ThreadCtxHookIsEnabled(pVCpu) \
+                                                          || pVCpu->hm.s.idEnteredCpu == RTMpCpuId(), \
+                                                          ("Illegal migration! Entered on CPU %u Current %u\n", \
+                                                          pVCpu->hm.s.idEnteredCpu, RTMpCpuId()))
+
+/** Asserts that the given CPUMCTX_EXTRN_XXX bits are present in the guest-CPU
+ *  context. */
+#define HMVMX_CPUMCTX_ASSERT(pVCpu, fExtrnMbz)  AssertMsg(!((pVCpu)->cpum.GstCtx.fExtrn & (fExtrnMbz)), \
+                                                          ("fExtrn=%#RX64 fExtrnMbz=%#RX64\n", (pVCpu)->cpum.GstCtx.fExtrn, \
+                                                          (fExtrnMbz)))
 
 /** Helper macro for VM-exit handlers called unexpectedly. */
@@ -3391 +3397 @@
     if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_APIC_TPR)
     {
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_APIC_TPR);
+
         if (   PDMHasApic(pVCpu->CTX_SUFF(pVM))
             && APICIsEnabled(pVCpu))
@@ -3535 +3543 @@
         Assert(pVCpu->hm.s.vmx.u32XcptBitmap & RT_BIT_32(X86_XCPT_DB));
 
+        /** @todo Optimize by checking cache before writing to VMCS. */
         int rc = VMXWriteVmcs32(VMX_VMCS32_CTRL_EXCEPTION_BITMAP, pVCpu->hm.s.vmx.u32XcptBitmap);
         AssertRCReturn(rc, rc);
@@ -3561 +3570 @@
     if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_RIP)
     {
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_RIP);
+
         rc = VMXWriteVmcsGstN(VMX_VMCS_GUEST_RIP, pMixedCtx->rip);
         AssertRCReturn(rc, rc);
@@ -3592 +3603 @@
     if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_RSP)
     {
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_RSP);
+
         int rc = VMXWriteVmcsGstN(VMX_VMCS_GUEST_RSP, pMixedCtx->rsp);
         AssertRCReturn(rc, rc);
@@ -3616 +3629 @@
     if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_RFLAGS)
     {
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_RFLAGS);
+
         /* Intel spec. 2.3.1 "System Flags and Fields in IA-32e Mode" claims the upper 32-bits of RFLAGS are reserved (MBZ).
            Let us assert it as such and use 32-bit VMWRITE. */
@@ -3666 +3681 @@
     {
         PVM pVM = pVCpu->CTX_SUFF(pVM);
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_CR0);
         Assert(!RT_HI_U32(pMixedCtx->cr0));
+
         uint32_t const u32ShadowCr0 = pMixedCtx->cr0;
         uint32_t       u32GuestCr0  = pMixedCtx->cr0;
@@ -3846 +3863 @@
     if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_CR3)
     {
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_CR3);
+
         RTGCPHYS GCPhysGuestCR3 = NIL_RTGCPHYS;
         if (pVM->hm.s.fNestedPaging)
@@ -3942 +3961 @@
     if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_CR4)
     {
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_CR4);
         Assert(!RT_HI_U32(pMixedCtx->cr4));
+
         uint32_t       u32GuestCr4  = pMixedCtx->cr4;
         uint32_t const u32ShadowCr4 = pMixedCtx->cr4;
@@ -4478 +4499 @@
         if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_CS)
         {
+            HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_CS);
             if (pVCpu->hm.s.vmx.RealMode.fRealOnV86Active)
                 pVCpu->hm.s.vmx.RealMode.AttrCS.u = pMixedCtx->cs.Attr.u;
@@ -4487 +4509 @@
         if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_SS)
         {
+            HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_SS);
             if (pVCpu->hm.s.vmx.RealMode.fRealOnV86Active)
                 pVCpu->hm.s.vmx.RealMode.AttrSS.u = pMixedCtx->ss.Attr.u;
@@ -4496 +4519 @@
         if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_DS)
         {
+            HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_DS);
             if (pVCpu->hm.s.vmx.RealMode.fRealOnV86Active)
                 pVCpu->hm.s.vmx.RealMode.AttrDS.u = pMixedCtx->ds.Attr.u;
@@ -4505 +4529 @@
         if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_ES)
         {
+            HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_ES);
             if (pVCpu->hm.s.vmx.RealMode.fRealOnV86Active)
                 pVCpu->hm.s.vmx.RealMode.AttrES.u = pMixedCtx->es.Attr.u;
@@ -4514 +4539 @@
         if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_FS)
         {
+            HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_FS);
             if (pVCpu->hm.s.vmx.RealMode.fRealOnV86Active)
                 pVCpu->hm.s.vmx.RealMode.AttrFS.u = pMixedCtx->fs.Attr.u;
@@ -4523 +4549 @@
         if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_GS)
         {
+            HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_GS);
             if (pVCpu->hm.s.vmx.RealMode.fRealOnV86Active)
                 pVCpu->hm.s.vmx.RealMode.AttrGS.u = pMixedCtx->gs.Attr.u;
@@ -4547 +4574 @@
     if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_TR)
     {
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_TR);
+
         /*
          * Real-mode emulation using virtual-8086 mode with CR4.VME. Interrupt redirection is
@@ -4614 +4643 @@
     if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_GDTR)
     {
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_GDTR);
+
         rc  = VMXWriteVmcs32(VMX_VMCS32_GUEST_GDTR_LIMIT, pMixedCtx->gdtr.cbGdt);
         rc |= VMXWriteVmcsGstN(VMX_VMCS_GUEST_GDTR_BASE,  pMixedCtx->gdtr.pGdt);
@@ -4630 +4661 @@
     if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_LDTR)
     {
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_LDTR);
+
         /* The unusable bit is specific to VT-x, if it's a null selector mark it as an unusable segment. */
         uint32_t u32Access = 0;
@@ -4667 +4700 @@
     if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_IDTR)
     {
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_IDTR);
+
         rc  = VMXWriteVmcs32(VMX_VMCS32_GUEST_IDTR_LIMIT, pMixedCtx->idtr.cbIdt);
         rc |= VMXWriteVmcsGstN(VMX_VMCS_GUEST_IDTR_BASE,  pMixedCtx->idtr.pIdt);
@@ -4716 +4751 @@
         {
 #if HC_ARCH_BITS == 32
+            HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_SYSCALL_MSRS | CPUMCTX_EXTRN_KERNEL_GS_BASE);
+
             int rc = hmR0VmxAddAutoLoadStoreMsr(pVCpu, MSR_K8_LSTAR,          pMixedCtx->msrLSTAR,        false, NULL);
             rc    |= hmR0VmxAddAutoLoadStoreMsr(pVCpu, MSR_K6_STAR,           pMixedCtx->msrSTAR,         false, NULL);
@@ -4738 +4775 @@
     if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_SYSENTER_MSR_MASK)
     {
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_SYSENTER_MSRS);
+
         if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_SYSENTER_CS_MSR)
         {
@@ -4762 +4801 @@
     if (ASMAtomicUoReadU64(&pVCpu->hm.s.fCtxChanged) & HM_CHANGED_GUEST_EFER_MSR)
     {
+        HMVMX_CPUMCTX_ASSERT(pVCpu, CPUMCTX_EXTRN_EFER);
+
         if (hmR0VmxShouldSwapEferMsr(pVCpu, pMixedCtx))
         {