Changeset 84292 in vbox for trunk

Timestamp:

May 13, 2020 4:06:42 PM (5 years ago)

Author:

vboxsync

Message:

AMD IOMMU: bugref:9654 Asynchronous IOMMU command processing.

File:

• trunk/src/VBox/Devices/Bus/DevIommuAmd.cpp (modified) (25 diffs)

trunk/src/VBox/Devices/Bus/DevIommuAmd.cpp

@@ -489 +489 @@
  * This will make a long jump to ring-3 to acquire the lock if necessary.
  */
-#define IOMMU_LOCK(a_pDevIns, a_pThis)  \
+#define IOMMU_LOCK(a_pDevIns)  \
     do { \
-        NOREF(pThis); \
         int rcLock = PDMDevHlpCritSectEnter((a_pDevIns), (a_pDevIns)->CTX_SUFF(pCritSectRo), VINF_SUCCESS); \
         if (RT_LIKELY(rcLock == VINF_SUCCESS)) \
@@ -500 +499 @@
 
 /**
- * Acquires the IOMMU PDM lock (no return, only asserts on failure).
+ * Acquires the IOMMU PDM lock (asserts on failure rather than returning an error).
  * This will make a long jump to ring-3 to acquire the lock if necessary.
  */
-#define IOMMU_LOCK_NORET(a_pDevIns, a_pThis)  \
+#define IOMMU_LOCK_NORET(a_pDevIns)  \
     do { \
-        NOREF(pThis); \
         int rcLock = PDMDevHlpCritSectEnter((a_pDevIns), (a_pDevIns)->CTX_SUFF(pCritSectRo), VINF_SUCCESS); \
         AssertRC(rcLock); \
@@ -513 +511 @@
  * Releases the IOMMU PDM lock.
  */
-#define IOMMU_UNLOCK(a_pDevIns, a_pThis) \
+#define IOMMU_UNLOCK(a_pDevIns) \
     do { \
         PDMDevHlpCritSectLeave((a_pDevIns), (a_pDevIns)->CTX_SUFF(pCritSectRo)); \
@@ -3545 +3543 @@
     PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
 
-    IOMMU_LOCK(pDevIns, pThis);
+    IOMMU_ASSERT_LOCKED(pDevIns);
 
     /* Check if event logging is active and the log has not overflowed. */
@@ -3593 +3591 @@
         }
     }
-
-    IOMMU_UNLOCK(pDevIns, pThis);
 }
 
@@ -3608 +3604 @@
 static void iommuAmdSetHwError(PPDMDEVINS pDevIns, PCEVT_GENERIC_T pEvent)
 {
+    IOMMU_ASSERT_LOCKED(pDevIns);
+
     PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
-    IOMMU_LOCK_NORET(pDevIns, pThis);
     if (pThis->ExtFeat.n.u1HwErrorSup)
     {
@@ -3619 +3616 @@
         Assert(pThis->HwEvtHi.n.u4EvtCode == IOMMU_EVT_DEV_TAB_HW_ERROR);
     }
-    IOMMU_UNLOCK(pDevIns, pThis);
 }
 
@@ -3639 +3635 @@
     pEvtPageTabHwErr->n.u16DevId           = uDevId;
     pEvtPageTabHwErr->n.u16DomainOrPasidLo = uDomainId;
-    //pEvtPageTabHwErr->n.u1GuestOrNested  = 0;
+    pEvtPageTabHwErr->n.u1GuestOrNested    = 0;
     pEvtPageTabHwErr->n.u1Interrupt        = RT_BOOL(enmOp == IOMMUOP_INTR_REQ);
     pEvtPageTabHwErr->n.u1ReadWrite        = RT_BOOL(enmOp == IOMMUOP_MEM_WRITE);
@@ -3656 +3652 @@
  * @param   pEvtPageTabHwErr    The page table hardware error event.
  * @param   enmEvtType          The hardware error event type.
