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

Timestamp:

Nov 16, 2022 2:34:26 AM (2 years ago)

Author:

vboxsync

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

154593

Message:

VMM/HMVMX,CPUM: Added a HM/AlwaysInterceptVmxMovDRx config for controlling how we deal with DR6.RTM & DR7.RTM and similar. Current default (-1) is the old behaviour of ignoring the issue, will change that to hide these new bits in a few hours.

Location:

trunk/src/VBox/VMM

Files:

• VMMAll/VMXAllTemplate.cpp.h (modified) (7 diffs)
• VMMR0/CPUMR0.cpp (modified) (1 diff)
• VMMR0/HMVMXR0.cpp (modified) (9 diffs)
• VMMR3/HM.cpp (modified) (3 diffs)
• include/HMInternal.h (modified) (5 diffs)

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

@@ -7144 +7144 @@
                         | (pVmxTransient->uExitQual & (  X86_DR6_B0 | X86_DR6_B1 | X86_DR6_B2 | X86_DR6_B3
                                                        | X86_DR6_BD | X86_DR6_BS));
+    Log6Func(("uDR6=%#RX64 uExitQual=%#RX64\n", uDR6, pVmxTransient->uExitQual));
 
     int rc;
@@ -7170 +7171 @@
          * The exception was for the guest.  Update DR6, DR7.GD and
          * IA32_DEBUGCTL.LBR before forwarding it.
-         * See Intel spec. 27.1 "Architectural State before a VM-Exit".
+         * See Intel spec. 27.1 "Architectural State before a VM-Exit"
+         * and @sdmv3{077,622,17.2.3,Debug Status Register (DR6)}.
          */
 #ifndef IN_NEM_DARWIN
@@ -9170 +9172 @@
     {
         /* We should -not- get this VM-exit if the guest's debug registers were active. */
-        if (pVmxTransient->fWasGuestDebugStateActive)
+        if (   pVmxTransient->fWasGuestDebugStateActive
+#ifdef VMX_WITH_MAYBE_ALWAYS_INTERCEPT_MOV_DRX
+            && !pVCpu->CTX_SUFF(pVM)->hmr0.s.vmx.fAlwaysInterceptMovDRx
+#endif
+           )
         {
             AssertMsgFailed(("Unexpected MOV DRx exit\n"));
@@ -9182 +9188 @@
             Assert(pVmcsInfo->u32XcptBitmap & RT_BIT(X86_XCPT_DB));
 
-            /* Don't intercept MOV DRx any more. */
-            pVmcsInfo->u32ProcCtls &= ~VMX_PROC_CTLS_MOV_DR_EXIT;
-            int rc = VMX_VMCS_WRITE_32(pVCpu, VMX_VMCS32_CTRL_PROC_EXEC, pVmcsInfo->u32ProcCtls);
-            AssertRC(rc);
+            /* Whether we disable intercepting MOV DRx instructions and resume
+               the current one, or emulate it and keep intercepting them is
+               configurable.  Though it usually comes down to whether there are
+               any new DR6 & DR7 bits (RTM) we want to hide from the guest. */
+#ifdef VMX_WITH_MAYBE_ALWAYS_INTERCEPT_MOV_DRX
+            bool const fResumeInstruction = !pVCpu->CTX_SUFF(pVM)->hmr0.s.vmx.fAlwaysInterceptMovDRx;
+#else
+            bool const fResumeInstruction = true;
+#endif
+            if (fResumeInstruction)
+            {
+                pVmcsInfo->u32ProcCtls &= ~VMX_PROC_CTLS_MOV_DR_EXIT;
+                int rc = VMX_VMCS_WRITE_32(pVCpu, VMX_VMCS32_CTRL_PROC_EXEC, pVmcsInfo->u32ProcCtls);
+                AssertRC(rc);
+            }
 
 #ifndef IN_NEM_DARWIN
@@ -9204 +9221 @@
 #endif
 
+            STAM_COUNTER_INC(&VCPU_2_VMXSTATS(pVCpu).StatDRxContextSwitch);
+            if (fResumeInstruction)
+            {
 #ifdef VBOX_WITH_STATISTICS
-            vmxHCReadToTransient<HMVMX_READ_EXIT_QUALIFICATION>(pVCpu, pVmxTransient);
-            if (VMX_EXIT_QUAL_DRX_DIRECTION(pVmxTransient->uExitQual) == VMX_EXIT_QUAL_DRX_DIRECTION_WRITE)
-                STAM_COUNTER_INC(&VCPU_2_VMXSTATS(pVCpu).StatExitDRxWrite);
-            else
-                STAM_COUNTER_INC(&VCPU_2_VMXSTATS(pVCpu).StatExitDRxRead);
-#endif
-            STAM_COUNTER_INC(&VCPU_2_VMXSTATS(pVCpu).StatDRxContextSwitch);
-            return VINF_SUCCESS;
+                vmxHCReadToTransient<HMVMX_READ_EXIT_QUALIFICATION>(pVCpu, pVmxTransient);
+                if (VMX_EXIT_QUAL_DRX_DIRECTION(pVmxTransient->uExitQual) == VMX_EXIT_QUAL_DRX_DIRECTION_WRITE)
+                    STAM_COUNTER_INC(&VCPU_2_VMXSTATS(pVCpu).StatExitDRxWrite);
+                else
+                    STAM_COUNTER_INC(&VCPU_2_VMXSTATS(pVCpu).StatExitDRxRead);
+#endif
+                return VINF_SUCCESS;
+            }
         }
     }
