Changeset 21615 in vbox for trunk/src/VBox/Devices/Storage/DevAHCI.cpp

Timestamp:

Jul 15, 2009 3:26:05 PM (15 years ago)

Author:

vboxsync

Message:

AHCI: fix race condition resulting in hanging I/O

File:

• trunk/src/VBox/Devices/Storage/DevAHCI.cpp (modified) (18 diffs)

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

@@ -294 +294 @@
     uint32_t                        regFBU;
     /** Interrupt Status. */
-    uint32_t                        regIS;
+    volatile uint32_t               regIS;
     /** Interrupt Enable. */
     uint32_t                        regIE;
@@ -1029 +1029 @@
 {
     ahciLog(("%s: write u32Value=%#010x\n", __FUNCTION__, u32Value));
-    pAhciPort->regSERR &= ~u32Value;
-    if (u32Value & AHCI_PORT_SERR_X)
-    {
+
+    if (   (u32Value & AHCI_PORT_SERR_X)
+        && (pAhciPort->regSERR & AHCI_PORT_SERR_X))
+    {
+        ASMAtomicAndU32(&pAhciPort->regIS, ~AHCI_PORT_IS_PCS);
         pAhciPort->regTFD |= ATA_STAT_ERR;
         pAhciPort->regTFD &= ~(ATA_STAT_DRQ | ATA_STAT_BUSY);
     }
+
+    pAhciPort->regSERR &= ~u32Value;
+
     return VINF_SUCCESS;
 }
@@ -1098 +1103 @@
 
             /* We received a COMINIT from the device. Tell the guest. */
-            pAhciPort->regIS   |= AHCI_PORT_IS_PCS;
+            ASMAtomicOrU32(&pAhciPort->regIS, AHCI_PORT_IS_PCS);
             pAhciPort->regSERR |= AHCI_PORT_SERR_X;
             pAhciPort->regTFD  |= ATA_STAT_BUSY;
@@ -1105 +1110 @@
             {
                 ahciPostFirstD2HFisIntoMemory(pAhciPort);
-                pAhciPort->regIS |= AHCI_PORT_IS_DHRS;
+                ASMAtomicOrU32(&pAhciPort->regIS, AHCI_PORT_IS_DHRS);
 
                 if (pAhciPort->regIE & AHCI_PORT_IE_DHRE)
@@ -1247 +1252 @@
 #else
                 ahciPostFirstD2HFisIntoMemory(pAhciPort);
-                pAhciPort->regIS |= AHCI_PORT_IS_DHRS;
+                ASMAtomicOrU32(&pAhciPort->regIS, AHCI_PORT_IS_DHRS);
 
                 if (pAhciPort->regIE & AHCI_PORT_IE_DHRE)
@@ -1364 +1369 @@
 {
     ahciLog(("%s: write u32Value=%#010x\n", __FUNCTION__, u32Value));
-    pAhciPort->regIS &= ~u32Value;
-
-    if (u32Value & AHCI_PORT_IS_PCS)
-        pAhciPort->regSERR &= ~AHCI_PORT_SERR_X;
+    ASMAtomicAndU32(&pAhciPort->regIS, ~(u32Value & AHCI_PORT_IS_READONLY));
+
     return VINF_SUCCESS;
 }
@@ -1502 +1505 @@
          * notification bits.
          */
+        bool fClear = true;
 
         ahci->regHbaIs &= ~(u32Value);
 
-        if ((!ahci->u32PortsInterrupted) && (!ahci->regHbaIs))
+        fClear = (!ahci->u32PortsInterrupted) && (!ahci->regHbaIs);
+        if (fClear)
+        {
+            unsigned i = 0;
+
+            /* Check if the cleared ports have a interrupt status bit set. */
+            while (u32Value > 0)
+            {
+                if (u32Value & 0x01)
+                {
+                    PAHCIPort pAhciPort = &ahci->ahciPort[i];
+
+                    if (pAhciPort->regIE & pAhciPort->regIS)
+                    {
+                        Log(("%s: Interrupt status of port %u set -> Set interrupt again\n", __FUNCTION__, i));
+                        ASMAtomicOrU32(&ahci->u32PortsInterrupted, 1 << i);
+                        fClear = false;
+                        break;
+                    }
+                }
+                u32Value = u32Value >> 1;
+                i++;
+            }
+        }
+
+        if (fClear)
             ahciHbaClearInterrupt(ahci);
         else
@@ -1791 +1820 @@
         pAhciPort->regCMD |= AHCI_PORT_CMD_CPS; /* Indicate that there is a device on that port */
         /* We received a COMINIT signal */
-        pAhciPort->regSERR |= AHCI_PORT_SERR_X;
-        pAhciPort->regIS   |= AHCI_PORT_IS_PCS;
         pAhciPort->regTFD  |= ATA_STAT_BUSY;
 
@@ -3822 +3849 @@
         {
             /* Error bit is set. */
-            pAhciPort->regIS |= AHCI_PORT_IS_TFES;
+            ASMAtomicOrU32(&pAhciPort->regIS, AHCI_PORT_IS_TFES);
             if (pAhciPort->regIE & AHCI_PORT_IE_TFEE)
                 fAssertIntr = true;
@@ -3829 +3856 @@
         if (fInterrupt)
         {
-            pAhciPort->regIS |= AHCI_PORT_IS_DHRS;
+            ASMAtomicOrU32(&pAhciPort->regIS, AHCI_PORT_IS_DHRS);
             /* Check if we should assert an interrupt */
             if (pAhciPort->regIE & AHCI_PORT_IE_DHRE)
@@ -3873 +3900 @@
         {
             /* Error bit is set. */
-            pAhciPort->regIS |= AHCI_PORT_IS_TFES;
+            ASMAtomicOrU32(&pAhciPort->regIS, AHCI_PORT_IS_TFES);
             if (pAhciPort->regIE & AHCI_PORT_IE_TFEE)
                 fAssertIntr = true;
@@ -3880 +3907 @@
         if (fInterrupt)
         {
-            pAhciPort->regIS |= AHCI_PORT_IS_SDBS;
+            ASMAtomicOrU32(&pAhciPort->regIS, AHCI_PORT_IS_SDBS);
             /* Check if we should assert an interrupt */
             if (pAhciPort->regIE & AHCI_PORT_IE_SDBE)
@@ -4943 +4970 @@
     /* Print some infos about the FIS. */
     ahciDumpFisInfo(pAhciPort, &pAhciPortTaskState->cmdFis[0]);
+
+    /* Print the PRDT */
+    RTGCPHYS GCPhysAddrPRDTLEntryStart = AHCI_RTGCPHYS_FROM_U32(pAhciPortTaskState->cmdHdr.u32CmdTblAddrUp, pAhciPortTaskState->cmdHdr.u32CmdTblAddr) + AHCI_CMDHDR_PRDT_OFFSET;
+
+    ahciLog(("PRDT address %RGp number of entries %u\n", GCPhysAddrPRDTLEntryStart, AHCI_CMDHDR_PRDTL_ENTRIES(pAhciPortTaskState->cmdHdr.u32DescInf)));
+
+    for (unsigned i = 0; i < AHCI_CMDHDR_PRDTL_ENTRIES(pAhciPortTaskState->cmdHdr.u32DescInf); i++)
+    {
+        SGLEntry SGEntry;
+
+        ahciLog(("Entry %u at address %RGp\n", i, GCPhysAddrPRDTLEntryStart));
+        PDMDevHlpPhysRead(pAhciPort->CTX_SUFF(pDevIns), GCPhysAddrPRDTLEntryStart, &SGEntry, sizeof(SGLEntry));
+
+        RTGCPHYS GCPhysDataAddr = AHCI_RTGCPHYS_FROM_U32(SGEntry.u32DBAUp, SGEntry.u32DBA);
+        ahciLog(("GCPhysAddr=%RGp Size=%u\n", GCPhysDataAddr, SGEntry.u32DescInf & SGLENTRY_DESCINF_DBC));
+
+        GCPhysAddrPRDTLEntryStart += sizeof(SGLEntry);
+    }
 #endif
 }
@@ -5180 +5225 @@
         }
 
-        ahciLog(("%s: Processing requests\n", __FUNCTION__));
         ASMAtomicXchgBool(&pAhciPort->fNotificationSend, false);
 
         uint32_t cTasksToProcess = ASMAtomicXchgU32(&pAhciPort->uActTasksActive, 0);
+
+        ahciLog(("%s: Processing %u requests\n", __FUNCTION__, cTasksToProcess));
 
         /* Process commands. */
@@ -5351 +5397 @@
             ahciLog(("%s: After uActReadPos=%u\n", __FUNCTION__, pAhciPort->uActReadPos));
             cTasksToProcess--;
+            if (!cTasksToProcess)
+                cTasksToProcess = ASMAtomicXchgU32(&pAhciPort->uActTasksActive, 0);
         }
 
@@ -5418 +5466 @@
      */
     pAhciPort->regCMD  |= AHCI_PORT_CMD_CPS;
-    pAhciPort->regIS   |= AHCI_PORT_IS_CPDS | AHCI_PORT_IS_PRCS;
+    ASMAtomicOrU32(&pAhciPort->regIS, AHCI_PORT_IS_CPDS | AHCI_PORT_IS_PRCS);
     pAhciPort->regSERR |= AHCI_PORT_SERR_N;
     if (pAhciPort->regIE & AHCI_PORT_IE_CPDE)
@@ -5440 +5488 @@
     pAhciPort->regSSTS = 0;
     pAhciPort->regCMD &= ~AHCI_PORT_CMD_CPS;
-    pAhciPort->regIS |= AHCI_PORT_IS_CPDS | AHCI_PORT_IS_PRCS;
+    ASMAtomicOrU32(&pAhciPort->regIS, AHCI_PORT_IS_CPDS | AHCI_PORT_IS_PRCS);
     pAhciPort->regSERR |= AHCI_PORT_SERR_N;
     if (pAhciPort->regIE & AHCI_PORT_IE_CPDE)
@@ -5853 +5901 @@
         SSMR3GetGCPhys(pSSMHandle, (RTGCPHYS *)&pAhci->ahciPort[i].GCPhysAddrClb);
         SSMR3GetGCPhys(pSSMHandle, (RTGCPHYS *)&pAhci->ahciPort[i].GCPhysAddrFb);
-        SSMR3GetU32(pSSMHandle, &pAhci->ahciPort[i].regIS);
+        SSMR3GetU32(pSSMHandle, (uint32_t *)&pAhci->ahciPort[i].regIS);
         SSMR3GetU32(pSSMHandle, &pAhci->ahciPort[i].regIE);
         SSMR3GetU32(pSSMHandle, &pAhci->ahciPort[i].regCMD);