Changeset 89849 in vbox for trunk/src/VBox/Devices/Audio

Timestamp:

Jun 23, 2021 9:18:30 AM (4 years ago)

Author:

vboxsync

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

145306

Message:

DevHda: Redid the locking in hdaMmioWrite, so that we take the lock before calling hdaWriteReg and hold it while supply missing bits for partial writes. bugref:9890

File:

• trunk/src/VBox/Devices/Audio/DevHda.cpp (modified) (9 diffs)

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

@@ -3049 +3049 @@
 DECLINLINE(VBOXSTRICTRC) hdaWriteReg(PPDMDEVINS pDevIns, PHDASTATE pThis, int idxRegDsc, uint32_t u32Value, char const *pszLog)
 {
-    DEVHDA_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
-
     if (   (HDA_REG(pThis, GCTL) & HDA_GCTL_CRST)
         || idxRegDsc == HDA_REG_GCTL)
@@ -3060 +3058 @@
                  g_aHdaRegMap[idxRegDsc].abbrev));
         STAM_COUNTER_INC(&pThis->StatRegWritesBlockedByReset);
-
-        DEVHDA_UNLOCK(pDevIns, pThis);
         return VINF_SUCCESS;
     }
@@ -3086 +3082 @@
                      g_aHdaRegMap[idxRegDsc].abbrev));
             STAM_COUNTER_INC(&pThis->StatRegWritesBlockedByRun);
-
-            DEVHDA_UNLOCK(pDevIns, pThis);
             return VINF_SUCCESS;
         }
@@ -3106 +3100 @@
         STAM_COUNTER_INC(&pThis->aStatRegWrites[idxRegDsc]);
 
-    DEVHDA_UNLOCK(pDevIns, pThis);
     RT_NOREF(pszLog);
     return rc;
@@ -3162 +3155 @@
     if (idxRegDsc >= 0 && g_aHdaRegMap[idxRegDsc].size == cb)
     {
+        DEVHDA_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
+
         Log3Func(("@%#05x u%u=%#0*RX64 %s\n", (uint32_t)off, cb * 8, 2 + cb * 2, u64Value, g_aHdaRegMap[idxRegDsc].abbrev));
         rc = hdaWriteReg(pDevIns, pThis, idxRegDsc, u64Value, "");
         Log3Func(("  %#x -> %#x\n", u32LogOldValue, idxRegMem != UINT32_MAX ? pThis->au32Regs[idxRegMem] : UINT32_MAX));
+
+        DEVHDA_UNLOCK(pDevIns, pThis);
     }
     /*
@@ -3172 +3169 @@
              && cb < g_aHdaRegMap[idxRegDsc].size)
     {
-        /** @todo r=bird: This is not correctly serialized!  Also we're not gaining
-         *        any advantage by not entering the critsect here already, because
-         *        hdaWriteReg will enter it! */
+        DEVHDA_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
+
         u64Value |=   pThis->au32Regs[g_aHdaRegMap[idxRegDsc].mem_idx]
                     & g_afMasks[g_aHdaRegMap[idxRegDsc].size]
@@ -3185 +3181 @@
         Log4Func(("  %#x -> %#x\n", u32LogOldValue, idxRegMem != UINT32_MAX ? pThis->au32Regs[idxRegMem] : UINT32_MAX));
         STAM_COUNTER_INC(&pThis->CTX_SUFF_Z(StatRegSubWrite));
+
+        DEVHDA_UNLOCK(pDevIns, pThis);
     }
     /*
@@ -3192 +3190 @@
     {
 #ifdef IN_RING3
-        /** @todo r=bird: This is not correctly serialized!  Also we're not gaining
-         *        much of an advantage by not entering the critsect here already,
-         *        becuase hdaWriteReg will eventually enter it, possibly multiple
-         *        times!   The only would be unknown wrights, which should be rare
-         *        and not something we need to optimize for. */
+        DEVHDA_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE);
+
         if (idxRegDsc == -1)
             Log4Func(("@%#05x u32=%#010x cb=%d\n", (uint32_t)off, *(uint32_t const *)pv, cb));
@@ -3280 +3275 @@
             }
         }
+
+        DEVHDA_UNLOCK(pDevIns, pThis);
 
 #else  /* !IN_RING3 */