From 0797164fac4faf4b46ea84f23f01ae84c1f4f6a6 Mon Sep 17 00:00:00 2001
From: Nekotekina <nekotekina@gmail.com>
Date: Sat, 7 Apr 2018 02:16:32 +0300
Subject: [PATCH] SPU: fix possible livelock

The bug affects TSX path
---
 Utilities/sysinfo.h          |  9 +++++++--
 rpcs3/Emu/Cell/PPUThread.cpp |  4 ++--
 rpcs3/Emu/Cell/SPUThread.cpp | 38 ++++++++++++++++++++++--------------
 rpcs3/Emu/Cell/SPUThread.h   |  1 +
 4 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/Utilities/sysinfo.h b/Utilities/sysinfo.h
index 54e07c559d..ad55ba724e 100644
--- a/Utilities/sysinfo.h
+++ b/Utilities/sysinfo.h
@@ -39,11 +39,11 @@ namespace utils
 
 	bool has_xop();
 
-	inline bool transaction_enter()
+	FORCE_INLINE bool transaction_enter(uint* out = nullptr)
 	{
 		while (true)
 		{
-			const auto status = _xbegin();
+			const uint status = _xbegin();
 
 			if (status == _XBEGIN_STARTED)
 			{
@@ -52,6 +52,11 @@ namespace utils
 
 			if (!(status & _XABORT_RETRY))
 			{
+				if (out)
+				{
+					*out = status;
+				}
+
 				return false;
 			}
 		}
diff --git a/rpcs3/Emu/Cell/PPUThread.cpp b/rpcs3/Emu/Cell/PPUThread.cpp
index 9f6f77c914..afdb55df14 100644
--- a/rpcs3/Emu/Cell/PPUThread.cpp
+++ b/rpcs3/Emu/Cell/PPUThread.cpp
@@ -964,7 +964,7 @@ extern bool ppu_stwcx(ppu_thread& ppu, u32 addr, u32 reg_value)
 
 	if (s_use_rtm && utils::transaction_enter())
 	{
-		if (!vm::g_mutex.is_lockable())
+		if (!vm::g_mutex.is_lockable() || vm::g_mutex.is_reading())
 		{
 			_xabort(0);
 		}
@@ -1008,7 +1008,7 @@ extern bool ppu_stdcx(ppu_thread& ppu, u32 addr, u64 reg_value)
 
 	if (s_use_rtm && utils::transaction_enter())
 	{
-		if (!vm::g_mutex.is_lockable())
+		if (!vm::g_mutex.is_lockable() || vm::g_mutex.is_reading())
 		{
 			_xabort(0);
 		}
diff --git a/rpcs3/Emu/Cell/SPUThread.cpp b/rpcs3/Emu/Cell/SPUThread.cpp
index 083122bd0a..253520581f 100644
--- a/rpcs3/Emu/Cell/SPUThread.cpp
+++ b/rpcs3/Emu/Cell/SPUThread.cpp
@@ -313,7 +313,7 @@ std::string SPUThread::dump() const
 	std::string ret = cpu_thread::dump();
 
 	// Print some transaction statistics
-	fmt::append(ret, "\nTX: %u; Fail: %u", tx_success, tx_failure);
+	fmt::append(ret, "\nTX: %u; Fail: %u (0x%x)", tx_success, tx_failure, tx_status);
 	fmt::append(ret, "\nRaddr: 0x%08x; R: 0x%x", raddr, raddr ? +vm::reservation_acquire(raddr, 128) : 0);
 	fmt::append(ret, "\nTag Mask: 0x%08x", ch_tag_mask);
 	fmt::append(ret, "\nMFC Stall: 0x%08x", ch_stall_mask);
@@ -855,7 +855,7 @@ void SPUThread::do_putlluc(const spu_mfc_cmd& args)
 	if (s_use_rtm && utils::transaction_enter())
 	{
 		// First transaction attempt
-		if (!vm::g_mutex.is_lockable())
+		if (!vm::g_mutex.is_lockable() || vm::g_mutex.is_reading())
 		{
 			_xabort(0);
 		}
@@ -864,12 +864,11 @@ void SPUThread::do_putlluc(const spu_mfc_cmd& args)
 		vm::reservation_update(addr, 128);
 		vm::notify(addr, 128);
 		_xend();
-		tx_success++;
 		return;
 	}
 	else if (s_use_rtm)
 	{
-		vm::reader_lock lock;
+		vm::writer_lock lock(0);
 
 		if (utils::transaction_enter())
 		{
@@ -877,15 +876,18 @@ void SPUThread::do_putlluc(const spu_mfc_cmd& args)
 			data = to_write;
 			vm::reservation_update(addr, 128);
 			_xend();
-			tx_success++;
-
-			vm::notify(addr, 128);
-			return;
 		}
 		else
 		{
-			tx_failure++;
+			vm::reservation_update(addr, 128, true);
+			_mm_sfence();
+			data = to_write;
+			_mm_sfence();
+			vm::reservation_update(addr, 128);
 		}
+
+		vm::notify(addr, 128);
+		return;
 	}
 
 	vm::writer_lock lock(0);
@@ -1102,7 +1104,6 @@ bool SPUThread::process_mfc_cmd(spu_mfc_cmd args)
 				if (LIKELY(vm::reservation_acquire(raddr, 128) == rtime))
 				{
 					// Copy to LS
-					tx_success++;
 					_ref<decltype(rdata)>(args.lsa & 0x3ffff) = rdata;
 					ch_atomic_stat.set_value(MFC_GETLLAR_SUCCESS);
 					return true;
@@ -1123,11 +1124,9 @@ bool SPUThread::process_mfc_cmd(spu_mfc_cmd args)
 
 			rdata = data;
 			_xend();
-			tx_success++;
 		}
 		else
 		{
-			tx_failure++;
 			vm::reader_lock lock;
 			rtime = vm::reservation_acquire(raddr, 128);
 			rdata = data;
@@ -1153,7 +1152,7 @@ bool SPUThread::process_mfc_cmd(spu_mfc_cmd args)
 			if (s_use_rtm && utils::transaction_enter())
 			{
 				// First transaction attempt
-				if (!vm::g_mutex.is_lockable())
+				if (!vm::g_mutex.is_lockable() || vm::g_mutex.is_reading())
 				{
 					_xabort(0);
 				}
@@ -1173,12 +1172,15 @@ bool SPUThread::process_mfc_cmd(spu_mfc_cmd args)
 			else if (s_use_rtm)
 			{
 				// Second transaction attempt
-				vm::reader_lock lock;
+				vm::writer_lock lock(0);
+
+				// Touch memory without modifying the value
+				vm::_ref<atomic_t<u32>>(args.eal) += 0;
 
 				// Touch reservation memory area as well
 				vm::reservation_acquire(raddr, 128) += 0;
 
-				if (utils::transaction_enter())
+				if (utils::transaction_enter(&tx_status))
 				{
 					if (rtime == vm::reservation_acquire(raddr, 128) && rdata == data)
 					{
@@ -1199,6 +1201,12 @@ bool SPUThread::process_mfc_cmd(spu_mfc_cmd args)
 				}
 				else
 				{
+					// Workaround MSVC
+					if (rtime == vm::reservation_acquire(raddr, 128) && rdata == data)
+					{
+						vm::reservation_update(raddr, 128);
+					}
+
 					// Don't fallback to heavyweight lock, just give up
 					tx_failure++;
 				}
diff --git a/rpcs3/Emu/Cell/SPUThread.h b/rpcs3/Emu/Cell/SPUThread.h
index 43eab7a945..adb2f8c614 100644
--- a/rpcs3/Emu/Cell/SPUThread.h
+++ b/rpcs3/Emu/Cell/SPUThread.h
@@ -593,6 +593,7 @@ public:
 
 	u64 tx_success = 0;
 	u64 tx_failure = 0;
+	uint tx_status = 0;
 
 	void push_snr(u32 number, u32 value);
 	void do_dma_transfer(const spu_mfc_cmd& args);