Changeset 72881 in vbox

Timestamp:

Jul 4, 2018 3:19:39 PM (7 years ago)

Author:

vboxsync

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

123426

Message:

EM,HM,IEM: Replaced EMInterpretRdmsr with IEMExecDecodedRdmsr.

Location:

trunk/src/VBox/VMM/VMMR0

Files:

• HMSVMR0.cpp (modified) (6 diffs)
• HMVMXR0.cpp (modified) (2 diffs)

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

@@ -6672 +6672 @@
  * \#VMEXIT helper for read MSRs, see hmR0SvmExitMsr.
  *
- * @returns VBox status code.
+ * @returns Strict VBox status code.
  * @param   pVCpu       The cross context virtual CPU structure.
  * @param   pVmcb       Pointer to the VM control block.
  */
-static int hmR0SvmExitReadMsr(PVMCPU pVCpu, PSVMVMCB pVmcb)
+static VBOXSTRICTRC hmR0SvmExitReadMsr(PVMCPU pVCpu, PSVMVMCB pVmcb)
 {
     PCPUMCTX pCtx  = &pVCpu->cpum.GstCtx;
-    HMSVM_CPUMCTX_IMPORT_STATE(pVCpu,   CPUMCTX_EXTRN_CR0
-                                      | CPUMCTX_EXTRN_RFLAGS
-                                      | CPUMCTX_EXTRN_SS
-                                      | CPUMCTX_EXTRN_ALL_MSRS);
-
     STAM_COUNTER_INC(&pVCpu->hm.s.StatExitRdmsr);
     Log4Func(("idMsr=%#RX32\n", pCtx->ecx));
 
+    VBOXSTRICTRC rcStrict;
     bool const fSupportsNextRipSave = hmR0SvmSupportsNextRipSave(pVCpu, pCtx);
     if (fSupportsNextRipSave)
     {
-        int rc = EMInterpretRdmsr(pVCpu->CTX_SUFF(pVM), pVCpu, CPUMCTX2CORE(pCtx));
-        if (RT_LIKELY(rc == VINF_SUCCESS))
-        {
-            pCtx->rip = pVmcb->ctrl.u64NextRIP;
-            HMSVM_CHECK_SINGLE_STEP(pVCpu, rc);
-            return VINF_SUCCESS;
-        }
-
-        AssertMsg(   rc == VERR_EM_INTERPRETER
-                  || rc == VINF_CPUM_R3_MSR_READ, ("EMInterpretRdmsr failed rc=%Rrc\n", rc));
-        return rc;
-    }
-
-    HMSVM_CPUMCTX_IMPORT_STATE(pVCpu, HMSVM_CPUMCTX_EXTRN_ALL);
-    int rc = VBOXSTRICTRC_TODO(EMInterpretInstruction(pVCpu, CPUMCTX2CORE(pCtx), 0));
-    if (RT_UNLIKELY(rc != VINF_SUCCESS))
-    {
-        AssertMsg(   rc == VERR_EM_INTERPRETER
-                  || rc == VINF_CPUM_R3_MSR_READ, ("EMInterpretInstruction failed rc=%Rrc\n", rc));
-    }
-    /* RIP updated by EMInterpretInstruction(). */
-    HMSVM_CHECK_SINGLE_STEP(pVCpu, rc);
-    return rc;
+        /** @todo Optimize this: Only retrieve the MSR bits we need here. CPUMAllMsrs.cpp
+         *  can ask for what it needs instead of using CPUMCTX_EXTRN_ALL_MSRS. */
+        HMSVM_CPUMCTX_IMPORT_STATE(pVCpu, IEM_CPUMCTX_EXTRN_EXEC_DECODED_NO_MEM_MASK | CPUMCTX_EXTRN_ALL_MSRS);
+        rcStrict = IEMExecDecodedRdmsr(pVCpu, pVmcb->ctrl.u64NextRIP - pCtx->rip);
+        if (RT_LIKELY(rcStrict == VINF_SUCCESS))
+            HMSVM_CHECK_SINGLE_STEP(pVCpu, rcStrict);     /* RIP updated by IEMExecDecodedRdmsr(). */
+        else
+            AssertMsg(   rcStrict == VINF_IEM_RAISED_XCPT
+                      || rcStrict == VINF_CPUM_R3_MSR_WRITE,
+                      ("Unexpected IEMExecDecodedWrmsr status: %Rrc\n", VBOXSTRICTRC_VAL(rcStrict)));
+    }
+    else
+    {
+        HMSVM_CPUMCTX_IMPORT_STATE(pVCpu, IEM_CPUMCTX_EXTRN_MUST_MASK | CPUMCTX_EXTRN_ALL_MSRS);
+        rcStrict = IEMExecOne(pVCpu);
+        if (RT_LIKELY(rcStrict == VINF_SUCCESS))
+            HMSVM_CHECK_SINGLE_STEP(pVCpu, rcStrict);     /* RIP updated by IEMExecOne(). */
+        else
+            AssertMsg(   rcStrict == VINF_IEM_RAISED_XCPT
+                      || rcStrict == VINF_CPUM_R3_MSR_READ, ("Unexpected IEMExecOne status: %Rrc\n", VBOXSTRICTRC_VAL(rcStrict)));
+    }
+    return rcStrict;
 }
 
