sys_event_flag improved

rpcs3/Emu/Cell/SPUThread.cpp

@@ -951,23 +951,19 @@ void SPUThread::set_ch_value(u32 ch, u32 value)
 					LOG_WARNING(SPU, "sys_event_flag_set_bit(id=%d, value=0x%x (flag=%d))", data, value, flag);
 				}
 
-				const auto ef = Emu.GetIdManager().get<lv2_event_flag_t>(data);
+				const auto eflag = Emu.GetIdManager().get<lv2_event_flag_t>(data);
 
-				if (!ef)
+				if (!eflag)
 				{
 					return ch_in_mbox.set_values(1, CELL_ESRCH);
 				}
 
-				while (ef->cancelled)
+				const u64 bitptn = 1ull << flag;
-				{
-					ef->cv.wait_for(lv2_lock, std::chrono::milliseconds(1));
-				}
 
-				ef->flags |= 1ull << flag;
+				if (~eflag->pattern.fetch_or(bitptn) & bitptn)
-
-				if (ef->waiters)
 				{
-					ef->cv.notify_all();
+					// notify if the bit was set
+					eflag->notify_all(lv2_lock);
 				}
 				
 				return ch_in_mbox.set_values(1, CELL_OK);
@@ -999,23 +995,19 @@ void SPUThread::set_ch_value(u32 ch, u32 value)
 					LOG_WARNING(SPU, "sys_event_flag_set_bit_impatient(id=%d, value=0x%x (flag=%d))", data, value, flag);
 				}
 
-				const auto ef = Emu.GetIdManager().get<lv2_event_flag_t>(data);
+				const auto eflag = Emu.GetIdManager().get<lv2_event_flag_t>(data);
 
-				if (!ef)
+				if (!eflag)
 				{
 					return;
 				}
 
-				while (ef->cancelled)
+				const u64 bitptn = 1ull << flag;
-				{
-					ef->cv.wait_for(lv2_lock, std::chrono::milliseconds(1));
-				}
 
-				ef->flags |= 1ull << flag;
+				if (~eflag->pattern.fetch_or(bitptn) & bitptn)
-
-				if (ef->waiters)
 				{
-					ef->cv.notify_all();
+					// notify if the bit was set
+					eflag->notify_all(lv2_lock);
 				}
 				
 				return;

rpcs3/Emu/Event.cpp

@@ -15,19 +15,6 @@ void EventManager::Clear()
 	m_map.clear();
 }
 
