Changeset 29827 in vbox for trunk/src

Timestamp:

May 26, 2010 6:27:38 PM (15 years ago)

Author:

vboxsync

Message:

LsiLogic: Fix a possible race condition in the interrupt handling code which can lead to I/O timeouts

File:

• trunk/src/VBox/Devices/Storage/DevLsiLogicSCSI.cpp (modified) (12 diffs)

trunk/src/VBox/Devices/Storage/DevLsiLogicSCSI.cpp

@@ -404 +404 @@
     uIntSts = (ASMAtomicReadU32(&pThis->uInterruptStatus) & ~LSILOGIC_REG_HOST_INTR_STATUS_DOORBELL_STS);
     /* Check maskable interrupts. */
-    uIntSts &= ~(pThis->uInterruptMask & ~LSILOGIC_REG_HOST_INTR_MASK_IRQ_ROUTING);
+    uIntSts &= ~(ASMAtomicReadU32(&pThis->uInterruptMask) & ~LSILOGIC_REG_HOST_INTR_MASK_IRQ_ROUTING);
 
     if (uIntSts)
@@ -584 +584 @@
     pLsiLogic->uReplyPostQueueNextEntryFreeWrite %= pLsiLogic->cReplyQueueEntries;
 
-    PDMCritSectLeave(&pLsiLogic->ReplyPostQueueCritSect);
-
     /* Set interrupt. */
     lsilogicSetInterrupt(pLsiLogic, LSILOGIC_REG_HOST_INTR_STATUS_REPLY_INTR);
+
+    PDMCritSectLeave(&pLsiLogic->ReplyPostQueueCritSect);
 }
 
@@ -702 +702 @@
         pLsiLogic->uReplyPostQueueNextEntryFreeWrite %= pLsiLogic->cReplyQueueEntries;
 
-        PDMCritSectLeave(&pLsiLogic->ReplyPostQueueCritSect);
-
         if (fForceReplyFifo)
         {
@@ -712 +710 @@
         /* Set interrupt. */
         lsilogicSetInterrupt(pLsiLogic, LSILOGIC_REG_HOST_INTR_STATUS_REPLY_INTR);
+
+        PDMCritSectLeave(&pLsiLogic->ReplyPostQueueCritSect);
 #else
         AssertMsgFailed(("This is not allowed to happen.\n"));
@@ -1060 +1060 @@
              * the latter one is read only.
              */
-            pThis->uInterruptStatus = pThis->uInterruptStatus & ~LSILOGIC_REG_HOST_INTR_STATUS_SYSTEM_DOORBELL;
+            ASMAtomicAndU32(&pThis->uInterruptStatus, ~LSILOGIC_REG_HOST_INTR_STATUS_SYSTEM_DOORBELL);
 
             /*
@@ -1085 +1085 @@
         case LSILOGIC_REG_HOST_INTR_MASK:
         {
-            pThis->uInterruptMask = (u32 & LSILOGIC_REG_HOST_INTR_MASK_W_MASK);
+            ASMAtomicWriteU32(&pThis->uInterruptMask, u32 & LSILOGIC_REG_HOST_INTR_MASK_W_MASK);
             lsilogicUpdateInterrupt(pThis);
             break;
@@ -1147 +1147 @@
 static int lsilogicRegisterRead(PLSILOGICSCSI pThis, uint32_t uOffset, void *pv, unsigned cb)
 {
+    int rc = VINF_SUCCESS;
     uint32_t u32 = 0;
 
@@ -1161 +1162 @@
                 LogFlowFunc((": cb is not 4 (%u)\n", cb));
 
-            if (pThis->uReplyPostQueueNextEntryFreeWrite != pThis->uReplyPostQueueNextAddressRead)
+            rc = PDMCritSectEnter(&pThis->ReplyPostQueueCritSect, VINF_IOM_HC_MMIO_READ);
+            if (rc != VINF_SUCCESS)
+                break;
+
+            uint32_t idxReplyPostQueueWrite = ASMAtomicUoReadU32(&pThis->uReplyPostQueueNextEntryFreeWrite);
+            uint32_t idxReplyPostQueueRead  = ASMAtomicUoReadU32(&pThis->uReplyPostQueueNextAddressRead);
+
+            if (idxReplyPostQueueWrite != idxReplyPostQueueRead)
             {
-                u32 = pThis->CTX_SUFF(pReplyPostQueueBase)[pThis->uReplyPostQueueNextAddressRead];
-                pThis->uReplyPostQueueNextAddressRead++;
-                pThis->uReplyPostQueueNextAddressRead %= pThis->cReplyQueueEntries;
+                u32 = pThis->CTX_SUFF(pReplyPostQueueBase)[idxReplyPostQueueRead];
+                idxReplyPostQueueRead++;
+                idxReplyPostQueueRead %= pThis->cReplyQueueEntries;
+                ASMAtomicWriteU32(&pThis->uReplyPostQueueNextAddressRead, idxReplyPostQueueRead);
             }
             else
@@ -1173 +1182 @@
                 lsilogicClearInterrupt(pThis, LSILOGIC_REG_HOST_INTR_STATUS_REPLY_INTR);
             }
+            PDMCritSectLeave(&pThis->ReplyPostQueueCritSect);
+
             Log(("%s: Returning address %#x\n", __FUNCTION__, u32));
             break;
@@ -1200 +1211 @@
         case LSILOGIC_REG_HOST_INTR_STATUS:
         {
-            u32 = pThis->uInterruptStatus;
+            u32 = ASMAtomicReadU32(&pThis->uInterruptStatus);
             break;
         }
         case LSILOGIC_REG_HOST_INTR_MASK:
         {
-            u32 = pThis->uInterruptMask;
+            u32 = ASMAtomicReadU32(&pThis->uInterruptMask);
             break;
         }
@@ -1255 +1266 @@
     LogFlowFunc(("pThis=%#p uOffset=%#x pv=%#p{%.*Rhxs} cb=%u\n", pThis, uOffset, pv, cb, pv, cb));
 
-    return VINF_SUCCESS;
+    return rc;
 }
 
@@ -1281 +1292 @@
     Assert(cb <= 4);
 
-    return lsilogicRegisterRead(pThis, uOffset, pu32, cb);
+    int rc = lsilogicRegisterRead(pThis, uOffset, pu32, cb);
+    if (rc == VINF_IOM_HC_MMIO_READ)
+        rc = VINF_IOM_HC_IOPORT_READ;
+
+    return rc;
 }