@@ -9226 +9246 @@
                                    | CPUMCTX_EXTRN_DR7>(pVCpu, pVmcsInfo, __FUNCTION__);
     AssertRCReturn(rc, rc);
-    Log4Func(("cs:rip=%#04x:%08RX64\n", pVCpu->cpum.GstCtx.cs.Sel, pVCpu->cpum.GstCtx.rip));
 
     uint8_t const iGReg  = VMX_EXIT_QUAL_DRX_GENREG(pVmxTransient->uExitQual);
     uint8_t const iDrReg = VMX_EXIT_QUAL_DRX_REGISTER(pVmxTransient->uExitQual);
+    Log4Func(("cs:rip=%#04x:%08RX64 r%d %s dr%d\n", pVCpu->cpum.GstCtx.cs.Sel, pVCpu->cpum.GstCtx.rip, iGReg,
+              VMX_EXIT_QUAL_DRX_DIRECTION(pVmxTransient->uExitQual) == VMX_EXIT_QUAL_DRX_DIRECTION_WRITE ? "->" : "<-", iDrReg));
 
     VBOXSTRICTRC  rcStrict;
@@ -9242 +9263 @@
 
         if (rcStrict == VINF_SUCCESS)
+       {
             /** @todo r=bird: Not sure why we always flag DR7 as modified here, but I've
              * kept it for now to avoid breaking something non-obvious. */
             ASMAtomicUoOrU64(&VCPU_2_VMXSTATE(pVCpu).fCtxChanged, HM_CHANGED_GUEST_RIP | HM_CHANGED_GUEST_RFLAGS
                                                                 | HM_CHANGED_GUEST_DR7);
+            /* Update the DR6 register if guest debug state is active, otherwise we'll
+               trash it when calling CPUMR0DebugStateMaybeSaveGuestAndRestoreHost. */
+            if (iDrReg == 6 && CPUMIsGuestDebugStateActive(pVCpu))
+                ASMSetDR6(pVCpu->cpum.GstCtx.dr[6]);
+            Log4Func(("r%d=%#RX64 => dr%d=%#RX64\n", iGReg, pVCpu->cpum.GstCtx.aGRegs[iGReg].u,
+                      iDrReg, pVCpu->cpum.GstCtx.dr[iDrReg]));
+        }
         else if (rcStrict == VINF_IEM_RAISED_XCPT)
         {

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

@@ -678 +678 @@
         pVCpu->cpum.s.Guest.dr[3] = ASMGetDR3();
         if (fDr6)
-            pVCpu->cpum.s.Guest.dr[6] = ASMGetDR6();
+            pVCpu->cpum.s.Guest.dr[6] = ASMGetDR6() | X86_DR6_RA1_MASK; /* ASSUMES no guest supprot for TSX-NI / RTM. */
     }
     ASMAtomicAndU32(&pVCpu->cpum.s.fUseFlags, ~(CPUM_USED_DEBUG_REGS_GUEST | CPUM_USED_DEBUG_REGS_HYPER));

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

