Index: /trunk/src/VBox/VMM/VMMR3/HWACCM.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMR3/HWACCM.cpp (revision 41774) +++ /trunk/src/VBox/VMM/VMMR3/HWACCM.cpp (revision 41775) @@ -1673,5 +1673,5 @@ for (unsigned i = 0; i < pVM->hwaccm.s.cPatches; i++) { - uint8_t szInstr[15]; + uint8_t abInstr[15]; PHWACCMTPRPATCH pPatch = &pVM->hwaccm.s.aPatches[i]; RTGCPTR pInstrGC = (RTGCPTR)pPatch->Core.Key; @@ -1688,5 +1688,5 @@ /* Check if the instruction is still the same. */ - rc = PGMPhysSimpleReadGCPtr(pVCpu, szInstr, pInstrGC, pPatch->cbNewOp); + rc = PGMPhysSimpleReadGCPtr(pVCpu, abInstr, pInstrGC, pPatch->cbNewOp); if (rc != VINF_SUCCESS) { @@ -1695,5 +1695,5 @@ } - if (memcmp(szInstr, pPatch->aNewOpcode, pPatch->cbNewOp)) + if (memcmp(abInstr, pPatch->aNewOpcode, pPatch->cbNewOp)) { Log(("Patched instruction was changed! (rc=%Rrc0\n", rc)); @@ -1799,22 +1799,40 @@ DECLCALLBACK(VBOXSTRICTRC) hwaccmR3ReplaceTprInstr(PVM pVM, PVMCPU pVCpu, void *pvUser) { - VMCPUID idCpu = (VMCPUID)(uintptr_t)pvUser; - PCPUMCTX pCtx = CPUMQueryGuestCtxPtr(pVCpu); - PDISCPUSTATE pDis = &pVCpu->hwaccm.s.DisState; - unsigned cbOp; - - /* Only execute the handler on the VCPU the original patch request was issued. (the other CPU(s) might not yet have switched to protected mode) */ + /* + * Only execute the handler on the VCPU the original patch request was + * issued. (The other CPU(s) might not yet have switched to protected + * mode, nor have the correct memory context.) + */ + VMCPUID idCpu = (VMCPUID)(uintptr_t)pvUser; if (pVCpu->idCpu != idCpu) return VINF_SUCCESS; - Log(("hwaccmR3ReplaceTprInstr: %RGv\n", pCtx->rip)); - - /* Two or more VCPUs were racing to patch this instruction. */ + /* + * We're racing other VCPUs here, so don't try patch the instruction twice + * and make sure there is still room for our patch record. + */ + PCPUMCTX pCtx = CPUMQueryGuestCtxPtr(pVCpu); PHWACCMTPRPATCH pPatch = (PHWACCMTPRPATCH)RTAvloU32Get(&pVM->hwaccm.s.PatchTree, (AVLOU32KEY)pCtx->eip); if (pPatch) + { + Log(("hwaccmR3ReplaceTprInstr: already patched %RGv\n", pCtx->rip)); return VINF_SUCCESS; - - Assert(pVM->hwaccm.s.cPatches < RT_ELEMENTS(pVM->hwaccm.s.aPatches)); - + } + uint32_t const idx = pVM->hwaccm.s.cPatches; + if (idx >= RT_ELEMENTS(pVM->hwaccm.s.aPatches)) + { + Log(("hwaccmR3ReplaceTprInstr: no available patch slots (%RGv)\n", pCtx->rip)); + return VINF_SUCCESS; + } + pPatch = &pVM->hwaccm.s.aPatches[idx]; + + Log(("hwaccmR3ReplaceTprInstr: rip=%RGv idxPatch=%u\n", pCtx->rip, idx)); + + /* + * Disassembler the instruction and get cracking. + */ + DBGFR3DisasInstrCurrentLog(pVCpu, "hwaccmR3ReplaceTprInstr"); + PDISCPUSTATE pDis = &pVCpu->hwaccm.s.DisState; + uint32_t cbOp; int rc = EMInterpretDisasOne(pVM, pVCpu, CPUMCTX2CORE(pCtx), pDis, &cbOp); AssertRC(rc); @@ -1823,13 +1841,10 @@ && cbOp >= 3) { - uint8_t aVMMCall[3] = { 0xf, 0x1, 0xd9}; - uint32_t idx = pVM->hwaccm.s.cPatches; - - pPatch = &pVM->hwaccm.s.aPatches[idx]; + static uint8_t const s_abVMMCall[3] = { 0x0f, 0x01, 0xd9 }; rc = PGMPhysSimpleReadGCPtr(pVCpu, pPatch->aOpcode, pCtx->rip, cbOp); AssertRC(rc); - pPatch->cbOp = cbOp; + pPatch->cbOp = cbOp; if (pDis->Param1.fUse == DISUSE_DISPLACEMENT32) @@ -1840,4 +1855,5 @@ pPatch->enmType = HWACCMTPRINSTR_WRITE_REG; pPatch->uSrcOperand = pDis->Param2.Base.idxGenReg; + Log(("hwaccmR3ReplaceTprInstr: HWACCMTPRINSTR_WRITE_REG %u\n", pDis->Param2.Base.idxGenReg)); } else @@ -1846,56 +1862,61 @@ pPatch->enmType = HWACCMTPRINSTR_WRITE_IMM; pPatch->uSrcOperand = pDis->Param2.uValue; + Log(("hwaccmR3ReplaceTprInstr: HWACCMTPRINSTR_WRITE_IMM %#llx\n", pDis->Param2.uValue)); } - rc = PGMPhysSimpleWriteGCPtr(pVCpu, pCtx->rip, aVMMCall, sizeof(aVMMCall)); + rc = PGMPhysSimpleWriteGCPtr(pVCpu, pCtx->rip, s_abVMMCall, sizeof(s_abVMMCall)); AssertRC(rc); - memcpy(pPatch->aNewOpcode, aVMMCall, sizeof(aVMMCall)); - pPatch->cbNewOp = sizeof(aVMMCall); + memcpy(pPatch->aNewOpcode, s_abVMMCall, sizeof(s_abVMMCall)); + pPatch->cbNewOp = sizeof(s_abVMMCall); } else { - RTGCPTR oldrip = pCtx->rip; - uint32_t oldcbOp = cbOp; - uint32_t uMmioReg = pDis->Param1.Base.idxGenReg; - - /* read */ - Assert(pDis->Param1.fUse == DISUSE_REG_GEN32); - - /* Found: + /* + * TPR Read. + * + * Found: * mov eax, dword [fffe0080] (5 bytes) * Check if next instruction is: * shr eax, 4 */ + Assert(pDis->Param1.fUse == DISUSE_REG_GEN32); + + uint8_t const idxMmioReg = pDis->Param1.Base.idxGenReg; + uint8_t const cbOpMmio = cbOp; + uint64_t const uSavedRip = pCtx->rip; + pCtx->rip += cbOp; rc = EMInterpretDisasOne(pVM, pVCpu, CPUMCTX2CORE(pCtx), pDis, &cbOp); - pCtx->rip = oldrip; + DBGFR3DisasInstrCurrentLog(pVCpu, "Following read"); + pCtx->rip = uSavedRip; + if ( rc == VINF_SUCCESS && pDis->pCurInstr->uOpcode == OP_SHR && pDis->Param1.fUse == DISUSE_REG_GEN32 - && pDis->Param1.Base.idxGenReg == uMmioReg + && pDis->Param1.Base.idxGenReg == idxMmioReg && pDis->Param2.fUse == DISUSE_IMMEDIATE8 && pDis->Param2.uValue == 4 - && oldcbOp + cbOp < sizeof(pVM->hwaccm.s.aPatches[idx].aOpcode)) + && cbOpMmio + cbOp < sizeof(pVM->hwaccm.s.aPatches[idx].aOpcode)) { - uint8_t szInstr[15]; + uint8_t abInstr[15]; /* Replacing two instructions now. */ - rc = PGMPhysSimpleReadGCPtr(pVCpu, &pPatch->aOpcode, pCtx->rip, oldcbOp + cbOp); + rc = PGMPhysSimpleReadGCPtr(pVCpu, &pPatch->aOpcode, pCtx->rip, cbOpMmio + cbOp); AssertRC(rc); - pPatch->cbOp = oldcbOp + cbOp; + pPatch->cbOp = cbOpMmio + cbOp; /* 0xF0, 0x0F, 0x20, 0xC0 = mov eax, cr8 */ - szInstr[0] = 0xF0; - szInstr[1] = 0x0F; - szInstr[2] = 0x20; - szInstr[3] = 0xC0 | pDis->Param1.Base.idxGenReg; + abInstr[0] = 0xF0; + abInstr[1] = 0x0F; + abInstr[2] = 0x20; + abInstr[3] = 0xC0 | pDis->Param1.Base.idxGenReg; for (unsigned i = 4; i < pPatch->cbOp; i++) - szInstr[i] = 0x90; /* nop */ - - rc = PGMPhysSimpleWriteGCPtr(pVCpu, pCtx->rip, szInstr, pPatch->cbOp); + abInstr[i] = 0x90; /* nop */ + + rc = PGMPhysSimpleWriteGCPtr(pVCpu, pCtx->rip, abInstr, pPatch->cbOp); AssertRC(rc); - memcpy(pPatch->aNewOpcode, szInstr, pPatch->cbOp); + memcpy(pPatch->aNewOpcode, abInstr, pPatch->cbOp); pPatch->cbNewOp = pPatch->cbOp; @@ -1906,11 +1927,12 @@ { pPatch->enmType = HWACCMTPRINSTR_READ; - pPatch->uDstOperand = pDis->Param1.Base.idxGenReg; - - rc = PGMPhysSimpleWriteGCPtr(pVCpu, pCtx->rip, aVMMCall, sizeof(aVMMCall)); + pPatch->uDstOperand = idxMmioReg; + + rc = PGMPhysSimpleWriteGCPtr(pVCpu, pCtx->rip, s_abVMMCall, sizeof(s_abVMMCall)); AssertRC(rc); - memcpy(pPatch->aNewOpcode, aVMMCall, sizeof(aVMMCall)); - pPatch->cbNewOp = sizeof(aVMMCall); + memcpy(pPatch->aNewOpcode, s_abVMMCall, sizeof(s_abVMMCall)); + pPatch->cbNewOp = sizeof(s_abVMMCall); + Log(("hwaccmR3ReplaceTprInstr: HWACCMTPRINSTR_READ %u\n", pPatch->uDstOperand)); } } @@ -1925,16 +1947,8 @@ } - /* Save invalid patch, so we will not try again. */ - uint32_t idx = pVM->hwaccm.s.cPatches; - -#ifdef LOG_ENABLED - char szOutput[256]; - rc = DBGFR3DisasInstrEx(pVM, pVCpu->idCpu, pCtx->cs, pCtx->rip, DBGF_DISAS_FLAGS_DEFAULT_MODE, - szOutput, sizeof(szOutput), NULL); - if (RT_SUCCESS(rc)) - Log(("Failed to patch instr: %s\n", szOutput)); -#endif - - pPatch = &pVM->hwaccm.s.aPatches[idx]; + /* + * Save invalid patch, so we will not try again. + */ + Log(("hwaccmR3ReplaceTprInstr: Failed to patch instr!\n")); pPatch->Core.Key = pCtx->eip; pPatch->enmType = HWACCMTPRINSTR_INVALID; @@ -1957,21 +1971,18 @@ DECLCALLBACK(VBOXSTRICTRC) hwaccmR3PatchTprInstr(PVM pVM, PVMCPU pVCpu, void *pvUser) { - VMCPUID idCpu = (VMCPUID)(uintptr_t)pvUser; - PCPUMCTX pCtx = CPUMQueryGuestCtxPtr(pVCpu); - PDISCPUSTATE pDis = &pVCpu->hwaccm.s.DisState; - unsigned cbOp; - int rc; -#ifdef LOG_ENABLED - RTGCPTR pInstr; - char szOutput[256]; -#endif - - /* Only execute the handler on the VCPU the original patch request was issued. (the other CPU(s) might not yet have switched to protected mode) */ + /* + * Only execute the handler on the VCPU the original patch request was + * issued. (The other CPU(s) might not yet have switched to protected + * mode, nor have the correct memory context.) + */ + VMCPUID idCpu = (VMCPUID)(uintptr_t)pvUser; if (pVCpu->idCpu != idCpu) return VINF_SUCCESS; - Assert(pVM->hwaccm.s.cPatches < RT_ELEMENTS(pVM->hwaccm.s.aPatches)); - - /* Two or more VCPUs were racing to patch this instruction. */ + /* + * We're racing other VCPUs here, so don't try patch the instruction twice + * and make sure there is still room for our patch record. + */ + PCPUMCTX pCtx = CPUMQueryGuestCtxPtr(pVCpu); PHWACCMTPRPATCH pPatch = (PHWACCMTPRPATCH)RTAvloU32Get(&pVM->hwaccm.s.PatchTree, (AVLOU32KEY)pCtx->eip); if (pPatch) @@ -1980,8 +1991,21 @@ return VINF_SUCCESS; } - - Log(("hwaccmR3PatchTprInstr %RGv\n", pCtx->rip)); - - rc = EMInterpretDisasOne(pVM, pVCpu, CPUMCTX2CORE(pCtx), pDis, &cbOp); + uint32_t const idx = pVM->hwaccm.s.cPatches; + if (idx >= RT_ELEMENTS(pVM->hwaccm.s.aPatches)) + { + Log(("hwaccmR3PatchTprInstr: no available patch slots (%RGv)\n", pCtx->rip)); + return VINF_SUCCESS; + } + pPatch = &pVM->hwaccm.s.aPatches[idx]; + + Log(("hwaccmR3PatchTprInstr: rip=%RGv idxPatch=%u\n", pCtx->rip, idx)); + DBGFR3DisasInstrCurrentLog(pVCpu, "hwaccmR3PatchTprInstr"); + + /* + * Disassemble the instruction and get cracking. + */ + PDISCPUSTATE pDis = &pVCpu->hwaccm.s.DisState; + uint32_t cbOp; + int rc = EMInterpretDisasOne(pVM, pVCpu, CPUMCTX2CORE(pCtx), pDis, &cbOp); AssertRC(rc); if ( rc == VINF_SUCCESS @@ -1989,16 +2013,6 @@ && cbOp >= 5) { - uint32_t idx = pVM->hwaccm.s.cPatches; uint8_t aPatch[64]; uint32_t off = 0; - - pPatch = &pVM->hwaccm.s.aPatches[idx]; - -#ifdef LOG_ENABLED - rc = DBGFR3DisasInstrEx(pVM, pVCpu->idCpu, pCtx->cs, pCtx->rip, DBGF_DISAS_FLAGS_DEFAULT_MODE, - szOutput, sizeof(szOutput), NULL); - if (RT_SUCCESS(rc)) - Log(("Original instr: %s\n", szOutput)); -#endif rc = PGMPhysSimpleReadGCPtr(pVCpu, pPatch->aOpcode, pCtx->rip, cbOp); @@ -2122,18 +2136,16 @@ #ifdef LOG_ENABLED - pInstr = pVM->hwaccm.s.pFreeGuestPatchMem; - while (true) + uint32_t cbCurInstr; + for (RTGCPTR GCPtrInstr = pVM->hwaccm.s.pFreeGuestPatchMem; + GCPtrInstr < pVM->hwaccm.s.pFreeGuestPatchMem + off; + GCPtrInstr += RT_MAX(cbCurInstr, 1)) { - uint32_t cb; - - rc = DBGFR3DisasInstrEx(pVM, pVCpu->idCpu, pCtx->cs, pInstr, DBGF_DISAS_FLAGS_DEFAULT_MODE, - szOutput, sizeof(szOutput), &cb); + char szOutput[256]; + rc = DBGFR3DisasInstrEx(pVM, pVCpu->idCpu, pCtx->cs, GCPtrInstr, DBGF_DISAS_FLAGS_DEFAULT_MODE, + szOutput, sizeof(szOutput), &cbCurInstr); if (RT_SUCCESS(rc)) Log(("Patch instr %s\n", szOutput)); - - pInstr += cb; - - if (pInstr >= pVM->hwaccm.s.pFreeGuestPatchMem + off) - break; + else + Log(("%RGv: rc=%Rrc\n", GCPtrInstr, rc)); } #endif @@ -2146,10 +2158,6 @@ AssertRC(rc); -#ifdef LOG_ENABLED - rc = DBGFR3DisasInstrEx(pVM, pVCpu->idCpu, pCtx->cs, pCtx->rip, DBGF_DISAS_FLAGS_DEFAULT_MODE, - szOutput, sizeof(szOutput), NULL); - if (RT_SUCCESS(rc)) - Log(("Jump: %s\n", szOutput)); -#endif + DBGFR3DisasInstrCurrentLog(pVCpu, "Jump"); + pVM->hwaccm.s.pFreeGuestPatchMem += off; pPatch->cbNewOp = 5; @@ -2164,18 +2172,14 @@ return VINF_SUCCESS; } - else - Log(("Ran out of space in our patch buffer!\n")); - } - - /* Save invalid patch, so we will not try again. */ - uint32_t idx = pVM->hwaccm.s.cPatches; - -#ifdef LOG_ENABLED - rc = DBGFR3DisasInstrEx(pVM, pVCpu->idCpu, pCtx->cs, pCtx->rip, DBGF_DISAS_FLAGS_DEFAULT_MODE, - szOutput, sizeof(szOutput), NULL); - if (RT_SUCCESS(rc)) - Log(("Failed to patch instr: %s\n", szOutput)); -#endif - + + Log(("Ran out of space in our patch buffer!\n")); + } + else + Log(("hwaccmR3PatchTprInstr: Failed to patch instr!\n")); + + + /* + * Save invalid patch, so we will not try again. + */ pPatch = &pVM->hwaccm.s.aPatches[idx]; pPatch->Core.Key = pCtx->eip;