From 5b69eda39aebbac1e81ae7343d7c50530c51efb8 Mon Sep 17 00:00:00 2001 From: Nekotekina Date: Sat, 14 Nov 2020 10:56:05 +0300 Subject: [PATCH] Fix suspend_all bug (TSX, TSX-FA) Could cause freezes. --- rpcs3/Emu/CPU/CPUThread.cpp | 48 +++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/rpcs3/Emu/CPU/CPUThread.cpp b/rpcs3/Emu/CPU/CPUThread.cpp index 7a4c86eae1..ca6a1e8341 100644 --- a/rpcs3/Emu/CPU/CPUThread.cpp +++ b/rpcs3/Emu/CPU/CPUThread.cpp @@ -24,6 +24,9 @@ LOG_CHANNEL(sys_log, "SYS"); static thread_local u64 s_tls_thread_slot = -1; +// Suspend counter stamp +static thread_local u64 s_tls_sctr = -1; + extern thread_local void(*g_tls_log_control)(const char* fmt, u64 progress); template <> @@ -585,7 +588,6 @@ bool cpu_thread::check_state() noexcept bool cpu_sleep_called = false; bool cpu_can_stop = true; bool escape, retval; - u64 susp_ctr = -1; while (true) { @@ -597,16 +599,26 @@ bool cpu_thread::check_state() noexcept if (flags & cpu_flag::pause) { // Save value before state is saved and cpu_flag::wait is observed - susp_ctr = g_suspend_counter; - - if (susp_ctr & 1 && flags & cpu_flag::wait) + if (s_tls_sctr == umax) { - susp_ctr = -1; + u64 ctr = g_suspend_counter; + + if (flags & cpu_flag::wait) + { + if ((ctr & 3) == 2) + { + s_tls_sctr = ctr; + } + } + else + { + s_tls_sctr = ctr; + } } } else { - susp_ctr = -1; + s_tls_sctr = -1; } if (flags & cpu_flag::temp) [[unlikely]] @@ -724,22 +736,22 @@ bool cpu_thread::check_state() noexcept // Wait for current suspend_all operation for (u64 i = 0;; i++) { - if (i < 20 || susp_ctr & 1) + u64 ctr = g_suspend_counter; + + if (i < 20 || ctr & 1) { busy_wait(300); } - else if (g_suspend_counter.load() >> 1 >= susp_ctr >> 1) + else if (ctr >> 2 == s_tls_sctr >> 2) { - g_suspend_counter.wait(susp_ctr, -2); + g_suspend_counter.wait(ctr, -4); } - - if (!(state & cpu_flag::pause)) + else { + s_tls_sctr = -1; break; } } - - susp_ctr = -1; } } } @@ -869,6 +881,9 @@ bool cpu_thread::suspend_work::push(cpu_thread* _this, bool cancel_if_not_suspen } }); + // Initialization (first increment) + g_suspend_counter += 2; + // Copy of thread bits decltype(ctr->cpu_copy_bits) copy2{}; @@ -910,6 +925,7 @@ bool cpu_thread::suspend_work::push(cpu_thread* _this, bool cancel_if_not_suspen _mm_pause(); } + // Second increment: all threads paused g_suspend_counter++; // Extract queue and reverse element order (FILO to FIFO) (TODO: maybe leave order as is?) @@ -964,7 +980,7 @@ bool cpu_thread::suspend_work::push(cpu_thread* _this, bool cancel_if_not_suspen } } - // Finalization (second increment) + // Finalization (last increment) verify(HERE), g_suspend_counter++ & 1; // Exact bitset for flag pause removal @@ -978,8 +994,10 @@ bool cpu_thread::suspend_work::push(cpu_thread* _this, bool cancel_if_not_suspen else { // Seems safe to set pause on self because wait flag hasn't been observed yet - _this->state += cpu_flag::pause + cpu_flag::temp; + s_tls_sctr = g_suspend_counter; + _this->state += cpu_flag::pause + cpu_flag::wait + cpu_flag::temp; _this->check_state(); + s_tls_sctr = -1; return true; }