Index: /trunk/src/VBox/Devices/Audio/DevHDA.cpp =================================================================== --- /trunk/src/VBox/Devices/Audio/DevHDA.cpp (revision 82398) +++ /trunk/src/VBox/Devices/Audio/DevHDA.cpp (revision 82399) @@ -3011,75 +3011,87 @@ Assert(cb == 4); Assert((off & 3) == 0); - DEVHDA_LOCK_RETURN(pDevIns, pThis, VINF_IOM_R3_MMIO_READ); - - if (!(HDA_REG(pThis, GCTL) & HDA_GCTL_CRST) && idxRegDsc != HDA_REG_GCTL) - LogFunc(("Access to registers except GCTL is blocked while resetting\n")); - - if (idxRegDsc >= 0) - { - /* ASSUMES gapless DWORD at end of map. */ - if (g_aHdaRegMap[idxRegDsc].size == 4) - { - /* - * Straight forward DWORD access. - */ - rc = g_aHdaRegMap[idxRegDsc].pfnRead(pDevIns, pThis, idxRegDsc, (uint32_t *)pv); - Log3Func(("\tRead %s => %x (%Rrc)\n", g_aHdaRegMap[idxRegDsc].abbrev, *(uint32_t *)pv, VBOXSTRICTRC_VAL(rc))); + rc = PDMDevHlpCritSectEnter(pDevIns, &pThis->CritSect, VINF_IOM_R3_MMIO_READ); + if (rc == VINF_SUCCESS) + { + if (!(HDA_REG(pThis, GCTL) & HDA_GCTL_CRST) && idxRegDsc != HDA_REG_GCTL) + LogFunc(("Access to registers except GCTL is blocked while resetting\n")); + + if (idxRegDsc >= 0) + { + /* ASSUMES gapless DWORD at end of map. */ + if (g_aHdaRegMap[idxRegDsc].size == 4) + { + /* + * Straight forward DWORD access. + */ + rc = g_aHdaRegMap[idxRegDsc].pfnRead(pDevIns, pThis, idxRegDsc, (uint32_t *)pv); + Log3Func(("\tRead %s => %x (%Rrc)\n", g_aHdaRegMap[idxRegDsc].abbrev, *(uint32_t *)pv, VBOXSTRICTRC_VAL(rc))); + STAM_COUNTER_INC(&pThis->aStatRegReads[idxRegDsc]); + } + else + { + /* + * Multi register read (unless there are trailing gaps). + * ASSUMES that only DWORD reads have sideeffects. + */ +#ifdef IN_RING3 + STAM_COUNTER_INC(&pThis->StatRegMultiReads); + uint32_t u32Value = 0; + unsigned cbLeft = 4; + do + { + uint32_t const cbReg = g_aHdaRegMap[idxRegDsc].size; + uint32_t u32Tmp = 0; + + rc = g_aHdaRegMap[idxRegDsc].pfnRead(pDevIns, pThis, idxRegDsc, &u32Tmp); + Log3Func(("\tRead %s[%db] => %x (%Rrc)*\n", g_aHdaRegMap[idxRegDsc].abbrev, cbReg, u32Tmp, VBOXSTRICTRC_VAL(rc))); + STAM_COUNTER_INC(&pThis->aStatRegReads[idxRegDsc]); + if (rc != VINF_SUCCESS) + break; + u32Value |= (u32Tmp & g_afMasks[cbReg]) << ((4 - cbLeft) * 8); + + cbLeft -= cbReg; + off += cbReg; + idxRegDsc++; + } while (cbLeft > 0 && g_aHdaRegMap[idxRegDsc].offset == off); + + if (rc == VINF_SUCCESS) + *(uint32_t *)pv = u32Value; + else + Assert(!IOM_SUCCESS(rc)); +#else /* !IN_RING3 */ + /* Take the easy way out. */ + STAM_COUNTER_INC(&pThis->aStatRegReadsToR3[idxRegDsc]); + rc = VINF_IOM_R3_MMIO_READ; +#endif /* !IN_RING3 */ + } } else { - /* - * Multi register read (unless there are trailing gaps). - * ASSUMES that only DWORD reads have sideeffects. - */ -#ifdef IN_RING3 - uint32_t u32Value = 0; - unsigned cbLeft = 4; - do - { - uint32_t const cbReg = g_aHdaRegMap[idxRegDsc].size; - uint32_t u32Tmp = 0; - - rc = g_aHdaRegMap[idxRegDsc].pfnRead(pDevIns, pThis, idxRegDsc, &u32Tmp); - Log3Func(("\tRead %s[%db] => %x (%Rrc)*\n", g_aHdaRegMap[idxRegDsc].abbrev, cbReg, u32Tmp, VBOXSTRICTRC_VAL(rc))); - if (rc != VINF_SUCCESS) - break; - u32Value |= (u32Tmp & g_afMasks[cbReg]) << ((4 - cbLeft) * 8); - - cbLeft -= cbReg; - off += cbReg; - idxRegDsc++; - } while (cbLeft > 0 && g_aHdaRegMap[idxRegDsc].offset == off); - - if (rc == VINF_SUCCESS) - *(uint32_t *)pv = u32Value; - else - Assert(!IOM_SUCCESS(rc)); -#else /* !IN_RING3 */ - /* Take the easy way out. */ - rc = VINF_IOM_R3_MMIO_READ; -#endif /* !IN_RING3 */ - } + LogRel(("HDA: Invalid read access @0x%x (bytes=%u)\n", (uint32_t)off, cb)); + Log3Func(("\tHole at %x is accessed for read\n", offRegLog)); + STAM_COUNTER_INC(&pThis->StatRegUnknownReads); + rc = VINF_IOM_MMIO_UNUSED_FF; + } + + DEVHDA_UNLOCK(pDevIns, pThis); + + /* + * Log the outcome. + */ +#ifdef LOG_ENABLED + if (cbLog == 4) + Log3Func(("\tReturning @%#05x -> %#010x %Rrc\n", offRegLog, *(uint32_t *)pv, VBOXSTRICTRC_VAL(rc))); + else if (cbLog == 2) + Log3Func(("\tReturning @%#05x -> %#06x %Rrc\n", offRegLog, *(uint16_t *)pv, VBOXSTRICTRC_VAL(rc))); + else if (cbLog == 1) + Log3Func(("\tReturning @%#05x -> %#04x %Rrc\n", offRegLog, *(uint8_t *)pv, VBOXSTRICTRC_VAL(rc))); +#endif } else { - LogRel(("HDA: Invalid read access @0x%x (bytes=%u)\n", (uint32_t)off, cb)); - Log3Func(("\tHole at %x is accessed for read\n", offRegLog)); - rc = VINF_IOM_MMIO_UNUSED_FF; - } - - DEVHDA_UNLOCK(pDevIns, pThis); - - /* - * Log the outcome. - */ -#ifdef LOG_ENABLED - if (cbLog == 4) - Log3Func(("\tReturning @%#05x -> %#010x %Rrc\n", offRegLog, *(uint32_t *)pv, VBOXSTRICTRC_VAL(rc))); - else if (cbLog == 2) - Log3Func(("\tReturning @%#05x -> %#06x %Rrc\n", offRegLog, *(uint16_t *)pv, VBOXSTRICTRC_VAL(rc))); - else if (cbLog == 1) - Log3Func(("\tReturning @%#05x -> %#04x %Rrc\n", offRegLog, *(uint8_t *)pv, VBOXSTRICTRC_VAL(rc))); -#endif + if (idxRegDsc >= 0) + STAM_COUNTER_INC(&pThis->aStatRegReadsToR3[idxRegDsc]); + } return rc; } @@ -3095,4 +3107,5 @@ LogRel2(("HDA: Warning: Access to register %s is blocked while controller is in reset mode\n", g_aHdaRegMap[idxRegDsc].abbrev)); + STAM_COUNTER_INC(&pThis->StatRegWritesBlockedByReset); DEVHDA_UNLOCK(pDevIns, pThis); @@ -3114,5 +3127,5 @@ */ - /* Is the RUN bit currently set? */ + /* Is the RUN bit currently set? */ if ( RT_BOOL(uSDCTL & HDA_SDCTL_RUN) /* Are writes to the register denied if RUN bit is set? */ @@ -3122,4 +3135,5 @@ LogRel2(("HDA: Warning: Access to register %s is blocked while the stream's RUN bit is set\n", g_aHdaRegMap[idxRegDsc].abbrev)); + STAM_COUNTER_INC(&pThis->StatRegWritesBlockedByRun); DEVHDA_UNLOCK(pDevIns, pThis); @@ -3135,4 +3149,10 @@ Log3Func(("Written value %#x to %s[%d byte]; %x => %x%s, rc=%d\n", u32Value, g_aHdaRegMap[idxRegDsc].abbrev, g_aHdaRegMap[idxRegDsc].size, u32OldValue, pThis->au32Regs[idxRegMem], pszLog, VBOXSTRICTRC_VAL(rc))); +#ifndef IN_RING3 + if (rc == VINF_IOM_R3_MMIO_WRITE) + STAM_COUNTER_INC(&pThis->aStatRegWritesToR3[idxRegDsc]); + else +#endif + STAM_COUNTER_INC(&pThis->aStatRegWrites[idxRegDsc]); DEVHDA_UNLOCK(pDevIns, pThis); @@ -3207,14 +3227,25 @@ * shifting out input values. */ - if (idxRegDsc < 0 && (idxRegDsc = hdaR3RegLookupWithin(off)) != -1) - { - uint32_t const cbBefore = (uint32_t)off - g_aHdaRegMap[idxRegDsc].offset; Assert(cbBefore > 0 && cbBefore < 4); - off -= cbBefore; - idxRegMem = g_aHdaRegMap[idxRegDsc].mem_idx; - u64Value <<= cbBefore * 8; - u64Value |= pThis->au32Regs[idxRegMem] & g_afMasks[cbBefore]; - Log3Func(("\tWithin register, supplied %u leading bits: %#llx -> %#llx ...\n", - cbBefore * 8, ~g_afMasks[cbBefore] & u64Value, u64Value)); - } + if (idxRegDsc < 0) + { + idxRegDsc = hdaR3RegLookupWithin(off); + if (idxRegDsc != -1) + { + uint32_t const cbBefore = (uint32_t)off - g_aHdaRegMap[idxRegDsc].offset; + Assert(cbBefore > 0 && cbBefore < 4); + off -= cbBefore; + idxRegMem = g_aHdaRegMap[idxRegDsc].mem_idx; + u64Value <<= cbBefore * 8; + u64Value |= pThis->au32Regs[idxRegMem] & g_afMasks[cbBefore]; + Log3Func(("\tWithin register, supplied %u leading bits: %#llx -> %#llx ...\n", + cbBefore * 8, ~g_afMasks[cbBefore] & u64Value, u64Value)); + STAM_COUNTER_INC(&pThis->StatRegMultiWrites); + } + else + STAM_COUNTER_INC(&pThis->StatRegUnknownWrites); + } + else + STAM_COUNTER_INC(&pThis->StatRegMultiWrites); + /* Loop thru the write area, it may cover multiple registers. */ @@ -5024,9 +5055,32 @@ * Register statistics. */ - PDMDevHlpSTAMRegister(pDevIns, &pThis->StatTimer, STAMTYPE_PROFILE, "/Devices/HDA/Timer", STAMUNIT_TICKS_PER_CALL, "Profiling hdaR3Timer."); - PDMDevHlpSTAMRegister(pDevIns, &pThis->StatIn, STAMTYPE_PROFILE, "/Devices/HDA/Input", STAMUNIT_TICKS_PER_CALL, "Profiling input."); - PDMDevHlpSTAMRegister(pDevIns, &pThis->StatOut, STAMTYPE_PROFILE, "/Devices/HDA/Output", STAMUNIT_TICKS_PER_CALL, "Profiling output."); - PDMDevHlpSTAMRegister(pDevIns, &pThis->StatBytesRead, STAMTYPE_COUNTER, "/Devices/HDA/BytesRead" , STAMUNIT_BYTES, "Bytes read from HDA emulation."); - PDMDevHlpSTAMRegister(pDevIns, &pThis->StatBytesWritten, STAMTYPE_COUNTER, "/Devices/HDA/BytesWritten", STAMUNIT_BYTES, "Bytes written to HDA emulation."); + PDMDevHlpSTAMRegister(pDevIns, &pThis->StatTimer, STAMTYPE_PROFILE, "Timer", STAMUNIT_TICKS_PER_CALL, "Profiling hdaR3Timer."); + PDMDevHlpSTAMRegister(pDevIns, &pThis->StatIn, STAMTYPE_PROFILE, "Input", STAMUNIT_TICKS_PER_CALL, "Profiling input."); + PDMDevHlpSTAMRegister(pDevIns, &pThis->StatOut, STAMTYPE_PROFILE, "Output", STAMUNIT_TICKS_PER_CALL, "Profiling output."); + PDMDevHlpSTAMRegister(pDevIns, &pThis->StatBytesRead, STAMTYPE_COUNTER, "BytesRead" , STAMUNIT_BYTES, "Bytes read from HDA emulation."); + PDMDevHlpSTAMRegister(pDevIns, &pThis->StatBytesWritten, STAMTYPE_COUNTER, "BytesWritten", STAMUNIT_BYTES, "Bytes written to HDA emulation."); + + AssertCompile(RT_ELEMENTS(g_aHdaRegMap) == HDA_NUM_REGS); + AssertCompile(RT_ELEMENTS(pThis->aStatRegReads) == HDA_NUM_REGS); + AssertCompile(RT_ELEMENTS(pThis->aStatRegReadsToR3) == HDA_NUM_REGS); + AssertCompile(RT_ELEMENTS(pThis->aStatRegWrites) == HDA_NUM_REGS); + AssertCompile(RT_ELEMENTS(pThis->aStatRegWritesToR3) == HDA_NUM_REGS); + for (size_t i = 0; i < RT_ELEMENTS(g_aHdaRegMap); i++) + { + PDMDevHlpSTAMRegisterF(pDevIns, &pThis->aStatRegReads[i], STAMTYPE_COUNTER, STAMVISIBILITY_ALWAYS, STAMUNIT_OCCURENCES, + g_aHdaRegMap[i].desc, "Regs/%03x-%s-Reads", g_aHdaRegMap[i].offset, g_aHdaRegMap[i].abbrev); + PDMDevHlpSTAMRegisterF(pDevIns, &pThis->aStatRegReadsToR3[i], STAMTYPE_COUNTER, STAMVISIBILITY_USED, STAMUNIT_OCCURENCES, + g_aHdaRegMap[i].desc, "Regs/%03x-%s-Reads-ToR3", g_aHdaRegMap[i].offset, g_aHdaRegMap[i].abbrev); + PDMDevHlpSTAMRegisterF(pDevIns, &pThis->aStatRegWrites[i], STAMTYPE_COUNTER, STAMVISIBILITY_ALWAYS, STAMUNIT_OCCURENCES, + g_aHdaRegMap[i].desc, "Regs/%03x-%s-Writes", g_aHdaRegMap[i].offset, g_aHdaRegMap[i].abbrev); + PDMDevHlpSTAMRegisterF(pDevIns, &pThis->aStatRegWritesToR3[i], STAMTYPE_COUNTER, STAMVISIBILITY_USED, STAMUNIT_OCCURENCES, + g_aHdaRegMap[i].desc, "Regs/%03x-%s-Writes-ToR3", g_aHdaRegMap[i].offset, g_aHdaRegMap[i].abbrev); + } + PDMDevHlpSTAMRegister(pDevIns, &pThis->StatRegMultiReads, STAMTYPE_COUNTER, "RegMultiReads", STAMUNIT_OCCURENCES, "Register read not targeting just one register"); + PDMDevHlpSTAMRegister(pDevIns, &pThis->StatRegMultiWrites, STAMTYPE_COUNTER, "RegMultiWrites", STAMUNIT_OCCURENCES, "Register writes not targeting just one register"); + PDMDevHlpSTAMRegister(pDevIns, &pThis->StatRegUnknownReads, STAMTYPE_COUNTER, "RegUnknownReads", STAMUNIT_OCCURENCES, "Reads of unknown registers."); + PDMDevHlpSTAMRegister(pDevIns, &pThis->StatRegUnknownWrites, STAMTYPE_COUNTER, "RegUnknownWrites", STAMUNIT_OCCURENCES, "Writes to unknown registers."); + PDMDevHlpSTAMRegister(pDevIns, &pThis->StatRegWritesBlockedByReset, STAMTYPE_COUNTER, "RegWritesBlockedByReset", STAMUNIT_OCCURENCES, "Writes blocked by pending reset (GCTL/CRST)"); + PDMDevHlpSTAMRegister(pDevIns, &pThis->StatRegWritesBlockedByRun, STAMTYPE_COUNTER, "RegWritesBlockedByRun", STAMUNIT_OCCURENCES, "Writes blocked by byte RUN bit."); # endif Index: /trunk/src/VBox/Devices/Audio/DevHDA.h =================================================================== --- /trunk/src/VBox/Devices/Audio/DevHDA.h (revision 82398) +++ /trunk/src/VBox/Devices/Audio/DevHDA.h (revision 82399) @@ -201,4 +201,19 @@ STAMCOUNTER StatBytesRead; STAMCOUNTER StatBytesWritten; + + /** @name Register statistics. + * The array members run parallel to g_aHdaRegMap. + * @{ */ + STAMCOUNTER aStatRegReads[HDA_NUM_REGS]; + STAMCOUNTER aStatRegReadsToR3[HDA_NUM_REGS]; + STAMCOUNTER aStatRegWrites[HDA_NUM_REGS]; + STAMCOUNTER aStatRegWritesToR3[HDA_NUM_REGS]; + STAMCOUNTER StatRegMultiReads; + STAMCOUNTER StatRegMultiWrites; + STAMCOUNTER StatRegUnknownReads; + STAMCOUNTER StatRegUnknownWrites; + STAMCOUNTER StatRegWritesBlockedByReset; + STAMCOUNTER StatRegWritesBlockedByRun; + /** @} */ #endif /** This is for checking that the build was correctly configured in all contexts. Index: /trunk/src/VBox/Devices/Audio/DevHDACommon.h =================================================================== --- /trunk/src/VBox/Devices/Audio/DevHDACommon.h (revision 82398) +++ /trunk/src/VBox/Devices/Audio/DevHDACommon.h (revision 82399) @@ -30,5 +30,9 @@ typedef VBOXSTRICTRC FNHDAREGWRITE(PPDMDEVINS pDevIns, PHDASTATE pThis, uint32_t iReg, uint32_t u32Value); -/** See 302349 p 6.2. */ +/** + * HDA register descriptor. + * + * See 302349 p 6.2. + */ typedef struct HDAREGDESC { @@ -56,4 +60,6 @@ const char *desc; } HDAREGDESC; +/** Pointer to a a const HDA register descriptor. */ +typedef HDAREGDESC const *PCHDAREGDESC; /** Index: /trunk/src/VBox/Devices/Audio/HDACodec.cpp =================================================================== --- /trunk/src/VBox/Devices/Audio/HDACodec.cpp (revision 82398) +++ /trunk/src/VBox/Devices/Audio/HDACodec.cpp (revision 82399) @@ -3028,4 +3028,5 @@ AssertPtrReturn(pThis, VERR_INVALID_POINTER); AssertPtrReturn(puResp, VERR_INVALID_POINTER); + STAM_COUNTER_INC(&pThis->StatLookups); if (CODEC_CAD(cmd) != pThis->id) @@ -3278,4 +3279,11 @@ #endif + /* + * Statistics + */ +#ifdef VBOX_WITH_STATISTICS + PDMDevHlpSTAMRegister(pDevIns, &pThis->StatLookups, STAMTYPE_COUNTER, "Codec/Lookups", STAMUNIT_OCCURENCES, "Number of codecLookup calls"); +#endif + LogFlowFuncLeaveRC(rc); return rc; Index: /trunk/src/VBox/Devices/Audio/HDACodec.h =================================================================== --- /trunk/src/VBox/Devices/Audio/HDACodec.h (revision 82398) +++ /trunk/src/VBox/Devices/Audio/HDACodec.h (revision 82399) @@ -122,4 +122,8 @@ DECLR3CALLBACKMEMBER(void, pfnDbgSelector, (PHDACODEC pThis, PCDBGFINFOHLP pHlp, const char *pszArgs)); /** @} */ + +#ifdef VBOX_WITH_STATISTICS + STAMCOUNTER StatLookups; +#endif } HDACODEC;