LV2: Unconditional Timer Synchronization Fix

rpcs3/Emu/Cell/lv2/sys_event.cpp

@@ -710,7 +710,7 @@ error_code sys_event_port_send(u32 eport_id, u64 data1, u64 data2, u64 data3)
 	{
 		if (lv2_obj::check(port.queue))
 		{
-			const u64 source = port.name ? port.name : (s64{process_getpid()} << 32) | u64{eport_id};
+			const u64 source = port.name ? port.name : (u64{process_getpid() + 0u} << 32) | u64{eport_id};
 
 			return port.queue->send(source, data1, data2, data3);
 		}

rpcs3/Emu/Cell/lv2/sys_timer.cpp

@@ -5,6 +5,8 @@
 #include "Emu/Cell/ErrorCodes.h"
 #include "Emu/Cell/PPUThread.h"
 #include "Emu/Cell/timers.hpp"
+
+#include "util/asm.hpp"
 #include "Emu/System.h"
 #include "Emu/system_config.h"
 #include "sys_event.h"
@@ -23,7 +25,7 @@ struct lv2_timer_thread
 	lv2_timer_thread();
 	void operator()();
 
-	SAVESTATE_INIT_POS(46); // Dependency ion LV2 objects (lv2_timer)
+	SAVESTATE_INIT_POS(46); // Dependency on LV2 objects (lv2_timer)
 
 	static constexpr auto thread_name = "Timer Thread"sv;
 };
@@ -46,7 +48,7 @@ void lv2_timer::save(utils::serial& ar)
 	ar(state), lv2_event_queue::save_ptr(ar, port.get()), ar(source, data1, data2, expire, period);
 }
 
-u64 lv2_timer::check()
+u64 lv2_timer::check() noexcept
 {
 	while (thread_ctrl::state() != thread_state::aborting)
 	{
@@ -63,11 +65,25 @@ u64 lv2_timer::check()
 				lv2_obj::notify_all_t notify;
 
 				std::lock_guard lock(mutex);
+				return check_unlocked(_now);
+			}
 
-				if (next = expire; _now < next)
+			return (next - _now);
+		}
+
+		break;
+	}
+
+	return umax;
+}
+
+u64 lv2_timer::check_unlocked(u64 _now) noexcept
+{
+	const u64 next = expire;
+
+	if (_now < next || state != SYS_TIMER_STATE_RUN)
 	{
-					// expire was updated in the middle, don't send an event
+		return;
-					continue;
 	}
 
 	if (port)
@@ -78,22 +94,13 @@ u64 lv2_timer::check()
 	if (period)
 	{
 		// Set next expiration time and check again
-					const u64 _next = next + period;
+		const u64 expire0 = utils::add_saturate<u64>(next, period);
-					expire.release(_next > next ? _next : umax);
+		expire.release(_expire0);
-					continue;
+		return expire0 - _now;
 	}
 
 	// Stop after oneshot
 	state.release(SYS_TIMER_STATE_STOP);
-				break;
-			}
-
-			return (next - _now);
-		}
-
-		break;
-	}
-
 	return umax;
 }
 
@@ -133,11 +140,11 @@ void lv2_timer_thread::operator()()
 		{
 			if (lv2_obj::check(timer))
 			{
-				const u64 adviced_sleep_time = timer->check();
+				const u64 advised_sleep_time = timer->check();
 
-				if (sleep_time > adviced_sleep_time)
+				if (sleep_time > advised_sleep_time)
 				{
-					sleep_time = adviced_sleep_time;
+					sleep_time = advised_sleep_time;
 				}
 			}
 		}
