From f798a23a85a79ca35f2b883787e521dd0ce29ee1 Mon Sep 17 00:00:00 2001 From: Luke Usher Date: Mon, 10 Sep 2018 11:24:53 +0100 Subject: [PATCH 1/8] Fix wrong calling convention around calls to KeGetPCR --- src/CxbxKrnl/EmuFS.cpp | 2 +- src/CxbxKrnl/EmuKrnl.cpp | 2 +- src/CxbxKrnl/EmuKrnlHal.cpp | 2 +- src/CxbxKrnl/EmuKrnlKe.cpp | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/CxbxKrnl/EmuFS.cpp b/src/CxbxKrnl/EmuFS.cpp index 20a2bdc66..fcec2b879 100644 --- a/src/CxbxKrnl/EmuFS.cpp +++ b/src/CxbxKrnl/EmuFS.cpp @@ -127,7 +127,7 @@ NT_TIB *GetNtTib() } -xboxkrnl::KPCR* KeGetPcr(); +xboxkrnl::KPCR* WINAPI KeGetPcr(); uint32_t fs_lock = 0; diff --git a/src/CxbxKrnl/EmuKrnl.cpp b/src/CxbxKrnl/EmuKrnl.cpp index d3800a53d..cf9d4a629 100644 --- a/src/CxbxKrnl/EmuKrnl.cpp +++ b/src/CxbxKrnl/EmuKrnl.cpp @@ -131,7 +131,7 @@ xboxkrnl::PLIST_ENTRY RemoveTailList(xboxkrnl::PLIST_ENTRY pListHead) // * Declaring this in a header causes errors with xboxkrnl // * namespace, so we must declare it within any file that uses it // ****************************************************************** -xboxkrnl::KPCR* KeGetPcr(); +xboxkrnl::KPCR* WINAPI KeGetPcr(); // Interrupts diff --git a/src/CxbxKrnl/EmuKrnlHal.cpp b/src/CxbxKrnl/EmuKrnlHal.cpp index c3c9943e0..a421cc113 100644 --- a/src/CxbxKrnl/EmuKrnlHal.cpp +++ b/src/CxbxKrnl/EmuKrnlHal.cpp @@ -76,7 +76,7 @@ LIST_ENTRY_DEFINE_HEAD(ShutdownRoutineList); // * Declaring this in a header causes errors with xboxkrnl // * namespace, so we must declare it within any file that uses it // ****************************************************************** -xboxkrnl::KPCR* KeGetPcr(); +xboxkrnl::KPCR* WINAPI KeGetPcr(); // ****************************************************************** // * 0x0009 - HalReadSMCTrayState() diff --git a/src/CxbxKrnl/EmuKrnlKe.cpp b/src/CxbxKrnl/EmuKrnlKe.cpp index f713cfeef..8d01fdf00 100644 --- a/src/CxbxKrnl/EmuKrnlKe.cpp +++ b/src/CxbxKrnl/EmuKrnlKe.cpp @@ -186,13 +186,13 @@ void FASTCALL KiWaitSatisfyAll // * NOTE: This is a macro on the Xbox, however we implement it // * as a function so it can suit our emulated KPCR structure // ****************************************************************** -xboxkrnl::KPCR* KeGetPcr() +xboxkrnl::KPCR* WINAPI KeGetPcr() { xboxkrnl::PKPCR Pcr; // See EmuKeSetPcr() Pcr = (xboxkrnl::PKPCR)__readfsdword(TIB_ArbitraryDataSlot); - + if (Pcr == nullptr) { EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "KeGetPCR returned nullptr: Was this called from a non-xbox thread?"); // Attempt to salvage the situation by calling InitXboxThread to setup KPCR in place From 1102133475dfa372d7a9c2bdd89a431a1e4fef59 Mon Sep 17 00:00:00 2001 From: Luke Usher Date: Sat, 15 Sep 2018 12:20:28 +0100 Subject: [PATCH 2/8] Attempt to interpret code blocks rather than single instructions, reducing MMIO overheads --- src/CxbxKrnl/EmuX86.cpp | 293 ++++++++++++++++++++++++++-------------- 1 file changed, 193 insertions(+), 100 deletions(-) diff --git a/src/CxbxKrnl/EmuX86.cpp b/src/CxbxKrnl/EmuX86.cpp index 7571ac2eb..db2756844 100644 --- a/src/CxbxKrnl/EmuX86.cpp +++ b/src/CxbxKrnl/EmuX86.cpp @@ -192,13 +192,8 @@ uint32_t EmuX86_Read(xbaddr addr, int size) return value; } - if (g_bEmuException) { - EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "EmuX86_Read(0x%08X, %d) [Unknown address]", addr, size); - value = 0; - } else { - // Outside EmuException, pass the memory-access through to normal memory : - value = EmuX86_Mem_Read(addr, size); - } + //pass the memory-access through to normal memory : + value = EmuX86_Mem_Read(addr, size); DbgPrintf(LOG_PREFIX, "Read(0x%08X, %d) = 0x%08X\n", addr, size, value); } @@ -224,12 +219,7 @@ void EmuX86_Write(xbaddr addr, uint32_t value, int size) return; } - if (g_bEmuException) { - EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "EmuX86_Write(0x%08X, 0x%08X) [Unknown address]", addr, value); - return; - } - - // Outside EmuException, pass the memory-access through to normal memory : + // Pass the memory-access through to normal memory : DbgPrintf(LOG_PREFIX, "Write(0x%.8X, 0x%.8X, %d)\n", addr, value, size); EmuX86_Mem_Write(addr, value, size); } @@ -860,6 +850,33 @@ bool EmuX86_Opcode_INC(LPEXCEPTION_POINTERS e, _DInst& info) /*EMUX86_EFLAG_PF*/PFCalc(result)); return true; +} + +bool EmuX86_Opcode_JMP(LPEXCEPTION_POINTERS e, _DInst& info) +{ + OperandAddress opAddr; + EmuX86_Operand_Addr_ForReadOnly(e, info, 0, opAddr); + + e->ContextRecord->Eip += opAddr.addr; + + return true; +} + +// Jump if condition is met +// Returns true if branch was taken +bool EmuX86_Opcode_JXX(LPEXCEPTION_POINTERS e, _DInst& info, DWORD flag, bool condition) +{ + OperandAddress opAddr; + EmuX86_Operand_Addr_ForReadOnly(e, info, 0, opAddr); + + // Branch if flag = condition + bool flagValue = (e->ContextRecord->EFlags & flag); + if (flagValue == condition) { + e->ContextRecord->Eip += opAddr.addr; + return true; + } + + return false; } bool EmuX86_Opcode_MOV(LPEXCEPTION_POINTERS e, _DInst& info) @@ -1035,6 +1052,37 @@ bool EmuX86_Opcode_TEST(LPEXCEPTION_POINTERS e, _DInst& info) return true; } +bool EmuX86_Opcode_XOR(LPEXCEPTION_POINTERS e, _DInst& info) +{ + // Read value from Source and Destination + uint32_t src = 0; + if (!EmuX86_Operand_Read(e, info, 1, &src)) + return false; + + // OR reads and writes the same operand : + OperandAddress opAddr; + if (!EmuX86_Operand_Addr_ForReadWrite(e, info, 0, OUT opAddr)) + return false; + + uint32_t dest = EmuX86_Addr_Read(opAddr); + + // OR Destination with src + uint32_t result = dest ^ src; + + // Write back the result + EmuX86_Addr_Write(opAddr, result); + + // The OF and CF flags are cleared; the SF, ZF, and PF flags are set according to the result. The state of the AF flag is undefined. + EmuX86_SetFlags_OSZPC(e, + /*EMUX86_EFLAG_OF*/0, + /*EMUX86_EFLAG_SF*/SFCalc(result), + /*EMUX86_EFLAG_ZF*/ZFCalc(result), + /*EMUX86_EFLAG_PF*/PFCalc(result), + /*EMUX86_EFLAG_CF*/0); + + return true; +} + void EmuX86_Opcode_CLI() { g_bEnableAllInterrupts = false; @@ -1081,98 +1129,143 @@ bool EmuX86_DecodeException(LPEXCEPTION_POINTERS e) // If an opcode or one of it's operand can't be decoded, that's a clear failure. // However, if for any reason, an opcode operand cannot be read from or written to, // that case may be logged, but it shouldn't fail the opcode handler. - _DInst info; - if (!EmuX86_DecodeOpcode((uint8_t*)e->ContextRecord->Eip, OUT info)) { - EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "Error decoding opcode at 0x%08X", e->ContextRecord->Eip); - return false; - } + _DInst info; + + xbaddr currentEip; // Used to detect taken branches/jumps + + // Execute op-codes until we hit an unhandled instruction, or an error occurs + while (true) { + currentEip = e->ContextRecord->Eip; - switch (info.opcode) { // Keep these cases alphabetically ordered and condensed - case I_ADD: - if (EmuX86_Opcode_ADD(e, info)) break; - goto opcode_error; - case I_AND: - if (EmuX86_Opcode_AND(e, info)) break; - goto opcode_error; - case I_CLI: { - // Disable all interrupts - EmuX86_Opcode_CLI(); - break; - } - case I_CMP: - if (EmuX86_Opcode_CMP(e, info)) break; - goto opcode_error; - case I_CMPXCHG: - if (EmuX86_Opcode_CMPXCHG(e, info)) break; - goto opcode_error; - case I_CPUID: - EmuX86_Opcode_CPUID(e, info); - break; - case I_DEC: - if (EmuX86_Opcode_DEC(e, info)) break; - goto opcode_error; - case I_IN: - if (EmuX86_Opcode_IN(e, info)) break; - goto opcode_error; - case I_INC: - if (EmuX86_Opcode_INC(e, info)) break; - goto opcode_error; - case I_INVD: // Flush internal caches; initiate flushing of external caches. - break; // We can safely ignore this - case I_INVLPG: { - // This instruction invalidates the TLB entry specified with the source operand. Since we don't emulate - // the TLB yet, we can safely ignore this. Test case: Fable. - break; - } - case I_MOV: - if (EmuX86_Opcode_MOV(e, info)) break; - goto opcode_error; - case I_MOVSX: - if (EmuX86_Opcode_MOVSX(e, info)) break; - goto opcode_error; - case I_MOVZX: - if (EmuX86_Opcode_MOVZX(e, info)) break; - goto opcode_error; - case I_OR: - if (EmuX86_Opcode_OR(e, info)) break; - goto opcode_error; - case I_OUT: - if (EmuX86_Opcode_OUT(e, info)) break; - goto opcode_error; - case I_STI: { - // Enable all interrupts - EmuX86_Opcode_STI(); - break; - } - case I_SUB: - if (EmuX86_Opcode_SUB(e, info)) break; - goto opcode_error; - case I_TEST: - if (EmuX86_Opcode_TEST(e, info)) break; - goto opcode_error; - case I_WBINVD: // Write back and flush internal caches; initiate writing-back and flushing of external caches. - break; // We can safely ignore this - case I_WRMSR: - // We do not emulate processor specific registers just yet - // Some titles attempt to manually set the TSC via this instruction - // This needs fixing eventually, but should be acceptible to ignore for now! - // Chase: Hollywood Stunt Driver hits this - EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "WRMSR instruction ignored"); - break; - default: - EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "Unhandled instruction : %u", info.opcode); - e->ContextRecord->Eip += info.size; - return false; - } + if (!EmuX86_DecodeOpcode((uint8_t*)e->ContextRecord->Eip, info)) { + EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "Error decoding opcode at 0x%08X", e->ContextRecord->Eip); + return false; + } - // When falling through here, the instruction was handled correctly, - // skip over the instruction and continue execution : - e->ContextRecord->Eip += info.size; - return true; + switch (info.opcode) { // Keep these cases alphabetically ordered and condensed + // Exit and branch Opcodes come first, for clarity/visibility + case I_JA: + if (EmuX86_Opcode_JXX(e, info, BITMASK(EMUX86_EFLAG_CF) & BITMASK(EMUX86_EFLAG_ZF), false)) { + continue; + } + break; + case I_JAE: + if (EmuX86_Opcode_JXX(e, info, BITMASK(EMUX86_EFLAG_CF), false)) { + continue; + } + break; + case I_JB: + if (EmuX86_Opcode_JXX(e, info, BITMASK(EMUX86_EFLAG_CF), true)) { + continue; + } + break; + case I_JBE: + if (EmuX86_Opcode_JXX(e, info, BITMASK(EMUX86_EFLAG_CF) & BITMASK(EMUX86_EFLAG_ZF), true)) { + continue; + } + break; + case I_JNZ: + if (EmuX86_Opcode_JXX(e, info, BITMASK(EMUX86_EFLAG_ZF), false)) { + continue; + } + break; + case I_JMP: + if (EmuX86_Opcode_JMP(e, info)) { + continue; + } + break; + case I_JZ: + if (EmuX86_Opcode_JXX(e, info, BITMASK(EMUX86_EFLAG_ZF), true)) { + continue; + } + break; + case I_RET: + // RET always signifies the end of a code block + return true; + case I_ADD: + if (EmuX86_Opcode_ADD(e, info)) break; + goto opcode_error; + case I_AND: + if (EmuX86_Opcode_AND(e, info)) break; + goto opcode_error; + case I_CLI: { + // Disable all interrupts + EmuX86_Opcode_CLI(); + break; + } + case I_CMP: + if (EmuX86_Opcode_CMP(e, info)) break; + goto opcode_error; + case I_CMPXCHG: + if (EmuX86_Opcode_CMPXCHG(e, info)) break; + goto opcode_error; + case I_CPUID: + EmuX86_Opcode_CPUID(e, info); + break; + case I_DEC: + if (EmuX86_Opcode_DEC(e, info)) break; + goto opcode_error; + case I_IN: + if (EmuX86_Opcode_IN(e, info)) break; + goto opcode_error; + case I_INC: + if (EmuX86_Opcode_INC(e, info)) break; + goto opcode_error; + case I_INVD: // Flush internal caches; initiate flushing of external caches. + break; // We can safely ignore this + case I_INVLPG: { + // This instruction invalidates the TLB entry specified with the source operand. Since we don't emulate + // the TLB yet, we can safely ignore this. Test case: Fable. + break; + } + case I_MOV: + if (EmuX86_Opcode_MOV(e, info)) break; + goto opcode_error; + case I_MOVSX: + if (EmuX86_Opcode_MOVSX(e, info)) break; + goto opcode_error; + case I_MOVZX: + if (EmuX86_Opcode_MOVZX(e, info)) break; + goto opcode_error; + case I_OR: + if (EmuX86_Opcode_OR(e, info)) break; + goto opcode_error; + case I_OUT: + if (EmuX86_Opcode_OUT(e, info)) break; + goto opcode_error; + case I_STI: { + // Enable all interrupts + EmuX86_Opcode_STI(); + break; + } + case I_SUB: + if (EmuX86_Opcode_SUB(e, info)) break; + goto opcode_error; + case I_TEST: + if (EmuX86_Opcode_TEST(e, info)) break; + goto opcode_error; + case I_WBINVD: // Write back and flush internal caches; initiate writing-back and flushing of external caches. + break; // We can safely ignore this + case I_WRMSR: + // We do not emulate processor specific registers just yet + // Some titles attempt to manually set the TSC via this instruction + // This needs fixing eventually, but should be acceptible to ignore for now! + // Chase: Hollywood Stunt Driver hits this + EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "WRMSR instruction ignored"); + break; + case I_XOR: + if (EmuX86_Opcode_XOR(e, info)) break; + goto opcode_error; + default: + EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "Unhandled instruction : %u", info.opcode); + return true; + } + + e->ContextRecord->Eip += info.size; + } opcode_error: EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "0x%08X: Error while handling instruction %u", e->ContextRecord->Eip, info.opcode); // TODO : format decodedInstructions[0] - e->ContextRecord->Eip += info.size; return false; } From b8889b6857af9795e62e7d7908644b2ab5cb29c0 Mon Sep 17 00:00:00 2001 From: Luke Usher Date: Sat, 15 Sep 2018 16:19:50 +0100 Subject: [PATCH 3/8] Fix a crash on invalid memory accesses --- src/CxbxKrnl/EmuX86.cpp | 69 +++++++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/src/CxbxKrnl/EmuX86.cpp b/src/CxbxKrnl/EmuX86.cpp index db2756844..573557f0e 100644 --- a/src/CxbxKrnl/EmuX86.cpp +++ b/src/CxbxKrnl/EmuX86.cpp @@ -118,35 +118,46 @@ void EmuX86_IOWrite(xbaddr addr, uint32_t value, int size) // uint32_t EmuX86_Mem_Read(xbaddr addr, int size) -{ - switch (size) { - case sizeof(uint32_t) : - return *(uint32_t*)addr; - case sizeof(uint16_t) : - return *(uint16_t*)addr; - case sizeof(uint8_t) : - return *(uint8_t*)addr; - default: - // UNREACHABLE(size); - return 0; +{ + __try { + + switch (size) { + case sizeof(uint32_t) : + return *(uint32_t*)addr; + case sizeof(uint16_t) : + return *(uint16_t*)addr; + case sizeof(uint8_t) : + return *(uint8_t*)addr; + default: + // UNREACHABLE(size); + return 0; + } + } + __except (true) { + EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "EmuX86_Mem_Read Failed (0x%08X, %d)", addr, size); } } void EmuX86_Mem_Write(xbaddr addr, uint32_t value, int size) -{ - switch (size) { - case sizeof(uint32_t) : - *(uint32_t*)addr = (uint32_t)value; - break; - case sizeof(uint16_t) : - *(uint16_t*)addr = (uint16_t)value; - break; - case sizeof(uint8_t) : - *(uint8_t*)addr = (uint8_t)value; - break; - default: - // UNREACHABLE(size); - return; +{ + __try { + switch (size) { + case sizeof(uint32_t) : + *(uint32_t*)addr = (uint32_t)value; + break; + case sizeof(uint16_t) : + *(uint16_t*)addr = (uint16_t)value; + break; + case sizeof(uint8_t) : + *(uint8_t*)addr = (uint8_t)value; + break; + default: + // UNREACHABLE(size); + return; + } + } + __except (true) { + EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "EmuX86_Mem_Write Failed (0x%08X, 0x%08X, %d)", addr, value, size); } } @@ -1179,8 +1190,12 @@ bool EmuX86_DecodeException(LPEXCEPTION_POINTERS e) continue; } break; - case I_RET: - // RET always signifies the end of a code block + case I_CALL: case I_RET: + // RET and CALL always signify the end of a code block + return true; + case I_PUSH: case I_POP: + // TODO: Implement these instructions + // currently stubbed to prevent firing the unimplemented instruction handler return true; case I_ADD: if (EmuX86_Opcode_ADD(e, info)) break; From b3338288a62e2131a2523618038a9e8127d093b2 Mon Sep 17 00:00:00 2001 From: Luke Usher Date: Sat, 15 Sep 2018 16:22:07 +0100 Subject: [PATCH 4/8] Fix OR->XOR in comment --- src/CxbxKrnl/EmuX86.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CxbxKrnl/EmuX86.cpp b/src/CxbxKrnl/EmuX86.cpp index 573557f0e..1acc451de 100644 --- a/src/CxbxKrnl/EmuX86.cpp +++ b/src/CxbxKrnl/EmuX86.cpp @@ -1070,14 +1070,14 @@ bool EmuX86_Opcode_XOR(LPEXCEPTION_POINTERS e, _DInst& info) if (!EmuX86_Operand_Read(e, info, 1, &src)) return false; - // OR reads and writes the same operand : + // XOR reads and writes the same operand : OperandAddress opAddr; if (!EmuX86_Operand_Addr_ForReadWrite(e, info, 0, OUT opAddr)) return false; uint32_t dest = EmuX86_Addr_Read(opAddr); - // OR Destination with src + // XOR Destination with src uint32_t result = dest ^ src; // Write back the result From 9bac79c44d86c3f079568130614704ae7a49b499 Mon Sep 17 00:00:00 2001 From: Luke Usher Date: Sat, 15 Sep 2018 16:24:38 +0100 Subject: [PATCH 5/8] Fix a compiler warning --- src/CxbxKrnl/EmuX86.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/CxbxKrnl/EmuX86.cpp b/src/CxbxKrnl/EmuX86.cpp index 1acc451de..9d85fb6c8 100644 --- a/src/CxbxKrnl/EmuX86.cpp +++ b/src/CxbxKrnl/EmuX86.cpp @@ -134,7 +134,8 @@ uint32_t EmuX86_Mem_Read(xbaddr addr, int size) } } __except (true) { - EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "EmuX86_Mem_Read Failed (0x%08X, %d)", addr, size); + EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "EmuX86_Mem_Read Failed (0x%08X, %d)", addr, size); + return 0; } } @@ -1070,14 +1071,14 @@ bool EmuX86_Opcode_XOR(LPEXCEPTION_POINTERS e, _DInst& info) if (!EmuX86_Operand_Read(e, info, 1, &src)) return false; - // XOR reads and writes the same operand : + // OR reads and writes the same operand : OperandAddress opAddr; if (!EmuX86_Operand_Addr_ForReadWrite(e, info, 0, OUT opAddr)) return false; uint32_t dest = EmuX86_Addr_Read(opAddr); - // XOR Destination with src + // OR Destination with src uint32_t result = dest ^ src; // Write back the result From 1413f2960e76a0ddad9a13fe3d1b92556f138739 Mon Sep 17 00:00:00 2001 From: Luke Usher Date: Sat, 15 Sep 2018 16:39:01 +0100 Subject: [PATCH 6/8] Fix comment again... Are these comments really needed? Or/Xor/Operands are self-explanatory --- src/CxbxKrnl/EmuKrnlDbg.cpp | 3 ++- src/CxbxKrnl/EmuX86.cpp | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/CxbxKrnl/EmuKrnlDbg.cpp b/src/CxbxKrnl/EmuKrnlDbg.cpp index d77b5e925..af040cf91 100644 --- a/src/CxbxKrnl/EmuKrnlDbg.cpp +++ b/src/CxbxKrnl/EmuKrnlDbg.cpp @@ -124,7 +124,8 @@ XBSYSAPI EXPORTNUM(8) xboxkrnl::ULONG _cdecl xboxkrnl::DbgPrint vsprintf(szBuffer, Format, argp); va_end(argp); - printf(szBuffer); // Note : missing newlines can occur + // Allow DbgPrint to be disabled + EmuLog(LOG_PREFIX, LOG_LEVEL::INFO, "%s", szBuffer); fflush(stdout); } diff --git a/src/CxbxKrnl/EmuX86.cpp b/src/CxbxKrnl/EmuX86.cpp index 9d85fb6c8..010e2596f 100644 --- a/src/CxbxKrnl/EmuX86.cpp +++ b/src/CxbxKrnl/EmuX86.cpp @@ -1071,14 +1071,14 @@ bool EmuX86_Opcode_XOR(LPEXCEPTION_POINTERS e, _DInst& info) if (!EmuX86_Operand_Read(e, info, 1, &src)) return false; - // OR reads and writes the same operand : + // XOR reads and writes the same operand : OperandAddress opAddr; if (!EmuX86_Operand_Addr_ForReadWrite(e, info, 0, OUT opAddr)) return false; uint32_t dest = EmuX86_Addr_Read(opAddr); - // OR Destination with src + // XOR Destination with src uint32_t result = dest ^ src; // Write back the result From 68f12930903e1012bce70217322db6882b6eba64 Mon Sep 17 00:00:00 2001 From: Luke Usher Date: Sat, 15 Sep 2018 22:37:35 +0100 Subject: [PATCH 7/8] Fix incorrect flag masks in jump instructions --- src/CxbxKrnl/EmuX86.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/CxbxKrnl/EmuX86.cpp b/src/CxbxKrnl/EmuX86.cpp index 010e2596f..a70248e12 100644 --- a/src/CxbxKrnl/EmuX86.cpp +++ b/src/CxbxKrnl/EmuX86.cpp @@ -1147,8 +1147,6 @@ bool EmuX86_DecodeException(LPEXCEPTION_POINTERS e) // Execute op-codes until we hit an unhandled instruction, or an error occurs while (true) { - currentEip = e->ContextRecord->Eip; - if (!EmuX86_DecodeOpcode((uint8_t*)e->ContextRecord->Eip, info)) { EmuLog(LOG_PREFIX, LOG_LEVEL::WARNING, "Error decoding opcode at 0x%08X", e->ContextRecord->Eip); return false; @@ -1157,7 +1155,7 @@ bool EmuX86_DecodeException(LPEXCEPTION_POINTERS e) switch (info.opcode) { // Keep these cases alphabetically ordered and condensed // Exit and branch Opcodes come first, for clarity/visibility case I_JA: - if (EmuX86_Opcode_JXX(e, info, BITMASK(EMUX86_EFLAG_CF) & BITMASK(EMUX86_EFLAG_ZF), false)) { + if (EmuX86_Opcode_JXX(e, info, BITMASK(EMUX86_EFLAG_CF) | BITMASK(EMUX86_EFLAG_ZF), false)) { continue; } break; @@ -1172,7 +1170,7 @@ bool EmuX86_DecodeException(LPEXCEPTION_POINTERS e) } break; case I_JBE: - if (EmuX86_Opcode_JXX(e, info, BITMASK(EMUX86_EFLAG_CF) & BITMASK(EMUX86_EFLAG_ZF), true)) { + if (EmuX86_Opcode_JXX(e, info, BITMASK(EMUX86_EFLAG_CF) | BITMASK(EMUX86_EFLAG_ZF), true)) { continue; } break; From 49e18513235586ca0cd288cb1c094cd86ee39c2a Mon Sep 17 00:00:00 2001 From: Luke Usher Date: Mon, 17 Sep 2018 20:35:14 +0100 Subject: [PATCH 8/8] More opcodes --- src/CxbxKrnl/EmuX86.cpp | 113 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 109 insertions(+), 4 deletions(-) diff --git a/src/CxbxKrnl/EmuX86.cpp b/src/CxbxKrnl/EmuX86.cpp index a70248e12..b9ce7f868 100644 --- a/src/CxbxKrnl/EmuX86.cpp +++ b/src/CxbxKrnl/EmuX86.cpp @@ -952,6 +952,32 @@ bool EmuX86_Opcode_MOVZX(LPEXCEPTION_POINTERS e, _DInst& info) return true; } + +bool EmuX86_Opcode_NOT(LPEXCEPTION_POINTERS e, _DInst& info) +{ + // NOT reads and writes the same operand : + OperandAddress opAddr; + if (!EmuX86_Operand_Addr_ForReadWrite(e, info, 0, OUT opAddr)) + return false; + + uint32_t dest = EmuX86_Addr_Read(opAddr); + + // NOT Destination with + uint32_t result = ~dest; + + // Write back the result + EmuX86_Addr_Write(opAddr, result); + + // The OF and CF flags are cleared; the SF, ZF, and PF flags are set according to the result. The state of the AF flag is undefined. + EmuX86_SetFlags_OSZPC(e, + /*EMUX86_EFLAG_OF*/0, + /*EMUX86_EFLAG_SF*/SFCalc(result), + /*EMUX86_EFLAG_ZF*/ZFCalc(result), + /*EMUX86_EFLAG_PF*/PFCalc(result), + /*EMUX86_EFLAG_CF*/0); + + return true; +} bool EmuX86_Opcode_OR(LPEXCEPTION_POINTERS e, _DInst& info) { @@ -1000,7 +1026,79 @@ bool EmuX86_Opcode_OUT(LPEXCEPTION_POINTERS e, _DInst& info) // Note : OUT instructions never update CPU flags -return true; + return true; +} + + +bool EmuX86_Opcode_SBB(LPEXCEPTION_POINTERS e, _DInst& info) +{ + // Read value from Source and Destination + uint32_t src = 0; + if (!EmuX86_Operand_Read(e, info, 1, &src)) + return false; + + // SBB reads and writes the same operand : + OperandAddress opAddr; + if (!EmuX86_Operand_Addr_ForReadWrite(e, info, 0, OUT opAddr)) + return false; + + uint32_t dest = EmuX86_Addr_Read(opAddr); + + // SUB Destination with src + uint64_t result = (uint64_t)dest - (uint64_t)src; + + // If the carry flag is set, subtract an additional 1 + if (e->ContextRecord->EFlags & BITMASK(EMUX86_EFLAG_CF)) { + result -= 1; + } + + // Write result back + EmuX86_Addr_Write(opAddr, static_cast(result)); + + // The OF, SF, ZF, AF, PF, and CF flags are set according to the result. + EmuX86_SetFlags_OSZAPC(e, + /*EMUX86_EFLAG_OF*/OF_Sub(result, src, dest), + /*EMUX86_EFLAG_SF*/SFCalc(result), + /*EMUX86_EFLAG_ZF*/ZFCalc(result), + /*EMUX86_EFLAG_AF*/AFCalc(result, src, dest), + /*EMUX86_EFLAG_PF*/PFCalc(result), + /*EMUX86_EFLAG_CF*/CFCalc(result)); + + return true; +} + + +bool EmuX86_Opcode_SHR(LPEXCEPTION_POINTERS e, _DInst& info) +{ + // Read value from Source and Destination + uint32_t src = 0; + if (!EmuX86_Operand_Read(e, info, 1, &src)) + return false; + + // SHR reads and writes the same operand : + OperandAddress opAddr; + if (!EmuX86_Operand_Addr_ForReadWrite(e, info, 0, OUT opAddr)) + return false; + + uint32_t dest = EmuX86_Addr_Read(opAddr); + + // Shift Destination with src + uint8_t carryBit = dest & 1; + uint64_t result = (uint64_t)dest >> (uint64_t)src; + + // Write result back + EmuX86_Addr_Write(opAddr, static_cast(result)); + + // The OF, SF, ZF, AF, PF, and CF flags are set according to the result. + EmuX86_SetFlags_OSZAPC(e, + /*EMUX86_EFLAG_OF*/OF_Sub(result, src, dest), + /*EMUX86_EFLAG_SF*/SFCalc(result), + /*EMUX86_EFLAG_ZF*/ZFCalc(result), + /*EMUX86_EFLAG_AF*/AFCalc(result, src, dest), + /*EMUX86_EFLAG_PF*/PFCalc(result), + /*EMUX86_EFLAG_CF*/carryBit); + + return true; } bool EmuX86_Opcode_SUB(LPEXCEPTION_POINTERS e, _DInst& info) @@ -1143,8 +1241,6 @@ bool EmuX86_DecodeException(LPEXCEPTION_POINTERS e) // that case may be logged, but it shouldn't fail the opcode handler. _DInst info; - xbaddr currentEip; // Used to detect taken branches/jumps - // Execute op-codes until we hit an unhandled instruction, or an error occurs while (true) { if (!EmuX86_DecodeOpcode((uint8_t*)e->ContextRecord->Eip, info)) { @@ -1240,6 +1336,9 @@ bool EmuX86_DecodeException(LPEXCEPTION_POINTERS e) goto opcode_error; case I_MOVZX: if (EmuX86_Opcode_MOVZX(e, info)) break; + goto opcode_error; + case I_NOT: + if (EmuX86_Opcode_NOT(e, info)) break; goto opcode_error; case I_OR: if (EmuX86_Opcode_OR(e, info)) break; @@ -1251,7 +1350,13 @@ bool EmuX86_DecodeException(LPEXCEPTION_POINTERS e) // Enable all interrupts EmuX86_Opcode_STI(); break; - } + } + case I_SBB: + if (EmuX86_Opcode_SBB(e, info)) break; + goto opcode_error; + case I_SHR: + if (EmuX86_Opcode_SHR(e, info)) break; + goto opcode_error; case I_SUB: if (EmuX86_Opcode_SUB(e, info)) break; goto opcode_error;