-bool EventManager::CheckKey(u64 key)
-{
-	if (!key)
-	{
-		// never exists
-		return false;
-	}
-
-	std::lock_guard<std::mutex> lock(m_mutex);
-
-	return m_map.find(key) != m_map.end();
-}
-
 bool EventManager::UnregisterKey(u64 key)
 {
 	if (!key)

rpcs3/Emu/Event.h

@@ -12,7 +12,6 @@ class EventManager
 public:
 	void Init();
 	void Clear();
-	bool CheckKey(u64 key);
 	bool UnregisterKey(u64 key);
 
 	template<typename... Args, typename = std::enable_if_t<std::is_constructible<lv2_event_queue_t, Args...>::value>> std::shared_ptr<lv2_event_queue_t> MakeEventQueue(u64 key, Args&&... args)

rpcs3/Emu/SysCalls/lv2/sys_event.h

@@ -20,7 +20,7 @@ enum : s32
 // Event Queue Ipc Key
 enum : u64
 {
-	SYS_EVENT_QUEUE_LOCAL = 0x00,
+	SYS_EVENT_QUEUE_LOCAL = 0,
 };
 
 // Event Port Type

rpcs3/Emu/SysCalls/lv2/sys_event_flag.cpp

@@ -5,17 +5,51 @@
 #include "Emu/SysCalls/SysCalls.h"
 
 #include "Emu/Cell/PPUThread.h"
-#include "sleep_queue.h"
 #include "sys_event_flag.h"
 
 SysCallBase sys_event_flag("sys_event_flag");
 
 extern u64 get_system_time();
 
+void lv2_event_flag_t::notify_all(lv2_lock_t& lv2_lock)
+{
+	CHECK_LV2_LOCK(lv2_lock);
+
+	auto pred = [this](sleep_queue_t::value_type& thread) -> bool
+	{
+		auto& ppu = static_cast<PPUThread&>(*thread);
+
+		// load pattern and mode from registers
+		const u64 bitptn = ppu.GPR[4];
+		const u32 mode = static_cast<u32>(ppu.GPR[5]);
+
+		// check specific pattern
+		if (check_pattern(bitptn, mode))
+		{
+			// save pattern
+			ppu.GPR[4] = clear_pattern(bitptn, mode);
+
+			if (!ppu.signal())
+			{
+				throw EXCEPTION("Thread already signaled");
+			}
+
+			return true;
+		}
+
+		return false;
+	};
+
+	// iterate over all waiters; protocol is ignored in current implementation
+	sq.erase(std::remove_if(sq.begin(), sq.end(), pred), sq.end());
+}
+
 s32 sys_event_flag_create(vm::ptr<u32> id, vm::ptr<sys_event_flag_attribute_t> attr, u64 init)
 {
 	sys_event_flag.Warning("sys_event_flag_create(id=*0x%x, attr=*0x%x, init=0x%llx)", id, attr, init);
 
+	LV2_LOCK;
+
 	if (!id || !attr)
 	{
 		return CELL_EFAULT;
@@ -23,13 +57,10 @@ s32 sys_event_flag_create(vm::ptr<u32> id, vm::ptr<sys_event_flag_attribute_t> a
 
 	const u32 protocol = attr->protocol;
 
-	switch (protocol)
+	if (protocol != SYS_SYNC_FIFO && protocol != SYS_SYNC_RETRY && protocol != SYS_SYNC_PRIORITY && protocol != SYS_SYNC_PRIORITY_INHERIT)
 	{
-	case SYS_SYNC_FIFO: break;
+		sys_event_flag.Error("sys_event_flag_create(): unknown protocol (0x%x)", protocol);
-	case SYS_SYNC_RETRY: break;
+		return CELL_EINVAL;
-	case SYS_SYNC_PRIORITY: break;
-	case SYS_SYNC_PRIORITY_INHERIT: break;
-	default: sys_event_flag.Error("sys_event_flag_create(): unknown protocol (0x%x)", attr->protocol); return CELL_EINVAL;
 	}
 
 	if (attr->pshared != SYS_SYNC_NOT_PROCESS_SHARED || attr->ipc_key.data() || attr->flags.data())
@@ -40,11 +71,10 @@ s32 sys_event_flag_create(vm::ptr<u32> id, vm::ptr<sys_event_flag_attribute_t> a
 
 	const u32 type = attr->type;
 
-	switch (type)
+	if (type != SYS_SYNC_WAITER_SINGLE && type != SYS_SYNC_WAITER_MULTIPLE)
 	{
-	case SYS_SYNC_WAITER_SINGLE: break;
+		sys_event_flag.Error("sys_event_flag_create(): unknown type (0x%x)", type);
-	case SYS_SYNC_WAITER_MULTIPLE: break;
+		return CELL_EINVAL;
-	default: sys_event_flag.Error("sys_event_flag_create(): unknown type (0x%x)", attr->type); return CELL_EINVAL;
 	}
 
 	*id = Emu.GetIdManager().make<lv2_event_flag_t>(init, protocol, type, attr->name_u64);
@@ -58,14 +88,14 @@ s32 sys_event_flag_destroy(u32 id)
 
 	LV2_LOCK;
 
-	const auto ef = Emu.GetIdManager().get<lv2_event_flag_t>(id);
+	const auto eflag = Emu.GetIdManager().get<lv2_event_flag_t>(id);
 
-	if (!ef)
+	if (!eflag)
 	{
 		return CELL_ESRCH;
 	}
 
-	if (ef->waiters)
+	if (!eflag->sq.empty())
 	{
 		return CELL_EBUSY;
 	}
@@ -75,106 +105,84 @@ s32 sys_event_flag_destroy(u32 id)
 	return CELL_OK;
 }
 
-s32 sys_event_flag_wait(u32 id, u64 bitptn, u32 mode, vm::ptr<u64> result, u64 timeout)
+s32 sys_event_flag_wait(PPUThread& ppu, u32 id, u64 bitptn, u32 mode, vm::ptr<u64> result, u64 timeout)
 {
 	sys_event_flag.Log("sys_event_flag_wait(id=0x%x, bitptn=0x%llx, mode=0x%x, result=*0x%x, timeout=0x%llx)", id, bitptn, mode, result, timeout);
 
 	const u64 start_time = get_system_time();
 
+	// If this syscall is called through the SC instruction, these registers must already contain corresponding values.
+	// But let's fixup them (in the case of explicit function call or something) because these values are used externally.
+	ppu.GPR[4] = bitptn;
+	ppu.GPR[5] = mode;
+
 	LV2_LOCK;
 
-	if (result)
+	if (result) *result = 0; // This is very annoying.
+
+	if (!lv2_event_flag_t::check_mode(mode))
 	{
-		*result = 0;
+		sys_event_flag.Error("sys_event_flag_wait(): unknown mode (0x%x)", mode);
+		return CELL_EINVAL;
 	}
 
-	switch (mode & 0xf)
+	const auto eflag = Emu.GetIdManager().get<lv2_event_flag_t>(id);
-	{
-	case SYS_EVENT_FLAG_WAIT_AND: break;
-	case SYS_EVENT_FLAG_WAIT_OR: break;
-	default: return CELL_EINVAL;
-	}
 
-	switch (mode & ~0xf)
+	if (!eflag)
-	{
-	case 0: break;
-	case SYS_EVENT_FLAG_WAIT_CLEAR: break;
-	case SYS_EVENT_FLAG_WAIT_CLEAR_ALL: break;
-	default: return CELL_EINVAL;
-	}
-
-	const auto ef = Emu.GetIdManager().get<lv2_event_flag_t>(id);
-
-	if (!ef)
 	{
 		return CELL_ESRCH;
 	}
 
-	if (ef->type == SYS_SYNC_WAITER_SINGLE && ef->waiters)
+	if (eflag->type == SYS_SYNC_WAITER_SINGLE && eflag->sq.size() > 0)
 	{
 		return CELL_EPERM;
 	}
 
-	while (ef->cancelled)
+	if (eflag->check_pattern(bitptn, mode))
 	{
-		// wait until other threads return CELL_ECANCELED (to prevent modifying bit pattern)
+		const u64 pattern = eflag->clear_pattern(bitptn, mode);
-		ef->cv.wait_for(lv2_lock, std::chrono::milliseconds(1));
+
+		if (result) *result = pattern;
+
+		return CELL_OK;
 	}
 
-	// protocol is ignored in current implementation
+	// add waiter; protocol is ignored in current implementation
-	ef->waiters++;
+	sleep_queue_entry_t waiter(ppu, eflag->sq);
 
-	while (true)
+	while (!ppu.unsignal())
 	{
-		if (result)
-		{
-			*result = ef->flags;
-		}
-
-		if (mode & SYS_EVENT_FLAG_WAIT_AND && (ef->flags & bitptn) == bitptn)
-		{
-			break;
-		}
-
-		if (mode & SYS_EVENT_FLAG_WAIT_OR && ef->flags & bitptn)
-		{
-			break;
-		}
-
 		CHECK_EMU_STATUS;
 
-		if (ef->cancelled)
+		if (timeout)
 		{
-			if (!--ef->cancelled)
+			const u64 passed = get_system_time() - start_time;
+
+			if (passed >= timeout)
 			{
-				ef->cv.notify_all();
+				if (result) *result = eflag->pattern;
+
+				return CELL_ETIMEDOUT;
 			}
 
-			return CELL_ECANCELED;
+			ppu.cv.wait_for(lv2_lock, std::chrono::microseconds(timeout - passed));
 		}
-
+		else
-		if (timeout && get_system_time() - start_time > timeout)
 		{
-			ef->waiters--;
+			ppu.cv.wait(lv2_lock);
-			return CELL_ETIMEDOUT;
 		}
-
+	}
-		ef->cv.wait_for(lv2_lock, std::chrono::milliseconds(1));
+	
+	// load pattern saved upon signaling
+	if (result)
+	{
+		*result = ppu.GPR[4];
 	}
 
-	if (mode & SYS_EVENT_FLAG_WAIT_CLEAR)
+	// check cause
+	if (ppu.GPR[5] == 0)
 	{
-		ef->flags &= ~bitptn;
+		return CELL_ECANCELED;
-	}
-
-	if (mode & SYS_EVENT_FLAG_WAIT_CLEAR_ALL)
-	{
-		ef->flags = 0;
-	}
-
-	if (--ef->waiters && ef->flags)
-	{
-		ef->cv.notify_one();
 	}
 
 	return CELL_OK;
@@ -186,58 +194,31 @@ s32 sys_event_flag_trywait(u32 id, u64 bitptn, u32 mode, vm::ptr<u64> result)
 
 	LV2_LOCK;
 
-	if (result)
+	if (result) *result = 0; // This is very annoying.
+
+	if (!lv2_event_flag_t::check_mode(mode))
 	{
-		*result = 0;
+		sys_event_flag.Error("sys_event_flag_trywait(): unknown mode (0x%x)", mode);
+		return CELL_EINVAL;
 	}
 
-	switch (mode & 0xf)
+	const auto eflag = Emu.GetIdManager().get<lv2_event_flag_t>(id);
-	{
-	case SYS_EVENT_FLAG_WAIT_AND: break;
-	case SYS_EVENT_FLAG_WAIT_OR: break;
-	default: return CELL_EINVAL;
-	}
 
-	switch (mode & ~0xf)
+	if (!eflag)
-	{
-	case 0: break;
-	case SYS_EVENT_FLAG_WAIT_CLEAR: break;
-	case SYS_EVENT_FLAG_WAIT_CLEAR_ALL: break;
-	default: return CELL_EINVAL;
-	}
-
-	const auto ef = Emu.GetIdManager().get<lv2_event_flag_t>(id);
-
-	if (!ef)
 	{
 		return CELL_ESRCH;
 	}
 
-	if (!((mode & SYS_EVENT_FLAG_WAIT_AND) && (ef->flags & bitptn) == bitptn) && !((mode & SYS_EVENT_FLAG_WAIT_OR) && (ef->flags & bitptn)))
+	if (eflag->check_pattern(bitptn, mode))
 	{
-		return CELL_EBUSY;
+		const u64 pattern = eflag->clear_pattern(bitptn, mode);
+
+		if (result) *result = pattern;
+
+		return CELL_OK;
 	}
 
-	while (ef->cancelled)
+	return CELL_EBUSY;
-	{
-		ef->cv.wait_for(lv2_lock, std::chrono::milliseconds(1));
-	}
-
-	if (mode & SYS_EVENT_FLAG_WAIT_CLEAR)
-	{
-		ef->flags &= ~bitptn;
-	}
-	else if (mode & SYS_EVENT_FLAG_WAIT_CLEAR_ALL)
-	{
-		ef->flags &= 0;
-	}
-
-	if (result)
-	{
-		*result = ef->flags;
-	}
-
-	return CELL_OK;
 }
 
 s32 sys_event_flag_set(u32 id, u64 bitptn)
@@ -246,23 +227,16 @@ s32 sys_event_flag_set(u32 id, u64 bitptn)
 
 	LV2_LOCK;
 
-	const auto ef = Emu.GetIdManager().get<lv2_event_flag_t>(id);
+	const auto eflag = Emu.GetIdManager().get<lv2_event_flag_t>(id);
 
-	if (!ef)
+	if (!eflag)
 	{
 		return CELL_ESRCH;
 	}
 
-	while (ef->cancelled)
+	if (bitptn && ~eflag->pattern.fetch_or(bitptn) & bitptn)
 	{
-		ef->cv.wait_for(lv2_lock, std::chrono::milliseconds(1));
+		eflag->notify_all(lv2_lock);
-	}
-
-	ef->flags |= bitptn;
-
-	if (ef->waiters)
-	{
-		ef->cv.notify_all();
 	}
 	
 	return CELL_OK;
@@ -274,19 +248,14 @@ s32 sys_event_flag_clear(u32 id, u64 bitptn)
 
 	LV2_LOCK;
 
-	const auto ef = Emu.GetIdManager().get<lv2_event_flag_t>(id);
+	const auto eflag = Emu.GetIdManager().get<lv2_event_flag_t>(id);
 
-	if (!ef)
+	if (!eflag)
 	{
 		return CELL_ESRCH;
 	}
 
-	while (ef->cancelled)
+	eflag->pattern &= bitptn;
-	{
-		ef->cv.wait_for(lv2_lock, std::chrono::milliseconds(1));
-	}
-
-	ef->flags &= bitptn;
 
 	return CELL_OK;
 }
@@ -302,27 +271,38 @@ s32 sys_event_flag_cancel(u32 id, vm::ptr<u32> num)
 		*num = 0;
 	}
 
-	const auto ef = Emu.GetIdManager().get<lv2_event_flag_t>(id);
+	const auto eflag = Emu.GetIdManager().get<lv2_event_flag_t>(id);
 
-	if (!ef)
+	if (!eflag)
 	{
 		return CELL_ESRCH;
 	}
 
-	while (ef->cancelled)
-	{
-		ef->cv.wait_for(lv2_lock, std::chrono::milliseconds(1));
-	}
-
 	if (num)
 	{
-		*num = ef->waiters;
+		*num = static_cast<u32>(eflag->sq.size());
 	}
 
-	if ((ef->cancelled = ef->waiters.exchange(0)))
+	const u64 pattern = eflag->pattern;
+
+	// signal all threads to return CELL_ECANCELED
+	for (auto& thread : eflag->sq)
 	{
-		ef->cv.notify_all();
+		auto& ppu = static_cast<PPUThread&>(*thread);
+
+		// save existing pattern
+		ppu.GPR[4] = pattern;
+
+		// clear "mode" as a sign of cancellation
+		ppu.GPR[5] = 0;
+
+		if (!thread->signal())
+		{
+			throw EXCEPTION("Thread already signaled");
+		}
 	}
+
+	eflag->sq.clear();
 	
 	return CELL_OK;
 }
@@ -338,16 +318,16 @@ s32 sys_event_flag_get(u32 id, vm::ptr<u64> flags)
 		return CELL_EFAULT;
 	}
 