+ *
+ * @thread  Any.
  */
 static void iommuAmdRaisePageTabHwErrorEvent(PPDMDEVINS pDevIns, IOMMUOP enmOp, PEVT_PAGE_TAB_HW_ERR_T pEvtPageTabHwErr,
@@ -3662 +3660 @@
     AssertCompile(sizeof(EVT_GENERIC_T) == sizeof(EVT_PAGE_TAB_HW_ERR_T));
     PCEVT_GENERIC_T pEvent = (PCEVT_GENERIC_T)pEvtPageTabHwErr;
+
+    IOMMU_LOCK_NORET(pDevIns);
 
     iommuAmdSetHwError(pDevIns, (PCEVT_GENERIC_T)pEvent);
@@ -3668 +3668 @@
         iommuAmdSetPciTargetAbort(pDevIns);
 
+    IOMMU_UNLOCK(pDevIns);
+
     Log((IOMMU_LOG_PFX ": Raised PAGE_TAB_HARDWARE_ERROR. uDevId=%#x uDomainId=%#x GCPhysPtEntity=%#RGp enmOp=%u enmType=%u\n",
          pEvtPageTabHwErr->n.u16DevId, pEvtPageTabHwErr->n.u16DomainOrPasidLo, pEvtPageTabHwErr->n.u64Addr, enmOp, enmEvtType));
@@ -3696 +3698 @@
  * @param   pEvtCmdHwErr    The command hardware error event.
  * @param   enmEvtType      The hardware error event type.
+ *
+ * @thread  Any.
  */
 static void iommuAmdRaiseCmdHwErrorEvent(PPDMDEVINS pDevIns, PCEVT_CMD_HW_ERR_T pEvtCmdHwErr, EVT_HW_ERR_TYPE_T enmEvtType)
@@ -3701 +3705 @@
     AssertCompile(sizeof(EVT_GENERIC_T) == sizeof(EVT_CMD_HW_ERR_T));
     PCEVT_GENERIC_T pEvent = (PCEVT_GENERIC_T)pEvtCmdHwErr;
+
+    IOMMU_LOCK_NORET(pDevIns);
 
     iommuAmdSetHwError(pDevIns, (PCEVT_GENERIC_T)pEvent);
     iommuAmdWriteEvtLogEntry(pDevIns, (PCEVT_GENERIC_T)pEvent);
     iommuAmdHaltCmdProcessing(pDevIns);
+
+    IOMMU_UNLOCK(pDevIns);
 
     Log((IOMMU_LOG_PFX ": Raised COMMAND_HARDWARE_ERROR. GCPhysCmd=%#RGp enmType=%u\n", pEvtCmdHwErr->n.u64Addr, enmEvtType));
@@ -3742 +3750 @@
  * @param   pEvtDevTabHwErr     The device table hardware error event.
  * @param   enmEvtType          The hardware error event type.
+ *
+ * @thread  Any.
  */
 static void iommuAmdRaiseDevTabHwErrorEvent(PPDMDEVINS pDevIns, IOMMUOP enmOp, PEVT_DEV_TAB_HW_ERROR_T pEvtDevTabHwErr,
@@ -3748 +3758 @@
     AssertCompile(sizeof(EVT_GENERIC_T) == sizeof(EVT_DEV_TAB_HW_ERROR_T));
     PCEVT_GENERIC_T pEvent = (PCEVT_GENERIC_T)pEvtDevTabHwErr;
+
+    IOMMU_LOCK_NORET(pDevIns);
+
     iommuAmdSetHwError(pDevIns, (PCEVT_GENERIC_T)pEvent);
     iommuAmdWriteEvtLogEntry(pDevIns, (PCEVT_GENERIC_T)pEvent);
     if (enmOp != IOMMUOP_CMD)
         iommuAmdSetPciTargetAbort(pDevIns);
+
+    IOMMU_UNLOCK(pDevIns);
 
     Log((IOMMU_LOG_PFX ": Raised DEV_TAB_HARDWARE_ERROR. uDevId=%#x GCPhysDte=%#RGp enmOp=%u enmType=%u\n",
@@ -3794 +3809 @@
  * @param   pEvtIllegalDte  The illegal device table entry event.
  * @param   enmEvtType      The illegal DTE event type.
+ *
+ * @thread  Any.
  */
 static void iommuAmdRaiseIllegalDteEvent(PPDMDEVINS pDevIns, IOMMUOP enmOp, PCEVT_ILLEGAL_DTE_T pEvtIllegalDte,
@@ -3800 +3817 @@
     AssertCompile(sizeof(EVT_GENERIC_T) == sizeof(EVT_ILLEGAL_DTE_T));
     PCEVT_GENERIC_T pEvent = (PCEVT_GENERIC_T)pEvtIllegalDte;
+
+    IOMMU_LOCK_NORET(pDevIns);
+
     iommuAmdWriteEvtLogEntry(pDevIns, pEvent);
     if (enmOp != IOMMUOP_CMD)
         iommuAmdSetPciTargetAbort(pDevIns);
+
+    IOMMU_UNLOCK(pDevIns);
 
     Log((IOMMU_LOG_PFX ": Raised ILLEGAL_DTE_EVENT. uDevId=%#x uIova=%#RX64 enmOp=%u enmEvtType=%u\n", pEvtIllegalDte->n.u16DevId,
@@ -3868 +3890 @@
     PCEVT_GENERIC_T pEvent = (PCEVT_GENERIC_T)pEvtIoPageFault;
 
+    IOMMU_LOCK_NORET(pDevIns);
+
     bool fSuppressEvtLogging = false;
     if (   enmOp == IOMMUOP_MEM_READ
@@ -3967 +3991 @@
         }
     }
+
+    IOMMU_UNLOCK(pDevIns);
 }
 
@@ -4630 +4656 @@
         if (Status.u64 & IOMMU_STATUS_CMD_BUF_RUNNING)
         {
-            IOMMU_LOCK(pDevIns, pThis);
+            IOMMU_LOCK(pDevIns);
 
             uint32_t const cbCmdBuf = iommuAmdGetBufLength(pThis->CmdBufBaseAddr.n.u4Len);
@@ -4648 +4674 @@
 
                     /* Process the fetched command. */
-                    IOMMU_UNLOCK(pDevIns, pThis);
+                    IOMMU_UNLOCK(pDevIns);
                     rc = iommuAmdR3ProcessCmd(pDevIns, &Cmd);
-                    IOMMU_LOCK(pDevIns, pThis);
+                    IOMMU_LOCK(pDevIns);
                     if (RT_SUCCESS(rc))
                     { /* likely */ }
@@ -4665 +4691 @@
                     /* Reporting this as a "data error". Maybe target abort is more appropriate? */
                     EVT_CMD_HW_ERR_T EvtCmdHwErr;
-                    iommuAmdInitCmdHwErrorEvent(GCPhysCmd, HWEVTTYPE_DATA_ERROR, & EvtCmdHwErr);
+                    iommuAmdInitCmdHwErrorEvent(GCPhysCmd, HWEVTTYPE_DATA_ERROR, &EvtCmdHwErr);
                     iommuAmdRaiseCmdHwErrorEvent(pDevIns, &EvtCmdHwErr, kHwErrType_PoisonedData);
                     break;
@@ -4671 +4697 @@
             }
 
-            IOMMU_UNLOCK(pDevIns, pThis);
+            IOMMU_UNLOCK(pDevIns);
         }
     }
@@ -4732 +4758 @@
     }
 
-    IOMMU_LOCK(pDevIns, pThis);
+    IOMMU_LOCK(pDevIns);
 
     VBOXSTRICTRC rcStrict;
@@ -4783 +4809 @@
     }
 
-    IOMMU_UNLOCK(pDevIns, pThis);
+    IOMMU_UNLOCK(pDevIns);
 
     Log3((IOMMU_LOG_PFX ": PCI config write: %#x -> To %#x (%u) %Rrc\n", u32Value, uAddress, cb, VBOXSTRICTRC_VAL(rcStrict)));