@@ -6719 +6714 @@
  * \#VMEXIT helper for write MSRs, see hmR0SvmExitMsr.
  *
- * @returns VBox status code.
+ * @returns Strict VBox status code.
  * @param   pVCpu           The cross context virtual CPU structure.
  * @param   pVmcb       Pointer to the VM control block.
  * @param   pSvmTransient   Pointer to the SVM-transient structure.
  */
-static int hmR0SvmExitWriteMsr(PVMCPU pVCpu, PSVMVMCB pVmcb, PSVMTRANSIENT pSvmTransient)
+static VBOXSTRICTRC  hmR0SvmExitWriteMsr(PVMCPU pVCpu, PSVMVMCB pVmcb, PSVMTRANSIENT pSvmTransient)
 {
     PCPUMCTX pCtx  = &pVCpu->cpum.GstCtx;
     uint32_t const idMsr = pCtx->ecx;
-    /** @todo Optimize this: We don't need to get much of the MSR state here
-     * since we're only updating.  CPUMAllMsrs.cpp can ask for what it needs and
-     * clear the applicable extern flags. */
-    HMSVM_CPUMCTX_IMPORT_STATE(pVCpu,   CPUMCTX_EXTRN_CR0
-                                      | CPUMCTX_EXTRN_RFLAGS
-                                      | CPUMCTX_EXTRN_SS
-                                      | CPUMCTX_EXTRN_ALL_MSRS
-                                      | IEM_CPUMCTX_EXTRN_EXEC_DECODED_NO_MEM_MASK);
-
     STAM_COUNTER_INC(&pVCpu->hm.s.StatExitWrmsr);
     Log4Func(("idMsr=%#RX32\n", idMsr));
@@ -6768 +6754 @@
     if (fSupportsNextRipSave)
     {
+        /** @todo Optimize this: We don't need to get much of the MSR state here
+         * since we're only updating.  CPUMAllMsrs.cpp can ask for what it needs and
+         * clear the applicable extern flags. */
+        HMSVM_CPUMCTX_IMPORT_STATE(pVCpu, IEM_CPUMCTX_EXTRN_EXEC_DECODED_NO_MEM_MASK | CPUMCTX_EXTRN_ALL_MSRS);
         rcStrict = IEMExecDecodedWrmsr(pVCpu, pVmcb->ctrl.u64NextRIP - pCtx->rip);
         if (RT_LIKELY(rcStrict == VINF_SUCCESS))
-            HMSVM_CHECK_SINGLE_STEP(pVCpu, rcStrict);
+            HMSVM_CHECK_SINGLE_STEP(pVCpu, rcStrict);     /* RIP updated by IEMExecDecodedWrmsr(). */
         else
             AssertMsg(   rcStrict == VINF_IEM_RAISED_XCPT
@@ -6778 +6768 @@
     else
     {
-        HMSVM_CPUMCTX_IMPORT_STATE(pVCpu, IEM_CPUMCTX_EXTRN_MUST_MASK);
+        HMSVM_CPUMCTX_IMPORT_STATE(pVCpu, IEM_CPUMCTX_EXTRN_MUST_MASK | CPUMCTX_EXTRN_ALL_MSRS);
         rcStrict = IEMExecOne(pVCpu);
         if (RT_LIKELY(rcStrict == VINF_SUCCESS))
-            HMSVM_CHECK_SINGLE_STEP(pVCpu, rcStrict);     /* RIP updated by EMInterpretInstruction(). */
+            HMSVM_CHECK_SINGLE_STEP(pVCpu, rcStrict);     /* RIP updated by IEMExecOne(). */
         else
             AssertMsg(   rcStrict == VINF_IEM_RAISED_XCPT
@@ -6815 +6805 @@
     }
 
-    return VBOXSTRICTRC_TODO(rcStrict);
+    return rcStrict;
 }
 
@@ -6829 +6819 @@
     PSVMVMCB pVmcb = hmR0SvmGetCurrentVmcb(pVCpu, pCtx);
     if (pVmcb->ctrl.u64ExitInfo1 == SVM_EXIT1_MSR_READ)
-        return hmR0SvmExitReadMsr(pVCpu, pVmcb);
+        return VBOXSTRICTRC_TODO(hmR0SvmExitReadMsr(pVCpu, pVmcb));
 
     Assert(pVmcb->ctrl.u64ExitInfo1 == SVM_EXIT1_MSR_WRITE);
-    return hmR0SvmExitWriteMsr(pVCpu, pVmcb, pSvmTransient);
+    return VBOXSTRICTRC_TODO(hmR0SvmExitWriteMsr(pVCpu, pVmcb, pSvmTransient));
 }
 

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

@@ -11839 +11839 @@
     HMVMX_VALIDATE_EXIT_HANDLER_PARAMS();
 
-    /* EMInterpretRdmsr() requires CR0, Eflags and SS segment register. FS, GS (base) can be accessed by MSR reads. */
-    int rc = hmR0VmxImportGuestState(pVCpu,   CPUMCTX_EXTRN_CR0
-                                            | CPUMCTX_EXTRN_RFLAGS
-                                            | CPUMCTX_EXTRN_SS
-                                            | CPUMCTX_EXTRN_FS
-                                            | CPUMCTX_EXTRN_GS);
-    if (!(pVCpu->hm.s.vmx.u32ProcCtls & VMX_VMCS_CTRL_PROC_EXEC_USE_MSR_BITMAPS))
-        rc |= hmR0VmxImportGuestState(pVCpu, CPUMCTX_EXTRN_ALL_MSRS);
+    /** @todo Optimize this: We currently drag in in the whole MSR state
+     * (CPUMCTX_EXTRN_ALL_MSRS) here.  We should optimize this to only get
+     * MSRs required.  That would require changes to IEM and possibly CPUM too.
+     * (Should probably do it lazy fashion from CPUMAllMsrs.cpp). */
+    uint32_t const idMsr = pMixedCtx->ecx;  NOREF(idMsr); /* Save it. */
+    int rc = hmR0VmxReadExitInstrLenVmcs(pVmxTransient);
+    rc    |= hmR0VmxImportGuestState(pVCpu, IEM_CPUMCTX_EXTRN_EXEC_DECODED_NO_MEM_MASK | CPUMCTX_EXTRN_ALL_MSRS);
     AssertRCReturn(rc, rc);
-    Log4Func(("ecx=%#RX32\n", pMixedCtx->ecx));
+
+    Log4Func(("ecx=%#RX32\n", idMsr));
 
 #ifdef VBOX_STRICT
     if (pVCpu->hm.s.vmx.u32ProcCtls & VMX_VMCS_CTRL_PROC_EXEC_USE_MSR_BITMAPS)
     {
-        if (   hmR0VmxIsAutoLoadStoreGuestMsr(pVCpu, pMixedCtx->ecx)
-            && pMixedCtx->ecx != MSR_K6_EFER)
-        {
-            AssertMsgFailed(("Unexpected RDMSR for an MSR in the auto-load/store area in the VMCS. ecx=%#RX32\n",
-                             pMixedCtx->ecx));
+        if (   hmR0VmxIsAutoLoadStoreGuestMsr(pVCpu, idMsr)
+            && idMsr != MSR_K6_EFER)
+        {
+            AssertMsgFailed(("Unexpected RDMSR for an MSR in the auto-load/store area in the VMCS. ecx=%#RX32\n", idMsr));
             HMVMX_UNEXPECTED_EXIT_RET(pVCpu, pVmxTransient);
         }
-        if (hmR0VmxIsLazyGuestMsr(pVCpu, pMixedCtx->ecx))
+        if (hmR0VmxIsLazyGuestMsr(pVCpu, idMsr))
         {
             VMXMSREXITREAD  enmRead;
             VMXMSREXITWRITE enmWrite;
-            int rc2 = hmR0VmxGetMsrPermission(pVCpu, pMixedCtx->ecx, &enmRead, &enmWrite);
+            int rc2 = hmR0VmxGetMsrPermission(pVCpu, idMsr, &enmRead, &enmWrite);
             AssertRCReturn(rc2, rc2);
             if (enmRead == VMXMSREXIT_PASSTHRU_READ)
             {
-                AssertMsgFailed(("Unexpected RDMSR for a passthru lazy-restore MSR. ecx=%#RX32\n", pMixedCtx->ecx));
+                AssertMsgFailed(("Unexpected RDMSR for a passthru lazy-restore MSR. ecx=%#RX32\n", idMsr));
                 HMVMX_UNEXPECTED_EXIT_RET(pVCpu, pVmxTransient);
             }
@@ -11875 +11874 @@
 #endif
 
-    PVM pVM = pVCpu->CTX_SUFF(pVM);
-    rc = EMInterpretRdmsr(pVM, pVCpu, CPUMCTX2CORE(pMixedCtx));
-    AssertMsg(rc == VINF_SUCCESS || rc == VERR_EM_INTERPRETER,
-              ("hmR0VmxExitRdmsr: failed, invalid error code %Rrc\n", rc));
+    VBOXSTRICTRC rcStrict = IEMExecDecodedRdmsr(pVCpu, pVmxTransient->cbInstr);
     STAM_COUNTER_INC(&pVCpu->hm.s.StatExitRdmsr);
-    if (RT_SUCCESS(rc))
-    {
-        rc = hmR0VmxAdvanceGuestRip(pVCpu, pMixedCtx, pVmxTransient);
-        Assert(pVmxTransient->cbInstr == 2);
-    }
-    return rc;
+    AssertMsg(   rcStrict == VINF_SUCCESS
+              || rcStrict == VINF_CPUM_R3_MSR_READ
+              || rcStrict == VINF_IEM_RAISED_XCPT,
+              ("Unexpected IEMExecDecodedRdmsr status: %Rrc\n", VBOXSTRICTRC_VAL(rcStrict)));
+
+    return rcStrict;
 }