Changeset 102737 in vbox

Timestamp:

Dec 31, 2023 4:05:24 PM (11 months ago)

Author:

vboxsync

Message:

VMM/IEM: Use correct TLBE flags when doing lookups. duh. bugref:10371

Location:

trunk/src/VBox/VMM

Files:

• VMMAll/IEMAllN8veHlpA.asm (modified) (2 diffs)
• VMMAll/IEMAllN8veRecompiler.cpp (modified) (8 diffs)
• include/IEMN8veRecompiler.h (modified) (1 diff)

trunk/src/VBox/VMM/VMMAll/IEMAllN8veHlpA.asm

@@ -36 +36 @@
 
 extern NAME(iemThreadedFunc_BltIn_LogCpuStateWorker)
+extern NAME(iemNativeHlpCheckTlbLookup)
 
 
@@ -100 +101 @@
 ENDPROC     iemNativeHlpAsmSafeWrapLogCpuState
 
+
+;;
+; This is wrapper function that saves and restores all volatile registers
+; so the impact of inserting CheckTlbLookup is minimal to the other TB code.
+;
+BEGINPROC   iemNativeHlpAsmSafeWrapCheckTlbLookup
+        push    xBP
+        SEH64_PUSH_xBP
+        mov     xBP, xSP
+        SEH64_SET_FRAME_xBP 0
+SEH64_END_PROLOGUE
+
+        ;
+        ; Save all volatile registers.
+        ;
+        push    xAX
+        push    xCX
+        push    xDX
+%ifdef RT_OS_WINDOWS
+        push    xSI
+        push    xDI
+%endif
+        push    r8
+        push    r9
+        push    r10
+        push    r11
+        sub     rsp, 8+20h
+
+        ;
+        ; Call C function to do the actual work.
+        ;
+%ifdef RT_OS_WINDOWS
+        mov     rcx, [rbp + 10h]
+        mov     rdx, [rbp + 18h]
+        mov     r8,  [rbp + 20h]
+        mov     r9,  [rbp + 28h]
+%else
+        mov     rdi, [rbp + 10h]
+        mov     rsi, [rbp + 18h]
+        mov     ecx, [rbp + 20h]
+        mov     edx, [rbp + 28h]
+%endif
+        call    NAME(iemNativeHlpCheckTlbLookup)
+
+        ;
+        ; Restore volatile registers and return to the TB code.
+        ;
+        add     rsp, 8+20h
+        pop     r11
+        pop     r10
+        pop     r9
+        pop     r8
+%ifdef RT_OS_WINDOWS
+        pop     xDI
+        pop     xSI
+%endif
+        pop     xDX
+        pop     xCX
+        pop     xAX
+        leave
+        ret     20h
+ENDPROC     iemNativeHlpAsmSafeWrapCheckTlbLookup
+

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

@@ -7143 +7143 @@
                 AssertFailed();
             }
+            else
+            {
+                /*
+                 * Allocate a temporary stack slot and spill the register to it.
+                 */
+                unsigned const idxStackSlot = ASMBitLastSetU32(~pReNative->Core.bmStack) - 1;
+                AssertStmt(idxStackSlot < IEMNATIVE_FRAME_VAR_SLOTS,
+                           IEMNATIVE_DO_LONGJMP(pReNative, VERR_IEM_VAR_OUT_OF_STACK_SLOTS));
+                pReNative->Core.bmStack |= RT_BIT_32(idxStackSlot);
+                pReNative->Core.aHstRegs[idxHstReg].idxStackSlot = (uint8_t)idxStackSlot;
+                Log12(("iemNativeVarSaveVolatileRegsPreHlpCall: spilling idxReg=%d onto the stack (slot %#x bp+%d, off=%#x)\n",
+                       idxHstReg, idxStackSlot, iemNativeStackCalcBpDisp(idxStackSlot), off));
+                off = iemNativeEmitStoreGprByBp(pReNative, off, iemNativeStackCalcBpDisp(idxStackSlot), idxHstReg);
+            }
         } while (fHstRegs);
     }
@@ -7201 +7215 @@
                 }
                 AssertFailed();
+            }
+            else
+            {
+                /*
+                 * Restore from temporary stack slot.
+                 */
+                uint8_t const idxStackSlot = pReNative->Core.aHstRegs[idxHstReg].idxStackSlot;
+                AssertContinue(idxStackSlot < IEMNATIVE_FRAME_VAR_SLOTS && (pReNative->Core.bmStack & RT_BIT_32(idxStackSlot)));
+                pReNative->Core.bmStack &= ~RT_BIT_32(idxStackSlot);
+                pReNative->Core.aHstRegs[idxHstReg].idxStackSlot = UINT8_MAX;
+
+                off = iemNativeEmitLoadGprByBp(pReNative, off, idxHstReg, iemNativeStackCalcBpDisp(idxStackSlot));
             }
         } while (fHstRegs);