@@ -216,9 +223,13 @@ error_code sys_timer_get_information(ppu_thread& ppu, u32 timer_id, vm::ptr<sys_
 	sys_timer.trace("sys_timer_get_information(timer_id=0x%x, info=*0x%x)", timer_id, info);
 
 	sys_timer_information_t _info{};
+	const u64 now = get_guest_system_time();
 
 	const auto timer = idm::check<lv2_obj, lv2_timer>(timer_id, [&](lv2_timer& timer)
 	{
+		std::lock_guard lock(timer.mutex);
+
+		timer.check_unlocked(now);
 		timer.get_information(_info);
 	});
 
@@ -255,6 +266,7 @@ error_code _sys_timer_start(ppu_thread& ppu, u32 timer_id, u64 base_time, u64 pe
 			return CELL_ENOTCONN;
 		}
 
+		timer.check_unlocked(start_time);
 		if (timer.state != SYS_TIMER_STATE_STOP)
 		{
 			return CELL_EBUSY;
@@ -266,9 +278,24 @@ error_code _sys_timer_start(ppu_thread& ppu, u32 timer_id, u64 base_time, u64 pe
 			return CELL_ETIMEDOUT;
 		}
 
-		// sys_timer_start_periodic() will use current time (TODO: is it correct?)
+		const u64 expire = period == 0 ? base_time : // oneshot
-		const u64 expire = base_time ? base_time : start_time + period;
+			base_time == 0 ? utils::add_saturate(start_time, period) : // periodic timer with no base (using start time as base)
-		timer.expire = expire > start_time ? expire : umax;
+			start_time < utils::add_saturate(base_time, period) ? utils::add_saturate(base_time, period) : // periodic with base time over start time
+			[&]() -> u64 // periodic timer base before start time (align to be at least a period over start time)
+			{
+				// Optimized from a loop in LV2:
+				// do
+				// {
+				// 	  base_time += period;
+				// }
+				// while (base_time < start_time);
+
+				const u64 start_time_with_base_time_reminder = utils::add_saturate(start_time - start_time % period, base_time % period);
+
+				return utils::add_saturate(start_time_with_base_time_reminder, start_time_with_base_time_reminder < start_time ? period : 0);
+			}();
+
+		timer.expire = expire;
 		timer.period = period;
 		timer.state  = SYS_TIMER_STATE_RUN;
 		return {};
@@ -300,10 +327,10 @@ error_code sys_timer_stop(ppu_thread& ppu, u32 timer_id)
 
 	sys_timer.trace("sys_timer_stop()");
 
-	const auto timer = idm::check<lv2_obj, lv2_timer>(timer_id, [](lv2_timer& timer)
+	const auto timer = idm::check<lv2_obj, lv2_timer>(timer_id, [now = get_guest_system_time(), notify = lv2_obj::notify_all_t()](lv2_timer& timer)
 	{
 		std::lock_guard lock(timer.mutex);
-
+		timer.check_unlocked(now);
 		timer.state = SYS_TIMER_STATE_STOP;
 	});
 
@@ -339,7 +366,7 @@ error_code sys_timer_connect_event_queue(ppu_thread& ppu, u32 timer_id, u32 queu
 
 		// Connect event queue
 		timer.port   = std::static_pointer_cast<lv2_event_queue>(found->second);
-		timer.source = name ? name : (s64{process_getpid()} << 32) | u64{timer_id};
+		timer.source = name ? name : (u64{process_getpid() + 0u} << 32) | u64{timer_id};
 		timer.data1  = data1;
 		timer.data2  = data2;
 		return {};
@@ -364,10 +391,11 @@ error_code sys_timer_disconnect_event_queue(ppu_thread& ppu, u32 timer_id)
 
 	sys_timer.warning("sys_timer_disconnect_event_queue(timer_id=0x%x)", timer_id);
 
-	const auto timer = idm::check<lv2_obj, lv2_timer>(timer_id, [](lv2_timer& timer) -> CellError
+	const auto timer = idm::check<lv2_obj, lv2_timer>(timer_id, [now = get_guest_system_time(), notify = lv2_obj::notify_all_t()](lv2_timer& timer) -> CellError
 	{
 		std::lock_guard lock(timer.mutex);
 
+		timer.check_unlocked(now);
 		timer.state = SYS_TIMER_STATE_STOP;
 
 		if (!lv2_obj::check(timer.port))

rpcs3/Emu/Cell/lv2/sys_timer.h

@@ -36,20 +36,19 @@ struct lv2_timer : lv2_obj
 	atomic_t<u64> expire{0}; // Next expiration time
 	atomic_t<u64> period{0}; // Period (oneshot if 0)
 
-	u64 check();
+	u64 check() noexcept;
+	u64 check_unlocked(u64 _now) noexcept;
 
 	lv2_timer() noexcept
 		: lv2_obj{1}
 	{
 	}
 
-	void get_information(sys_timer_information_t& info)
+	void get_information(sys_timer_information_t& info) const
 	{
-		reader_lock lock(mutex);
-
 		if (state == SYS_TIMER_STATE_RUN)
 		{
-			info.timer_state = SYS_TIMER_STATE_RUN;
+			info.timer_state = state;
 			info.next_expire = expire;
 			info.period      = period;
 		}

rpcs3/rpcs3qt/kernel_explorer.cpp

@@ -527,11 +527,30 @@ void kernel_explorer::update()
 		{
 			auto& timer = static_cast<lv2_timer&>(obj);
 
-			sys_timer_information_t info;
+			u32 timer_state{SYS_TIMER_STATE_STOP};
-			timer.get_information(info);
+			std::shared_ptr<lv2_event_queue> port;
+			u64 source = 0;
+			u64 data1 = 0;
+			u64 data2 = 0;
 
-			add_leaf(node, qstr(fmt::format("Timer 0x%08x: State: %s, Period: 0x%llx, Next Expire: 0x%llx", id, info.timer_state ? "Running" : "Stopped"
+			u64 expire = 0; // Next expiration time
-				, info.period, info.next_expire)));
+			u64 period = 0; // Period (oneshot if 0)
+
+			if (reader_lock r_lock(timer.mutex); true)
+			{
+				timer_state = timer.state;
+
+				port = timer.port;
+				source = timer.source;
+				data1 = timer.data1;
+				data2 = timer.data2;
+
+				expire = timer.expire; // Next expiration time
+				period = timer.period; // Period (oneshot if 0)
+			}
+
+			add_leaf(node, qstr(fmt::format("Timer 0x%08x: State: %s, Period: 0x%llx, Next Expire: 0x%llx, Queue ID: 0x%08x, Source: 0x%08x, Data1: 0x%08x, Data2: 0x%08x", id, timer_state ? "Running" : "Stopped"
+				, period, expire, port ? port->id : 0, source, data1, data2)));
 			break;
 		}
 		case SYS_SEMAPHORE_OBJECT:

rpcs3/util/asm.hpp

@@ -389,6 +389,25 @@ namespace utils
 		return static_cast<T>(value / align + (value > 0 ? T{(value % align) > (align / 2)} : 0 - T{(value % align) < (align / 2)}));
 	}
 
+	template <UnsignedInt T>
+	constexpr T add_saturate(T addend1, T addend2)
+	{
+		return static_cast<T>(~addend1) < addend2 ? T{umax} : static_cast<T>(addend1 + addend2);
+	}
+
+	template <UnsignedInt T>
+	constexpr T sub_saturate(T minuend, T subtrahend)
+	{
+		return minuend < subtrahend ? T{0} : static_cast<T>(minuend - subtrahend);
+	}
+
+
+	template <UnsignedInt T>
+	constexpr T mul_saturate(T factor1, T factor2)
+	{
+		return T{umax} / factor1 < factor2 ? T{umax} : static_cast<T>(factor1 * factor2);
+	}
+
 	// Hack. Pointer cast util to workaround UB. Use with extreme care.
 	template <typename T, typename U>
 	[[nodiscard]] T* bless(U* ptr)