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

Timestamp:

Jun 10, 2016 10:14:16 AM (9 years ago)

Author:

vboxsync

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

107998

Message:

VMM/HM: Better handling of edge-cases during exceptions caused as a result of event injection.

Location:

trunk/src/VBox/VMM

Files:

• VMMR0/HMSVMR0.cpp (modified) (11 diffs)
• VMMR0/HMVMXR0.cpp (modified) (9 diffs)
• VMMR3/HM.cpp (modified) (1 diff)
• include/HMInternal.h (modified) (1 diff)

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

@@ -2269 +2269 @@
     /* If we're emulating an instruction, we shouldn't have any TRPM traps pending
        and if we're injecting an event we should have a TRPM trap pending. */
-    AssertMsg(rcExit != VINF_EM_RAW_INJECT_TRPM_EVENT || TRPMHasTrap(pVCpu), ("rcExit=%Rrc\n", rcExit));
-    AssertMsg(rcExit != VINF_EM_RAW_EMULATE_INSTR || !TRPMHasTrap(pVCpu), ("rcExit=%Rrc\n", rcExit));
+    AssertMsg(rcExit != VINF_EM_RAW_INJECT_TRPM_EVENT ||  TRPMHasTrap(pVCpu), ("rcExit=%Rrc\n", rcExit));
+    AssertMsg(rcExit != VINF_EM_RAW_EMULATE_INSTR     || !TRPMHasTrap(pVCpu), ("rcExit=%Rrc\n", rcExit));
 
     /* Sync. the necessary state for going back to ring-3. */
@@ -2366 +2366 @@
     Log4(("hmR0SvmSetPendingEvent: u=%#RX64 u8Vector=%#x Type=%#x ErrorCodeValid=%RTbool ErrorCode=%#RX32\n", pEvent->u,
           pEvent->n.u8Vector, (uint8_t)pEvent->n.u3Type, !!pEvent->n.u1ErrorCodeValid, pEvent->n.u32ErrorCode));
-
-    STAM_COUNTER_INC(&pVCpu->hm.s.StatInjectPendingReflect);
 }
 
@@ -2461 +2459 @@
 
     hmR0SvmSetPendingEvent(pVCpu, &Event, GCPtrFaultAddress);
-    STAM_COUNTER_DEC(&pVCpu->hm.s.StatInjectPendingReflect);
 }
 
@@ -4198 +4195 @@
 
                 Assert(pVmcb->ctrl.ExitIntInfo.n.u3Type != SVM_EVENT_SOFTWARE_INT);
+                STAM_COUNTER_INC(&pVCpu->hm.s.StatInjectPendingReflect);
                 hmR0SvmSetPendingEvent(pVCpu, &pVmcb->ctrl.ExitIntInfo, 0 /* GCPtrFaultAddress */);
 
