From af305aa1686ed972c953d5a19e3e8b600c74f2b5 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sun, 10 May 2015 20:56:52 -0500 Subject: [PATCH] Fix a race condition when pausing the CPU core. This affects enabling and disabling block profiling on the fly. The block profiling pauses the CPU cores and then flushes the JIT's block cache and enables block profile. The issue with this is that when we pause the CPU core, we don't have a way to tell if the JIT recompiler has actually left. So if the secondary thread that is clearing the JIT block cache is too quick, it will clear the cache as a recompiler is still running that block that has been cleared. --- .../Core/PowerPC/Interpreter/Interpreter.cpp | 3 +++ Source/Core/Core/PowerPC/Jit64/JitAsm.cpp | 6 ++++++ Source/Core/Core/PowerPC/JitArm32/JitAsm.cpp | 4 ++++ Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp | 4 ++++ Source/Core/Core/PowerPC/PowerPC.cpp | 17 +++++++++++++++++ Source/Core/Core/PowerPC/PowerPC.h | 1 + 6 files changed, 35 insertions(+) diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp b/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp index 55be204115..f57ff17c41 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp @@ -277,6 +277,9 @@ void Interpreter::Run() PC = NPC; } } + + // Let the waiting thread know we are done leaving + PowerPC::FinishStateMove(); } void Interpreter::unknown_instruction(UGeckoInstruction _inst) diff --git a/Source/Core/Core/PowerPC/Jit64/JitAsm.cpp b/Source/Core/Core/PowerPC/Jit64/JitAsm.cpp index 9b64e433ba..6abfbf366d 100644 --- a/Source/Core/Core/PowerPC/Jit64/JitAsm.cpp +++ b/Source/Core/Core/PowerPC/Jit64/JitAsm.cpp @@ -216,6 +216,12 @@ void Jit64AsmRoutineManager::Generate() ADD(64, R(RSP), Imm8(0x18)); POP(RSP); } + + // Let the waiting thread know we are done leaving + ABI_PushRegistersAndAdjustStack({}, 0); + ABI_CallFunction(reinterpret_cast(&PowerPC::FinishStateMove)); + ABI_PopRegistersAndAdjustStack({}, 0); + ABI_PopRegistersAndAdjustStack(ABI_ALL_CALLEE_SAVED, 8, 16); RET(); diff --git a/Source/Core/Core/PowerPC/JitArm32/JitAsm.cpp b/Source/Core/Core/PowerPC/JitArm32/JitAsm.cpp index a971f7ad3e..416de95680 100644 --- a/Source/Core/Core/PowerPC/JitArm32/JitAsm.cpp +++ b/Source/Core/Core/PowerPC/JitArm32/JitAsm.cpp @@ -136,6 +136,10 @@ void JitArmAsmRoutineManager::Generate() if (SConfig::GetInstance().m_LocalCoreStartupParameter.bEnableDebugging) SetJumpTarget(dbg_exit); + // Let the waiting thread know we are done leaving + MOVI2R(R0, (u32)&PowerPC::FinishStateMove); + BL(R0); + ADD(_SP, _SP, 4); POP(9, R4, R5, R6, R7, R8, R9, R10, R11, _PC); // Returns diff --git a/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp b/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp index 053d7ebf05..d894170116 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp @@ -87,6 +87,10 @@ void JitArm64AsmRoutineManager::Generate() SetJumpTarget(Exit); + // Let the waiting thread know we are done leaving + MOVI2R(X0, (u64)&PowerPC::FinishStateMove); + BLR(X0); + ABI_PopRegisters(regs_to_save); RET(X30); diff --git a/Source/Core/Core/PowerPC/PowerPC.cpp b/Source/Core/Core/PowerPC/PowerPC.cpp index b9ec4ca085..f5594cd69b 100644 --- a/Source/Core/Core/PowerPC/PowerPC.cpp +++ b/Source/Core/Core/PowerPC/PowerPC.cpp @@ -4,6 +4,7 @@ #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" +#include "Common/Event.h" #include "Common/FPURoundMode.h" #include "Common/MathUtil.h" @@ -33,6 +34,7 @@ static volatile CPUState state = CPU_POWERDOWN; Interpreter * const interpreter = Interpreter::getInstance(); static CoreMode mode; +static Common::Event s_state_change; Watches watches; BreakPoints breakpoints; @@ -238,16 +240,31 @@ void Start() void Pause() { + volatile CPUState old_state = state; state = CPU_STEPPING; + + // Wait for the CPU core to leave + if (old_state == CPU_RUNNING) + s_state_change.WaitFor(std::chrono::seconds(1)); Host_UpdateDisasmDialog(); } void Stop() { + volatile CPUState old_state = state; state = CPU_POWERDOWN; + + // Wait for the CPU core to leave + if (old_state == CPU_RUNNING) + s_state_change.WaitFor(std::chrono::seconds(1)); Host_UpdateDisasmDialog(); } +void FinishStateMove() +{ + s_state_change.Set(); +} + void UpdatePerformanceMonitor(u32 cycles, u32 num_load_stores, u32 num_fp_inst) { switch (MMCR0.PMC1SELECT) diff --git a/Source/Core/Core/PowerPC/PowerPC.h b/Source/Core/Core/PowerPC/PowerPC.h index bcdec4c573..98606b420b 100644 --- a/Source/Core/Core/PowerPC/PowerPC.h +++ b/Source/Core/Core/PowerPC/PowerPC.h @@ -156,6 +156,7 @@ void RunLoop(); void Start(); void Pause(); void Stop(); +void FinishStateMove(); CPUState GetState(); volatile CPUState *GetStatePtr(); // this oddity is here instead of an extern declaration to easily be able to find all direct accesses throughout the code.