@@ -10093 +10119 @@
 
 
+/**
+ * This is called via iemNativeHlpAsmSafeWrapCheckTlbLookup.
+ */
+DECLASM(void) iemNativeHlpCheckTlbLookup(PVMCPU pVCpu, uintptr_t uResult, uint64_t GCPtr, uint32_t uSegAndSizeAndAccess)
+{
+    uint8_t const  iSegReg = RT_BYTE1(uSegAndSizeAndAccess);
+    uint8_t const  cbMem   = RT_BYTE2(uSegAndSizeAndAccess);
+    uint32_t const fAccess = uSegAndSizeAndAccess >> 16;
+    Log(("iemNativeHlpCheckTlbLookup: %x:%#RX64 LB %#x fAccess=%#x -> %#RX64\n", iSegReg, GCPtr, cbMem, fAccess, uResult));
+
+    /* Do the lookup manually. */
+    RTGCPTR const      GCPtrFlat = iSegReg == UINT8_MAX ? GCPtr : GCPtr + pVCpu->cpum.GstCtx.aSRegs[iSegReg].u64Base;
+    uint64_t const     uTag      = IEMTLB_CALC_TAG(    &pVCpu->iem.s.DataTlb, GCPtrFlat);
+    PIEMTLBENTRY const pTlbe     = IEMTLB_TAG_TO_ENTRY(&pVCpu->iem.s.DataTlb, uTag);
+    if (RT_LIKELY(pTlbe->uTag == uTag))
+    {
+        /*
+         * Check TLB page table level access flags.
+         */
+        AssertCompile(IEMTLBE_F_PT_NO_USER == 4);
+        uint64_t const fNoUser          = (IEM_GET_CPL(pVCpu) + 1) & IEMTLBE_F_PT_NO_USER;
+        uint64_t const fNoWriteNoDirty  = !(fAccess & IEM_ACCESS_TYPE_WRITE) ? 0
+                                        : IEMTLBE_F_PT_NO_WRITE | IEMTLBE_F_PT_NO_DIRTY | IEMTLBE_F_PG_NO_WRITE;
+        uint64_t const fFlagsAndPhysRev = pTlbe->fFlagsAndPhysRev & (  IEMTLBE_F_PHYS_REV       | IEMTLBE_F_NO_MAPPINGR3
+                                                                     | IEMTLBE_F_PG_UNASSIGNED
+                                                                     | IEMTLBE_F_PT_NO_ACCESSED
+                                                                     | fNoWriteNoDirty          | fNoUser);
+        uint64_t const uTlbPhysRev      = pVCpu->iem.s.DataTlb.uTlbPhysRev;
+        if (RT_LIKELY(fFlagsAndPhysRev == uTlbPhysRev))
+        {
+            /*
+             * Return the address.
+             */
+            uint8_t const * const pbAddr = &pTlbe->pbMappingR3[GCPtrFlat & GUEST_PAGE_OFFSET_MASK];
+            if ((uintptr_t)pbAddr == uResult)
+                return;
+            AssertFailed();
+        }
+        else
+            AssertMsgFailed(("fFlagsAndPhysRev=%#RX64 vs uTlbPhysRev=%#RX64: %#RX64\n",
+                             fFlagsAndPhysRev, uTlbPhysRev, fFlagsAndPhysRev ^ uTlbPhysRev));
+    }
+    else
+        AssertFailed();
+    __debugbreak();
+}
+DECLASM(void) iemNativeHlpAsmSafeWrapCheckTlbLookup(void);
+
+
 #ifdef IEMNATIVE_WITH_TLB_LOOKUP
 DECL_INLINE_THROW(uint32_t)
@@ -10427 +10502 @@
     AssertCompile(IEMTLBE_F_PT_NO_USER == 4);
     uint64_t const fNoUser = (((pReNative->fExec >> IEM_F_X86_CPL_SHIFT) & IEM_F_X86_CPL_SMASK) + 1) & IEMTLBE_F_PT_NO_USER;
-    off = iemNativeEmitLoadGprImmEx(pCodeBuf, off, pTlbState->idxReg1,
-                                      IEMTLBE_F_PHYS_REV       | IEMTLBE_F_NO_MAPPINGR3
-                                    | IEMTLBE_F_PG_UNASSIGNED  | IEMTLBE_F_PG_NO_READ
-                                    | IEMTLBE_F_PT_NO_ACCESSED | fNoUser);
+    uint64_t       fTlbe   = IEMTLBE_F_PHYS_REV | IEMTLBE_F_NO_MAPPINGR3 | IEMTLBE_F_PG_UNASSIGNED | IEMTLBE_F_PT_NO_ACCESSED
+                           | fNoUser;
+    if (fAccess & IEM_ACCESS_TYPE_READ)
+        fTlbe |= IEMTLBE_F_PG_NO_READ;
+    if (fAccess & IEM_ACCESS_TYPE_WRITE)
+        fTlbe |= IEMTLBE_F_PT_NO_WRITE | IEMTLBE_F_PG_NO_WRITE | IEMTLBE_F_PT_NO_DIRTY;
+    off = iemNativeEmitLoadGprImmEx(pCodeBuf, off, pTlbState->idxReg1, fTlbe);
 # if defined(RT_ARCH_AMD64)
     /* and reg1, [reg2->fFlagsAndPhysRev] */
