Changeset 89620 in vbox for trunk/src/VBox/Devices

Timestamp:

Jun 11, 2021 8:51:10 AM (4 years ago)

Author:

vboxsync

Message:

Intel IOMMU: bugref:9967 Allow PDMIoApicSetEoi operation to be queued to ring-3 if I/O APIC isn't available in R0.
This key change is taking the PDM lock such that it doesn't fail (VINF_SUCCESS instead of VINF_IOM_R3_MMIO_WRITE for rcBusy, which wouldn't have worked anyway when called via APICHvSetEoi for instance).
Also cleaned up the prototype of PDMIoApicSendMsi a bit (use PVMCC instead of PPDMDEVINS).

File:

• trunk/src/VBox/Devices/PC/DevIoApic.cpp (modified) (4 diffs)

trunk/src/VBox/Devices/PC/DevIoApic.cpp

@@ -313 +313 @@
     /** Number of IOMMU denied or failed MSIs. */
     STAMCOUNTER             StatIommuDiscardedMsi;
-    /** Number of returns to ring-3 due to EOI broadcast lock contention. */
-    STAMCOUNTER             StatEoiContention;
     /** Number of returns to ring-3 due to Set RTE lock contention. */
     STAMCOUNTER             StatSetRteContention;
@@ -845 +843 @@
  * @interface_method_impl{PDMIOAPICREG,pfnSetEoi}
  */
-static DECLCALLBACK(VBOXSTRICTRC) ioapicSetEoi(PPDMDEVINS pDevIns, uint8_t u8Vector)
+static DECLCALLBACK(void) ioapicSetEoi(PPDMDEVINS pDevIns, uint8_t u8Vector)
 {
     PIOAPIC   pThis   = PDMDEVINS_2_DATA(pDevIns, PIOAPIC);
     PIOAPICCC pThisCC = PDMDEVINS_2_DATA_CC(pDevIns, PIOAPICCC);
+
+    LogFlow(("IOAPIC: ioapicSetEoi: u8Vector=%#x (%u)\n", u8Vector, u8Vector));
     STAM_COUNTER_INC(&pThis->CTX_SUFF_Z(StatSetEoi));
-    LogFlow(("IOAPIC: ioapicSetEoi: u8Vector=%#x (%u)\n", u8Vector, u8Vector));
 
     bool fRemoteIrrCleared = false;
-    VBOXSTRICTRC rc = IOAPIC_LOCK(pDevIns, pThis, pThisCC, VINF_IOM_R3_MMIO_WRITE);
-    if (rc == VINF_SUCCESS)
-    {
-        for (uint8_t idxRte = 0; idxRte < RT_ELEMENTS(pThis->au64RedirTable); idxRte++)
+    int rc = IOAPIC_LOCK(pDevIns, pThis, pThisCC, VINF_SUCCESS);
+    AssertRC(rc);
+
+    for (uint8_t idxRte = 0; idxRte < RT_ELEMENTS(pThis->au64RedirTable); idxRte++)
+    {
+        uint64_t const u64Rte = pThis->au64RedirTable[idxRte];
+        if (IOAPIC_RTE_GET_VECTOR(u64Rte) == u8Vector)
         {
-            uint64_t const u64Rte = pThis->au64RedirTable[idxRte];
-            if (IOAPIC_RTE_GET_VECTOR(u64Rte) == u8Vector)
-            {
 #ifdef DEBUG_ramshankar
-                /* This assertion may trigger when restoring saved-states created using the old, incorrect I/O APIC code. */
-                Assert(IOAPIC_RTE_GET_REMOTE_IRR(u64Rte));
+            /* This assertion may trigger when restoring saved-states created using the old, incorrect I/O APIC code. */
+            Assert(IOAPIC_RTE_GET_REMOTE_IRR(u64Rte));
 #endif
-                pThis->au64RedirTable[idxRte] &= ~IOAPIC_RTE_REMOTE_IRR;
-                fRemoteIrrCleared = true;
-                STAM_COUNTER_INC(&pThis->StatEoiReceived);
-                Log2(("IOAPIC: ioapicSetEoi: Cleared remote IRR, idxRte=%u vector=%#x (%u)\n", idxRte, u8Vector, u8Vector));
-
-                /*
-                 * Signal the next pending interrupt for this RTE.
-                 */
-                uint32_t const uPinMask = UINT32_C(1) << idxRte;
-                if (pThis->uIrr & uPinMask)
-                    ioapicSignalIntrForRte(pDevIns, pThis, pThisCC, idxRte);
-            }
+            pThis->au64RedirTable[idxRte] &= ~IOAPIC_RTE_REMOTE_IRR;
+            fRemoteIrrCleared = true;
+            STAM_COUNTER_INC(&pThis->StatEoiReceived);
+            Log2(("IOAPIC: ioapicSetEoi: Cleared remote IRR, idxRte=%u vector=%#x (%u)\n", idxRte, u8Vector, u8Vector));
+
+            /*
+             * Signal the next pending interrupt for this RTE.
+             */
+            uint32_t const uPinMask = UINT32_C(1) << idxRte;
+            if (pThis->uIrr & uPinMask)
+                ioapicSignalIntrForRte(pDevIns, pThis, pThisCC, idxRte);
         }
-
-        IOAPIC_UNLOCK(pDevIns, pThis, pThisCC);
+    }
+
+    IOAPIC_UNLOCK(pDevIns, pThis, pThisCC);
+
 #ifndef VBOX_WITH_IOMMU_AMD
-        AssertMsg(fRemoteIrrCleared, ("Failed to clear remote IRR for vector %#x (%u)\n", u8Vector, u8Vector));
+    AssertMsg(fRemoteIrrCleared, ("Failed to clear remote IRR for vector %#x (%u)\n", u8Vector, u8Vector));
 #endif
-    }
-    else
-        STAM_COUNTER_INC(&pThis->StatEoiContention);
-
-    return rc;
 }
 
@@ -1119 +1114 @@
         case IOAPIC_DIRECT_OFF_EOI:
             if (pThis->u8ApicVer == IOAPIC_VERSION_ICH9)
-                rc = ioapicSetEoi(pDevIns, uValue);
+                ioapicSetEoi(pDevIns, uValue);
             else
                 Log(("IOAPIC: ioapicMmioWrite: Write to EOI register ignored!\n"));
@@ -1668 +1663 @@
     PDMDevHlpSTAMRegister(pDevIns, &pThis->StatIommuDiscardedMsi,  STAMTYPE_COUNTER, "Iommu/DiscardedMsi",  STAMUNIT_OCCURENCES, "Number of MSIs discarded by the IOMMU.");
 
-    PDMDevHlpSTAMRegister(pDevIns, &pThis->StatEoiContention,    STAMTYPE_COUNTER, "CritSect/ContentionSetEoi", STAMUNIT_OCCURENCES, "Number of times the critsect is busy during EOI writes causing trips to R3.");
     PDMDevHlpSTAMRegister(pDevIns, &pThis->StatSetRteContention, STAMTYPE_COUNTER, "CritSect/ContentionSetRte", STAMUNIT_OCCURENCES, "Number of times the critsect is busy during RTE writes causing trips to R3.");