-	const auto ef = Emu.GetIdManager().get<lv2_event_flag_t>(id);
+	const auto eflag = Emu.GetIdManager().get<lv2_event_flag_t>(id);
 
-	if (!ef)
+	if (!eflag)
 	{
-		*flags = 0;
+		*flags = 0; // This is very annoying.
 
 		return CELL_ESRCH;
 	}
 
-	*flags = ef->flags;
+	*flags = eflag->pattern;
 
 	return CELL_OK;
 }

rpcs3/Emu/SysCalls/lv2/sys_event_flag.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include "sleep_queue.h"
+
 namespace vm { using namespace ps3; }
 
 enum
@@ -35,29 +37,86 @@ struct lv2_event_flag_t
 	const s32 type;
 	const u64 name;
 
-	std::atomic<u64> flags;
+	std::atomic<u64> pattern;
-	std::atomic<u32> cancelled;
 
-	// TODO: use sleep queue, possibly remove condition variable
+	sleep_queue_t sq;
-	std::condition_variable cv;
-	std::atomic<u32> waiters;
 
 	lv2_event_flag_t(u64 pattern, u32 protocol, s32 type, u64 name)
-		: flags(pattern)
+		: pattern(pattern)
 		, protocol(protocol)
 		, type(type)
 		, name(name)
