Reservations improved

Utilities/Thread.cpp

@@ -213,7 +213,7 @@ static const reg_table_t reg_table[17] =
 
 #endif
 
-bool handle_access_violation(const u32 addr, x64_context* context)
+bool handle_access_violation(const u32 addr, bool is_writing, x64_context* context)
 {
 	// check if address is RawSPU MMIO register
 	if (addr - RAW_SPU_BASE_ADDR < (6 * RAW_SPU_OFFSET) && (addr % RAW_SPU_OFFSET) >= RAW_SPU_PROB_OFFSET)
@@ -279,7 +279,7 @@ bool handle_access_violation(const u32 addr, x64_context* context)
 	}
 
 	// check if fault is caused by reservation
-	if (vm::reservation_query(addr))
+	if (vm::reservation_query(addr, is_writing))
 	{
 		return true;
 	}
@@ -304,11 +304,12 @@ void _se_translator(unsigned int u, EXCEPTION_POINTERS* pExp)
 const PVOID exception_handler = (atexit([]{ RemoveVectoredExceptionHandler(exception_handler); }), AddVectoredExceptionHandler(1, [](PEXCEPTION_POINTERS pExp) -> LONG
 {
 	const u64 addr64 = (u64)pExp->ExceptionRecord->ExceptionInformation[1] - (u64)vm::g_base_addr;
+	const bool is_writing = pExp->ExceptionRecord->ExceptionInformation[0] != 0;
 
 	if (pExp->ExceptionRecord->ExceptionCode == EXCEPTION_ACCESS_VIOLATION &&
 		(u32)addr64 == addr64 &&
 		GetCurrentNamedThread() &&
-		handle_access_violation((u32)addr64, pExp->ContextRecord))
+		handle_access_violation((u32)addr64, is_writing, pExp->ContextRecord))
 	{
 		return EXCEPTION_CONTINUE_EXECUTION;
 	}
@@ -327,7 +328,7 @@ void signal_handler(int sig, siginfo_t* info, void* uct)
 
 	if ((u32)addr64 == addr64 && GetCurrentNamedThread())
 	{
-		if (handle_access_violation((u32)addr64, (ucontext_t*)uct))
+		if (handle_access_violation((u32)addr64, is_writing, (ucontext_t*)uct))
 		{
 			return; // proceed execution
 		}

rpcs3/Emu/Cell/SPUThread.cpp

@@ -468,9 +468,10 @@ void SPUThread::EnqMfcCmd(MFCReg& MFCArgs)
 		}
 		else // store unconditional (may be wrong)
 		{
-			vm::reservation_break(ea);
-
-			ProcessCmd(MFC_PUT_CMD, tag, lsa, ea, 128);
+			vm::reservation_op(ea, 128, [this, tag, lsa, ea]()
+			{
+				ProcessCmd(MFC_PUT_CMD, tag, lsa, ea, 128);
+			});
 
 			if (op == MFC_PUTLLUC_CMD)
 			{

rpcs3/Emu/Memory/vm.cpp

@@ -141,15 +141,15 @@ namespace vm
 
 	reservation_mutex_t g_reservation_mutex;
 
-	void _reservation_set(u32 addr)
+	void _reservation_set(u32 addr, bool no_access = false)
 	{
 		//const auto stamp0 = get_time();
 
 #ifdef _WIN32
 		DWORD old;
-		if (!VirtualProtect(vm::get_ptr(addr & ~0xfff), 4096, PAGE_READONLY, &old))
+		if (!VirtualProtect(vm::get_ptr(addr & ~0xfff), 4096, no_access ? PAGE_NOACCESS : PAGE_READONLY, &old))
 #else
-		if (mprotect(vm::get_ptr(addr & ~0xfff), 4096, PROT_READ))
+		if (mprotect(vm::get_ptr(addr & ~0xfff), 4096, no_access ? PROT_NONE : PROT_READ))
 #endif
 		{
 			throw fmt::format("vm::_reservation_set() failed (addr=0x%x)", addr);
@@ -223,7 +223,7 @@ namespace vm
 			// change memory protection to read-only
 			_reservation_set(addr);
 
-			// may not be necessary, just for sure:
+			// may not be necessary
 			_mm_mfence();
 
 			// set additional information
@@ -252,9 +252,15 @@ namespace vm
 			return false;
 		}
 
+		// change memory protection to no access
+		_reservation_set(addr, true);
+
 		// update memory using privileged access
 		memcpy(vm::get_priv_ptr(addr), data, size);
 
+		// remove callback to not call it on successful update
+		g_reservation_cb = nullptr;
+
 		// free the reservation and restore memory protection
 		_reservation_break(addr);
 
@@ -262,7 +268,7 @@ namespace vm
 		return true;
 	}
 
-	bool reservation_query(u32 addr)
+	bool reservation_query(u32 addr, bool is_writing)
 	{
 		std::lock_guard<reservation_mutex_t> lock(g_reservation_mutex);
 
@@ -275,9 +281,12 @@ namespace vm
 			}
 		}
 
-		// break the reservation
-		_reservation_break(addr);
-
+		if (is_writing)
+		{
+			// break the reservation
+			_reservation_break(addr);
+		}
+		
 		return true;
 	}
 
@@ -299,13 +308,16 @@ namespace vm
 		std::lock_guard<reservation_mutex_t> lock(g_reservation_mutex);
 
 		// break previous reservation
-		if (g_reservation_owner)
+		if (g_reservation_owner != GetCurrentNamedThread() || g_reservation_addr != addr || g_reservation_size != size)
 		{
-			_reservation_break(g_reservation_addr);
+			if (g_reservation_owner)
+			{
+				_reservation_break(g_reservation_addr);
+			}
 		}
 
-		// change memory protection to read-only
-		_reservation_set(addr);
+		// change memory protection to no access
+		_reservation_set(addr, true);
 
 		// set additional information
 		g_reservation_addr = addr;
@@ -313,7 +325,7 @@ namespace vm
 		g_reservation_owner = GetCurrentNamedThread();
 		g_reservation_cb = nullptr;
 
-		// may not be necessary, just for sure:
+		// may not be necessary
 		_mm_mfence();
 
 		// do the operation

rpcs3/Emu/Memory/vm.h

@@ -34,7 +34,7 @@ namespace vm
 	bool reservation_acquire(void* data, u32 addr, u32 size, const std::function<void()>& callback = nullptr);
 	// attempt to atomically update reserved memory
 	bool reservation_update(u32 addr, const void* data, u32 size);
-	bool reservation_query(u32 addr);
+	bool reservation_query(u32 addr, bool is_writing);
 	void reservation_free();
 	// perform complete operation
 	void reservation_op(u32 addr, u32 size, std::function<void()> proc);