Changeset 87692 in vbox for trunk/src/VBox/Devices/Bus

Timestamp:

Feb 10, 2021 4:50:02 PM (4 years ago)

Author:

vboxsync

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

142735

Message:

AMD IOMMU: bugref:9654 Drop the atomic reads when holding the lock.

File:

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

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

@@ -1221 +1221 @@
 
 
-DECLINLINE(IOMMU_STATUS_T) iommuAmdGetStatus(PCIOMMU pThis)
-{
-    IOMMU_STATUS_T Status;
-    Status.u64 = ASMAtomicReadU64((volatile uint64_t *)&pThis->Status.u64);
-    return Status;
-}
-
-
-DECLINLINE(IOMMU_CTRL_T) iommuAmdGetCtrl(PCIOMMU pThis)
+DECL_FORCE_INLINE(IOMMU_CTRL_T) iommuAmdGetCtrlUnlocked(PCIOMMU pThis)
 {
     IOMMU_CTRL_T Ctrl;
@@ -1272 +1264 @@
  * @param   pDevIns     The IOMMU device instance.
  *
- * @remarks The IOMMU lock must be held.
+ * @remarks The IOMMU lock must be held while calling this!
  */
 static void iommuAmdCmdThreadWakeUpIfNeeded(PPDMDEVINS pDevIns)
 {
-    IOMMU_ASSERT_LOCKED(pDevIns);
     Log4Func(("\n"));
 
     PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
-    IOMMU_STATUS_T const Status = iommuAmdGetStatus(pThis);
-    if (Status.n.u1CmdBufRunning)
+    if (pThis->Status.n.u1CmdBufRunning)
     {
         Log4Func(("Signaling command thread\n"));
@@ -1622 +1612 @@
      * Inconsistent specs :/
      */
-    IOMMU_STATUS_T const Status = iommuAmdGetStatus(pThis);
-    if (Status.n.u1CmdBufRunning)
+    if (pThis->Status.n.u1CmdBufRunning)
     {
         LogFunc(("Setting CmdBufBar (%#RX64) when command buffer is running -> Ignored\n", u64Value));
@@ -1665 +1654 @@
      * See AMD IOMMU spec. "Event Log Base Address Register".
      */
-    IOMMU_STATUS_T const Status = iommuAmdGetStatus(pThis);
-    if (Status.n.u1EvtLogRunning)
+    if (pThis->Status.n.u1EvtLogRunning)
     {
         LogFunc(("Setting EvtLogBar (%#RX64) when event logging is running -> Ignored\n", u64Value));
@@ -1712 +1700 @@
     if (NewCtrl.n.u3DevTabSegEn <= pThis->ExtFeat.n.u2DevTabSegSup)
     {
-        IOMMU_CTRL_T const OldCtrl = iommuAmdGetCtrl(pThis);
+        IOMMU_CTRL_T const OldCtrl = pThis->Ctrl;
 
         /* Update the register. */
@@ -1962 +1950 @@
      * See AMD IOMMU spec. 3.3.13 "Command and Event Log Pointer Registers".
      */
-    IOMMU_STATUS_T const Status = iommuAmdGetStatus(pThis);
-    if (Status.n.u1CmdBufRunning)
+    if (pThis->Status.n.u1CmdBufRunning)
     {
         LogFunc(("Setting CmdBufHeadPtr (%#RX64) when command buffer is running -> Ignored\n", u64Value));
@@ -2075 +2062 @@
      * See AMD IOMMU spec. 3.3.13 "Command and Event Log Pointer Registers".
      */
-    IOMMU_STATUS_T const Status = iommuAmdGetStatus(pThis);
-    if (Status.n.u1EvtLogRunning)
+    if (pThis->Status.n.u1EvtLogRunning)
     {
         LogFunc(("Setting EvtLogTailPtr (%#RX64) when event log is running -> Ignored\n", u64Value));
@@ -2117 +2103 @@
      * Writing 0 to an RW1C bit has no effect. Writing 1 to an RW1C bit, clears the bit if it's already 1.
      */
-    IOMMU_STATUS_T const OldStatus = iommuAmdGetStatus(pThis);
+    IOMMU_STATUS_T const OldStatus = pThis->Status;
     uint64_t const fOldRw1cBits = (OldStatus.u64 &  IOMMU_STATUS_RW1C_MASK);
     uint64_t const fOldRoBits   = (OldStatus.u64 & ~IOMMU_STATUS_RW1C_MASK);
@@ -2523 +2509 @@
 
     /* Check if event logging is active and the log has not overflowed. */
-    IOMMU_STATUS_T const Status = iommuAmdGetStatus(pThis);
+    IOMMU_STATUS_T const Status = pThis->Status;
     if (   Status.n.u1EvtLogRunning
         && !Status.n.u1EvtOverflow)
@@ -2553 +2539 @@
 
             /* Check and signal an interrupt if software wants to receive one when an event log entry is written. */
-            IOMMU_CTRL_T const Ctrl = iommuAmdGetCtrl(pThis);
-            if (Ctrl.n.u1EvtIntrEn)
+            if (pThis->Ctrl.n.u1EvtIntrEn)
                 iommuAmdMsiInterruptRaise(pDevIns);
         }
@@ -2563 +2548 @@
 
             /* Check and signal an interrupt if software wants to receive one when the event log has overflowed. */
-            IOMMU_CTRL_T const Ctrl = iommuAmdGetCtrl(pThis);
-            if (Ctrl.n.u1EvtIntrEn)
+            if (pThis->Ctrl.n.u1EvtIntrEn)
                 iommuAmdMsiInterruptRaise(pDevIns);
         }
@@ -3062 +3046 @@
 
     /* Figure out which device table segment is being accessed. */
-    IOMMU_CTRL_T const Ctrl = iommuAmdGetCtrl(pThis);
-    uint8_t const idxSegsEn = Ctrl.n.u3DevTabSegEn;
+    uint8_t const idxSegsEn = pThis->Ctrl.n.u3DevTabSegEn;
     Assert(idxSegsEn < RT_ELEMENTS(g_auDevTabSegShifts));
 
@@ -3776 +3759 @@
 
     PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
-    IOMMU_CTRL_T const Ctrl = iommuAmdGetCtrl(pThis);
+    IOMMU_CTRL_T const Ctrl = iommuAmdGetCtrlUnlocked(pThis);
     if (Ctrl.n.u1IommuEn)
     {
@@ -3852 +3835 @@
 
     PIOMMU pThis = PDMDEVINS_2_DATA(pDevIns, PIOMMU);
-    IOMMU_CTRL_T const Ctrl = iommuAmdGetCtrl(pThis);
+    IOMMU_CTRL_T const Ctrl = iommuAmdGetCtrlUnlocked(pThis);
     if (Ctrl.n.u1IommuEn)
     {
@@ -4207 +4190 @@
 
     /* Interrupts are forwarded with remapping when the IOMMU is disabled. */
-    IOMMU_CTRL_T const Ctrl = iommuAmdGetCtrl(pThis);
+    IOMMU_CTRL_T const Ctrl = iommuAmdGetCtrlUnlocked(pThis);
     if (Ctrl.n.u1IommuEn)
     {
@@ -4313 +4296 @@
                     IOMMU_LOCK(pDevIns, pThisCC);
                     ASMAtomicOrU64(&pThis->Status.u64, IOMMU_STATUS_COMPLETION_WAIT_INTR);
-                    IOMMU_CTRL_T const Ctrl = iommuAmdGetCtrl(pThis);
-                    bool const fRaiseInt = Ctrl.n.u1CompWaitIntrEn;
+                    bool const fRaiseInt = pThis->Ctrl.n.u1CompWaitIntrEn;
                     IOMMU_UNLOCK(pDevIns, pThisCC);
 
@@ -4547 +4529 @@
         IOMMU_LOCK(pDevIns, pThisCC);
 
-        IOMMU_STATUS_T const Status = iommuAmdGetStatus(pThis);
-        if (Status.n.u1CmdBufRunning)
+        if (pThis->Status.n.u1CmdBufRunning)
         {
             /* Get the offsets we need to read commands from memory (circular buffer offset). */
@@ -5916 +5897 @@
     PDMDevHlpSTAMRegister(pDevIns, &pThis->StatIotlbeCacheMiss, STAMTYPE_COUNTER, "IOTLB/CacheMiss", STAMUNIT_OCCURENCES, "Number of IOTLB cache misses.");
     PDMDevHlpSTAMRegister(pDevIns, &pThis->StatIotlbeLazyEvictReuse, STAMTYPE_COUNTER, "IOTLB/LazyEvictReuse", STAMUNIT_OCCURENCES, "Number of IOTLB entries reused after lazy eviction.");
-    PDMDevHlpSTAMRegister(pDevIns, &pThis->StatIotlbeLookup, STAMTYPE_PROFILE, "IOTLB/Lookup", STAMUNIT_TICKS_PER_CALL, "Profiling IOTLB entry lookup.");
+    PDMDevHlpSTAMRegister(pDevIns, &pThis->StatIotlbeLookup, STAMTYPE_PROFILE, "Profile/IotlbeLookup", STAMUNIT_TICKS_PER_CALL, "Profiling IOTLB entry lookup.");
 
     PDMDevHlpSTAMRegister(pDevIns, &pThis->StatDteLookupNonContig, STAMTYPE_COUNTER, "DTE/LookupNonContig", STAMUNIT_OCCURENCES, "DTE lookups that resulted in non-contiguous translated regions.");
-    PDMDevHlpSTAMRegister(pDevIns, &pThis->StatIoPageWalkLookup, STAMTYPE_PROFILE, "DTE/Lookup", STAMUNIT_TICKS_PER_CALL, "Profiling I/O page walk lookup.");
+    PDMDevHlpSTAMRegister(pDevIns, &pThis->StatIoPageWalkLookup, STAMTYPE_PROFILE, "Profile/IoPageWalk", STAMUNIT_TICKS_PER_CALL, "Profiling I/O page walk lookup.");
 # endif