-		, cancelled(0)
-		, waiters(0)
 	{
 	}
+
+	static inline bool check_mode(u32 mode)
+	{
+		switch (mode & 0xf)
+		{
+		case SYS_EVENT_FLAG_WAIT_AND: break;
+		case SYS_EVENT_FLAG_WAIT_OR: break;
+		default: return false;
+		}
+
+		switch (mode & ~0xf)
+		{
+		case 0: break;
+		case SYS_EVENT_FLAG_WAIT_CLEAR: break;
+		case SYS_EVENT_FLAG_WAIT_CLEAR_ALL: break;
+		default: return false;
+		}
+
+		return true;
+	}
+
+	inline bool check_pattern(u64 bitptn, u32 mode)
+	{
+		if ((mode & 0xf) == SYS_EVENT_FLAG_WAIT_AND)
+		{
+			return (pattern & bitptn) == bitptn;
+		}
+		else if ((mode & 0xf) == SYS_EVENT_FLAG_WAIT_OR)
+		{
+			return (pattern & bitptn) != 0;
+		}
+		else
+		{
+			throw EXCEPTION("Unknown mode (0x%x)", mode);
+		}
+	}
+
+	inline u64 clear_pattern(u64 bitptn, u32 mode)
+	{
+		if ((mode & ~0xf) == SYS_EVENT_FLAG_WAIT_CLEAR)
+		{
+			return pattern.fetch_and(~bitptn);
+		}
+		else if ((mode & ~0xf) == SYS_EVENT_FLAG_WAIT_CLEAR_ALL)
+		{
+			return pattern.exchange(0);
+		}
+		else if ((mode & ~0xf) == 0)
+		{
+			return pattern;
+		}
+		else
+		{
+			throw EXCEPTION("Unknown mode (0x%x)", mode);
+		}
+	}
+
+	void notify_all(lv2_lock_t& lv2_lock);
 };
 
 REG_ID_TYPE(lv2_event_flag_t, 0x98); // SYS_EVENT_FLAG_OBJECT
 
