Index: /trunk/include/VBox/dis.h =================================================================== --- /trunk/include/VBox/dis.h (revision 41759) +++ /trunk/include/VBox/dis.h (revision 41760) @@ -497,15 +497,23 @@ /** - * Callback for reading opcode bytes. - * - * @param pDisState Pointer to the CPU state. The primary user argument - * can be retrived from DISCPUSTATE::pvUser. If - * more is required these can be passed in the - * subsequent slots. - * @param pbDst Pointer to output buffer. - * @param uSrcAddr The address to start reading at. - * @param cbToRead The number of bytes to read. - */ -typedef DECLCALLBACK(int) FNDISREADBYTES(PDISCPUSTATE pDisState, uint8_t *pbDst, RTUINTPTR uSrcAddr, uint32_t cbToRead); + * Callback for reading instruction bytes. + * + * @returns VBox status code, bytes in DISCPUSTATE::abInstr and byte count in + * DISCPUSTATE::cbCachedInstr. + * @param pDis Pointer to the disassembler state. The user + * argument can be found in DISCPUSTATE::pvUser if + * needed. + * @param offInstr The offset relative to the start of the instruction. + * + * To get the source address, add this to + * DISCPUSTATE::uInstrAddr. + * + * To calculate the destination buffer address, use it + * as an index into DISCPUSTATE::abInstr. + * + * @param cbMinRead The minimum number of bytes to read. + * @param cbMaxRead The maximum number of bytes that may be read. + */ +typedef DECLCALLBACK(int) FNDISREADBYTES(PDISCPUSTATE pDis, uint8_t offInstr, uint8_t cbMinRead, uint8_t cbMaxRead); /** Pointer to a opcode byte reader. */ typedef FNDISREADBYTES *PFNDISREADBYTES; Index: /trunk/src/VBox/Devices/PC/BIOS-new/MakeDebianBiosAssembly.cpp =================================================================== --- /trunk/src/VBox/Devices/PC/BIOS-new/MakeDebianBiosAssembly.cpp (revision 41759) +++ /trunk/src/VBox/Devices/PC/BIOS-new/MakeDebianBiosAssembly.cpp (revision 41760) @@ -908,16 +908,17 @@ * @remarks @a uSrcAddr is the flat address. */ -static DECLCALLBACK(int) disReadOpcodeBytes(PDISCPUSTATE pDisState, uint8_t *pbDst, RTUINTPTR uSrcAddr, uint32_t cbToRead) -{ - if (uSrcAddr + cbToRead >= VBOX_BIOS_BASE + _64K) - { - RT_BZERO(pbDst, cbToRead); - if (uSrcAddr >= VBOX_BIOS_BASE + _64K) +static DECLCALLBACK(int) disReadOpcodeBytes(PDISCPUSTATE pDis, uint8_t offInstr, uint8_t cbMinRead, uint8_t cbMaxRead) +{ + RTUINTPTR offBios = pDis->uInstrAddr + offInstr - VBOX_BIOS_BASE; + size_t cbToRead = cbMaxRead; + if (offBios + cbToRead > _64K) + { + if (offBios >= _64K) cbToRead = 0; else - cbToRead = VBOX_BIOS_BASE + _64K - uSrcAddr; - } - memcpy(pbDst, &g_pbImg[uSrcAddr - VBOX_BIOS_BASE], cbToRead); - NOREF(pDisState); + cbToRead = _64K - offBios; + } + memcpy(&pDis->abInstr[offInstr], &g_pbImg[offBios], cbToRead); + pDis->cbCachedInstr = offInstr + cbToRead; return VINF_SUCCESS; } Index: /trunk/src/VBox/Disassembler/DisasmCore.cpp =================================================================== --- /trunk/src/VBox/Disassembler/DisasmCore.cpp (revision 41759) +++ /trunk/src/VBox/Disassembler/DisasmCore.cpp (revision 41760) @@ -65,5 +65,5 @@ static uint32_t disReadDWord(PDISCPUSTATE pCpu, RTUINTPTR pAddress); static uint64_t disReadQWord(PDISCPUSTATE pCpu, RTUINTPTR pAddress); -static DECLCALLBACK(int) disReadBytesDefault(PDISCPUSTATE pCpu, uint8_t *pbDst, RTUINTPTR uSrcAddr, uint32_t cbToRead); +static DECLCALLBACK(int) disReadBytesDefault(PDISCPUSTATE pDis, uint8_t offInstr, uint8_t cbMinRead, uint8_t cbMaxRead); @@ -2390,12 +2390,14 @@ -static DECLCALLBACK(int) disReadBytesDefault(PDISCPUSTATE pCpu, uint8_t *pbDst, RTUINTPTR uSrcAddr, uint32_t cbToRead) +static DECLCALLBACK(int) disReadBytesDefault(PDISCPUSTATE pDis, uint8_t offInstr, uint8_t cbMinRead, uint8_t cbMaxRead) { #ifdef IN_RING0 AssertMsgFailed(("disReadWord with no read callback in ring 0!!\n")); - RT_BZERO(pbDst, cbToRead); + RT_BZERO(&pDis->abInstr[offInstr], cbMaxRead); + pDis->cbCachedInstr = offInstr + cbMaxRead; return VERR_DIS_NO_READ_CALLBACK; #else - memcpy(pbDst, (void const *)(uintptr_t)uSrcAddr, cbToRead); + memcpy(&pDis->abInstr[offInstr], (uint8_t const *)(uintptr_t)pDis->uInstrAddr + offInstr, cbMaxRead); + pDis->cbCachedInstr = offInstr + cbMaxRead; return VINF_SUCCESS; #endif @@ -2437,16 +2439,19 @@ /* - * Do the read. No need to zero anything, abInstr is already zeroed by the - * DISInstrEx API. + * Do the read. + * (No need to zero anything on failure as abInstr is already zeroed by the + * DISInstrEx API.) */ - /** @todo Change the callback API so it can read more, thus avoid lots of - * calls or it doing its own caching. */ - int rc = pCpu->pfnReadBytes(pCpu, &pCpu->abInstr[off], pCpu->uInstrAddr + off, cbMin); - if (RT_FAILURE(rc)) + int rc = pCpu->pfnReadBytes(pCpu, off, cbMin, sizeof(pCpu->abInstr) - off); + if (RT_SUCCESS(rc)) + { + Assert(pCpu->cbCachedInstr >= off + cbMin); + Assert(pCpu->cbCachedInstr <= sizeof(pCpu->abInstr)); + } + else { Log(("disReadMore failed with rc=%Rrc!!\n", rc)); pCpu->rc = VERR_DIS_MEM_READ; } - pCpu->cbCachedInstr = off + cbMin; } Index: /trunk/src/VBox/Disassembler/testcase/tstDisasm-2.cpp =================================================================== --- /trunk/src/VBox/Disassembler/testcase/tstDisasm-2.cpp (revision 41759) +++ /trunk/src/VBox/Disassembler/testcase/tstDisasm-2.cpp (revision 41760) @@ -167,97 +167,53 @@ /** * Callback for reading bytes. - * - * @todo This should check that the disassembler doesn't do unnecessary reads, - * however the current doesn't do this and is just complicated... - */ -static DECLCALLBACK(int) MyDisasInstrRead(PDISCPUSTATE pDisState, uint8_t *pbDst, RTUINTPTR uSrcAddr, uint32_t cbToRead) -{ - PMYDISSTATE pState = (PMYDISSTATE)pDisState; + */ +static DECLCALLBACK(int) MyDisasInstrRead(PDISCPUSTATE pDis, uint8_t offInstr, uint8_t cbMinRead, uint8_t cbMaxRead) +{ + PMYDISSTATE pState = (PMYDISSTATE)pDis; + RTUINTPTR uSrcAddr = pState->Cpu.uInstrAddr + offInstr; if (RT_LIKELY( pState->uNextAddr == uSrcAddr - && pState->cbLeft >= cbToRead)) + && pState->cbLeft >= cbMinRead)) { /* * Straight forward reading. */ - if (cbToRead == 1) + //size_t cbToRead = cbMaxRead; + size_t cbToRead = cbMinRead; + memcpy(&pState->Cpu.abInstr[offInstr], pState->pbNext, cbToRead); + pState->Cpu.cbCachedInstr = offInstr + cbToRead; + pState->pbNext += cbToRead; + pState->cbLeft -= cbToRead; + pState->uNextAddr += cbToRead; + return VINF_SUCCESS; + } + + if (pState->uNextAddr == uSrcAddr) + { + /* + * Reading too much. + */ + if (pState->cbLeft > 0) { - pState->cbLeft--; - *pbDst = *pState->pbNext++; - pState->uNextAddr++; + memcpy(&pState->Cpu.abInstr[offInstr], pState->pbNext, pState->cbLeft); + offInstr += (uint8_t)pState->cbLeft; + cbMinRead -= (uint8_t)pState->cbLeft; + pState->pbNext += pState->cbLeft; + pState->uNextAddr += pState->cbLeft; + pState->cbLeft = 0; } - else - { - memcpy(pbDst, pState->pbNext, cbToRead); - pState->pbNext += cbToRead; - pState->cbLeft -= cbToRead; - pState->uNextAddr += cbToRead; - } + memset(&pState->Cpu.abInstr[offInstr], 0xcc, cbMinRead); + pState->rc = VERR_EOF; } else { /* - * Jumping up the stream. - * This occurs when the byte sequence is added to the output string. + * Non-sequential read, that's an error. */ - uint64_t offReq64 = uSrcAddr - pState->uAddress; - if (offReq64 < 32) - { - uint32_t offReq = offReq64; - uintptr_t off = pState->pbNext - pState->pbInstr; - if (off + pState->cbLeft <= offReq) - { - pState->pbNext += pState->cbLeft; - pState->uNextAddr += pState->cbLeft; - pState->cbLeft = 0; - - memset(pbDst, 0xcc, cbToRead); - pState->rc = VERR_EOF; - return VERR_EOF; - } - - /* reset the stream. */ - pState->cbLeft += off; - pState->pbNext = pState->pbInstr; - pState->uNextAddr = pState->uAddress; - - /* skip ahead. */ - pState->cbLeft -= offReq; - pState->pbNext += offReq; - pState->uNextAddr += offReq; - - /* do the reading. */ - if (pState->cbLeft >= cbToRead) - { - memcpy(pbDst, pState->pbNext, cbToRead); - pState->cbLeft -= cbToRead; - pState->pbNext += cbToRead; - pState->uNextAddr += cbToRead; - } - else - { - if (pState->cbLeft > 0) - { - memcpy(pbDst, pState->pbNext, pState->cbLeft); - pbDst += pState->cbLeft; - cbToRead -= (uint32_t)pState->cbLeft; - pState->pbNext += pState->cbLeft; - pState->uNextAddr += pState->cbLeft; - pState->cbLeft = 0; - } - memset(pbDst, 0xcc, cbToRead); - pState->rc = VERR_EOF; - return VERR_EOF; - } - } - else - { - RTStrmPrintf(g_pStdErr, "Reading before current instruction!\n"); - memset(pbDst, 0x90, cbToRead); - pState->rc = VERR_INTERNAL_ERROR; - return VERR_INTERNAL_ERROR; - } - } - - return VINF_SUCCESS; + RTStrmPrintf(g_pStdErr, "Reading before current instruction!\n"); + memset(&pState->Cpu.abInstr[offInstr], 0x90, cbMinRead); + pState->rc = VERR_INTERNAL_ERROR; + } + pState->Cpu.cbCachedInstr = offInstr + cbMinRead; + return pState->rc; } @@ -340,5 +296,5 @@ || State.Cpu.pCurInstr->uOpcode == OP_INVALID || State.Cpu.pCurInstr->uOpcode == OP_ILLUD2 - || ( State.enmUndefOp == kUndefOp_DefineByte + || ( State.enmUndefOp == kUndefOp_DefineByte && !MyDisasIsValidInstruction(&State.Cpu)); if (State.fUndefOp && State.enmUndefOp == kUndefOp_DefineByte) @@ -347,5 +303,5 @@ { State.Cpu.abInstr[0] = 0; - State.Cpu.pfnReadBytes(&State.Cpu, &State.Cpu.abInstr[0], State.uAddress, 1); + State.Cpu.pfnReadBytes(&State.Cpu, 0, 1, 1); State.cbInstr = 1; } Index: /trunk/src/VBox/Runtime/testcase/tstLdr-3.cpp =================================================================== --- /trunk/src/VBox/Runtime/testcase/tstLdr-3.cpp (revision 41759) +++ /trunk/src/VBox/Runtime/testcase/tstLdr-3.cpp (revision 41760) @@ -147,7 +147,9 @@ * @callback_method_impl{FNDISREADBYTES} */ -static DECLCALLBACK(int) MyReadBytes(PDISCPUSTATE pDisState, uint8_t *pbDst, RTUINTPTR uSrcAddr, uint32_t cbToRead) -{ - memcpy(pbDst, (uint8_t const *)((uintptr_t)uSrcAddr + (uintptr_t)pDisState->pvUser), cbToRead); +static DECLCALLBACK(int) MyReadBytes(PDISCPUSTATE pDis, uint8_t offInstr, uint8_t cbMinRead, uint8_t cbMaxRead) +{ + uint8_t const *pbSrc = (uint8_t const *)((uintptr_t)pDis->uInstrAddr + (uintptr_t)pDis->pvUser + offInstr); + memcpy(&pDis->abInstr[offInstr], pbSrc, cbMinRead); + pDis->cbCachedInstr = offInstr + cbMinRead; return VINF_SUCCESS; } Index: /trunk/src/VBox/Runtime/testcase/tstLdr-4.cpp =================================================================== --- /trunk/src/VBox/Runtime/testcase/tstLdr-4.cpp (revision 41759) +++ /trunk/src/VBox/Runtime/testcase/tstLdr-4.cpp (revision 41760) @@ -61,4 +61,12 @@ else if (!strcmp(pszSymbol, "RTAssertMsg2Weak") || !strcmp(pszSymbol, "_RTAssertMsg2Weak")) *pValue = (uintptr_t)RTAssertMsg1Weak; + else if (!strcmp(pszSymbol, "RTAssertMsg1") || !strcmp(pszSymbol, "_RTAssertMsg1")) + *pValue = (uintptr_t)RTAssertMsg1; + else if (!strcmp(pszSymbol, "RTAssertMsg2") || !strcmp(pszSymbol, "_RTAssertMsg2")) + *pValue = (uintptr_t)RTAssertMsg2; + else if (!strcmp(pszSymbol, "RTAssertMsg2V") || !strcmp(pszSymbol, "_RTAssertMsg2V")) + *pValue = (uintptr_t)RTAssertMsg2V; + else if (!strcmp(pszSymbol, "RTAssertMayPanic") || !strcmp(pszSymbol, "_RTAssertMayPanic")) + *pValue = (uintptr_t)RTAssertMayPanic; else if (!strcmp(pszSymbol, "RTLogDefaultInstance") || !strcmp(pszSymbol, "_RTLogDefaultInstance")) *pValue = (uintptr_t)RTLogDefaultInstance; Index: /trunk/src/VBox/Runtime/testcase/tstLdrDisasmTest.cpp =================================================================== --- /trunk/src/VBox/Runtime/testcase/tstLdrDisasmTest.cpp (revision 41759) +++ /trunk/src/VBox/Runtime/testcase/tstLdrDisasmTest.cpp (revision 41760) @@ -83,17 +83,12 @@ * @callback_method_impl{FNDISREADBYTES} */ -static DECLCALLBACK(int) DisasmTest1ReadCode(PDISCPUSTATE pDisState, uint8_t *pbDst, RTUINTPTR uSrcAddr, uint32_t cbToRead) +static DECLCALLBACK(int) DisasmTest1ReadCode(PDISCPUSTATE pDis, uint8_t offInstr, uint8_t cbMinRead, uint8_t cbMaxRead) { - NOREF(pDisState); - while (cbToRead > 0) - { - *pbDst = g_ab32BitCode[uSrcAddr]; - - /* next */ - pbDst++; - uSrcAddr++; - cbToRead--; - } - return 0; + size_t cb = cbMaxRead; + if (cb + pDis->uInstrAddr + offInstr > sizeof(g_ab32BitCode)) + cb = cbMinRead; + memcpy(&pDis->abInstr[offInstr], &g_ab32BitCode[pDis->uInstrAddr + offInstr], cb); + pDis->cbCachedInstr = offInstr + cb; + return VINF_SUCCESS; } Index: /trunk/src/VBox/VMM/VMMAll/EMAll.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMAll/EMAll.cpp (revision 41759) +++ /trunk/src/VBox/VMM/VMMAll/EMAll.cpp (revision 41760) @@ -284,14 +284,19 @@ * @callback_method_impl{FNDISREADBYTES} */ -static DECLCALLBACK(int) emReadBytes(PDISCPUSTATE pDisState, uint8_t *pbDst, RTUINTPTR uSrcAddr, uint32_t cbToRead) -{ - PEMDISSTATE pState = (PEMDISSTATE)pDisState->pvUser; +static DECLCALLBACK(int) emReadBytes(PDISCPUSTATE pDis, uint8_t offInstr, uint8_t cbMinRead, uint8_t cbMaxRead) +{ + PEMDISSTATE pState = (PEMDISSTATE)pDis->pvUser; # ifndef IN_RING0 PVM pVM = pState->pVM; # endif PVMCPU pVCpu = pState->pVCpu; + /** @todo Rewrite this to make full use of the abInstr buffer and drop our extra + * caching buffer. Just playing safe at first... */ + uint8_t *pbDst = &pDis->abInstr[offInstr]; + RTUINTPTR uSrcAddr = pDis->uInstrAddr + offInstr; + size_t cbToRead = cbMinRead; # ifdef IN_RING0 - int rc; + int rc; if ( pState->GCPtr @@ -303,4 +308,5 @@ for (unsigned i = 0; i < cbToRead; i++) pbDst[i] = pState->aOpcode[offset + i]; + pDis->cbCachedInstr = offInstr + cbToRead; return VINF_SUCCESS; } @@ -308,4 +314,5 @@ rc = PGMPhysSimpleReadGCPtr(pVCpu, pbDst, uSrcAddr, cbToRead); AssertMsgRC(rc, ("PGMPhysSimpleReadGCPtr failed for uSrcAddr=%RTptr cbToRead=%x rc=%d\n", uSrcAddr, cbToRead, rc)); + # elif defined(IN_RING3) if (!PATMIsPatchGCAddr(pVM, uSrcAddr)) @@ -332,4 +339,5 @@ # endif /* IN_RING3 */ + pDis->cbCachedInstr = offInstr + cbToRead; return VINF_SUCCESS; } Index: /trunk/src/VBox/VMM/VMMR3/CPUM.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMR3/CPUM.cpp (revision 41759) +++ /trunk/src/VBox/VMM/VMMR3/CPUM.cpp (revision 41760) @@ -5,5 +5,5 @@ /* - * Copyright (C) 2006-2010 Oracle Corporation + * Copyright (C) 2006-2012 Oracle Corporation * * This file is part of VirtualBox Open Source Edition (OSE), as @@ -3527,15 +3527,16 @@ * @callback_method_impl{FNDISREADBYTES} */ -static DECLCALLBACK(int) cpumR3DisasInstrRead(PDISCPUSTATE pDisState, uint8_t *pbDst, RTUINTPTR uSrcAddr, uint32_t cbToRead) -{ - PCPUMDISASSTATE pState = (PCPUMDISASSTATE)pDisState->pvUser; - Assert(cbToRead > 0); +static DECLCALLBACK(int) cpumR3DisasInstrRead(PDISCPUSTATE pDis, uint8_t offInstr, uint8_t cbMinRead, uint8_t cbMaxRead) +{ + PCPUMDISASSTATE pState = (PCPUMDISASSTATE)pDis->pvUser; for (;;) { - RTGCUINTPTR GCPtr = uSrcAddr + pState->GCPtrSegBase; - - /* Need to update the page translation? */ - if ( !pState->pvPageR3 - || (GCPtr >> PAGE_SHIFT) != (pState->pvPageGC >> PAGE_SHIFT)) + RTGCUINTPTR GCPtr = pDis->uInstrAddr + offInstr + pState->GCPtrSegBase; + + /* + * Need to update the page translation? + */ + if ( !pState->pvPageR3 + || (GCPtr >> PAGE_SHIFT) != (pState->pvPageGC >> PAGE_SHIFT)) { int rc = VINF_SUCCESS; @@ -3565,9 +3566,13 @@ } - /* check the segment limit */ - if (!pState->f64Bits && uSrcAddr > pState->cbSegLimit) + /* + * Check the segment limit. + */ + if (!pState->f64Bits && pDis->uInstrAddr + offInstr > pState->cbSegLimit) return VERR_OUT_OF_SELECTOR_BOUNDS; - /* calc how much we can read */ + /* + * Calc how much we can read. + */ uint32_t cb = PAGE_SIZE - (GCPtr & PAGE_OFFSET_MASK); if (!pState->f64Bits) @@ -3577,14 +3582,22 @@ cb = cbSeg; } - if (cb > cbToRead) - cb = cbToRead; - - /* read and advance */ - memcpy(pbDst, (char *)pState->pvPageR3 + (GCPtr & PAGE_OFFSET_MASK), cb); - cbToRead -= cb; - if (!cbToRead) + /** @todo read more later. */ + //if (cb > cbMaxRead) - later + // cb = cbMaxRead; + if (cb > cbMinRead) + cb = cbMinRead; + + /* + * Read and advance or exit. + */ + memcpy(&pDis->abInstr[offInstr], (uint8_t *)pState->pvPageR3 + (GCPtr & PAGE_OFFSET_MASK), cb); + offInstr += (uint8_t)cb; + if (cb >= cbMinRead) + { + pDis->cbCachedInstr = offInstr; return VINF_SUCCESS; - pbDst += cb; - uSrcAddr += cb; + } + cbMinRead -= (uint8_t)cb; + cbMaxRead -= (uint8_t)cb; } } Index: /trunk/src/VBox/VMM/VMMR3/CSAM.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMR3/CSAM.cpp (revision 41759) +++ /trunk/src/VBox/VMM/VMMR3/CSAM.cpp (revision 41760) @@ -5,5 +5,5 @@ /* - * Copyright (C) 2006-2007 Oracle Corporation + * Copyright (C) 2006-2012 Oracle Corporation * * This file is part of VirtualBox Open Source Edition (OSE), as @@ -726,37 +726,49 @@ * @callback_method_impl{FNDISREADBYTES} */ -static DECLCALLBACK(int) CSAMR3ReadBytes(PDISCPUSTATE pDisState, uint8_t *pbDst, RTUINTPTR uSrcAddr, uint32_t cbToRead) -{ - PVM pVM = (PVM)pDisState->pvUser; - RTHCUINTPTR pInstrHC = (RTHCUINTPTR)pDisState->pvUser2; - RTGCUINTPTR32 pInstrGC = pDisState->uInstrAddr; - int orgsize = cbToRead; - PVMCPU pVCpu = VMMGetCpu0(pVM); +static DECLCALLBACK(int) CSAMR3ReadBytes(PDISCPUSTATE pDis, uint8_t offInstr, uint8_t cbMinRead, uint8_t cbMaxRead) +{ + PVM pVM = (PVM)pDis->pvUser; + int rc = VINF_SUCCESS; + +/** @todo Optimize this code to read more when possible! Add a new PATM call for + * reading more than one byte. */ /* We are not interested in patched instructions, so read the original opcode bytes. Note! single instruction patches (int3) are checked in CSAMR3AnalyseCallback */ - for (int i = 0; i < orgsize; i++) - { - int rc = PATMR3QueryOpcode(pVM, (RTRCPTR)uSrcAddr, pbDst); - if (RT_FAILURE(rc)) + while (cbMinRead > 0) + { + int rc2 = PATMR3QueryOpcode(pVM, (RTRCPTR)pDis->uInstrAddr + offInstr, &pDis->abInstr[offInstr]); + if (RT_FAILURE(rc2)) break; - uSrcAddr++; - pbDst++; - cbToRead--; - } - if (cbToRead == 0) - return VINF_SUCCESS; - - if (PAGE_ADDRESS(pInstrGC) != PAGE_ADDRESS(uSrcAddr + cbToRead - 1) && !PATMIsPatchGCAddr(pVM, uSrcAddr)) - return PGMPhysSimpleReadGCPtr(pVCpu, pbDst, uSrcAddr, cbToRead); - - Assert(pInstrHC); - - /* pInstrHC is the base address; adjust according to the GC pointer. */ - pInstrHC = pInstrHC + (uSrcAddr - pInstrGC); - - memcpy(pbDst, (void *)pInstrHC, cbToRead); - - return VINF_SUCCESS; + offInstr++; + cbMinRead--; + cbMaxRead--; + } + if (cbMinRead > 0) + { + /* + * The current byte isn't a patch instruction byte... + */ + uint8_t const *pbInstr = (uint8_t const *)pDis->pvUser2; + RTUINTPTR uSrcAddr = pDis->uInstrAddr + offInstr; + if ( PAGE_ADDRESS(pDis->uInstrAddr) != PAGE_ADDRESS(uSrcAddr + cbMinRead - 1) + && !PATMIsPatchGCAddr(pVM, uSrcAddr)) /** @todo does CSAM actually analyze patch code, or is this just a copy&past check? */ + { + /* Crossed page boundrary, pbInstr is no good.. */ + PVMCPU pVCpu = VMMGetCpu0(pVM); + rc = PGMPhysSimpleReadGCPtr(pVCpu, &pDis->abInstr[offInstr], uSrcAddr, cbMinRead); + offInstr += cbMinRead; + } + else + { + /* pbInstr is eqvivalent to uInstrAddr. */ + AssertPtr(pbInstr); + memcpy(&pDis->abInstr[offInstr], &pbInstr[offInstr], cbMinRead); + offInstr += cbMinRead; + } + } + + pDis->cbCachedInstr = offInstr; + return rc; } Index: /trunk/src/VBox/VMM/VMMR3/DBGFDisas.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMR3/DBGFDisas.cpp (revision 41759) +++ /trunk/src/VBox/VMM/VMMR3/DBGFDisas.cpp (revision 41760) @@ -67,5 +67,5 @@ void const *pvPageR3; /** Pointer to the current page - GC Ptr. */ - RTGCPTR pvPageGC; + RTGCPTR GCPtrPage; /** Pointer to the next instruction (relative to GCPtrSegBase). */ RTGCUINTPTR GCPtrNext; @@ -105,5 +105,5 @@ pState->cbSegLimit = pSelInfo->cbLimit; pState->enmMode = enmMode; - pState->pvPageGC = 0; + pState->GCPtrPage = 0; pState->pvPageR3 = NULL; pState->hAs = pSelInfo->fFlags & DBGFSELINFO_FLAGS_HYPER /** @todo Deal more explicitly with RC in DBGFR3Disas*. */ @@ -201,26 +201,27 @@ * @callback_method_impl{FNDISREADBYTES} * - * @remarks @a uSrcAddr is relative to the base address indicated by + * @remarks The source is relative to the base address indicated by * DBGFDISASSTATE::GCPtrSegBase. */ -static DECLCALLBACK(int) dbgfR3DisasInstrRead(PDISCPUSTATE pDisState, uint8_t *pbDst, RTUINTPTR uSrcAddr, uint32_t cbToRead) -{ - PDBGFDISASSTATE pState = (PDBGFDISASSTATE)pDisState; - Assert(cbToRead > 0); +static DECLCALLBACK(int) dbgfR3DisasInstrRead(PDISCPUSTATE pDis, uint8_t offInstr, uint8_t cbMinRead, uint8_t cbMaxRead) +{ + PDBGFDISASSTATE pState = (PDBGFDISASSTATE)pDis; for (;;) { - RTGCUINTPTR GCPtr = uSrcAddr + pState->GCPtrSegBase; - - /* Need to update the page translation? */ + RTGCUINTPTR GCPtr = pDis->uInstrAddr + offInstr + pState->GCPtrSegBase; + + /* + * Need to update the page translation? + */ if ( !pState->pvPageR3 - || (GCPtr >> PAGE_SHIFT) != (pState->pvPageGC >> PAGE_SHIFT)) + || (GCPtr >> PAGE_SHIFT) != (pState->GCPtrPage >> PAGE_SHIFT)) { int rc = VINF_SUCCESS; /* translate the address */ - pState->pvPageGC = GCPtr & PAGE_BASE_GC_MASK; - if (MMHyperIsInsideArea(pState->pVM, pState->pvPageGC)) + pState->GCPtrPage = GCPtr & PAGE_BASE_GC_MASK; + if (MMHyperIsInsideArea(pState->pVM, pState->GCPtrPage)) { - pState->pvPageR3 = MMHyperRCToR3(pState->pVM, (RTRCPTR)pState->pvPageGC); + pState->pvPageR3 = MMHyperRCToR3(pState->pVM, (RTRCPTR)pState->GCPtrPage); if (!pState->pvPageR3) rc = VERR_INVALID_POINTER; @@ -232,7 +233,7 @@ if (pState->enmMode <= PGMMODE_PROTECTED) - rc = PGMPhysGCPhys2CCPtrReadOnly(pState->pVM, pState->pvPageGC, &pState->pvPageR3, &pState->PageMapLock); + rc = PGMPhysGCPhys2CCPtrReadOnly(pState->pVM, pState->GCPtrPage, &pState->pvPageR3, &pState->PageMapLock); else - rc = PGMPhysGCPtr2CCPtrReadOnly(pState->pVCpu, pState->pvPageGC, &pState->pvPageR3, &pState->PageMapLock); + rc = PGMPhysGCPtr2CCPtrReadOnly(pState->pVCpu, pState->GCPtrPage, &pState->pvPageR3, &pState->PageMapLock); pState->fLocked = RT_SUCCESS_NP(rc); } @@ -244,9 +245,13 @@ } - /* check the segment limit */ - if (!pState->f64Bits && uSrcAddr > pState->cbSegLimit) + /* + * Check the segment limit. + */ + if (!pState->f64Bits && pDis->uInstrAddr + offInstr > pState->cbSegLimit) return VERR_OUT_OF_SELECTOR_BOUNDS; - /* calc how much we can read */ + /* + * Calc how much we can read, maxing out the read. + */ uint32_t cb = PAGE_SIZE - (GCPtr & PAGE_OFFSET_MASK); if (!pState->f64Bits) @@ -256,14 +261,19 @@ cb = cbSeg; } - if (cb > cbToRead) - cb = cbToRead; - - /* read and advance */ - memcpy(pbDst, (char *)pState->pvPageR3 + (GCPtr & PAGE_OFFSET_MASK), cb); - cbToRead -= cb; - if (!cbToRead) + if (cb > cbMaxRead) + cb = cbMaxRead; + + /* + * Read and advance, + */ + memcpy(&pDis->abInstr[offInstr], (char *)pState->pvPageR3 + (GCPtr & PAGE_OFFSET_MASK), cb); + offInstr += (uint8_t)cb; + if (cb >= cbMinRead) + { + pDis->cbCachedInstr = offInstr; return VINF_SUCCESS; - pbDst += cb; - uSrcAddr += cb; + } + cbMaxRead -= (uint8_t)cb; + cbMinRead -= (uint8_t)cb; } } @@ -500,16 +510,14 @@ else { - uint32_t cbBits = State.Cpu.cbInstr; - uint8_t *pau8Bits = (uint8_t *)alloca(cbBits); - rc = dbgfR3DisasInstrRead(&State.Cpu, pau8Bits, GCPtr, cbBits); - AssertRC(rc); + uint32_t cbInstr = State.Cpu.cbInstr; + uint8_t const *pabInstr = State.Cpu.abInstr; if (fFlags & DBGF_DISAS_FLAGS_NO_ADDRESS) RTStrPrintf(pszOutput, cbOutput, "%.*Rhxs%*s %s", - cbBits, pau8Bits, cbBits < 8 ? (8 - cbBits) * 3 : 0, "", + cbInstr, pabInstr, cbInstr < 8 ? (8 - cbInstr) * 3 : 0, "", szBuf); else if (fRealModeAddress) RTStrPrintf(pszOutput, cbOutput, "%04x:%04x %.*Rhxs%*s %s", Sel, (unsigned)GCPtr, - cbBits, pau8Bits, cbBits < 8 ? (8 - cbBits) * 3 : 0, "", + cbInstr, pabInstr, cbInstr < 8 ? (8 - cbInstr) * 3 : 0, "", szBuf); else if (Sel == DBGF_SEL_FLAT) @@ -518,10 +526,10 @@ RTStrPrintf(pszOutput, cbOutput, "%RGv %.*Rhxs%*s %s", GCPtr, - cbBits, pau8Bits, cbBits < 8 ? (8 - cbBits) * 3 : 0, "", + cbInstr, pabInstr, cbInstr < 8 ? (8 - cbInstr) * 3 : 0, "", szBuf); else RTStrPrintf(pszOutput, cbOutput, "%08RX32 %.*Rhxs%*s %s", (uint32_t)GCPtr, - cbBits, pau8Bits, cbBits < 8 ? (8 - cbBits) * 3 : 0, "", + cbInstr, pabInstr, cbInstr < 8 ? (8 - cbInstr) * 3 : 0, "", szBuf); } @@ -531,10 +539,10 @@ RTStrPrintf(pszOutput, cbOutput, "%04x:%RGv %.*Rhxs%*s %s", Sel, GCPtr, - cbBits, pau8Bits, cbBits < 8 ? (8 - cbBits) * 3 : 0, "", + cbInstr, pabInstr, cbInstr < 8 ? (8 - cbInstr) * 3 : 0, "", szBuf); else RTStrPrintf(pszOutput, cbOutput, "%04x:%08RX32 %.*Rhxs%*s %s", Sel, (uint32_t)GCPtr, - cbBits, pau8Bits, cbBits < 8 ? (8 - cbBits) * 3 : 0, "", + cbInstr, pabInstr, cbInstr < 8 ? (8 - cbInstr) * 3 : 0, "", szBuf); } Index: /trunk/src/VBox/VMM/VMMR3/PATM.cpp =================================================================== --- /trunk/src/VBox/VMM/VMMR3/PATM.cpp (revision 41759) +++ /trunk/src/VBox/VMM/VMMR3/PATM.cpp (revision 41760) @@ -80,5 +80,5 @@ PVM pVM; PPATCHINFO pPatchInfo; - R3PTRTYPE(uint8_t *) pInstrHC; + R3PTRTYPE(uint8_t *) pbInstrHC; RTRCPTR pInstrGC; uint32_t fReadFlags; @@ -534,13 +534,12 @@ } -DECLCALLBACK(int) patmReadBytes(PDISCPUSTATE pDisState, uint8_t *pbDst, RTUINTPTR uSrcAddr, uint32_t cbToRead) -{ - PATMDISASM *pDisInfo = (PATMDISASM *)pDisState->pvUser; - int orgsize = cbToRead; - - Assert(cbToRead); - if (cbToRead == 0) - return VERR_INVALID_PARAMETER; - +/** + * @callback_method_impl{FNDISREADBYTES} + */ +static DECLCALLBACK(int) patmReadBytes(PDISCPUSTATE pDis, uint8_t offInstr, uint8_t cbMinRead, uint8_t cbMaxRead) +{ + PATMDISASM *pDisInfo = (PATMDISASM *)pDis->pvUser; + +/** @todo change this to read more! */ /* * Trap/interrupt handler typically call common code on entry. Which might already have patches inserted. @@ -550,45 +549,51 @@ if (pDisInfo->fReadFlags & PATMREAD_ORGCODE) { - for (int i = 0; i < orgsize; i++) - { - int rc = PATMR3QueryOpcode(pDisInfo->pVM, (RTRCPTR)uSrcAddr, pbDst); + for (;;) + { + int rc = PATMR3QueryOpcode(pDisInfo->pVM, (RTGCPTR32)pDis->uInstrAddr + offInstr, &pDis->abInstr[offInstr]); if (RT_FAILURE(rc)) - break; - uSrcAddr++; - pbDst++; - cbToRead--; - } - if (cbToRead == 0) - return VINF_SUCCESS; + break; /* VERR_PATCH_NOT_FOUND */ + offInstr++; + cbMinRead--; + if (cbMinRead == 0) + { + pDis->cbCachedInstr = offInstr; + return VINF_SUCCESS; + } + cbMaxRead--; + } + #ifdef VBOX_STRICT - if ( !(pDisInfo->pPatchInfo->flags & (PATMFL_DUPLICATE_FUNCTION|PATMFL_IDTHANDLER)) - && !(pDisInfo->fReadFlags & PATMREAD_NOCHECK)) - { - Assert(PATMR3IsInsidePatchJump(pDisInfo->pVM, uSrcAddr, NULL) == false); - Assert(PATMR3IsInsidePatchJump(pDisInfo->pVM, uSrcAddr+cbToRead-1, NULL) == false); + if ( !(pDisInfo->pPatchInfo->flags & (PATMFL_DUPLICATE_FUNCTION|PATMFL_IDTHANDLER)) + && !(pDisInfo->fReadFlags & PATMREAD_NOCHECK)) + { + Assert(PATMR3IsInsidePatchJump(pDisInfo->pVM, pDis->uInstrAddr + offInstr, NULL) == false); + Assert(PATMR3IsInsidePatchJump(pDisInfo->pVM, pDis->uInstrAddr + offInstr + cbMinRead-1, NULL) == false); } #endif } - if ( !pDisInfo->pInstrHC - || ( PAGE_ADDRESS(pDisInfo->pInstrGC) != PAGE_ADDRESS(uSrcAddr + cbToRead - 1) - && !PATMIsPatchGCAddr(pDisInfo->pVM, uSrcAddr))) + int rc = VINF_SUCCESS; + RTGCPTR32 uSrcAddr = (RTGCPTR32)pDis->uInstrAddr + offInstr; + if ( !pDisInfo->pbInstrHC + || ( PAGE_ADDRESS(pDisInfo->pInstrGC) != PAGE_ADDRESS(uSrcAddr + cbMinRead - 1) + && !PATMIsPatchGCAddr(pDisInfo->pVM, uSrcAddr))) { Assert(!PATMIsPatchGCAddr(pDisInfo->pVM, uSrcAddr)); - return PGMPhysSimpleReadGCPtr(&pDisInfo->pVM->aCpus[0], pbDst, uSrcAddr, cbToRead); - } - - Assert(pDisInfo->pInstrHC); - - uint8_t *pInstrHC = pDisInfo->pInstrHC; - - Assert(pInstrHC); - - /* pInstrHC is the base address; adjust according to the GC pointer. */ - pInstrHC = pInstrHC + (uSrcAddr - pDisInfo->pInstrGC); - - memcpy(pbDst, (void *)pInstrHC, cbToRead); - - return VINF_SUCCESS; + rc = PGMPhysSimpleReadGCPtr(&pDisInfo->pVM->aCpus[0], &pDis->abInstr[offInstr], uSrcAddr, cbMinRead); + offInstr += cbMinRead; + } + else + { + /* pbInstrHC is the base address; adjust according to the GC pointer. */ + uint8_t const *pbInstrHC = pDisInfo->pbInstrHC; AssertPtr(pbInstrHC); + pbInstrHC += uSrcAddr - pDisInfo->pInstrGC; + + memcpy(&pDis->abInstr[offInstr], pbInstrHC, cbMinRead); + offInstr += cbMinRead; + } + + pDis->cbCachedInstr = offInstr; + return rc; } @@ -600,5 +605,5 @@ disinfo.pVM = pVM; disinfo.pPatchInfo = pPatch; - disinfo.pInstrHC = pbInstrHC; + disinfo.pbInstrHC = pbInstrHC; disinfo.pInstrGC = InstrGCPtr32; disinfo.fReadFlags = fReadFlags; @@ -616,5 +621,5 @@ disinfo.pVM = pVM; disinfo.pPatchInfo = pPatch; - disinfo.pInstrHC = pbInstrHC; + disinfo.pbInstrHC = pbInstrHC; disinfo.pInstrGC = InstrGCPtr32; disinfo.fReadFlags = fReadFlags; @@ -633,5 +638,5 @@ disinfo.pVM = pVM; disinfo.pPatchInfo = pPatch; - disinfo.pInstrHC = pbInstrHC; + disinfo.pbInstrHC = pbInstrHC; disinfo.pInstrGC = InstrGCPtr32; disinfo.fReadFlags = fReadFlags; Index: /trunk/src/VBox/VMM/include/PATMInternal.h =================================================================== --- /trunk/src/VBox/VMM/include/PATMInternal.h (revision 41759) +++ /trunk/src/VBox/VMM/include/PATMInternal.h (revision 41760) @@ -5,5 +5,5 @@ /* - * Copyright (C) 2006-2007 Oracle Corporation + * Copyright (C) 2006-2012 Oracle Corporation * * This file is part of VirtualBox Open Source Edition (OSE), as @@ -675,6 +675,4 @@ -FNDISREADBYTES patmReadBytes; - RT_C_DECLS_BEGIN