@@ -4208 +4206 @@
             case SVMREFLECTXCPT_DF:
             {
+                STAM_COUNTER_INC(&pVCpu->hm.s.StatInjectPendingReflect);
                 hmR0SvmSetPendingXcptDF(pVCpu);
                 rc = VINF_HM_DOUBLE_FAULT;
@@ -5108 +5107 @@
     if ((u32ErrCode & (X86_TRAP_PF_RSVD | X86_TRAP_PF_P)) == (X86_TRAP_PF_RSVD | X86_TRAP_PF_P))
     {
+        /* If event delivery causes an MMIO #NPF, go back to instruction emulation as
+           otherwise injecting the original pending event would most likely cause the same MMIO #NPF. */
+        if (RT_UNLIKELY(pVCpu->hm.s.Event.fPending))
+            return VERR_EM_INTERPRETER;
+
         VBOXSTRICTRC rc2 = PGMR0Trap0eHandlerNPMisconfig(pVM, pVCpu, enmNestedPagingMode, CPUMCTX2CORE(pCtx), GCPhysFaultAddr,
                                                          u32ErrCode);
@@ -5189 +5193 @@
 
     /* Check if this task-switch occurred while delivering an event through the guest IDT. */
-    PSVMVMCB pVmcb = (PSVMVMCB)pVCpu->hm.s.svm.pvVmcb;
-    if (   !(pVmcb->ctrl.u64ExitInfo2 & (SVM_EXIT2_TASK_SWITCH_IRET | SVM_EXIT2_TASK_SWITCH_JMP))
-        && pVCpu->hm.s.Event.fPending)  /**  Can happen with exceptions/NMI. See @bugref{8411}.*/
+    if (pVCpu->hm.s.Event.fPending)  /* Can happen with exceptions/NMI. See @bugref{8411}. */
     {
         /*
@@ -5416 +5418 @@
     HMSVM_VALIDATE_EXIT_HANDLER_PARAMS();
 
-    HMSVM_CHECK_EXIT_DUE_TO_EVENT_DELIVERY();
+    /* Paranoia; Ensure we cannot be called as a result of event delivery. */
+    PSVMVMCB pVmcb = (PSVMVMCB)pVCpu->hm.s.svm.pvVmcb;
+    Assert(!pVmcb->ctrl.ExitIntInfo.n.u1Valid);
 
     /* We're playing with the host CPU state here, make sure we don't preempt or longjmp. */
@@ -5468 +5472 @@
     HMSVM_VALIDATE_EXIT_HANDLER_PARAMS();
 
-    HMSVM_CHECK_EXIT_DUE_TO_EVENT_DELIVERY();
+    /* Paranoia; Ensure we cannot be called as a result of event delivery. */
+    PSVMVMCB pVmcb = (PSVMVMCB)pVCpu->hm.s.svm.pvVmcb;
+    Assert(!pVmcb->ctrl.ExitIntInfo.n.u1Valid);
 
     int rc = VERR_SVM_UNEXPECTED_XCPT_EXIT;
@@ -5509 +5515 @@
     HMSVM_VALIDATE_EXIT_HANDLER_PARAMS();
 
-    HMSVM_CHECK_EXIT_DUE_TO_EVENT_DELIVERY();
+    /* Paranoia; Ensure we cannot be called as a result of event delivery. */
+    PSVMVMCB pVmcb = (PSVMVMCB)pVCpu->hm.s.svm.pvVmcb;
+    Assert(!pVmcb->ctrl.ExitIntInfo.n.u1Valid);
 
     STAM_COUNTER_INC(&pVCpu->hm.s.StatExitGuestMF);
@@ -5544 +5552 @@
     HMSVM_VALIDATE_EXIT_HANDLER_PARAMS();
 
+    /* If this #DB is the result of delivering an event, go back to the interpreter. */
     HMSVM_CHECK_EXIT_DUE_TO_EVENT_DELIVERY();
+    if (RT_UNLIKELY(pVCpu->hm.s.Event.fPending))
+    {
+        STAM_COUNTER_INC(&pVCpu->hm.s.StatInjectPendingInterpret);
+        return VERR_EM_INTERPRETER;
+    }
 
     STAM_COUNTER_INC(&pVCpu->hm.s.StatExitGuestDB);

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

@@ -5659 +5659 @@
     pVCpu->hm.s.Event.cbInstr           = cbInstr;
     pVCpu->hm.s.Event.GCPtrFaultAddress = GCPtrFaultAddress;
-
-    STAM_COUNTER_INC(&pVCpu->hm.s.StatInjectPendingReflect);
 }
 
@@ -5819 +5817 @@
 
                 /* If uExitVector is #PF, CR2 value will be updated from the VMCS if it's a guest #PF. See hmR0VmxExitXcptPF(). */
+                STAM_COUNTER_INC(&pVCpu->hm.s.StatInjectPendingReflect);
                 hmR0VmxSetPendingEvent(pVCpu, VMX_ENTRY_INT_INFO_FROM_EXIT_IDT_INFO(pVmxTransient->uIdtVectoringInfo),
                                        0 /* cbInstr */,  u32ErrCode, pMixedCtx->cr2);
@@ -5830 +5829 @@
             case VMXREFLECTXCPT_DF:
             {
+                STAM_COUNTER_INC(&pVCpu->hm.s.StatInjectPendingReflect);
                 hmR0VmxSetPendingXcptDF(pVCpu, pMixedCtx);
                 rcStrict = VINF_HM_DOUBLE_FAULT;
@@ -6913 +6913 @@
 
     hmR0VmxSetPendingEvent(pVCpu, u32IntInfo, cbInstr, uErrCode, GCPtrFaultAddress);
-    STAM_COUNTER_DEC(&pVCpu->hm.s.StatInjectPendingReflect);
 }
 
@@ -11168 +11167 @@
         case VMX_EXIT_INTERRUPTION_INFO_TYPE_HW_XCPT:
         {
+            /*
+             * If there's any exception caused as a result of event injection, go back to
+             * the interpreter. The page-fault case is complicated and we manually handle
+             * any currently pending event in hmR0VmxExitXcptPF. Nested #ACs are already
+             * handled in hmR0VmxCheckExitDueToEventDelivery.
+             */
+            if (!pVCpu->hm.s.Event.fPending)
+            { /* likely */ }
+            else if (   uVector != X86_XCPT_PF
+                     && uVector != X86_XCPT_AC)
+            {
+                STAM_COUNTER_INC(&pVCpu->hm.s.StatInjectPendingInterpret);
+                rc = VERR_EM_INTERPRETER;
+                break;
+            }
+
             switch (uVector)
             {
@@ -12520 +12535 @@
     HMVMX_VALIDATE_EXIT_HANDLER_PARAMS();
 
+    STAM_COUNTER_INC(&pVCpu->hm.s.StatExitApicAccess);
+
     /* If this VM-exit occurred while delivering an event through the guest IDT, handle it accordingly. */
     VBOXSTRICTRC rcStrict1 = hmR0VmxCheckExitDueToEventDelivery(pVCpu, pMixedCtx, pVmxTransient);
     if (RT_LIKELY(rcStrict1 == VINF_SUCCESS))
-    { /* likely */ }
+    {
+        /* For some crazy guest, if an event delivery causes an APIC-access VM-exit, go to instruction emulation. */
+        if (RT_UNLIKELY(pVCpu->hm.s.Event.fPending))
+        {
+            STAM_COUNTER_INC(&pVCpu->hm.s.StatInjectPendingInterpret);
+            return VERR_EM_INTERPRETER;
+        }
+    }
     else
     {
@@ -12586 +12610 @@
     }
 
-    STAM_COUNTER_INC(&pVCpu->hm.s.StatExitApicAccess);
     if (rcStrict2 != VINF_SUCCESS)
         STAM_COUNTER_INC(&pVCpu->hm.s.StatSwitchApicAccessToR3);
@@ -12693 +12716 @@
     VBOXSTRICTRC rcStrict1 = hmR0VmxCheckExitDueToEventDelivery(pVCpu, pMixedCtx, pVmxTransient);
     if (RT_LIKELY(rcStrict1 == VINF_SUCCESS))
-    { /* likely */ }
+    {
+        /* If event delivery causes an EPT misconfig (MMIO), go back to instruction emulation as otherwise
+           injecting the original pending event would most likely cause the same EPT misconfig VM-exit. */
+        if (RT_UNLIKELY(pVCpu->hm.s.Event.fPending))
+        {
+            STAM_COUNTER_INC(&pVCpu->hm.s.StatInjectPendingInterpret);
+            return VERR_EM_INTERPRETER;
+        }
+    }
     else
     {
@@ -12751 +12782 @@
     VBOXSTRICTRC rcStrict1 = hmR0VmxCheckExitDueToEventDelivery(pVCpu, pMixedCtx, pVmxTransient);
     if (RT_LIKELY(rcStrict1 == VINF_SUCCESS))
-    { /* likely */ }
+    {
+        /* In the unlikely case that the EPT violation happened as a result of delivering an event, log it. */
+        if (RT_UNLIKELY(pVCpu->hm.s.Event.fPending))
+            Log4(("EPT violation with an event pending u64IntInfo=%#RX64\n", pVCpu->hm.s.Event.u64IntInfo));
+    }
     else
     {

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

@@ -897 +897 @@
         HM_REG_COUNTER(&pVCpu->hm.s.StatInjectInterrupt,        "/HM/CPU%d/EventInject/Interrupt", "Injected an external interrupt into the guest.");
         HM_REG_COUNTER(&pVCpu->hm.s.StatInjectXcpt,             "/HM/CPU%d/EventInject/Trap", "Injected an exception into the guest.");
-        HM_REG_COUNTER(&pVCpu->hm.s.StatInjectPendingReflect,   "/HM/CPU%d/EventInject/PendingReflect", "Reflecting an exception back to the guest.");
+        HM_REG_COUNTER(&pVCpu->hm.s.StatInjectPendingReflect,   "/HM/CPU%d/EventInject/PendingReflect", "Reflecting an exception (or #DF) caused due to event injection.");
+        HM_REG_COUNTER(&pVCpu->hm.s.StatInjectPendingInterpret, "/HM/CPU%d/EventInject/PendingInterpret", "Falling to interpreter for handling exception caused due to event injection.");
 
         HM_REG_COUNTER(&pVCpu->hm.s.StatFlushPage,              "/HM/CPU%d/Flush/Page", "Invalidating a guest page on all guest CPUs.");

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

@@ -929 +929 @@
     STAMCOUNTER             StatInjectXcpt;
     STAMCOUNTER             StatInjectPendingReflect;
+    STAMCOUNTER             StatInjectPendingInterpret;
 
     STAMCOUNTER             StatExitAll;