Index: /trunk/src/VBox/Devices/Audio/DevHDA.cpp =================================================================== --- /trunk/src/VBox/Devices/Audio/DevHDA.cpp (revision 82322) +++ /trunk/src/VBox/Devices/Audio/DevHDA.cpp (revision 82323) @@ -155,47 +155,16 @@ * Acquires the TM lock and HDA lock, returns on failure. */ -#define DEVHDA_LOCK_BOTH_RETURN_VOID(a_pDevIns, a_pThis, a_SD) \ - do { \ - int rcLock = TMTimerLock((a_pThis)->pTimer[a_SD], VERR_IGNORED); \ - if (rcLock == VINF_SUCCESS) \ - { /* likely */ } \ - else \ - { \ - AssertRC(rcLock); \ - return; \ - } \ - rcLock = PDMDevHlpCritSectEnter((a_pDevIns), &(a_pThis)->CritSect, VERR_IGNORED); \ - if (rcLock == VINF_SUCCESS) \ - { /* likely */ } \ - else \ - { \ - AssertRC(rcLock); \ - TMTimerUnlock((a_pThis)->pTimer[a_SD]); \ - return; \ - } \ - } while (0) - -/** - * Acquires the TM lock and HDA lock, returns on failure. - */ #define DEVHDA_LOCK_BOTH_RETURN(a_pDevIns, a_pThis, a_SD, a_rcBusy) \ do { \ int rcLock = TMTimerLock((a_pThis)->pTimer[a_SD], (a_rcBusy)); \ if (rcLock == VINF_SUCCESS) \ - { /* likely */ } \ - else \ { \ - AssertRC(rcLock); \ - return rcLock; \ + rcLock = PDMDevHlpCritSectEnter((a_pDevIns), &(a_pThis)->CritSect, (a_rcBusy)); \ + if (rcLock == VINF_SUCCESS) \ + break; \ + TMTimerUnlock((a_pThis)->pTimer[a_SD]); \ } \ - rcLock = PDMDevHlpCritSectEnter((a_pDevIns), &(a_pThis)->CritSect, (a_rcBusy)); \ - if (rcLock == VINF_SUCCESS) \ - { /* likely */ } \ - else \ - { \ - AssertRC(rcLock); \ - TMTimerUnlock((a_pThis)->pTimer[a_SD]); \ - return rcLock; \ - } \ + AssertRC(rcLock); \ + return rcLock; \ } while (0) @@ -1312,6 +1281,4 @@ const uint8_t uSD = HDA_SD_NUM_FROM_REG(pThis, CTL, iReg); - DEVHDA_LOCK_BOTH_RETURN(pDevIns, pThis, uSD, VINF_IOM_R3_MMIO_WRITE); - /* * Some guests write too much (that is, 32-bit with the top 8 bit being junk) @@ -1319,13 +1286,4 @@ */ u32Value &= 0x00ffffff; - - const bool fRun = RT_BOOL(u32Value & HDA_SDCTL_RUN); - const bool fInRun = RT_BOOL(HDA_REG_IND(pThis, iReg) & HDA_SDCTL_RUN); - - const bool fReset = RT_BOOL(u32Value & HDA_SDCTL_SRST); - const bool fInReset = RT_BOOL(HDA_REG_IND(pThis, iReg) & HDA_SDCTL_SRST); - - /*LogFunc(("[SD%RU8] fRun=%RTbool, fInRun=%RTbool, fReset=%RTbool, fInReset=%RTbool, %R[sdctl]\n", - uSD, fRun, fInRun, fReset, fInReset, u32Value));*/ /* @@ -1337,21 +1295,26 @@ */ uint8_t uTag = (u32Value >> HDA_SDCTL_NUM_SHIFT) & HDA_SDCTL_NUM_MASK; - if (uTag > HDA_MAX_TAGS) - { - LogFunc(("[SD%RU8] Warning: Invalid stream tag %RU8 specified!\n", uSD, uTag)); - - DEVHDA_UNLOCK_BOTH(pDevIns, pThis, uSD); - return VINF_SUCCESS; /* Always return success to the MMIO handler. */ - } + ASSERT_GUEST_MSG_RETURN(uTag < RT_ELEMENTS(pThis->aTags), + ("SD%RU8: Invalid stream tag %RU8 (u32Value=%#x)!\n", uSD, uTag, u32Value), + VINF_SUCCESS /* Always return success to the MMIO handler. */); PHDASTREAM pStream = hdaGetStreamFromSD(pThis, uSD); - if (!pStream) - { - ASSERT_GUEST_LOGREL_MSG_FAILED(("Guest tried writing SDCTL (0x%x) to unhandled stream #%RU8\n", u32Value, uSD)); - - DEVHDA_UNLOCK_BOTH(pDevIns, pThis, uSD); - return VINF_SUCCESS; /* Always return success to the MMIO handler. */ - } - + ASSERT_GUEST_LOGREL_MSG_RETURN(pStream, ("Guest tried writing SDCTL (0x%x) to unhandled stream #%RU8\n", u32Value, uSD), + VINF_SUCCESS /* Always return success to the MMIO handler. */); + + const bool fRun = RT_BOOL(u32Value & HDA_SDCTL_RUN); + const bool fReset = RT_BOOL(u32Value & HDA_SDCTL_SRST); + + /** + * @todo r=bird: Must reduce the time we holding the virtual sync + * clock lock here! + */ + DEVHDA_LOCK_BOTH_RETURN(pDevIns, pThis, uSD, VINF_IOM_R3_MMIO_WRITE); + + const bool fInRun = RT_BOOL(HDA_REG_IND(pThis, iReg) & HDA_SDCTL_RUN); + const bool fInReset = RT_BOOL(HDA_REG_IND(pThis, iReg) & HDA_SDCTL_SRST); + + /*LogFunc(("[SD%RU8] fRun=%RTbool, fInRun=%RTbool, fReset=%RTbool, fInReset=%RTbool, %R[sdctl]\n", + uSD, fRun, fInRun, fReset, fInReset, u32Value));*/ if (fInReset) { @@ -1514,14 +1477,13 @@ #ifdef IN_RING3 const uint8_t uSD = HDA_SD_NUM_FROM_REG(pThis, STS, iReg); - + PHDASTREAM pStream = hdaGetStreamFromSD(pThis, uSD); + ASSERT_GUEST_LOGREL_MSG_RETURN(pStream, ("Guest tried writing SDSTS (0x%x) to unhandled stream #%RU8\n", u32Value, uSD), + VINF_SUCCESS); + + /** + * @todo r=bird: Must reduce the time we holding the virtual sync + * clock lock here! + */ DEVHDA_LOCK_BOTH_RETURN(pDevIns, pThis, uSD, VINF_IOM_R3_MMIO_WRITE); - - PHDASTREAM pStream = hdaGetStreamFromSD(pThis, uSD); - if (!pStream) - { - ASSERT_GUEST_LOGREL_MSG_FAILED(("Guest tried writing SDSTS (0x%x) to unhandled stream #%RU8\n", u32Value, uSD)); - DEVHDA_UNLOCK_BOTH(pDevIns, pThis, uSD); - return VINF_SUCCESS; /* Always return success to the MMIO handler. */ - } hdaR3StreamLock(pStream); @@ -2891,12 +2853,11 @@ static DECLCALLBACK(void) hdaR3Timer(PPDMDEVINS pDevIns, PTMTIMER pTimer, void *pvUser) { + PHDASTATE pThis = PDMDEVINS_2_DATA(pDevIns, PHDASTATE); + PHDASTREAM pStream = (PHDASTREAM)pvUser; RT_NOREF(pTimer); - PHDASTREAM pStream = (PHDASTREAM)pvUser; AssertPtr(pStream); - - PHDASTATE pThis = pStream->pHDAState; - - DEVHDA_LOCK_BOTH_RETURN_VOID(pDevIns, pStream->pHDAState, pStream->u8SD); + Assert(PDMDevHlpCritSectIsOwner(pDevIns, &pThis->CritSect)); + Assert(TMTimerIsLockOwner(pStream->pTimer)); hdaR3StreamUpdate(pStream, true /* fInTimer */); @@ -2919,6 +2880,4 @@ else Log3Func(("fSinksActive=%RTbool\n", fSinkActive)); - - DEVHDA_UNLOCK_BOTH(pDevIns, pThis, pStream->u8SD); } @@ -5236,5 +5195,8 @@ PHDASTATE pThis = PDMDEVINS_2_DATA(pDevIns, PHDASTATE); - int rc = PDMDevHlpMmioSetUpContext(pDevIns, pThis->hMmio, hdaMMIOWrite, hdaMMIORead, NULL /*pvUser*/); + int rc = PDMDevHlpSetDeviceCritSect(pDevIns, PDMDevHlpCritSectGetNop(pDevIns)); + AssertRCReturn(rc, rc); + + rc = PDMDevHlpMmioSetUpContext(pDevIns, pThis->hMmio, hdaMMIOWrite, hdaMMIORead, NULL /*pvUser*/); AssertRCReturn(rc, rc); Index: /trunk/src/VBox/Devices/Audio/DevHDACommon.cpp =================================================================== --- /trunk/src/VBox/Devices/Audio/DevHDACommon.cpp (revision 82322) +++ /trunk/src/VBox/Devices/Audio/DevHDACommon.cpp (revision 82323) @@ -256,13 +256,6 @@ PHDASTREAM hdaGetStreamFromSD(PHDASTATE pThis, uint8_t uSD) { - AssertPtrReturn(pThis, NULL); - AssertReturn(uSD < HDA_MAX_STREAMS, NULL); - - if (uSD >= HDA_MAX_STREAMS) - { - ASSERT_GUEST_LOGREL_MSG_FAILED(("Stream #%RU8 is invalid\n", uSD)); - return NULL; - } - + AssertPtr(pThis); + ASSERT_GUEST_MSG_RETURN(uSD < HDA_MAX_STREAMS, ("uSD=%u (%#x)\n", uSD, uSD), NULL); return &pThis->aStreams[uSD]; } Index: /trunk/src/VBox/Devices/Audio/DevIchAc97.cpp =================================================================== --- /trunk/src/VBox/Devices/Audio/DevIchAc97.cpp (revision 82322) +++ /trunk/src/VBox/Devices/Audio/DevIchAc97.cpp (revision 82323) @@ -549,7 +549,7 @@ * Acquires the AC'97 lock. */ -#define DEVAC97_LOCK(a_pThis) \ +#define DEVAC97_LOCK(a_pDevIns, a_pThis) \ do { \ - int rcLock = PDMCritSectEnter(&(a_pThis)->CritSect, VERR_IGNORED); \ + int rcLock = PDMDevHlpCritSectEnter((a_pDevIns), &(a_pThis)->CritSect, VERR_IGNORED); \ AssertRC(rcLock); \ } while (0) @@ -558,79 +558,37 @@ * Acquires the AC'97 lock or returns. */ -# define DEVAC97_LOCK_RETURN(a_pThis, a_rcBusy) \ +# define DEVAC97_LOCK_RETURN(a_pDevIns, a_pThis, a_rcBusy) \ do { \ - int rcLock = PDMCritSectEnter(&(a_pThis)->CritSect, a_rcBusy); \ - if (rcLock != VINF_SUCCESS) \ - { \ - AssertRC(rcLock); \ - return rcLock; \ - } \ + int rcLock = PDMDevHlpCritSectEnter((a_pDevIns), &(a_pThis)->CritSect, a_rcBusy); \ + if (rcLock == VINF_SUCCESS) \ + break; \ + AssertRC(rcLock); \ + return rcLock; \ } while (0) -/** - * Acquires the AC'97 lock or returns. - */ -# define DEVAC97_LOCK_RETURN_VOID(a_pThis) \ - do { \ - int rcLock = PDMCritSectEnter(&(a_pThis)->CritSect, VERR_IGNORED); \ - if (rcLock != VINF_SUCCESS) \ - { \ - AssertRC(rcLock); \ - return; \ - } \ - } while (0) - -#ifdef IN_RC /** Retrieves an attribute from a specific audio stream in RC. */ -# define DEVAC97_CTX_SUFF_SD(a_Var, a_SD) a_Var##RC[a_SD] -#elif defined(IN_RING0) -/** Retrieves an attribute from a specific audio stream in R0. */ -# define DEVAC97_CTX_SUFF_SD(a_Var, a_SD) a_Var##R0[a_SD] -#else -/** Retrieves an attribute from a specific audio stream in R3. */ -# define DEVAC97_CTX_SUFF_SD(a_Var, a_SD) a_Var##R3[a_SD] -#endif +#define DEVAC97_CTX_SUFF_SD(a_Var, a_SD) CTX_SUFF(a_Var)[a_SD] /** * Releases the AC'97 lock. */ -#define DEVAC97_UNLOCK(a_pThis) \ - do { PDMCritSectLeave(&(a_pThis)->CritSect); } while (0) +#define DEVAC97_UNLOCK(a_pDevIns, a_pThis) \ + do { PDMDevHlpCritSectLeave((a_pDevIns), &(a_pThis)->CritSect); } while (0) /** * Acquires the TM lock and AC'97 lock, returns on failure. */ -#define DEVAC97_LOCK_BOTH_RETURN_VOID(a_pThis, a_SD) \ - do { \ - int rcLock = TMTimerLock((a_pThis)->DEVAC97_CTX_SUFF_SD(pTimer, a_SD), VERR_IGNORED); \ - if (rcLock != VINF_SUCCESS) \ - { \ - AssertRC(rcLock); \ - return; \ - } \ - rcLock = PDMCritSectEnter(&(a_pThis)->CritSect, VERR_IGNORED); \ - if (rcLock != VINF_SUCCESS) \ - { \ - AssertRC(rcLock); \ - TMTimerUnlock((a_pThis)->DEVAC97_CTX_SUFF_SD(pTimer, a_SD)); \ - return; \ - } \ - } while (0) - -/** - * Acquires the TM lock and AC'97 lock, returns on failure. - */ -#define DEVAC97_LOCK_BOTH_RETURN(a_pThis, a_SD, a_rcBusy) \ +#define DEVAC97_LOCK_BOTH_RETURN(a_pDevIns, a_pThis, a_SD, a_rcBusy) \ do { \ int rcLock = TMTimerLock((a_pThis)->DEVAC97_CTX_SUFF_SD(pTimer, a_SD), (a_rcBusy)); \ - if (rcLock != VINF_SUCCESS) \ - return rcLock; \ - rcLock = PDMCritSectEnter(&(a_pThis)->CritSect, (a_rcBusy)); \ - if (rcLock != VINF_SUCCESS) \ + if (rcLock == VINF_SUCCESS) \ { \ - AssertRC(rcLock); \ + rcLock = PDMDevHlpCritSectEnter((a_pDevIns), &(a_pThis)->CritSect, (a_rcBusy)); \ + if (rcLock == VINF_SUCCESS) \ + break; \ TMTimerUnlock((a_pThis)->DEVAC97_CTX_SUFF_SD(pTimer, a_SD)); \ - return rcLock; \ } \ + AssertRC(rcLock); \ + return rcLock; \ } while (0) @@ -638,7 +596,7 @@ * Releases the AC'97 lock and TM lock. */ -#define DEVAC97_UNLOCK_BOTH(a_pThis, a_SD) \ +#define DEVAC97_UNLOCK_BOTH(a_pDevIns, a_pThis, a_SD) \ do { \ - PDMCritSectLeave(&(a_pThis)->CritSect); \ + PDMDevHlpCritSectLeave((a_pDevIns), &(a_pThis)->CritSect); \ TMTimerUnlock((a_pThis)->DEVAC97_CTX_SUFF_SD(pTimer, a_SD)); \ } while (0) @@ -2629,45 +2587,33 @@ /** - * Timer callback which handles the audio data transfers on a periodic basis. - * - * @param pDevIns Device instance. - * @param pTimer Timer which was used when calling this. - * @param pvUser User argument as PAC97STATE. + * @callback_method_impl{FNTMTIMERDEV, + * Timer callback which handles the audio data transfers on a periodic basis.} */ static DECLCALLBACK(void) ichac97R3Timer(PPDMDEVINS pDevIns, PTMTIMER pTimer, void *pvUser) { - RT_NOREF(pDevIns, pTimer); - + PAC97STATE pThis = PDMDEVINS_2_DATA(pDevIns, PAC97STATE); + STAM_PROFILE_START(&pThis->StatTimer, a); PAC97STREAM pStream = (PAC97STREAM)pvUser; + RT_NOREF(pTimer); + AssertPtr(pStream); - - PAC97STATE pThis = pStream->pAC97State; - AssertPtr(pThis); - - STAM_PROFILE_START(&pThis->StatTimer, a); - - DEVAC97_LOCK_BOTH_RETURN_VOID(pThis, pStream->u8SD); + Assert(PDMDevHlpCritSectIsOwner(pDevIns, &pThis->CritSect)); + Assert(TMTimerIsLockOwner((pThis)->DEVAC97_CTX_SUFF_SD(pTimer, pStream->u8SD))); ichac97R3StreamUpdate(pThis, pStream, true /* fInTimer */); PAUDMIXSINK pSink = ichac97R3IndexToSink(pThis, pStream->u8SD); - - bool fSinkActive = false; - if (pSink) - fSinkActive = AudioMixerSinkIsActive(pSink); - - if (fSinkActive) + if (pSink && AudioMixerSinkIsActive(pSink)) { ichac97R3StreamTransferUpdate(pThis, pStream, pStream->Regs.picb << 1); /** @todo r=andy Assumes 16-bit samples. */ - ichac97TimerSet(pThis,pStream, + ichac97TimerSet(pThis, pStream, TMTimerGet((pThis)->DEVAC97_CTX_SUFF_SD(pTimer, pStream->u8SD)) + pStream->State.cTransferTicks, false /* fForce */); } - DEVAC97_UNLOCK_BOTH(pThis, pStream->u8SD); - STAM_PROFILE_STOP(&pThis->StatTimer, a); } + #endif /* IN_RING3 */ @@ -2940,5 +2886,5 @@ RT_NOREF(pvUser); - DEVAC97_LOCK_RETURN(pThis, VINF_IOM_R3_IOPORT_READ); + DEVAC97_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_IOPORT_READ); /* Get the index of the NABMBAR port. */ @@ -3091,5 +3037,5 @@ } - DEVAC97_UNLOCK(pThis); + DEVAC97_UNLOCK(pDevIns, pThis); return rc; @@ -3113,5 +3059,5 @@ pRegs = &pStream->Regs; - DEVAC97_LOCK_BOTH_RETURN(pThis, pStream->u8SD, VINF_IOM_R3_IOPORT_WRITE); + DEVAC97_LOCK_BOTH_RETURN(pDevIns, pThis, pStream->u8SD, VINF_IOM_R3_IOPORT_WRITE); } @@ -3288,5 +3234,5 @@ if (pStream) - DEVAC97_UNLOCK_BOTH(pThis, pStream->u8SD); + DEVAC97_UNLOCK_BOTH(pDevIns, pThis, pStream->u8SD); return rc; @@ -3301,11 +3247,9 @@ PAC97STATE pThis = PDMDEVINS_2_DATA(pDevIns, PAC97STATE); RT_NOREF(pvUser); - - DEVAC97_LOCK_RETURN(pThis, VINF_IOM_R3_IOPORT_READ); + Assert(offPort < 256); + + DEVAC97_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_IOPORT_READ); VBOXSTRICTRC rc = VINF_SUCCESS; - - Assert(offPort < 256); - switch (cb) { @@ -3340,5 +3284,5 @@ } - DEVAC97_UNLOCK(pThis); + DEVAC97_UNLOCK(pDevIns, pThis); return rc; } @@ -3353,5 +3297,5 @@ RT_NOREF(pvUser); - DEVAC97_LOCK_RETURN(pThis, VINF_IOM_R3_IOPORT_WRITE); + DEVAC97_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_IOPORT_WRITE); VBOXSTRICTRC rc = VINF_SUCCESS; @@ -3548,5 +3492,5 @@ } - DEVAC97_UNLOCK(pThis); + DEVAC97_UNLOCK(pDevIns, pThis); return rc; } @@ -3935,5 +3879,5 @@ LogFunc(("iLUN=%u, fFlags=0x%x\n", iLUN, fFlags)); - DEVAC97_LOCK(pThis); + DEVAC97_LOCK(pDevIns, pThis); PAC97DRIVER pDrv; @@ -3945,5 +3889,5 @@ LogFunc(("Failed with %Rrc\n", rc2)); - DEVAC97_UNLOCK(pThis); + DEVAC97_UNLOCK(pDevIns, pThis); return VINF_SUCCESS; @@ -3959,5 +3903,5 @@ LogFunc(("iLUN=%u, fFlags=0x%x\n", iLUN, fFlags)); - DEVAC97_LOCK(pThis); + DEVAC97_LOCK(pDevIns, pThis); PAC97DRIVER pDrv, pDrvNext; @@ -3978,5 +3922,5 @@ } - DEVAC97_UNLOCK(pThis); + DEVAC97_UNLOCK(pDevIns, pThis); } @@ -4372,5 +4316,8 @@ PAC97STATE pThis = PDMDEVINS_2_DATA(pDevIns, PAC97STATE); - int rc = PDMDevHlpIoPortSetUpContext(pDevIns, pThis->hIoPortsNam, ichac97IoPortNamWrite, ichac97IoPortNamRead, NULL /*pvUser*/); + int rc = PDMDevHlpSetDeviceCritSect(pDevIns, PDMDevHlpCritSectGetNop(pDevIns)); + AssertRCReturn(rc, rc); + + rc = PDMDevHlpIoPortSetUpContext(pDevIns, pThis->hIoPortsNam, ichac97IoPortNamWrite, ichac97IoPortNamRead, NULL /*pvUser*/); AssertRCReturn(rc, rc); rc = PDMDevHlpIoPortSetUpContext(pDevIns, pThis->hIoPortsNabm, ichac97IoPortNabmWrite, ichac97IoPortNabmRead, NULL /*pvUser*/); Index: /trunk/src/VBox/Devices/Audio/HDAStream.cpp =================================================================== --- /trunk/src/VBox/Devices/Audio/HDAStream.cpp (revision 82322) +++ /trunk/src/VBox/Devices/Audio/HDAStream.cpp (revision 82323) @@ -1375,10 +1375,6 @@ Log3Func(("[SD%RU8] Scheduling timer\n", pStream->u8SD)); - TMTimerUnlock(pStream->pTimer); - LogFunc(("Timer set SD%RU8\n", pStream->u8SD)); hdaR3TimerSet(pStream->pHDAState, pStream, tsTransferNext, false /* fForce */); - - TMTimerLock(pStream->pTimer, VINF_SUCCESS); pStream->State.tsTransferNext = tsTransferNext;