Changeset 60646 in vbox

Timestamp:

Apr 22, 2016 12:59:40 PM (9 years ago)

Author:

vboxsync

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

106813

Message:

VMM/APIC: Go back to ring-3 for all base MSR writes for now. Logging and a couple of Windows build fixes with size_t and '==' instead of '=' (thanks Michal).

Location:

trunk

Files:

• include/VBox/vmm/apic.h (modified) (1 diff)
• src/VBox/VMM/VMMAll/APICAll.cpp (modified) (12 diffs)
• src/VBox/VMM/VMMR3/APIC.cpp (modified) (8 diffs)
• src/VBox/VMM/include/APICInternal.h (modified) (1 diff)

trunk/include/VBox/vmm/apic.h

@@ -926 +926 @@
  */
 VMMR3_INT_DECL(void)        APICR3InitIpi(PVMCPU pVCpu);
-VMMR3_INT_DECL(void)        APICR3Reset(PVMCPU pVCpu);
+VMMR3_INT_DECL(void)        APICR3Reset(PVMCPU pVCpu, bool fResetApicBaseMsr);
 /** @} */
 #endif /* IN_RING3 */

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

@@ -373 +373 @@
  * @param   uApicBaseMsr        The APIC Base MSR value.
  */
-static APICMODE apicGetMode(uint64_t uApicBaseMsr)
+APICMODE apicGetMode(uint64_t uApicBaseMsr)
 {
     uint32_t const uMode   = MSR_APICBASE_GET_MODE(uApicBaseMsr);
@@ -1970 +1970 @@
 {
     Assert(pVCpu);
+    NOREF(pDevIns);
+
+#ifdef IN_RING3
     PAPICCPU pApicCpu   = VMCPU_TO_APICCPU(pVCpu);
     PAPIC    pApic      = VM_TO_APIC(pVCpu->CTX_SUFF(pVM));
@@ -1976 +1979 @@
     uint64_t uBaseMsr   = pApicCpu->uApicBaseMsr;
 
-    /** @todo probably go back to ring-3 for all cases regardless of
-     *        fRZEnabled. Writing this MSR is not something guests
-     *        typically do often, and therefore is not performance
-     *        critical. We'll have better diagnostics in ring-3. */
-    if (!pApic->fRZEnabled)
-        return VINF_CPUM_R3_MSR_WRITE;
+    Log4(("APIC%u: ApicSetBaseMsr: u64BaseMsr=%#RX64 enmNewMode=%s enmOldMode=%s\n", pVCpu->idCpu, u64BaseMsr,
+          apicGetModeName(enmNewMode), apicGetModeName(enmOldMode)));
 
     /*
@@ -1993 +1992 @@
     if (MSR_APICBASE_GET_PHYSADDR(uBaseMsr) != XAPIC_APICBASE_PHYSADDR)
     {
-#ifdef IN_RING3
         LogRelMax(5, ("APIC%u: Attempt to relocate base to %#RGp, unsupported -> #GP(0)\n", pVCpu->idCpu,
                       MSR_APICBASE_GET_PHYSADDR(uBaseMsr)));
         return VERR_CPUM_RAISE_GP_0;
-#else
-        return VINF_CPUM_R3_MSR_WRITE;
-#endif
     }
 
@@ -2014 +2009 @@
             case APICMODE_DISABLED:
             {
-#ifdef IN_RING3
                 /*
                  * The APIC state needs to be reset (especially the APIC ID as x2APIC APIC ID bit layout
@@ -2021 +2015 @@
                  * See Intel spec. 10.4.3 "Enabling or Disabling the Local APIC".
                  */
-                APICR3Reset(pVCpu);
+                APICR3Reset(pVCpu, false /* fResetApicBaseMsr */);
                 uBaseMsr &= ~(MSR_APICBASE_XAPIC_ENABLE_BIT | MSR_APICBASE_X2APIC_ENABLE_BIT);
                 Log4(("APIC%u: Switched mode to disabled\n", pVCpu->idCpu));
-#else
-                return VINF_CPUM_R3_MSR_WRITE;
-#endif
                 break;
             }
@@ -2044 +2035 @@
             case APICMODE_X2APIC:
             {
-                uBaseMsr |= MSR_APICBASE_X2APIC_ENABLE_BIT;
+                uBaseMsr |= MSR_APICBASE_XAPIC_ENABLE_BIT | MSR_APICBASE_X2APIC_ENABLE_BIT;
 
                 /*
@@ -2078 +2069 @@
     ASMAtomicWriteU64(&pApicCpu->uApicBaseMsr, uBaseMsr);
     return VINF_SUCCESS;
+#else  /* !IN_RING3 */
+    return VINF_CPUM_R3_MSR_WRITE;
+#endif /* IN_RING3 */
 }
 
@@ -2195 +2189 @@
             switch (enmDeliveryMode)
             {
+                case XAPICDELIVERYMODE_INIT:
+                {
+                    /** @todo won't work in R0/RC because callers don't care about rcRZ. */
+                    AssertMsgFailed(("INIT through LINT0/LINT1 is not yet supported\n"));
+                    /* fallthru */
+                }
                 case XAPICDELIVERYMODE_FIXED:
                 {
@@ -2208 +2208 @@
                 case XAPICDELIVERYMODE_SMI:
                 case XAPICDELIVERYMODE_NMI:
-                case XAPICDELIVERYMODE_INIT:    /** @todo won't work in R0/RC because callers don't care about rcRZ. */
                 {
                     VMCPUSET DestCpuSet;
@@ -2221 +2220 @@
                 case XAPICDELIVERYMODE_EXTINT:
                 {
-                    Log4(("APIC%u: APICLocalInterrupt: External interrupt. u8Pin=%u u8Level=%u\n", pVCpu->idCpu, u8Pin, u8Level));
+                    Log4(("APIC%u: APICLocalInterrupt: %s ExtINT through LINT%u\n", pVCpu->idCpu,
+                          u8Level ? "Raising" : "Lowering", u8Pin));
                     if (u8Level)
                         APICSetInterruptFF(pVCpu, PDMAPICIRQ_EXTINT);
@@ -2244 +2244 @@
     else
     {
-        /* The APIC is disabled, pass it through the CPU. */
-        LogFlow(("APIC%u: APICLocalInterrupt: APIC hardware-disabled, passing interrupt to CPU. u8Pin=%u u8Level=%u\n",
-                 pVCpu->idCpu, u8Pin, u8Level));
-        if (u8Level)
-            APICSetInterruptFF(pVCpu, PDMAPICIRQ_EXTINT);
+        /* The APIC is hardware disabled. The CPU behaves as though there is no on-chip APIC. */
+        if (u8Pin == 0)
+        {
+            /* LINT0 behaves as an external interrupt pin. */
+            Log4(("APIC%u: APICLocalInterrupt: APIC hardware-disabled, %s ExtINT through LINT0\n", pVCpu->idCpu,
+                  u8Level ? "raising" : "lowering"));
+            if (u8Level)
+                APICSetInterruptFF(pVCpu, PDMAPICIRQ_EXTINT);
+            else
+                APICClearInterruptFF(pVCpu, PDMAPICIRQ_EXTINT);
+        }
         else
-            APICClearInterruptFF(pVCpu, PDMAPICIRQ_EXTINT);
+        {
+            /* LINT1 behaves as NMI. */
+            Log4(("APIC%u: APICLocalInterrupt: APIC hardware-disabled, raising NMI through LINT1\n", pVCpu->idCpu));
+            APICSetInterruptFF(pVCpu, PDMAPICIRQ_NMI);
+        }
     }
 

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

@@ -253 +253 @@
  * reset.
  *
- * @param   pVCpu           The cross context virtual CPU structure.
- */
-VMMR3_INT_DECL(void) APICR3Reset(PVMCPU pVCpu)
+ * @param   pVCpu               The cross context virtual CPU structure.
+ * @param   fResetApicBaseMsr   Whether to reset the APIC base MSR.
+ */
+VMMR3_INT_DECL(void) APICR3Reset(PVMCPU pVCpu, bool fResetApicBaseMsr)
 {
     VMCPU_ASSERT_EMT_OR_NOT_RUNNING(pVCpu);
@@ -291 +292 @@
     /** @todo It isn't clear in the spec. where exactly the default base address
      *        is (re)initialized, atm we do it here in Reset. */
-    apicR3ResetBaseMsr(pVCpu);
+    if (fResetApicBaseMsr)
+        apicR3ResetBaseMsr(pVCpu);
 
     /*
@@ -373 +375 @@
     PCXAPICPAGE  pXApicPage  = VMCPU_TO_CXAPICPAGE(pVCpu);
     PCX2APICPAGE pX2ApicPage = VMCPU_TO_CX2APICPAGE(pVCpu);
+
+    uint64_t const uBaseMsr  = pApicCpu->uApicBaseMsr;
+    APICMODE const enmMode   = apicGetMode(uBaseMsr);
     bool const   fX2ApicMode = XAPIC_IN_X2APIC_MODE(pVCpu);
 
-    pHlp->pfnPrintf(pHlp, "VCPU[%u] APIC at %#RGp\n", pVCpu->idCpu, MSR_APICBASE_GET_PHYSADDR(pApicCpu->uApicBaseMsr));
-    pHlp->pfnPrintf(pHlp, "  Mode                          = %s\n", fX2ApicMode ? "x2Apic" : "xApic");
+    pHlp->pfnPrintf(pHlp, "VCPU[%u] APIC:\n", pVCpu->idCpu);
+    pHlp->pfnPrintf(pHlp, "  APIC Base MSR                 = %#RX64 (Addr=%#RX64)\n", uBaseMsr,
+                    MSR_APICBASE_GET_PHYSADDR(uBaseMsr));
+    pHlp->pfnPrintf(pHlp, "  Mode                          = %#x (%s)\n", enmMode, apicGetModeName(enmMode));
     if (fX2ApicMode)
     {
@@ -386 +393 @@
     pHlp->pfnPrintf(pHlp, "  Version                       = %#x\n",      pXApicPage->version.all.u32Version);
     pHlp->pfnPrintf(pHlp, "    APIC Version                = %#x\n",      pXApicPage->version.u.u8Version);
-    pHlp->pfnPrintf(pHlp, "    Max LVT entries             = %u\n",       pXApicPage->version.u.u8MaxLvtEntry);
+    pHlp->pfnPrintf(pHlp, "    Max LVT entry index (0..N)  = %u\n",       pXApicPage->version.u.u8MaxLvtEntry);
     pHlp->pfnPrintf(pHlp, "    EOI Broadcast supression    = %RTbool\n",  pXApicPage->version.u.fEoiBroadcastSupression);
     if (!fX2ApicMode)
@@ -929 +936 @@
             TMTimerStop(pApicCpu->pTimerR3);
 
-        APICR3Reset(pVCpuDest);
+        APICR3Reset(pVCpuDest, true /* fResetApicBaseMsr */);
 
         /* Clear the interrupt pending force flag. */
@@ -1070 +1077 @@
         if (fNeedsGCMapping)
         {
-            pApic->pvApicPibRC == NIL_RTRCPTR;
+            pApic->pvApicPibRC = NIL_RTRCPTR;
             int rc = MMR3HyperMapHCPhys(pVM, (void *)pApic->pvApicPibR3, NIL_RTR0PTR, pApic->HCPhysApicPib, pApic->cbApicPib,
                                         "APIC PIB", (PRTGCPTR)&pApic->pvApicPibRC);
@@ -1127 +1134 @@
 
                 /* Associate the per-VCPU PIB pointers to the per-VM PIB mapping. */
-                size_t const offApicPib    = idCpu * sizeof(APICPIB);
+                uint32_t const offApicPib  = idCpu * sizeof(APICPIB);
                 pApicCpu->pvApicPibR0      = (RTR0PTR)((RTR0UINTPTR)pApic->pvApicPibR0 + offApicPib);
                 pApicCpu->pvApicPibR3      = (RTR3PTR)((RTR3UINTPTR)pApic->pvApicPibR3 + offApicPib);
@@ -1135 +1142 @@
                 /* Initialize the virtual-APIC state. */
                 memset((void *)pApicCpu->pvApicPageR3, 0, pApicCpu->cbApicPage);
-                APICR3Reset(pVCpu);
+                APICR3Reset(pVCpu, true /* fResetApicBaseMsr */);
 
 #ifdef DEBUG_ramshankar

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

@@ -629 +629 @@
 const char             *apicGetTimerModeName(XAPICTIMERMODE enmTimerMode);
 void                    apicHintTimerFreq(PAPICCPU pApicCpu, uint32_t uInitialCount, uint8_t uTimerShift);
+APICMODE                apicGetMode(uint64_t uApicBaseMsr);
 
 VMMDECL(uint64_t)       APICGetBaseMsr(PPDMDEVINS pDevIns, PVMCPU pVCpu);