@@ -71 +71 @@
 #endif
 
+/** Enables the fAlwaysInterceptMovDRx related code. */
+#define VMX_WITH_MAYBE_ALWAYS_INTERCEPT_MOV_DRX 1
+
 
 /*********************************************************************************************************************************
@@ -97 +100 @@
 static bool     hmR0VmxShouldSwapEferMsr(PCVMCPUCC pVCpu, PCVMXTRANSIENT pVmxTransient);
 static int      hmR0VmxExitHostNmi(PVMCPUCC pVCpu, PCVMXVMCSINFO pVmcsInfo);
+
+
+/*********************************************************************************************************************************
+*   Global Variables                                                                                                             *
+*********************************************************************************************************************************/
+/** The DR6 value after writing zero to the register.
+ * Set by VMXR0GlobalInit(). */
+static uint64_t g_fDr6Zeroed = 0;
 
 
@@ -3044 +3055 @@
 # endif
 #endif
+
+    /*
+     * For detecting whether DR6.RTM is writable or not (done in VMXR0InitVM).
+     */
+    RTTHREADPREEMPTSTATE Preempt = RTTHREADPREEMPTSTATE_INITIALIZER;
+    RTThreadPreemptDisable(&Preempt);
+    RTCCUINTXREG const fSavedDr6 = ASMGetDR6();
+    ASMSetDR6(0);
+    RTCCUINTXREG const fZeroDr6  = ASMGetDR6();
+    ASMSetDR6(fSavedDr6);
+    RTThreadPreemptRestore(&Preempt);
+
+    g_fDr6Zeroed = fZeroDr6;
+
     return VINF_SUCCESS;
 }
@@ -3151 +3176 @@
     *(uint64_t *)(pVM->hmr0.s.vmx.pbScratch + 16) = UINT64_C(0xdeadbeefdeadbeef);
 #endif
+
+    /*
+     * Copy out stuff that's for ring-3 and determin default configuration.
+     */
+    pVM->hm.s.ForR3.vmx.u64HostDr6Zeroed = g_fDr6Zeroed;
+
+    /* Since we do not emulate RTM, make sure DR6.RTM cannot be cleared by the
+       guest and cause confusion there.  It appears that the DR6.RTM bit can be
+       cleared even if TSX-NI is disabled (microcode update / system / whatever). */
+#ifdef VMX_WITH_MAYBE_ALWAYS_INTERCEPT_MOV_DRX
+    if (pVM->hm.s.vmx.fAlwaysInterceptMovDRxCfg == 0)
+        pVM->hmr0.s.vmx.fAlwaysInterceptMovDRx = g_fDr6Zeroed != X86_DR6_RA1_MASK;
+    else
+#endif
+        pVM->hmr0.s.vmx.fAlwaysInterceptMovDRx = pVM->hm.s.vmx.fAlwaysInterceptMovDRxCfg > 0;
+    pVM->hm.s.ForR3.vmx.fAlwaysInterceptMovDRx = pVM->hmr0.s.vmx.fAlwaysInterceptMovDRx;
+
     return VINF_SUCCESS;
 }
@@ -3802 +3844 @@
 
     bool     fSteppingDB      = false;
-    bool     fInterceptMovDRx = false;
     uint32_t uProcCtls        = pVmcsInfo->u32ProcCtls;
     if (pVCpu->hm.s.fSingleInstruction)