@@ -10462 +10540 @@
     off = iemNativeEmitLoadGprByGprEx(pCodeBuf, off, pTlbState->idxReg1, pTlbState->idxReg2,
                                       RT_UOFFSETOF(IEMTLBENTRY, pbMappingR3));
-    /* if (!reg1) jmp tlbmiss */
+    /* if (!reg1) goto tlbmiss; */
     /** @todo eliminate the need for this test? */
     off = iemNativeEmitTestIfGprIsZeroAndJmpToLabelEx(pReNative, pCodeBuf, off, pTlbState->idxReg1,
@@ -10480 +10558 @@
     /* add result, reg1 */
     off = iemNativeEmitAddTwoGprsEx(pCodeBuf, off, idxRegMemResult, pTlbState->idxReg1);
+
+# if 0
+    /*
+     * To verify the result we call a helper function.
+     *
+     * It's like the state logging, so parameters are passed on the stack.
+     * iemNativeHlpAsmSafeWrapCheckTlbLookup(pVCpu, result, addr, seg | (cbMem << 8) | (fAccess << 16))
+     */
+#  ifdef RT_ARCH_AMD64
+    /* push     seg | (cbMem << 8) | (fAccess << 16) */
+    pCodeBuf[off++] = 0x68;
+    pCodeBuf[off++] = iSegReg;
+    pCodeBuf[off++] = cbMem;
+    pCodeBuf[off++] = RT_BYTE1(fAccess);
+    pCodeBuf[off++] = RT_BYTE2(fAccess);
+    /* push     pTlbState->idxRegPtr / immediate address. */
+    if (pTlbState->idxRegPtr != UINT8_MAX)
+    {
+        if (pTlbState->idxRegPtr >= 8)
+            pCodeBuf[off++] = X86_OP_REX_B;
+        pCodeBuf[off++] = 0x50 + (pTlbState->idxRegPtr & 7);
+    }
+    else
+    {
+        off = iemNativeEmitLoadGprImmEx(pCodeBuf, off, pTlbState->idxReg1, pTlbState->uAbsPtr);
+        if (pTlbState->idxReg1 >= 8)
+            pCodeBuf[off++] = X86_OP_REX_B;
+        pCodeBuf[off++] = 0x50 + (pTlbState->idxReg1 & 7);
+    }
+    /* push     idxRegMemResult */
+    if (idxRegMemResult >= 8)
+        pCodeBuf[off++] = X86_OP_REX_B;
+    pCodeBuf[off++] = 0x50 + (idxRegMemResult & 7);
+    /* push     pVCpu */
+    pCodeBuf[off++] = 0x50 + IEMNATIVE_REG_FIXED_PVMCPU;
+    /* mov      reg1, helper */
+    off = iemNativeEmitLoadGprImmEx(pCodeBuf, off, pTlbState->idxReg1, (uintptr_t)iemNativeHlpAsmSafeWrapCheckTlbLookup);
+    /* call     [reg1] */
+    pCodeBuf[off++] = X86_OP_REX_W | (pTlbState->idxReg1 < 8 ? 0 : X86_OP_REX_B);
+    pCodeBuf[off++] = 0xff;
+    pCodeBuf[off++] = X86_MODRM_MAKE(X86_MOD_REG, 2, pTlbState->idxReg1 & 7);
+    /* The stack is cleaned up by helper function. */
+
+#  else
+#   error "Port me"
+#  endif
+# endif
 
     IEMNATIVE_ASSERT_INSTR_BUF_ENSURE(pReNative, off);
@@ -11746 +11871 @@
         off = iemNativeEmitTlbLookup(pReNative, off, &TlbState, iSegReg, cbMem, fAlignMask, fAccess,
                                      idxLabelTlbLookup, idxLabelTlbMiss, idxRegMemResult);
-        TlbState.freeRegsAndReleaseVars(pReNative, idxVarGCPtrMem);
 
         /*
@@ -11758 +11882 @@
          */
         iemNativeLabelDefine(pReNative, idxLabelTlbDone, off);
+
+        TlbState.freeRegsAndReleaseVars(pReNative, idxVarGCPtrMem);
 
 # ifndef IEMNATIVE_WITH_FREE_AND_FLUSH_VOLATILE_REGS_AT_TLB_LOOKUP

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

@@ -563 +563 @@
     /** Variable index if holding a variable, otherwise UINT8_MAX. */
     uint8_t         idxVar;
+    /** Stack slot assigned by iemNativeVarSaveVolatileRegsPreHlpCall and freed
+     * by iemNativeVarRestoreVolatileRegsPostHlpCall.  This is not valid outside
+     * that scope. */
+    uint8_t         idxStackSlot;
     /** Alignment padding. */
-    uint8_t         abAlign[6];
+    uint8_t         abAlign[5];
 } IEMNATIVEHSTREG;