Changeset 69719 in vbox

Timestamp:

Nov 16, 2017 3:11:55 PM (7 years ago)

Author:

vboxsync

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

119100

Message:

Audio/HDA: Forward ported / integrated r119000 + r119062 (Fixed CORB / RIRB buffer handling when issuing multiple commands at once; fixes device detection on some OSes (e.g. Haiku)).

Location:

trunk/src/VBox/Devices

Files:

• Audio/DevHDA.cpp (modified) (9 diffs)
• Audio/DevHDA.h (modified) (1 diff)
• Audio/DevHDACommon.h (modified) (1 diff)
• testcase/tstDeviceStructSizeRC.cpp (modified) (1 diff)

trunk/src/VBox/Devices/Audio/DevHDA.cpp

@@ -731 +731 @@
         if (RT_FAILURE(rc))
             AssertRCReturn(rc, rc);
-# ifdef DEBUG_CMD_BUFFER
+    }
+    else
+    {
+        Assert((HDA_REG(pThis, RIRBCTL) & HDA_RIRBCTL_RDMAEN));
+        rc = PDMDevHlpPCIPhysWrite(pThis->CTX_SUFF(pDevIns), pThis->u64RIRBBase, pThis->pu64RirbBuf, pThis->cbRirbBuf);
+        if (RT_FAILURE(rc))
+            AssertRCReturn(rc, rc);
+    }
+
+#ifdef DEBUG_CMD_BUFFER
+        LogFunc(("fLocal=%RTbool\n", fLocal));
+
         uint8_t i = 0;
         do
@@ -740 +751 @@
             {
                 const char *pszPrefix;
-                if ((i + j) == HDA_REG(pThis, CORBRP));
+                if ((i + j) == HDA_REG(pThis, CORBRP))
                     pszPrefix = "[R]";
-                else if ((i + j) == HDA_REG(pThis, CORBWP));
+                else if ((i + j) == HDA_REG(pThis, CORBWP))
                     pszPrefix = "[W]";
                 else
                     pszPrefix = "   "; /* three spaces */
-                LogFunc(("%s%08x", pszPrefix, pThis->pu32CorbBuf[i + j]));
+                Log((" %s%08x", pszPrefix, pThis->pu32CorbBuf[i + j]));
                 j++;
             } while (j < 8);
-            LogFunc(("\n"));
+            Log(("\n"));
             i += 8;
         } while(i != 0);
-# endif
-    }
-    else
-    {
-        Assert((HDA_REG(pThis, RIRBCTL) & HDA_RIRBCTL_RDMAEN));
-        rc = PDMDevHlpPCIPhysWrite(pThis->CTX_SUFF(pDevIns), pThis->u64RIRBBase, pThis->pu64RirbBuf, pThis->cbRirbBuf);
-        if (RT_FAILURE(rc))
-            AssertRCReturn(rc, rc);
-# ifdef DEBUG_CMD_BUFFER
-        uint8_t i = 0;
+
         do {
             LogFunc(("RIRB%02x: ", i));
@@ -771 +773 @@
                 else
                     prefix = "   ";
-                LogFunc((" %s%016lx", prefix, pThis->pu64RirbBuf[i + j]));
+                Log((" %s%016lx", prefix, pThis->pu64RirbBuf[i + j]));
             } while (++j < 8);
-            LogFunc(("\n"));
+            Log(("\n"));
             i += 8;
         } while (i != 0);
-# endif
-    }
+#endif
     return rc;
 }