@@ -3821 +3862 @@
     }
 
+#ifdef VMX_WITH_MAYBE_ALWAYS_INTERCEPT_MOV_DRX
+    bool     fInterceptMovDRx = pVCpu->CTX_SUFF(pVM)->hmr0.s.vmx.fAlwaysInterceptMovDRx;
+#else
+    bool     fInterceptMovDRx = false;
+#endif
     uint64_t u64GuestDr7;
     if (   fSteppingDB
@@ -3860 +3906 @@
                 STAM_COUNTER_INC(&pVCpu->hm.s.StatDRxArmed);
             }
+#ifndef VMX_WITH_MAYBE_ALWAYS_INTERCEPT_MOV_DRX
             Assert(!fInterceptMovDRx);
+#endif
         }
         else if (!CPUMIsGuestDebugStateActive(pVCpu))
@@ -4607 +4655 @@
 
     /* Restore host debug registers if necessary. We will resync on next R0 reentry. */
-#ifdef VBOX_STRICT
-    if (CPUMIsHyperDebugStateActive(pVCpu))
-        Assert(pVmcsInfo->u32ProcCtls & VMX_PROC_CTLS_MOV_DR_EXIT);
+#ifdef VMX_WITH_MAYBE_ALWAYS_INTERCEPT_MOV_DRX
+    Assert(   (pVmcsInfo->u32ProcCtls & VMX_PROC_CTLS_MOV_DR_EXIT)
+           || (!CPUMIsHyperDebugStateActive(pVCpu) && !pVCpu->CTX_SUFF(pVM)->hmr0.s.vmx.fAlwaysInterceptMovDRx));
+#else
+    Assert((pVmcsInfo->u32ProcCtls & VMX_PROC_CTLS_MOV_DR_EXIT) || !CPUMIsHyperDebugStateActive(pVCpu));
 #endif
     CPUMR0DebugStateMaybeSaveGuestAndRestoreHost(pVCpu, true /* save DR6 */);
@@ -5497 +5547 @@
                                | (pVmcsInfoGst->u32ProcCtls & ~(  VMX_PROC_CTLS_INT_WINDOW_EXIT
                                                                 | VMX_PROC_CTLS_NMI_WINDOW_EXIT
-                                                                | VMX_PROC_CTLS_MOV_DR_EXIT
+                                                                | VMX_PROC_CTLS_MOV_DR_EXIT /* hmR0VmxExportSharedDebugState makes
+                                                                                               sure guest DRx regs are loaded. */
                                                                 | VMX_PROC_CTLS_USE_TPR_SHADOW
                                                                 | VMX_PROC_CTLS_MONITOR_TRAP_FLAG));

trunk/src/VBox/VMM/VMMR3/HM.cpp

@@ -281 +281 @@
                               "|SvmVGif"
                               "|LovelyMesaDrvWorkaround"
-                              "|MissingOS2TlbFlushWorkaround",
-                              "" /* pszValidNodes */, "HM" /* pszWho */, 0 /* uInstance */);
+                              "|MissingOS2TlbFlushWorkaround"
+                              "|AlwaysInterceptVmxMovDRx"
+                              , "" /* pszValidNodes */, "HM" /* pszWho */, 0 /* uInstance */);
     if (RT_FAILURE(rc))
         return rc;
@@ -526 +527 @@
      * (TESTCFG.SYS typically crashes).  See ticketref:20625 for details. */
     rc = CFGMR3QueryBoolDef(pCfgHm, "MissingOS2TlbFlushWorkaround", &pVM->hm.s.fMissingOS2TlbFlushWorkaround, false);
