Index: /trunk/src/VBox/Devices/Audio/DevHDA.cpp =================================================================== --- /trunk/src/VBox/Devices/Audio/DevHDA.cpp (revision 87318) +++ /trunk/src/VBox/Devices/Audio/DevHDA.cpp (revision 87319) @@ -536,26 +536,4 @@ - - -/** - * Retrieves the number of bytes of a FIFOW register. - * - * @return Number of bytes of a given FIFOW register. - */ -DECLINLINE(uint8_t) hdaSDFIFOWToBytes(uint32_t u32RegFIFOW) -{ - uint32_t cb; - switch (u32RegFIFOW) - { - case HDA_SDFIFOW_8B: cb = 8; break; - case HDA_SDFIFOW_16B: cb = 16; break; - case HDA_SDFIFOW_32B: cb = 32; break; - default: cb = 0; break; - } - - Assert(RT_IS_POWER_OF_TWO(cb)); - return cb; -} - #ifdef IN_RING3 /** @@ -1563,5 +1541,5 @@ } - uint32_t u32FIFOW = 0; + uint32_t u16FIFOW = 0; switch (u32Value) { @@ -1569,16 +1547,16 @@ case HDA_SDFIFOW_16B: case HDA_SDFIFOW_32B: - u32FIFOW = u32Value; + u16FIFOW = u32Value; break; default: ASSERT_GUEST_LOGREL_MSG_FAILED(("Guest tried writing unsupported FIFOW (0x%zx) to stream #%RU8, defaulting to 32 bytes\n", u32Value, idxStream)); - u32FIFOW = HDA_SDFIFOW_32B; + u16FIFOW = HDA_SDFIFOW_32B; break; } - pThis->aStreams[idxStream].u16FIFOW = hdaSDFIFOWToBytes(u32FIFOW); - LogFunc(("[SD%zu] Updating FIFOW to %u bytes\n", idxStream, pThis->aStreams[idxStream].u16FIFOW)); - return hdaRegWriteU16(pDevIns, pThis, iReg, u32FIFOW); + pThis->aStreams[idxStream].u8FIFOW = hdaSDFIFOWToBytes(u16FIFOW); + LogFunc(("[SD%zu] Updating FIFOW to %RU8 bytes\n", idxStream, pThis->aStreams[idxStream].u8FIFOW)); + return hdaRegWriteU16(pDevIns, pThis, iReg, u16FIFOW); } Index: /trunk/src/VBox/Devices/Audio/DevHDACommon.cpp =================================================================== --- /trunk/src/VBox/Devices/Audio/DevHDACommon.cpp (revision 87318) +++ /trunk/src/VBox/Devices/Audio/DevHDACommon.cpp (revision 87319) @@ -79,4 +79,27 @@ pThis->u8IRQL = 0; } +} + +/** + * Retrieves the number of bytes of a FIFOW register. + * + * @return Number of bytes of a given FIFOW register. + * @param u16RegFIFOS FIFOW register to convert. + */ +uint8_t hdaSDFIFOWToBytes(uint16_t u16RegFIFOW) +{ + uint32_t cb; + switch (u16RegFIFOW) + { + case HDA_SDFIFOW_8B: cb = 8; break; + case HDA_SDFIFOW_16B: cb = 16; break; + case HDA_SDFIFOW_32B: cb = 32; break; + default: + AssertFailedStmt(cb = 32); /* Paranoia. */ + break; + } + + Assert(RT_IS_POWER_OF_TWO(cb)); + return cb; } @@ -334,5 +357,5 @@ while (cbLeft) { - uint32_t cbChunk = RT_MIN(cbLeft, pStreamShared->u16FIFOS); + uint32_t cbChunk = RT_MIN(cbLeft, pStreamShared->u8FIFOS); rc = PDMDevHlpPhysRead(pDevIns, GCPhysChunk, (uint8_t *)pvBuf + cbReadTotal, cbChunk); @@ -409,5 +432,5 @@ while (cbLeft) { - uint32_t cbChunk = RT_MIN(cbLeft, pStreamShared->u16FIFOS); + uint32_t cbChunk = RT_MIN(cbLeft, pStreamShared->u8FIFOS); /* Sanity checks. */ Index: /trunk/src/VBox/Devices/Audio/HDAStream.cpp =================================================================== --- /trunk/src/VBox/Devices/Audio/HDAStream.cpp (revision 87318) +++ /trunk/src/VBox/Devices/Audio/HDAStream.cpp (revision 87319) @@ -242,5 +242,6 @@ const uint16_t u16LVI = HDA_STREAM_REG(pThis, LVI, uSD); const uint32_t u32CBL = HDA_STREAM_REG(pThis, CBL, uSD); - const uint16_t u16FIFOS = HDA_STREAM_REG(pThis, FIFOS, uSD) + 1; + const uint8_t u8FIFOS = HDA_STREAM_REG(pThis, FIFOS, uSD) + 1; + uint8_t u8FIFOW = hdaSDFIFOWToBytes(HDA_STREAM_REG(pThis, FIFOW, uSD)); const uint16_t u16FMT = HDA_STREAM_REG(pThis, FMT, uSD); @@ -250,5 +251,6 @@ || !u16LVI || !u32CBL - || !u16FIFOS + || !u8FIFOS + || !u8FIFOW || !u16FMT) { @@ -326,5 +328,6 @@ && u16LVI == pStreamShared->u16LVI && u32CBL == pStreamShared->u32CBL - && u16FIFOS == pStreamShared->u16FIFOS + && u8FIFOS == pStreamShared->u8FIFOS + && u8FIFOW == pStreamShared->u8FIFOW && u16FMT == pStreamShared->u16FMT) { @@ -332,4 +335,9 @@ return VINF_NO_CHANGE; } + + /* Make sure the guest behaves regarding the stream's FIFO. */ + ASSERT_GUEST_LOGREL_MSG_STMT(u8FIFOW <= u8FIFOS, + ("Guest tried setting a bigger FIFOW (%RU8) than FIFOS (%RU8), limiting\n", u8FIFOW, u8FIFOS), + u8FIFOW = u8FIFOS /* ASSUMES that u8FIFOS has been validated. */); pStreamShared->u8SD = uSD; @@ -339,5 +347,6 @@ pStreamShared->u16LVI = u16LVI; pStreamShared->u32CBL = u32CBL; - pStreamShared->u16FIFOS = u16FIFOS; + pStreamShared->u8FIFOS = u8FIFOS; + pStreamShared->u8FIFOW = u8FIFOW; pStreamShared->u16FMT = u16FMT; @@ -387,6 +396,6 @@ pCfg->Device.cMsSchedulingHint = 1000 /* ms */ / pStreamShared->State.uTimerHz; - LogFunc(("[SD%RU8] DMA @ 0x%x (%RU32 bytes), LVI=%RU16, FIFOS=%RU16\n", - uSD, pStreamShared->u64BDLBase, pStreamShared->u32CBL, pStreamShared->u16LVI, pStreamShared->u16FIFOS)); + LogFunc(("[SD%RU8] DMA @ 0x%x (%RU32 bytes), LVI=%RU16, FIFOS=%RU8\n", + uSD, pStreamShared->u64BDLBase, pStreamShared->u32CBL, pStreamShared->u16LVI, pStreamShared->u8FIFOS)); if (RT_SUCCESS(rc)) @@ -987,5 +996,5 @@ Assert(pStreamShared->u64BDLBase); Assert(pStreamShared->u32CBL); - Assert(pStreamShared->u16FIFOS); + Assert(pStreamShared->u8FIFOS); /* State sanity checks. */ @@ -1021,5 +1030,5 @@ { /* Limit the chunk to the stream's FIFO size and what's left to process. */ - uint32_t cbChunk = RT_MIN(cbLeft, pStreamShared->u16FIFOS); + uint32_t cbChunk = RT_MIN(cbLeft, pStreamShared->u8FIFOS); /* Limit the chunk to the remaining data of the current BDLE. */ @@ -1238,4 +1247,9 @@ if (cbDMA) { + const size_t cbCircBufUsed = RTCircBufUsed(pCircBuf); + + Log3Func(("[SD%RU8] cbDMA=%RU32, cbUsed=%zu, uFIFOW=%RU8, uFIFOS=%RU8\n", + uSD, cbDMA, cbCircBufUsed, pStreamShared->u8FIFOW, pStreamShared->u8FIFOS)); + /* We always increment the position of DMA buffer counter because we're always reading * into an intermediate DMA buffer. */ @@ -1253,14 +1267,19 @@ } - /* - * Update the stream's current position. - * Do this as accurate and close to the actual data transfer as possible. - * All guetsts rely on this, depending on the mechanism they use (LPIB register or DMA counters). - */ - uint32_t cbStreamPos = hdaR3StreamGetPosition(pThis, pStreamShared); - if (cbStreamPos == pStreamShared->u32CBL) - cbStreamPos = 0; - - hdaR3StreamSetPosition(pStreamShared, pDevIns, pThis, cbStreamPos + cbDMA); + /* Only set the new DMA position if we at least reached the stream's FIFO watermark. + * This by default is 32 bytes. */ + if (cbCircBufUsed >= pStreamShared->u8FIFOW) + { + /* + * Update the stream's current position. + * Do this as accurate and close to the actual data transfer as possible. + * All guetsts rely on this, depending on the mechanism they use (LPIB register or DMA counters). + */ + uint32_t cbStreamPos = hdaR3StreamGetPosition(pThis, pStreamShared); + if (cbStreamPos == pStreamShared->u32CBL) + cbStreamPos = 0; + + hdaR3StreamSetPosition(pStreamShared, pDevIns, pThis, cbStreamPos + cbDMA); + } } Index: /trunk/src/VBox/Devices/Audio/HDAStream.h =================================================================== --- /trunk/src/VBox/Devices/Audio/HDAStream.h (revision 87318) +++ /trunk/src/VBox/Devices/Audio/HDAStream.h (revision 87319) @@ -203,12 +203,13 @@ * Will be updated in hdaR3StreamInit(). */ uint16_t u16FMT; - /** FIFO Size (FIFOS). + /** FIFO Size (checked + translated in bytes, FIFOS). * Maximum number of bytes that may have been DMA'd into * memory but not yet transmitted on the link. * * Will be updated in hdaR3StreamInit(). */ - uint16_t u16FIFOS; - /** FIFO Watermark. */ - uint16_t u16FIFOW; + uint8_t u8FIFOS; + /** FIFO Watermark (checked + translated in bytes, FIFOW). */ + uint8_t u8FIFOW; + uint8_t abPadding1[2]; /** FIFO scratch buffer, to avoid intermediate (re-)allocations. */ uint8_t abFIFO[HDA_FIFO_MAX + 1]; @@ -216,5 +217,4 @@ * Will be updated in hdaR3StreamInit(). */ uint16_t u16LVI; - uint16_t au16Padding1[2]; /** The timer for pumping data thru the attached LUN drivers. */ TMTIMERHANDLE hTimer;