@@ -790 +791 @@
 static int hdaCORBCmdProcess(PHDASTATE pThis)
 {
-    int rc = hdaCmdSync(pThis, true);
-    if (RT_FAILURE(rc))
-        AssertRCReturn(rc, rc);
+    int rc = hdaCmdSync(pThis, true /* Sync from guest */);
+    AssertRCReturn(rc, rc);
 
     uint8_t corbRp = HDA_REG(pThis, CORBRP);
@@ -798 +798 @@
     uint8_t rirbWp = HDA_REG(pThis, RIRBWP);
 
-    Assert((corbWp != corbRp));
-    Log3Func(("CORB(RP:%x, WP:%x) RIRBWP:%x\n", HDA_REG(pThis, CORBRP), HDA_REG(pThis, CORBWP), HDA_REG(pThis, RIRBWP)));
+    Log3Func(("CORB(RP:%x, WP:%x) RIRBWP:%x\n", corbRp, corbWp, rirbWp));
 
     while (corbRp != corbWp)
     {
-        uint64_t uResp;
-        uint32_t uCmd = pThis->pu32CorbBuf[++corbRp];
-
-        int rc2 = pThis->pCodec->pfnLookup(pThis->pCodec, HDA_CODEC_CMD(uCmd, 0 /* Codec index */), &uResp);
-        if (RT_FAILURE(rc2))
-            LogFunc(("Codec lookup failed with rc=%Rrc\n", rc2));
-
-        (rirbWp)++;
+        corbRp = (corbRp + 1) % HDA_CORB_SIZE; /* Advance +1 as the first command(s) are at CORBWP + 1. */
+
+        uint64_t uResp = 0;
+        uint32_t uCmd = pThis->pu32CorbBuf[corbRp];
+
+        rc = pThis->pCodec->pfnLookup(pThis->pCodec, HDA_CODEC_CMD(uCmd, 0 /* Codec index */), &uResp);
+        if (RT_FAILURE(rc))
+            LogFunc(("Codec lookup failed with rc=%Rrc\n", rc));
+
+        LogFunc(("verb:%08x -> %016lx\n", uCmd, uResp));
 
         if (   (uResp & CODEC_RESPONSE_UNSOLICITED)
             && !(HDA_REG(pThis, GCTL) & HDA_GCTL_UNSOL))
         {
-            LogFunc(("Unexpected unsolicited response\n"));
+            LogFunc(("Unexpected unsolicited response.\n"));
             HDA_REG(pThis, CORBRP) = corbRp;
+
+            /** @todo r=andy No CORB/RIRB syncing to guest required in that case? */
             return rc;
         }
 
+        rirbWp = (rirbWp + 1) % HDA_RIRB_SIZE;
+
         pThis->pu64RirbBuf[rirbWp] = uResp;
 
-        pThis->u8RespIntCnt++;
-        if (pThis->u8RespIntCnt == RINTCNT_N(pThis))
+        pThis->u16RespIntCnt++;
+        if (pThis->u16RespIntCnt > HDA_MAX_RINTCNT) /* Make sure that the guest can't hang the host. */
+        {
+            LogRel(("HDA: Maximum response interrupt count (%d) reached, bailing out\n", HDA_MAX_RINTCNT));
+            pThis->u16RespIntCnt = HDA_MAX_RINTCNT;
             break;
+        }
     }
 
@@ -830 +839 @@
     HDA_REG(pThis, RIRBWP) = rirbWp;
 
-    rc = hdaCmdSync(pThis, false);
-
-    Log3Func(("CORB(RP:%x, WP:%x) RIRBWP:%x\n",
-              HDA_REG(pThis, CORBRP), HDA_REG(pThis, CORBWP), HDA_REG(pThis, RIRBWP)));
-
-    if (HDA_REG(pThis, RIRBCTL) & HDA_RIRBCTL_RINTCTL) /* Response Interrupt Control (RINTCTL) enabled? */
-    {
-        if (pThis->u8RespIntCnt)
-        {
-            pThis->u8RespIntCnt = 0;
-
+    rc = hdaCmdSync(pThis, false /* Sync to guest */);
+    AssertRCReturn(rc, rc);
+
+    Log3Func(("CORB(RP:%x, WP:%x) RIRBWP:%x, uRespIntCnt=%RU16\n", corbRp, corbWp, rirbWp, pThis->u16RespIntCnt));
+
+    if (pThis->u16RespIntCnt)
+    {
+        if (HDA_REG(pThis, RIRBCTL) & HDA_RIRBCTL_RINTCTL) /* Response Interrupt Control (RINTCTL) enabled? */
+        {
             HDA_REG(pThis, RIRBSTS) |= HDA_RIRBSTS_RINTFL;
+            HDA_REG(pThis, RINTCNT)  = RT_LO_U8(pThis->u16RespIntCnt);
 
 #ifndef DEBUG
@@ -848 +856 @@
             rc = hdaProcessInterrupt(pThis, __FUNCTION__);
 #endif
-        }
+            pThis->u16RespIntCnt--;
+        }
+        else /* Not enabled -- just reset our internal counter. */
+            pThis->u16RespIntCnt = 0;
     }
 
@@ -2830 +2841 @@
     HDA_REG(pThis, CORBSIZE) = 0x42;                     /* see 6.2.1 */
     HDA_REG(pThis, RIRBSIZE) = 0x42;                     /* see 6.2.1 */
+    HDA_REG(pThis, CORBRP)   = 0x0;
+    HDA_REG(pThis, RIRBWP)   = 0x0;
+    HDA_REG(pThis, RINTCNT)  = 0x0;
 
     /*
@@ -2888 +2902 @@
     else
         pThis->pu64RirbBuf = (uint64_t *)RTMemAllocZ(pThis->cbRirbBuf);
+
+    /* Clear our internal response interrupt counter. */
+    pThis->u16RespIntCnt = 0;
 
     for (uint8_t uSD = 0; uSD < HDA_MAX_STREAMS; ++uSD)

trunk/src/VBox/Devices/Audio/DevHDA.h

@@ -204 +204 @@
 #endif
     /** Response Interrupt Count (RINTCNT). */
-    uint8_t                            u8RespIntCnt;
+    uint16_t                           u16RespIntCnt;
     /** Current IRQ level. */
     uint8_t                            u8IRQL;
     /** Padding for alignment. */
-    uint8_t                            au8Padding3[6];
+    uint8_t                            au8Padding3[5];
 #ifdef DEBUG
     HDASTATEDBGINFO                    Dbg;

trunk/src/VBox/Devices/Audio/DevHDACommon.h

@@ -249 +249 @@
 #define HDA_REG_RINTCNT             25          /* 0x5A */
 #define HDA_RMX_RINTCNT             23
-#define RINTCNT_N(pThis)            (HDA_REG(pThis, RINTCNT) & 0xff)
+
+/** Maximum number of Response Interrupts. */
+#define HDA_MAX_RINTCNT             256
 
 #define HDA_REG_RIRBCTL             26          /* 0x5C */

trunk/src/VBox/Devices/testcase/tstDeviceStructSizeRC.cpp

@@ -1899 +1899 @@
 #endif
     GEN_CHECK_OFF(HDASTATE, u64WalClk);
-    GEN_CHECK_OFF(HDASTATE, u8RespIntCnt);
+    GEN_CHECK_OFF(HDASTATE, u16RespIntCnt);
     GEN_CHECK_OFF(HDASTATE, u8IRQL);