+// Aux
+class PPUThread;
+
+// SysCalls
 s32 sys_event_flag_create(vm::ptr<u32> id, vm::ptr<sys_event_flag_attribute_t> attr, u64 init);
 s32 sys_event_flag_destroy(u32 id);
-s32 sys_event_flag_wait(u32 id, u64 bitptn, u32 mode, vm::ptr<u64> result, u64 timeout);
+s32 sys_event_flag_wait(PPUThread& ppu, u32 id, u64 bitptn, u32 mode, vm::ptr<u64> result, u64 timeout);
 s32 sys_event_flag_trywait(u32 id, u64 bitptn, u32 mode, vm::ptr<u64> result);
 s32 sys_event_flag_set(u32 id, u64 bitptn);
 s32 sys_event_flag_clear(u32 id, u64 bitptn);

rpcs3/Emu/SysCalls/lv2/sys_lwmutex.cpp

@@ -16,17 +16,15 @@ s32 _sys_lwmutex_create(vm::ptr<u32> lwmutex_id, u32 protocol, vm::ptr<sys_lwmut
 {
 	sys_lwmutex.Warning("_sys_lwmutex_create(lwmutex_id=*0x%x, protocol=0x%x, control=*0x%x, arg4=0x%x, name=0x%llx, arg6=0x%x)", lwmutex_id, protocol, control, arg4, name, arg6);
 
-	switch (protocol)
+	if (protocol != SYS_SYNC_FIFO && protocol != SYS_SYNC_RETRY && protocol != SYS_SYNC_PRIORITY)
 	{
-	case SYS_SYNC_FIFO: break;
+		sys_lwmutex.Error("_sys_lwmutex_create(): unknown protocol (0x%x)", protocol);
-	case SYS_SYNC_RETRY: break;
+		return CELL_EINVAL;
-	case SYS_SYNC_PRIORITY: break;
-	default: sys_lwmutex.Error("_sys_lwmutex_create(): invalid protocol (0x%x)", protocol); return CELL_EINVAL;
 	}
 
 	if (arg4 != 0x80000001 || arg6)
 	{
-		sys_lwmutex.Error("_sys_lwmutex_create(): unknown parameters (arg4=0x%x, arg6=0x%x)", arg4, arg6);
+		throw EXCEPTION("Unknown arguments (arg4=0x%x, arg6=0x%x)", arg4, arg6);
 	}
 
 	*lwmutex_id = Emu.GetIdManager().make<lv2_lwmutex_t>(protocol, name);