Index: /trunk/src/VBox/Devices/Audio/DevHDA.cpp =================================================================== --- /trunk/src/VBox/Devices/Audio/DevHDA.cpp (revision 81180) +++ /trunk/src/VBox/Devices/Audio/DevHDA.cpp (revision 81181) @@ -402,6 +402,6 @@ /* Offset 0x88 (SD0) */ \ { offset + 0x8, 0x00004, 0xFFFFFFFF, 0xFFFFFFFF, HDA_RD_FLAG_NONE, hdaRegReadU32 , hdaRegWriteSDCBL , HDA_REG_IDX_STRM(name, CBL) , #name " Cyclic Buffer Length" }, \ - /* Offset 0x8C (SD0) */ \ - { offset + 0xC, 0x00002, 0x0000FFFF, 0x0000FFFF, HDA_RD_FLAG_NONE, hdaRegReadU16 , hdaRegWriteSDLVI , HDA_REG_IDX_STRM(name, LVI) , #name " Last Valid Index" }, \ + /* Offset 0x8C (SD0) -- upper 8 bits are reserved */ \ + { offset + 0xC, 0x00002, 0x0000FFFF, 0x000000FF, HDA_RD_FLAG_NONE, hdaRegReadU16 , hdaRegWriteSDLVI , HDA_REG_IDX_STRM(name, LVI) , #name " Last Valid Index" }, \ /* Reserved: FIFO Watermark. ** @todo Document this! */ \ { offset + 0xE, 0x00002, 0x00000007, 0x00000007, HDA_RD_FLAG_NONE, hdaRegReadU16 , hdaRegWriteSDFIFOW, HDA_REG_IDX_STRM(name, FIFOW), #name " FIFO Watermark" }, \ @@ -1368,5 +1368,5 @@ hdaR3StreamLock(pStream); - int rc2; + int rc2 = VINF_SUCCESS; # ifdef VBOX_WITH_AUDIO_HDA_ASYNC_IO @@ -1420,36 +1420,40 @@ } - /* Enable/disable the stream. */ - rc2 = hdaR3StreamEnable(pStream, fRun /* fEnable */); - AssertRC(rc2); - - if (fRun) + if (RT_SUCCESS(rc2)) { - /* Keep track of running streams. */ - pThis->cStreamsActive++; - - /* (Re-)init the stream's period. */ - hdaR3StreamPeriodInit(&pStream->State.Period, - pStream->u8SD, pStream->u16LVI, pStream->u32CBL, &pStream->State.Cfg); - - /* Begin a new period for this stream. */ - rc2 = hdaR3StreamPeriodBegin(&pStream->State.Period, hdaWalClkGetCurrent(pThis)/* Use current wall clock time */); + /* Enable/disable the stream. */ + rc2 = hdaR3StreamEnable(pStream, fRun /* fEnable */); AssertRC(rc2); - rc2 = hdaR3TimerSet(pThis, pStream, TMTimerGet(pThis->pTimer[pStream->u8SD]) + pStream->State.cTransferTicks, false /* fForce */); - AssertRC(rc2); - } - else - { - /* Keep track of running streams. */ - Assert(pThis->cStreamsActive); - if (pThis->cStreamsActive) - pThis->cStreamsActive--; - - /* Make sure to (re-)schedule outstanding (delayed) interrupts. */ - hdaR3ReschedulePendingInterrupts(pThis); - - /* Reset the period. */ - hdaR3StreamPeriodReset(&pStream->State.Period); + if (fRun) + { + /* Keep track of running streams. */ + pThis->cStreamsActive++; + + /* (Re-)init the stream's period. */ + hdaR3StreamPeriodInit(&pStream->State.Period, + pStream->u8SD, pStream->u16LVI, pStream->u32CBL, &pStream->State.Cfg); + + /* Begin a new period for this stream. */ + rc2 = hdaR3StreamPeriodBegin(&pStream->State.Period, hdaWalClkGetCurrent(pThis)/* Use current wall clock time */); + AssertRC(rc2); + + rc2 = hdaR3TimerSet(pThis, pStream, TMTimerGet(pThis->pTimer[pStream->u8SD]) + pStream->State.cTransferTicks, + false /* fForce */); + AssertRC(rc2); + } + else + { + /* Keep track of running streams. */ + Assert(pThis->cStreamsActive); + if (pThis->cStreamsActive) + pThis->cStreamsActive--; + + /* Make sure to (re-)schedule outstanding (delayed) interrupts. */ + hdaR3ReschedulePendingInterrupts(pThis); + + /* Reset the period. */ + hdaR3StreamPeriodReset(&pStream->State.Period); + } } @@ -1597,7 +1601,7 @@ DEVHDA_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_WRITE); + const uint8_t uSD = HDA_SD_NUM_FROM_REG(pThis, LVI, iReg); + #ifdef HDA_USE_DMA_ACCESS_HANDLER - uint8_t uSD = HDA_SD_NUM_FROM_REG(pThis, LVI, iReg); - if (hdaGetDirFromSD(uSD) == PDMAUDIODIR_OUT) { @@ -1613,4 +1617,7 @@ } #endif + + ASSERT_GUEST_LOGREL_MSG(u32Value <= UINT8_MAX, /* Should be covered by the register write mask, but just to make sure. */ + ("LVI for stream #%RU8 must not be bigger than %RU8\n", uSD, UINT8_MAX - 1)); int rc2 = hdaRegWriteU16(pDevIns, pThis, iReg, u32Value); Index: /trunk/src/VBox/Devices/Audio/HDAStream.cpp =================================================================== --- /trunk/src/VBox/Devices/Audio/HDAStream.cpp (revision 81180) +++ /trunk/src/VBox/Devices/Audio/HDAStream.cpp (revision 81181) @@ -26,4 +26,5 @@ #include +#include #include #include @@ -239,4 +240,8 @@ AssertRCReturn(rc, rc); + ASSERT_GUEST_LOGREL_MSG_RETURN(u32CBL % pStream->State.Mapping.cbFrameSize == 0, + ("CBL for stream #%RU8 does not align to frame size\n", pStream->u8SD), + VERR_INVALID_PARAMETER); + /* * Set the stream's timer Hz rate, based on the stream channel count. @@ -344,8 +349,4 @@ pStream->u8SD, pStream->u64BDLBase, pStream->u32CBL, pStream->u16LVI, pStream->u16FIFOS)); - /* Make sure that mandatory parameters are set up correctly. */ - AssertStmt(pStream->u32CBL % pStream->State.Mapping.cbFrameSize == 0, rc = VERR_INVALID_PARAMETER); - AssertStmt(pStream->u16LVI >= 1, rc = VERR_INVALID_PARAMETER); - if (RT_SUCCESS(rc)) { @@ -357,5 +358,4 @@ /* Figure out how many transfer fragments we're going to use for this stream. */ /** @todo Use a more dynamic fragment size? */ - Assert(pStream->u16LVI <= UINT8_MAX - 1); uint8_t cFragments = pStream->u16LVI + 1; if (cFragments <= 1) @@ -446,36 +446,45 @@ Assert(pStream->State.cbTransferSize); Assert(pStream->State.cbTransferSize % pStream->State.Mapping.cbFrameSize == 0); - - /* Calculate the bytes we need to transfer to / from the stream's DMA per iteration. - * This is bound to the device's Hz rate and thus to the (virtual) timing the device expects. */ - pStream->State.cbTransferChunk = (pStream->State.Cfg.Props.uHz / pStream->State.uTimerHz) * pStream->State.Mapping.cbFrameSize; - Assert(pStream->State.cbTransferChunk); - Assert(pStream->State.cbTransferChunk % pStream->State.Mapping.cbFrameSize == 0); - - /* Make sure that the transfer chunk does not exceed the overall transfer size. */ - if (pStream->State.cbTransferChunk > pStream->State.cbTransferSize) - pStream->State.cbTransferChunk = pStream->State.cbTransferSize; - - const uint64_t cTicksPerHz = TMTimerGetFreq(pStream->pTimer) / pStream->State.uTimerHz; - - /* Calculate the timer ticks per byte for this stream. */ - pStream->State.cTicksPerByte = cTicksPerHz / pStream->State.cbTransferChunk; - Assert(pStream->State.cTicksPerByte); - - /* Calculate timer ticks per transfer. */ - pStream->State.cTransferTicks = pStream->State.cbTransferChunk * pStream->State.cTicksPerByte; - Assert(pStream->State.cTransferTicks); - - LogFunc(("[SD%RU8] Timer %uHz (%RU64 ticks per Hz), cTicksPerByte=%RU64, cbTransferChunk=%RU32, cTransferTicks=%RU64, " \ - "cbTransferSize=%RU32\n", - pStream->u8SD, pStream->State.uTimerHz, cTicksPerHz, pStream->State.cTicksPerByte, - pStream->State.cbTransferChunk, pStream->State.cTransferTicks, pStream->State.cbTransferSize)); - - /* Make sure to also update the stream's DMA counter (based on its current LPIB value). */ - hdaR3StreamSetPosition(pStream, HDA_STREAM_REG(pThis, LPIB, pStream->u8SD)); + ASSERT_GUEST_LOGREL_MSG_STMT(pStream->State.cbTransferSize, + ("Transfer size for stream #%RU8 is invalid\n", pStream->u8SD), rc = VERR_INVALID_PARAMETER); + if (RT_SUCCESS(rc)) + { + /* Calculate the bytes we need to transfer to / from the stream's DMA per iteration. + * This is bound to the device's Hz rate and thus to the (virtual) timing the device expects. */ + pStream->State.cbTransferChunk = (pStream->State.Cfg.Props.uHz / pStream->State.uTimerHz) * pStream->State.Mapping.cbFrameSize; + Assert(pStream->State.cbTransferChunk); + Assert(pStream->State.cbTransferChunk % pStream->State.Mapping.cbFrameSize == 0); + ASSERT_GUEST_LOGREL_MSG_STMT(pStream->State.cbTransferChunk, + ("Transfer chunk for stream #%RU8 is invalid\n", pStream->u8SD), + rc = VERR_INVALID_PARAMETER); + if (RT_SUCCESS(rc)) + { + /* Make sure that the transfer chunk does not exceed the overall transfer size. */ + if (pStream->State.cbTransferChunk > pStream->State.cbTransferSize) + pStream->State.cbTransferChunk = pStream->State.cbTransferSize; + + const uint64_t cTicksPerHz = TMTimerGetFreq(pStream->pTimer) / pStream->State.uTimerHz; + + /* Calculate the timer ticks per byte for this stream. */ + pStream->State.cTicksPerByte = cTicksPerHz / pStream->State.cbTransferChunk; + Assert(pStream->State.cTicksPerByte); + + /* Calculate timer ticks per transfer. */ + pStream->State.cTransferTicks = pStream->State.cbTransferChunk * pStream->State.cTicksPerByte; + Assert(pStream->State.cTransferTicks); + + LogFunc(("[SD%RU8] Timer %uHz (%RU64 ticks per Hz), cTicksPerByte=%RU64, cbTransferChunk=%RU32, " \ + "cTransferTicks=%RU64, cbTransferSize=%RU32\n", + pStream->u8SD, pStream->State.uTimerHz, cTicksPerHz, pStream->State.cTicksPerByte, + pStream->State.cbTransferChunk, pStream->State.cTransferTicks, pStream->State.cbTransferSize)); + + /* Make sure to also update the stream's DMA counter (based on its current LPIB value). */ + hdaR3StreamSetPosition(pStream, HDA_STREAM_REG(pThis, LPIB, pStream->u8SD)); #ifdef LOG_ENABLED - hdaR3BDLEDumpAll(pThis, pStream->u64BDLBase, pStream->u16LVI + 1); + hdaR3BDLEDumpAll(pThis, pStream->u64BDLBase, pStream->u16LVI + 1); #endif + } + } }