Changeset 41760 in vbox

Timestamp:

Jun 15, 2012 3:56:20 PM (12 years ago)

Author:

vboxsync

Message:

DIS: Chagned FNDISREADBYTES to permit reading more bytes that the immeidate request. Not using the read-ahead feature in any important code path yet, that's comming next, bit by bit.

Location:

trunk

Files:

• include/VBox/dis.h (modified) (1 diff)
• src/VBox/Devices/PC/BIOS-new/MakeDebianBiosAssembly.cpp (modified) (1 diff)
• src/VBox/Disassembler/DisasmCore.cpp (modified) (3 diffs)
• src/VBox/Disassembler/testcase/tstDisasm-2.cpp (modified) (3 diffs)
• src/VBox/Runtime/testcase/tstLdr-3.cpp (modified) (1 diff)
• src/VBox/Runtime/testcase/tstLdr-4.cpp (modified) (1 diff)
• src/VBox/Runtime/testcase/tstLdrDisasmTest.cpp (modified) (1 diff)
• src/VBox/VMM/VMMAll/EMAll.cpp (modified) (4 diffs)
• src/VBox/VMM/VMMR3/CPUM.cpp (modified) (4 diffs)
• src/VBox/VMM/VMMR3/CSAM.cpp (modified) (2 diffs)
• src/VBox/VMM/VMMR3/DBGFDisas.cpp (modified) (9 diffs)
• src/VBox/VMM/VMMR3/PATM.cpp (modified) (6 diffs)
• src/VBox/VMM/include/PATMInternal.h (modified) (2 diffs)

trunk/include/VBox/dis.h

@@ -497 +497 @@
 
 /**
- * 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;

trunk/src/VBox/Devices/PC/BIOS-new/MakeDebianBiosAssembly.cpp

@@ -908 +908 @@
  * @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;
 }

trunk/src/VBox/Disassembler/DisasmCore.cpp

@@ -65 +65 @@
 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 +2390 @@
 
 
-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 +2439 @@
 
     /*
-     * 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;
 }
 

trunk/src/VBox/Disassembler/testcase/tstDisasm-2.cpp

@@ -167 +167 @@
 /**
  * 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 +296 @@
                           || 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 +303 @@
                 {
                     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;
                 }

trunk/src/VBox/Runtime/testcase/tstLdr-3.cpp

@@ -147 +147 @@
  * @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;
 }

trunk/src/VBox/Runtime/testcase/tstLdr-4.cpp

@@ -61 +61 @@
     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;

trunk/src/VBox/Runtime/testcase/tstLdrDisasmTest.cpp

@@ -83 +83 @@
  * @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;
 }
 

trunk/src/VBox/VMM/VMMAll/EMAll.cpp

@@ -284 +284 @@
  * @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 +308 @@
         for (unsigned i = 0; i < cbToRead; i++)
             pbDst[i] = pState->aOpcode[offset + i];
+        pDis->cbCachedInstr = offInstr + cbToRead;
         return VINF_SUCCESS;
     }
@@ -308 +314 @@
     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 +339 @@
 
 # endif /* IN_RING3 */
+    pDis->cbCachedInstr = offInstr + cbToRead;
     return VINF_SUCCESS;
 }

trunk/src/VBox/VMM/VMMR3/CPUM.cpp

@@ -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 +3527 @@
  * @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 +3566 @@
         }
 
-        /* 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 +3582 @@
                 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;
     }
 }

trunk/src/VBox/VMM/VMMR3/CSAM.cpp

@@ -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 +726 @@
  * @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;
 }
 

trunk/src/VBox/VMM/VMMR3/DBGFDisas.cpp

@@ -67 +67 @@
     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 +105 @@
     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 +201 @@
  * @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 +233 @@
 
                 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 +245 @@
         }
 
-        /* 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 +261 @@
                 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 +510 @@
     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 +526 @@
                 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 +539 @@
                 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);
         }

trunk/src/VBox/VMM/VMMR3/PATM.cpp

@@ -80 +80 @@
     PVM                  pVM;
     PPATCHINFO           pPatchInfo;
-    R3PTRTYPE(uint8_t *) pInstrHC;
+    R3PTRTYPE(uint8_t *) pbInstrHC;
     RTRCPTR              pInstrGC;
     uint32_t             fReadFlags;
@@ -534 +534 @@
 }
 
-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 +549 @@
     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 +605 @@
     disinfo.pVM         = pVM;
     disinfo.pPatchInfo  = pPatch;
-    disinfo.pInstrHC    = pbInstrHC;
+    disinfo.pbInstrHC   = pbInstrHC;
     disinfo.pInstrGC    = InstrGCPtr32;
     disinfo.fReadFlags  = fReadFlags;
@@ -616 +621 @@
     disinfo.pVM         = pVM;
     disinfo.pPatchInfo  = pPatch;
-    disinfo.pInstrHC    = pbInstrHC;
+    disinfo.pbInstrHC   = pbInstrHC;
     disinfo.pInstrGC    = InstrGCPtr32;
     disinfo.fReadFlags  = fReadFlags;
@@ -633 +638 @@
     disinfo.pVM         = pVM;
     disinfo.pPatchInfo  = pPatch;
-    disinfo.pInstrHC    = pbInstrHC;
+    disinfo.pbInstrHC   = pbInstrHC;
     disinfo.pInstrGC    = InstrGCPtr32;
     disinfo.fReadFlags  = fReadFlags;

trunk/src/VBox/VMM/include/PATMInternal.h

@@ -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 +675 @@
 
 
-FNDISREADBYTES patmReadBytes;
-
 
 RT_C_DECLS_BEGIN