+    AssertLogRelRCReturn(rc, rc);
+
+    /** @cfgm{/HM/AlwaysInterceptVmxMovDRx,int8_t,-1}
+     * Whether to always intercept MOV DRx when using VMX.
+     * The value is a tristate: 1 for always intercepting, -1 for lazy intercept,
+     * and 0 for default.  The default means that it's always intercepted when the
+     * host DR6 contains bits not known to the guest.
+     *
+     * With the introduction of transactional synchronization extensions new
+     * instructions, aka TSX-NI or RTM, bit 16 in DR6 is cleared to indicate that a
+     * \#DB was related to a transaction.  The bit is also cleared when writing zero
+     * to it, so guest lazily resetting DR6 by writing 0 to it, ends up with an
+     * unexpected value.  Similiarly, bit 11 in DR7 is used to enabled RTM
+     * debugging support and therefore writable by the guest.
+     *
+     * Out of caution/paranoia, we will by default intercept DRx moves when setting
+     * DR6 to zero (on the host) doesn't result in 0xffff0ff0 (X86_DR6_RA1_MASK).
+     * Note that it seems DR6.RTM remains writable even after the microcode updates
+     * disabling TSX. */
+    rc = CFGMR3QueryS8Def(pCfgHm, "AlwaysInterceptVmxMovDRx", &pVM->hm.s.vmx.fAlwaysInterceptMovDRxCfg, -1);
     AssertLogRelRCReturn(rc, rc);
 
@@ -1602 +1623 @@
     LogRel(("HM: Host EFER                         = %#RX64\n", pVM->hm.s.ForR3.vmx.u64HostMsrEfer));
     LogRel(("HM: MSR_IA32_SMM_MONITOR_CTL          = %#RX64\n", pVM->hm.s.ForR3.vmx.u64HostSmmMonitorCtl));
+    LogRel(("HM: Host DR6 zero'ed                  = %#RX64%s\n", pVM->hm.s.ForR3.vmx.u64HostDr6Zeroed,
+            pVM->hm.s.ForR3.vmx.fAlwaysInterceptMovDRx ? " - always intercept MOV DRx" : ""));
 
     hmR3VmxReportFeatCtlMsr(pVM->hm.s.ForR3.vmx.u64HostFeatCtrl);

trunk/src/VBox/VMM/include/HMInternal.h

@@ -276 +276 @@
         /** The shift mask employed by the VMX-Preemption timer (set by ring-0). */
         uint8_t                     cPreemptTimerShift;
-        bool                        fAlignment1;
 
         /** @name Configuration (gets copied if problematic)
@@ -290 +289 @@
          * quietly clears this if the hardware doesn't support the preemption timer. */
         bool                        fUsePreemptTimerCfg;
+        /** Whether to always intercept MOV DRx: 1 (always), 0 (default), -1 (lazy).
+         * In the default case it is only always intercepted when setting DR6 to 0 on
+         * the host results in a value different from X86_DR6_RA1_MASK. */
+        int8_t                      fAlwaysInterceptMovDRxCfg;
         /** @} */
 
@@ -365 +368 @@
             /** Whether to use VMCS shadowing. */
             bool                        fUseVmcsShadowing;
-            bool                        fAlignment2;
+            /** Whether MOV DRx is always intercepted or not (set by ring-0 VMX init, for
+             * logging). */
+            bool                        fAlwaysInterceptMovDRx;
 
             /** Host CR4 value (set by ring-0 VMX init, for logging). */
@@ -375 +380 @@
             /** Host IA32_FEATURE_CONTROL MSR (set by ring-0 VMX init, for logging). */
             uint64_t                    u64HostFeatCtrl;
+            /** Host zero'ed DR6 value (set by ring-0 VMX init, for logging). */
+            uint64_t                    u64HostDr6Zeroed;
 
             /** The first valid host LBR branch-from-IP stack range. */
@@ -512 +519 @@
         /** Set if Last Branch Record (LBR) is enabled. */
         bool                        fLbr;
-        bool                        afAlignment2[3];
+        /** Set always intercept MOV DRx. */
+        bool                        fAlwaysInterceptMovDRx;
+        bool                        afAlignment2[2];
 
         /** Set if VPID is supported (copy in HM::vmx::fVpidForRing3). */