From 1ade90cb0c6bb1e9ed37b23673a99e9cb80154e1 Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Mon, 6 Jul 2026 21:14:09 -0400 Subject: [PATCH 1/7] feat(runtime): deferred INTC pending-raise latch with per-tick drain submitDmaSend completes DMA transfers synchronously but never raised the completion interrupt, so the standard sce-libdma kick-and-wait protocol (sceDmaSend -> CreateSema -> AddIntcHandler -> EnableIntc -> WaitSema) could never be released: a synchronous dispatch at the raise site would fire before the handler is registered and be lost, hanging the guest. Add a level-triggered pending-cause latch: - raisePendingIntc(cause): atomically sets a per-cause bit in an atomic pending bitmask (safe from any thread; vblank causes 2/3 and out-of-range causes are excluded -- vblank stays the worker's own periodic dispatch). - drainPendingIntc(rdram, runtime): called once per tick by the interrupt worker; dispatches each pending cause, clearing the bit only once at least one registered+enabled handler actually ran, or after it ages out. A per-cause age array tracks undelivered ticks: a pending cause survives 120 drains and drops on the 121st (~2 s @ 60 Hz), with the age reset on re-raise. This covers the raise-vs-registration race in both directions. - dispatchIntcHandlersForCause is renamed dispatchAndCountIntcHandlersForCause and changes return type void -> int, returning the number of handlers dispatched (0 when the cause is disabled or unregistered) so the drain can tell delivery from a miss. submitDmaSend raises INTC cause 5 after a VIF1 (0x10009000) kick; delivery is intentionally deferred to the next drain tick, which is hardware-plausible DMA-completion latency. The raise is an unconditional over-approximation rather than a per-kick edge: the VIFcode i-bit and DMAtag irq bit are decoded, but the resulting INT stat bit is sticky (cleared only by FBRST/CPU write) and has no consumer, so it is inert and cannot gate a clean edge. Over-raising is benign when unused (the cause ages out), whereas gating on the inert bit would drop legitimate interrupts. resetInterruptHandlerState() (called from cleanupRuntime) resets the handler tables, masks, and latch so runtime state does not leak between tests. Regression tests cover delivery on the next drain tick, the age-out boundary (survives exactly 120 undelivered drains, drops on the 121st, age resets on re-raise), pending persistence across the registration race, and vblank/out-of-range exclusion. raisePendingIntc, drainPendingIntc, and resetInterruptHandlerState are exposed in the runtime header for use by the interrupt worker and tests. --- .../src/lib/Kernel/Stubs/Helpers/Support.h | 13 ++ .../src/lib/Kernel/Syscalls/Interrupt.cpp | 112 ++++++++++- .../src/lib/Kernel/Syscalls/Interrupt.h | 9 + ps2xTest/src/ps2_runtime_interrupt_tests.cpp | 175 ++++++++++++++++++ 4 files changed, 304 insertions(+), 5 deletions(-) diff --git a/ps2xRuntime/src/lib/Kernel/Stubs/Helpers/Support.h b/ps2xRuntime/src/lib/Kernel/Stubs/Helpers/Support.h index a98e548d4..25e60e2e3 100644 --- a/ps2xRuntime/src/lib/Kernel/Stubs/Helpers/Support.h +++ b/ps2xRuntime/src/lib/Kernel/Stubs/Helpers/Support.h @@ -1,6 +1,11 @@ #include #include +namespace ps2_syscalls +{ + void raisePendingIntc(uint32_t cause); +} + namespace { constexpr uint32_t kCdSectorSize = 2048; @@ -1436,6 +1441,14 @@ namespace ps2_syscalls::dispatchDmacHandlersForCause(rdram, runtime, completedCause); } + // Defer cause-5 (VIF1 completion) to the next drain: sce libdma registers + // the handler only after the kick returns. Raised unconditionally per kick -- + // a deliberate over-approximation; an unconsumed raise ages out harmlessly. + if (channelBase == 0x10009000u) // VIF1 + { + ps2_syscalls::raisePendingIntc(5u); + } + return 0; } diff --git a/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp b/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp index 220eadf18..a05165eb7 100644 --- a/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp +++ b/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp @@ -3,6 +3,8 @@ #include "ps2_log.h" #include "Stubs/GS.h" +#include + namespace ps2_syscalls { namespace interrupt_state @@ -23,6 +25,11 @@ namespace ps2_syscalls uint32_t g_enabled_dmac_mask = 0xFFFFFFFFu; uint64_t g_vsync_tick_counter = 0u; VSyncFlagRegistration g_vsync_registration{}; + + std::atomic g_pending_intc_causes{0u}; // bitmask, one pending bit per cause + std::atomic g_pending_intc_age[32] = {}; // drain ticks since raise, per cause + // The age entries are atomic because raisePendingIntc (any thread) resets + // an age while the interrupt worker thread increments it in drainPendingIntc. } using namespace interrupt_state; @@ -95,11 +102,11 @@ namespace ps2_syscalls return (s_cachedStackTop != 0u) ? s_cachedStackTop : (PS2_RAM_SIZE - 0x10u); } - static void dispatchIntcHandlersForCause(uint8_t *rdram, PS2Runtime *runtime, uint32_t cause) + static int dispatchAndCountIntcHandlersForCause(uint8_t *rdram, PS2Runtime *runtime, uint32_t cause) { if (!rdram || !runtime) { - return; + return 0; } std::vector handlers; @@ -107,7 +114,7 @@ namespace ps2_syscalls std::lock_guard lock(g_irq_handler_mutex); if (cause < 32u && (g_enabled_intc_mask & (1u << cause)) == 0u) { - return; + return 0; } handlers.reserve(g_intcHandlers.size()); @@ -132,6 +139,7 @@ namespace ps2_syscalls { return a.order < b.order; }); } + int dispatchedCount = 0; for (const IrqHandlerInfo &info : handlers) { if (!runtime->hasFunction(info.handler)) @@ -156,6 +164,7 @@ namespace ps2_syscalls continue; } + ++dispatchedCount; try { R5900Context irqCtx{}; @@ -194,6 +203,98 @@ namespace ps2_syscalls } } } + + return dispatchedCount; + } + + void raisePendingIntc(uint32_t cause) + { + if (cause >= 32u || cause == kIntcVblankStart || cause == kIntcVblankEnd) + { + return; + } + + const uint32_t bit = 1u << cause; + const uint32_t prev = g_pending_intc_causes.fetch_or(bit, std::memory_order_acq_rel); + if ((prev & bit) == 0u) + { + g_pending_intc_age[cause].store(0u, std::memory_order_relaxed); // fresh raise: age restarts + PS2_IF_AGRESSIVE_LOGS({ + static std::atomic s_raiseLogCount{0u}; + const uint32_t logIndex = s_raiseLogCount.fetch_add(1u, std::memory_order_relaxed); + if (logIndex < 16u || (logIndex % 256u) == 0u) + { + RUNTIME_LOG("[INTC:raise] cause=" << cause); + } + }); + } + } + + void drainPendingIntc(uint8_t *rdram, PS2Runtime *runtime) + { + uint32_t pending = g_pending_intc_causes.load(std::memory_order_acquire); + while (pending != 0u) + { + const uint32_t cause = static_cast(std::countr_zero(pending)); + const uint32_t bit = 1u << cause; + pending &= ~bit; + + const int ran = dispatchAndCountIntcHandlersForCause(rdram, runtime, cause); + if (ran > 0) + { + // Level-triggered by design: delivery clears the single pending + // bit. If another raise of this same cause lands mid-drain + // (between the dispatch above and this fetch_and), the two raises + // collapse into one delivery. Accepted under the + // level-triggered design -- a set bit means "at least one pending", + // not a count -- a deliberate tradeoff, not a lost-wakeup bug. + g_pending_intc_causes.fetch_and(~bit, std::memory_order_acq_rel); + g_pending_intc_age[cause].store(0u, std::memory_order_relaxed); + PS2_IF_AGRESSIVE_LOGS({ + static std::atomic s_deliverLogCount{0u}; + const uint32_t logIndex = s_deliverLogCount.fetch_add(1u, std::memory_order_relaxed); + if (logIndex < 16u || (logIndex % 256u) == 0u) + { + RUNTIME_LOG("[INTC:deliver] cause=" << cause << " handlers=" << ran); + } + }); + } + else if (g_pending_intc_age[cause].fetch_add(1u, std::memory_order_relaxed) + 1u > kPendingIntcMaxAgeTicks) + { + g_pending_intc_causes.fetch_and(~bit, std::memory_order_acq_rel); + g_pending_intc_age[cause].store(0u, std::memory_order_relaxed); + PS2_IF_AGRESSIVE_LOGS({ + static std::atomic s_dropLogCount{0u}; + const uint32_t logIndex = s_dropLogCount.fetch_add(1u, std::memory_order_relaxed); + if (logIndex < 16u || (logIndex % 256u) == 0u) + { + RUNTIME_LOG("[INTC:drop] cause=" << cause << " aged out with no registered/enabled handler"); + } + }); + } + } + } + + void resetInterruptHandlerState() + { + { + std::lock_guard lock(g_irq_handler_mutex); + g_intcHandlers.clear(); + g_dmacHandlers.clear(); + g_nextIntcHandlerId = 1; + g_nextDmacHandlerId = 1; + g_intc_head_order = 0; + g_intc_tail_order = 1000; + g_dmac_head_order = 0; + g_dmac_tail_order = 1000; + g_enabled_intc_mask = 0xFFFFFFFFu; + g_enabled_dmac_mask = 0xFFFFFFFFu; + } + g_pending_intc_causes.store(0u, std::memory_order_release); + for (auto &age : g_pending_intc_age) + { + age.store(0u, std::memory_order_relaxed); + } } void dispatchDmacHandlersForCause(uint8_t *rdram, PS2Runtime *runtime, uint32_t cause) @@ -355,9 +456,10 @@ namespace ps2_syscalls { const uint64_t tickValue = signalVSyncFlag(rdram, runtime); ps2_stubs::dispatchGsSyncVCallback(rdram, runtime, tickValue); - dispatchIntcHandlersForCause(rdram, runtime, kIntcVblankStart); + dispatchAndCountIntcHandlersForCause(rdram, runtime, kIntcVblankStart); std::this_thread::sleep_for(std::chrono::microseconds(500)); - dispatchIntcHandlersForCause(rdram, runtime, kIntcVblankEnd); + dispatchAndCountIntcHandlersForCause(rdram, runtime, kIntcVblankEnd); + drainPendingIntc(rdram, runtime); } } diff --git a/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h b/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h index eb9ba5f03..e30fb66a3 100644 --- a/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h +++ b/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h @@ -1,6 +1,8 @@ #pragma once +#include #include +#include #include "ps2_syscalls.h" namespace ps2_syscalls @@ -24,9 +26,16 @@ namespace ps2_syscalls extern uint32_t g_enabled_dmac_mask; extern uint64_t g_vsync_tick_counter; extern VSyncFlagRegistration g_vsync_registration; + extern std::atomic g_pending_intc_causes; + constexpr uint32_t kPendingIntcMaxAgeTicks = 120u; } void dispatchDmacHandlersForCause(uint8_t *rdram, PS2Runtime *runtime, uint32_t cause); + void raisePendingIntc(uint32_t cause); + void drainPendingIntc(uint8_t *rdram, PS2Runtime *runtime); + // Test-support: reset all INTC/DMAC handler bookkeeping to process-start + // defaults so regression tests are order-independent. + void resetInterruptHandlerState(); void EnsureVSyncWorkerRunning(uint8_t *rdram, PS2Runtime *runtime); uint64_t GetCurrentVSyncTick(); void stopInterruptWorker(); diff --git a/ps2xTest/src/ps2_runtime_interrupt_tests.cpp b/ps2xTest/src/ps2_runtime_interrupt_tests.cpp index 7254973d9..6be0e8f1f 100644 --- a/ps2xTest/src/ps2_runtime_interrupt_tests.cpp +++ b/ps2xTest/src/ps2_runtime_interrupt_tests.cpp @@ -2,6 +2,7 @@ #include "ps2_runtime.h" #include "ps2_syscalls.h" #include "Stubs/DMA.h" +#include "Syscalls/Interrupt.h" #include "runtime/ps2_gs_gpu.h" #include @@ -52,6 +53,8 @@ namespace std::atomic g_dmacSendHits{0u}; std::atomic g_dmacSendLastCause{0u}; std::atomic g_dmacSendLastChcr{0u}; + std::atomic g_pendingIntcHits{0u}; + std::atomic g_pendingIntcLastCause{0xFFFFFFFFu}; void setRegU32(R5900Context &ctx, int reg, uint32_t value) { @@ -125,6 +128,9 @@ namespace { env.runtime.requestStop(); notifyRuntimeStop(); + // Defensive cleanup: reset the global INTC handler tables, enable masks, + // and pending latch so each test starts from a known INTC state. + resetInterruptHandlerState(); } void testIntcHandler(uint8_t *rdram, R5900Context *ctx, PS2Runtime *runtime) @@ -177,6 +183,18 @@ namespace ctx->pc = 0u; } + + void testPendingIntcHandler(uint8_t *rdram, R5900Context *ctx, PS2Runtime *runtime) + { + (void)rdram; + (void)runtime; + + const uint32_t cause = getRegU32(ctx, 4); + g_pendingIntcHits.fetch_add(1u, std::memory_order_relaxed); + g_pendingIntcLastCause.store(cause, std::memory_order_relaxed); + + ctx->pc = 0u; + } } void register_ps2_runtime_interrupt_tests() @@ -824,5 +842,162 @@ void register_ps2_runtime_interrupt_tests() cleanupRuntime(env); }); + + tc.Run("raisePendingIntc delivers to a registered handler on the next drain tick", [](TestCase &t) + { + notifyRuntimeStop(); + TestEnv env; + stopInterruptWorker(); + interrupt_state::g_pending_intc_causes.store(0u); + + constexpr uint32_t kCause = 5u; + constexpr uint32_t kHandlerAddr = 0x00ABE100u; + + g_pendingIntcHits.store(0u, std::memory_order_relaxed); + g_pendingIntcLastCause.store(0xFFFFFFFFu, std::memory_order_relaxed); + + env.runtime.registerFunction(kHandlerAddr, &testPendingIntcHandler); + + R5900Context addCtx{}; + setRegU32(addCtx, 4, kCause); + setRegU32(addCtx, 5, kHandlerAddr); + setRegU32(addCtx, 6, 0u); + setRegU32(addCtx, 7, 0u); + t.IsTrue(callSyscall(0x10u, env.rdram.data(), &addCtx, &env.runtime), "AddIntcHandler syscall should dispatch"); + t.IsTrue(getRegS32(addCtx, 2) > 0, "AddIntcHandler for cause 5 should return handler id"); + + R5900Context enableCtx{}; + setRegU32(enableCtx, 4, kCause); + t.IsTrue(callSyscall(0x14u, env.rdram.data(), &enableCtx, &env.runtime), "EnableIntc syscall should dispatch"); + + // AddIntcHandler auto-starts the interrupt worker thread; stop it so + // this test thread is the only drainer (avoids a double-dispatch race + // between the worker's own per-tick drain and the manual drain below). + stopInterruptWorker(); + + raisePendingIntc(kCause); + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), + "pending bit 5 should be set after raise"); + + drainPendingIntc(env.rdram.data(), &env.runtime); + + t.Equals(g_pendingIntcHits.load(std::memory_order_relaxed), 1u, "handler should run exactly once"); + t.Equals(g_pendingIntcLastCause.load(std::memory_order_relaxed), kCause, + "handler should observe cause 5 in $a0"); + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), 0u, + "pending bit 5 should be cleared after delivery"); + + cleanupRuntime(env); + }); + + tc.Run("undelivered pending cause survives the age window then drops", [](TestCase &t) + { + notifyRuntimeStop(); + TestEnv env; + stopInterruptWorker(); + interrupt_state::g_pending_intc_causes.store(0u); + + // No cause-5 handler is registered in this test, and cleanupRuntime + // resets the process-global INTC handler table between tests, so the + // handler table starts empty here by construction (order-independent). + // Every drain therefore finds no registered+enabled handler for + // cause 5 and is a no-op until the age-out threshold fires. + constexpr uint32_t kCause = 5u; + + raisePendingIntc(kCause); + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), + "pending bit should be set after raise"); + + for (uint32_t i = 0; i < interrupt_state::kPendingIntcMaxAgeTicks; ++i) + { + drainPendingIntc(env.rdram.data(), &env.runtime); + } + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), + "pending bit should survive exactly kPendingIntcMaxAgeTicks undelivered drain ticks"); + + drainPendingIntc(env.rdram.data(), &env.runtime); + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), 0u, + "pending bit should age out and drop on the 121st undelivered drain"); + + raisePendingIntc(kCause); + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), + "re-raise after drop should set the pending bit again"); + + drainPendingIntc(env.rdram.data(), &env.runtime); + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), + "pending bit should survive a single drain after re-raise since its age restarted"); + + cleanupRuntime(env); + }); + + tc.Run("pending cause persists across the raise-vs-registration race", [](TestCase &t) + { + notifyRuntimeStop(); + TestEnv env; + stopInterruptWorker(); + interrupt_state::g_pending_intc_causes.store(0u); + + constexpr uint32_t kCause = 5u; + constexpr uint32_t kHandlerAddr = 0x00ABE200u; + + g_pendingIntcHits.store(0u, std::memory_order_relaxed); + g_pendingIntcLastCause.store(0xFFFFFFFFu, std::memory_order_relaxed); + + raisePendingIntc(kCause); + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), + "pending bit should be set after raise"); + + drainPendingIntc(env.rdram.data(), &env.runtime); + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), + "pending bit should survive a drain while no handler is registered yet"); + + env.runtime.registerFunction(kHandlerAddr, &testPendingIntcHandler); + + R5900Context addCtx{}; + setRegU32(addCtx, 4, kCause); + setRegU32(addCtx, 5, kHandlerAddr); + setRegU32(addCtx, 6, 0u); + setRegU32(addCtx, 7, 0u); + t.IsTrue(callSyscall(0x10u, env.rdram.data(), &addCtx, &env.runtime), "AddIntcHandler syscall should dispatch"); + t.IsTrue(getRegS32(addCtx, 2) > 0, "AddIntcHandler for cause 5 should return handler id"); + + R5900Context enableCtx{}; + setRegU32(enableCtx, 4, kCause); + t.IsTrue(callSyscall(0x14u, env.rdram.data(), &enableCtx, &env.runtime), "EnableIntc syscall should dispatch"); + + // AddIntcHandler auto-starts the worker thread; it may legitimately + // deliver the still-pending cause on its own tick before we stop it. + // Either that path or the manual drain below is correct -- the + // level-triggered bit guarantees exactly one delivery either way. + stopInterruptWorker(); + + drainPendingIntc(env.rdram.data(), &env.runtime); + + t.Equals(g_pendingIntcHits.load(std::memory_order_relaxed), 1u, + "handler should run exactly once total, delivered by the worker or the manual drain"); + t.Equals(g_pendingIntcLastCause.load(std::memory_order_relaxed), kCause, + "delivered handler should observe cause 5 in $a0"); + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), 0u, + "pending bit should be cleared after delivery"); + + cleanupRuntime(env); + }); + + tc.Run("vblank causes are excluded from the pending latch", [](TestCase &t) + { + notifyRuntimeStop(); + TestEnv env; + stopInterruptWorker(); + interrupt_state::g_pending_intc_causes.store(0u); + + raisePendingIntc(2u); // VBLANK start + raisePendingIntc(3u); // VBLANK end + raisePendingIntc(32u); // out of range + + t.Equals(interrupt_state::g_pending_intc_causes.load(), 0u, + "vblank causes and out-of-range causes must never set the pending latch"); + + cleanupRuntime(env); + }); }); } From 32ed26d4e8cdc336f01897e03d9e3ebbb2fa2776 Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Wed, 15 Jul 2026 15:04:24 -0400 Subject: [PATCH 2/7] fix(recomp): include before elfio.hpp to fix build with newer libstdc++ elfio's elf_types.hpp relies on uint16_t/uint32_t/uint64_t being transitively available without including itself. This build environment's libstdc++ no longer pulls those in transitively, causing elf_parser.cpp to fail to compile with 'does not name a type' errors. --- ps2xRecomp/include/ps2recomp/elf_parser.h | 1 + 1 file changed, 1 insertion(+) diff --git a/ps2xRecomp/include/ps2recomp/elf_parser.h b/ps2xRecomp/include/ps2recomp/elf_parser.h index c8cafe5ee..10f9e4ce8 100644 --- a/ps2xRecomp/include/ps2recomp/elf_parser.h +++ b/ps2xRecomp/include/ps2recomp/elf_parser.h @@ -1,6 +1,7 @@ #ifndef PS2RECOMP_ELF_PARSER_H #define PS2RECOMP_ELF_PARSER_H +#include #include #include #include From c993b021f22872d5a28e525e08a91c85d8c2eb1b Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Wed, 15 Jul 2026 16:18:59 -0400 Subject: [PATCH 3/7] test(runtime): make contended semaphore poll/signal test deterministic The "Semaphore poll/signal remains stable under host-thread contention" test asserted signalOkCount > 0, but SignalSema only returns success when count < max_count. With init_count == max_count == 1 the counter starts full, so the signaler thread only succeeds after the poller has drained the count. Whether that interleaving occurs depends entirely on host thread scheduling, so under load (as on the linux-clang CI runner) the signaler can run all 64 iterations while the counter is full, hitting KE_SEMA_OVF every time and leaving signalOkCount == 0 -> spurious failure. Replace the non-deterministic signalOk > 0 assertion with the conservation invariant finalCount == init_count - successful_polls + successful_signals, which holds for every interleaving and is a stronger stability check: it detects any lost or double-applied acquire/release under contention. The guaranteed pollOk > 0 assertion (the first poll always observes count == 1) and the count-range check are retained. --- ps2xTest/src/ps2_runtime_expansion_tests.cpp | 21 +++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/ps2xTest/src/ps2_runtime_expansion_tests.cpp b/ps2xTest/src/ps2_runtime_expansion_tests.cpp index dcfe8ac4f..d90384c42 100644 --- a/ps2xTest/src/ps2_runtime_expansion_tests.cpp +++ b/ps2xTest/src/ps2_runtime_expansion_tests.cpp @@ -1810,10 +1810,19 @@ void register_ps2_runtime_expansion_tests() "PollSema worker thread should not throw"); t.IsFalse(signalerThrew.load(std::memory_order_acquire), "SignalSema worker thread should not throw"); - t.IsTrue(pollOkCount.load(std::memory_order_relaxed) > 0, + const int32_t pollOk = pollOkCount.load(std::memory_order_relaxed); + const int32_t signalOk = signalOkCount.load(std::memory_order_relaxed); + t.IsTrue(pollOk > 0, "contended PollSema should observe at least one successful acquire"); - t.IsTrue(signalOkCount.load(std::memory_order_relaxed) > 0, - "contended SignalSema should observe successful releases"); + // SignalSema only returns success while the counter has headroom + // (count < max_count). With init_count == max_count == 1 the counter + // starts full, so the signaler can only succeed after the poller has + // drained it. Whether that interleaving happens is up to the host + // scheduler, so signalOk is NOT guaranteed to be > 0 and must not be + // asserted directly (doing so made this test flaky under CI load). + // The meaningful stability property under contention is that no + // acquire/release is ever lost or double-applied, which the + // conservation invariant below verifies. constexpr uint32_t kStatusAddr = 0x2100u; R5900Context referCtx{}; @@ -1825,6 +1834,12 @@ void register_ps2_runtime_expansion_tests() int32_t finalCount = 0; std::memcpy(&finalCount, rdram.data() + kStatusAddr + 0u, sizeof(finalCount)); t.IsTrue(finalCount >= 0 && finalCount <= 1, "semaphore count should remain within [0, max_count]"); + // Conservation invariant: starting from init_count (1), every + // successful poll decrements and every successful signal increments + // the counter under the semaphore mutex. This holds for any thread + // interleaving; if contention had corrupted the count it would not. + t.Equals(finalCount, 1 - pollOk + signalOk, + "semaphore count must account for every successful acquire/release under contention"); runtime.requestStop(); notifyRuntimeStop(); From 542022256bc3e170c51f26129f4d8259dedac189 Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Sun, 26 Jul 2026 08:15:37 -0400 Subject: [PATCH 4/7] feat(runtime): gate VIF1 cause-5 on a decoded VIFcode I-bit edge via a PS2Memory consume API Cause 5 is a real hardware event defined by a VIFcode I bit, not by DMA completion. processVIF1Data already decodes that bit; record it as an edge on PS2Memory and consume it from submitDmaSend before raising the deferred cause-5, mirroring the existing consumeCompletedDmacCauses() pattern one level of abstraction up. An ordinary VIF1 packet no longer trips a persistent cause-5 handler. Dropping the channelBase == VIF1 guard on the raise (the edge counter is only ever incremented by processVIF1Data) opens a narrow gap: an edge parsed via processVIF1Data during a non-VIF1 kick's processPendingTransfers, or via the GS/Font/GIF-chain paths, would otherwise wait for the next unrelated submitDmaSend to be consumed. Add a second consume site in drainCompletedDmacHandlers, the runtime's existing "consume queued DMAC events and dispatch" chokepoint, so those edges are delivered within the frame instead of being misattributed to a later kick. --- ps2xRuntime/include/runtime/ps2_memory.h | 2 ++ ps2xRuntime/src/lib/Kernel/Stubs/Helpers/Support.h | 9 +++++---- ps2xRuntime/src/lib/ps2_memory.cpp | 6 ++++++ ps2xRuntime/src/lib/ps2_runtime.cpp | 10 ++++++++++ ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp | 4 ++++ 5 files changed, 27 insertions(+), 4 deletions(-) diff --git a/ps2xRuntime/include/runtime/ps2_memory.h b/ps2xRuntime/include/runtime/ps2_memory.h index 30fc455bd..be45a68bf 100644 --- a/ps2xRuntime/include/runtime/ps2_memory.h +++ b/ps2xRuntime/include/runtime/ps2_memory.h @@ -343,6 +343,7 @@ class PS2Memory void processVIF1Data(const uint8_t *data, uint32_t sizeBytes); void processPendingTransfers(); std::vector consumeCompletedDmacCauses(); + uint32_t consumeVif1InterruptEdges(); int pollDmaRegisters(); @@ -420,6 +421,7 @@ class PS2Memory std::vector m_pendingVif1Transfers; std::mutex m_completedDmacMutex; std::vector m_completedDmacCauses; + std::atomic m_vif1InterruptEdges{0u}; // I-bit VIFcodes seen since last consume struct CodeRegion { diff --git a/ps2xRuntime/src/lib/Kernel/Stubs/Helpers/Support.h b/ps2xRuntime/src/lib/Kernel/Stubs/Helpers/Support.h index 8c05a984b..b9e1dcd4e 100644 --- a/ps2xRuntime/src/lib/Kernel/Stubs/Helpers/Support.h +++ b/ps2xRuntime/src/lib/Kernel/Stubs/Helpers/Support.h @@ -1441,10 +1441,11 @@ namespace ps2_syscalls::dispatchDmacHandlersForCause(rdram, runtime, completedCause); } - // Defer cause-5 (VIF1 completion) to the next drain: sce libdma registers - // the handler only after the kick returns. Raised unconditionally per kick -- - // a deliberate over-approximation; an unconsumed raise ages out harmlessly. - if (channelBase == 0x10009000u) // VIF1 + // VIF1 INTC cause 5 is asserted by a VIFcode I bit, decoded during the + // processPendingTransfers() parse above. Raise only when such an edge was + // actually seen. Delivery stays deferred to the next drain: sce libdma + // registers the cause-5 handler after this kick returns. + if (mem.consumeVif1InterruptEdges() != 0u) { ps2_syscalls::raisePendingIntc(5u); } diff --git a/ps2xRuntime/src/lib/ps2_memory.cpp b/ps2xRuntime/src/lib/ps2_memory.cpp index 3d593285a..8407025a9 100644 --- a/ps2xRuntime/src/lib/ps2_memory.cpp +++ b/ps2xRuntime/src/lib/ps2_memory.cpp @@ -297,6 +297,7 @@ bool PS2Memory::initialize(size_t ramSize) std::lock_guard lock(m_completedDmacMutex); m_completedDmacCauses.clear(); } + m_vif1InterruptEdges.store(0u, std::memory_order_relaxed); m_codeRegions.clear(); m_path3Masked = false; m_path3MaskedFifo.clear(); @@ -1665,6 +1666,11 @@ std::vector PS2Memory::consumeCompletedDmacCauses() return causes; } +uint32_t PS2Memory::consumeVif1InterruptEdges() +{ + return m_vif1InterruptEdges.exchange(0u, std::memory_order_relaxed); +} + void PS2Memory::flushMaskedPath3Packets(bool drainImmediately) { if (m_path3Masked || m_path3MaskedFifo.empty()) diff --git a/ps2xRuntime/src/lib/ps2_runtime.cpp b/ps2xRuntime/src/lib/ps2_runtime.cpp index b79e386a5..b8976e366 100644 --- a/ps2xRuntime/src/lib/ps2_runtime.cpp +++ b/ps2xRuntime/src/lib/ps2_runtime.cpp @@ -9,6 +9,7 @@ #include "Kernel/Stubs/Audio.h" #include "Kernel/Stubs/GS.h" #include "Kernel/Stubs/MPEG.h" +#include "Kernel/Syscalls/Interrupt.h" #include "ps2_host_backend.h" #include "ps2_iop_host.h" #include "ps2x/iop/iop_subsystem.h" @@ -1404,6 +1405,15 @@ void PS2Runtime::drainCompletedDmacHandlers(uint8_t *rdram) { ps2_syscalls::dispatchDmacHandlersForCause(rdram, this, cause); } + + // Catch-all for VIF1 I-bit edges parsed outside submitDmaSend's own + // consume (GS/Font/GIF-chain paths that call processVIF1Data within the + // frame) so they get delivered here instead of leaking into a later, + // unrelated submitDmaSend kick. + if (m_memory.consumeVif1InterruptEdges() != 0u) + { + ps2_syscalls::raisePendingIntc(5u); + } } void PS2Runtime::handleTrap(uint8_t *rdram, R5900Context *ctx) diff --git a/ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp b/ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp index c0af2a3ac..7bb6fd80c 100644 --- a/ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp +++ b/ps2xRuntime/src/lib/ps2_vif1_interpreter.cpp @@ -309,7 +309,11 @@ void PS2Memory::processVIF1Data(const uint8_t *data, uint32_t sizeBytes) vif1_regs.code = cmd; vif1_regs.num = num; if (irq) + { vif1_regs.stat |= (1u << 11); // INT + // Record the VIF1 INT edge for the deferred cause-5 raise. + m_vif1InterruptEdges.fetch_add(1u, std::memory_order_relaxed); + } if (opcode == VIF_NOP) { From f766a22889375d5ff6fb40e2900a7731a573a149 Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Sun, 26 Jul 2026 08:15:53 -0400 Subject: [PATCH 5/7] fix(runtime): expire pending INTC causes by monotonic wall-clock, refreshed on every raise Replace the per-cause drain-tick counter with a per-cause steady_clock timestamp captured at raise time. Two problems with the tick counter: its window scales with how often something happens to call drainPendingIntc, not with real time, so a cause raised before the interrupt worker starts could survive far longer (or shorter) than intended; and the timestamp was previously only stamped on the 0->1 edge, so a cause re-raised while already pending kept aging from its first raise instead of its most recent one, letting a live, repeatedly re-raised cause get dropped out from under an about-to-register handler. The timestamp is now stored on every raise, independent of whether the pending bit was already set, and drainPendingIntc compares wall-clock age against a fixed ~2s window instead of counting ticks. Add an injectable clock (setPendingIntcClockForTest) so the expiry window can be exercised deterministically in tests without sleeping; resetInterruptHandlerState restores the real clock so a test that installs a fake one can't leak it into the next test. --- .../src/lib/Kernel/Syscalls/Interrupt.cpp | 59 +++++++++++++------ .../src/lib/Kernel/Syscalls/Interrupt.h | 6 +- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp b/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp index 8fc2c5d05..84fff50d3 100644 --- a/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp +++ b/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp @@ -28,13 +28,33 @@ namespace ps2_syscalls VSyncFlagRegistration g_vsync_registration{}; std::atomic g_pending_intc_causes{0u}; // bitmask, one pending bit per cause - std::atomic g_pending_intc_age[32] = {}; // drain ticks since raise, per cause - // The age entries are atomic because raisePendingIntc (any thread) resets - // an age while the interrupt worker thread increments it in drainPendingIntc. + std::atomic g_pending_intc_raise_ns[32] = {}; // steady_clock ns at last raise, per cause + // The raise-timestamp entries are atomic because raisePendingIntc (any + // thread) stores a fresh timestamp while the interrupt worker thread + // reads it in drainPendingIntc. } using namespace interrupt_state; + namespace + { + SteadyNowFn g_pendingIntcNowFn{}; // empty = real clock + + int64_t now_ns() + { + const auto tp = g_pendingIntcNowFn ? g_pendingIntcNowFn() + : std::chrono::steady_clock::now(); + return std::chrono::duration_cast( + tp.time_since_epoch()) + .count(); + } + } + + void setPendingIntcClockForTest(SteadyNowFn fn) + { + g_pendingIntcNowFn = std::move(fn); + } + static void writeGuestU32NoThrow(uint8_t *rdram, uint32_t addr, uint32_t value) { if (addr == 0u) @@ -237,10 +257,10 @@ namespace ps2_syscalls } const uint32_t bit = 1u << cause; + g_pending_intc_raise_ns[cause].store(now_ns(), std::memory_order_relaxed); // every raise const uint32_t prev = g_pending_intc_causes.fetch_or(bit, std::memory_order_acq_rel); if ((prev & bit) == 0u) { - g_pending_intc_age[cause].store(0u, std::memory_order_relaxed); // fresh raise: age restarts PS2_IF_AGRESSIVE_LOGS({ static std::atomic s_raiseLogCount{0u}; const uint32_t logIndex = s_raiseLogCount.fetch_add(1u, std::memory_order_relaxed); @@ -271,7 +291,6 @@ namespace ps2_syscalls // level-triggered design -- a set bit means "at least one pending", // not a count -- a deliberate tradeoff, not a lost-wakeup bug. g_pending_intc_causes.fetch_and(~bit, std::memory_order_acq_rel); - g_pending_intc_age[cause].store(0u, std::memory_order_relaxed); PS2_IF_AGRESSIVE_LOGS({ static std::atomic s_deliverLogCount{0u}; const uint32_t logIndex = s_deliverLogCount.fetch_add(1u, std::memory_order_relaxed); @@ -281,18 +300,21 @@ namespace ps2_syscalls } }); } - else if (g_pending_intc_age[cause].fetch_add(1u, std::memory_order_relaxed) + 1u > kPendingIntcMaxAgeTicks) + else { - g_pending_intc_causes.fetch_and(~bit, std::memory_order_acq_rel); - g_pending_intc_age[cause].store(0u, std::memory_order_relaxed); - PS2_IF_AGRESSIVE_LOGS({ - static std::atomic s_dropLogCount{0u}; - const uint32_t logIndex = s_dropLogCount.fetch_add(1u, std::memory_order_relaxed); - if (logIndex < 16u || (logIndex % 256u) == 0u) - { - RUNTIME_LOG("[INTC:drop] cause=" << cause << " aged out with no registered/enabled handler"); - } - }); + const int64_t age = now_ns() - g_pending_intc_raise_ns[cause].load(std::memory_order_relaxed); + if (age > kPendingIntcMaxAgeNs) + { + g_pending_intc_causes.fetch_and(~bit, std::memory_order_acq_rel); + PS2_IF_AGRESSIVE_LOGS({ + static std::atomic s_dropLogCount{0u}; + const uint32_t logIndex = s_dropLogCount.fetch_add(1u, std::memory_order_relaxed); + if (logIndex < 16u || (logIndex % 256u) == 0u) + { + RUNTIME_LOG("[INTC:drop] cause=" << cause << " aged out with no registered/enabled handler"); + } + }); + } } } } @@ -313,10 +335,11 @@ namespace ps2_syscalls g_enabled_dmac_mask = 0xFFFFFFFFu; } g_pending_intc_causes.store(0u, std::memory_order_release); - for (auto &age : g_pending_intc_age) + for (auto &ts : g_pending_intc_raise_ns) { - age.store(0u, std::memory_order_relaxed); + ts.store(0, std::memory_order_relaxed); } + setPendingIntcClockForTest(nullptr); // a leaked fake clock would leak into the next test } void dispatchDmacHandlersForCause(uint8_t *rdram, PS2Runtime *runtime, uint32_t cause) diff --git a/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h b/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h index e30fb66a3..e0c42b6c8 100644 --- a/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h +++ b/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h @@ -1,8 +1,10 @@ #pragma once #include +#include #include #include +#include #include "ps2_syscalls.h" namespace ps2_syscalls @@ -27,7 +29,7 @@ namespace ps2_syscalls extern uint64_t g_vsync_tick_counter; extern VSyncFlagRegistration g_vsync_registration; extern std::atomic g_pending_intc_causes; - constexpr uint32_t kPendingIntcMaxAgeTicks = 120u; + constexpr int64_t kPendingIntcMaxAgeNs = 2'000'000'000; // ~2 s wall-clock (see Q1) } void dispatchDmacHandlersForCause(uint8_t *rdram, PS2Runtime *runtime, uint32_t cause); @@ -36,6 +38,8 @@ namespace ps2_syscalls // Test-support: reset all INTC/DMAC handler bookkeeping to process-start // defaults so regression tests are order-independent. void resetInterruptHandlerState(); + using SteadyNowFn = std::function; + void setPendingIntcClockForTest(SteadyNowFn fn); // nullptr restores steady_clock::now void EnsureVSyncWorkerRunning(uint8_t *rdram, PS2Runtime *runtime); uint64_t GetCurrentVSyncTick(); void stopInterruptWorker(); From ec26ccd69988c62e05fc9b767d769802b5df3a3f Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Sun, 26 Jul 2026 08:16:04 -0400 Subject: [PATCH 6/7] test(runtime): cover the VIF1 producer path and clock-based expiration Two new tests drive the real producer (ps2_stubs::sceDmaSend, not a direct raisePendingIntc call) with a CNT-then-END VIF1 DMA chain: one with an I-bit VIFcode in the payload, one without. They cover the exact DQ8 boot hazard -- a kick that runs before sce libdma registers its cause-5 handler -- and the negative case the old unconditional raise got wrong: an ordinary VIF1 packet must not trip a persistent cause-5 handler. Rewrite the drain-tick age-out test to drive the injectable clock instead of looping a tick counter (the counter it referenced is gone), and extend it to re-raise mid-window and confirm the bit survives because freshness is measured from the most recent raise, not the first. Add a test for a raise that predates worker startup, confirming it cannot outlive the expiry window once a drain finally runs. --- ps2xTest/src/ps2_runtime_interrupt_tests.cpp | 177 +++++++++++++++++-- 1 file changed, 166 insertions(+), 11 deletions(-) diff --git a/ps2xTest/src/ps2_runtime_interrupt_tests.cpp b/ps2xTest/src/ps2_runtime_interrupt_tests.cpp index dd1e7a5c9..3feb4bd75 100644 --- a/ps2xTest/src/ps2_runtime_interrupt_tests.cpp +++ b/ps2xTest/src/ps2_runtime_interrupt_tests.cpp @@ -536,6 +536,122 @@ void register_ps2_runtime_interrupt_tests() cleanupRuntime(env); }); + tc.Run("I-bit VIF1 packet raises cause 5, delivered to a handler registered after the kick", [](TestCase &t) + { + notifyRuntimeStop(); + TestEnv env; + t.IsTrue(env.runtime.memory().initialize(), "runtime memory initialize should succeed"); + + constexpr uint32_t kHandlerAddr = 0x00ABD300u; + constexpr uint32_t kVif1Ch = 0x10009000u; + constexpr uint32_t kTag0 = 0x00028800u; + constexpr uint32_t kTag1 = kTag0 + 0x20u; + constexpr uint32_t kCause = 5u; + + uint8_t *rdram = env.runtime.memory().getRDRAM(); + writeDmaTag(rdram, kTag0, makeDmaTag(1u, 1u, 0u, false)); // CNT, qwc=1 + writeGuestU64(rdram, kTag0 + 0x10u, 0x0000000080000000ull); // I-bit VIF_NOP + writeGuestU64(rdram, kTag0 + 0x18u, 0u); + writeDmaTag(rdram, kTag1, makeDmaTag(0u, 7u, 0u, false)); // END + + stopInterruptWorker(); + interrupt_state::g_pending_intc_causes.store(0u); + g_pendingIntcHits.store(0u, std::memory_order_relaxed); + g_pendingIntcLastCause.store(0xFFFFFFFFu, std::memory_order_relaxed); + + // Kick before registering: sce libdma only registers the cause-5 + // handler after the kick returns, so this is the DQ8 boot order. + R5900Context sendCtx{}; + setRegU32(sendCtx, 4, kVif1Ch); + setRegU32(sendCtx, 5, kTag0); + ps2_stubs::sceDmaSend(rdram, &sendCtx, &env.runtime); + + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), + "an I-bit VIFcode parsed by the kick should raise cause 5 before any handler is registered"); + + env.runtime.registerFunction(kHandlerAddr, &testPendingIntcHandler); + + R5900Context addCtx{}; + setRegU32(addCtx, 4, kCause); + setRegU32(addCtx, 5, kHandlerAddr); + setRegU32(addCtx, 6, 0u); + setRegU32(addCtx, 7, 0u); + t.IsTrue(callSyscall(0x10u, rdram, &addCtx, &env.runtime), "AddIntcHandler syscall should dispatch"); + t.IsTrue(getRegS32(addCtx, 2) > 0, "AddIntcHandler for cause 5 should return handler id"); + + R5900Context enableCtx{}; + setRegU32(enableCtx, 4, kCause); + t.IsTrue(callSyscall(0x14u, rdram, &enableCtx, &env.runtime), "EnableIntc syscall should dispatch"); + + // AddIntcHandler auto-starts the interrupt worker; stop it so this + // test thread is the only drainer. + stopInterruptWorker(); + + drainPendingIntc(rdram, &env.runtime); + + t.Equals(g_pendingIntcHits.load(std::memory_order_relaxed), 1u, "late-registered handler should run exactly once"); + t.Equals(g_pendingIntcLastCause.load(std::memory_order_relaxed), kCause, "handler should observe cause 5 in $a0"); + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), 0u, + "pending bit 5 should be cleared after delivery"); + + cleanupRuntime(env); + }); + + tc.Run("VIF1 packet without an I bit does not raise cause 5", [](TestCase &t) + { + notifyRuntimeStop(); + TestEnv env; + t.IsTrue(env.runtime.memory().initialize(), "runtime memory initialize should succeed"); + + constexpr uint32_t kHandlerAddr = 0x00ABD380u; + constexpr uint32_t kVif1Ch = 0x10009000u; + constexpr uint32_t kTag0 = 0x00028A00u; + constexpr uint32_t kTag1 = kTag0 + 0x20u; + constexpr uint32_t kCause = 5u; + + uint8_t *rdram = env.runtime.memory().getRDRAM(); + writeDmaTag(rdram, kTag0, makeDmaTag(1u, 1u, 0u, false)); // CNT, qwc=1 + writeGuestU64(rdram, kTag0 + 0x10u, 0u); // plain VIF_NOP, no I bit + writeGuestU64(rdram, kTag0 + 0x18u, 0u); + writeDmaTag(rdram, kTag1, makeDmaTag(0u, 7u, 0u, false)); // END + + stopInterruptWorker(); + interrupt_state::g_pending_intc_causes.store(0u); + g_pendingIntcHits.store(0u, std::memory_order_relaxed); + g_pendingIntcLastCause.store(0xFFFFFFFFu, std::memory_order_relaxed); + + R5900Context sendCtx{}; + setRegU32(sendCtx, 4, kVif1Ch); + setRegU32(sendCtx, 5, kTag0); + ps2_stubs::sceDmaSend(rdram, &sendCtx, &env.runtime); + + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), 0u, + "an ordinary VIF1 packet with no I bit must not raise cause 5"); + + env.runtime.registerFunction(kHandlerAddr, &testPendingIntcHandler); + + R5900Context addCtx{}; + setRegU32(addCtx, 4, kCause); + setRegU32(addCtx, 5, kHandlerAddr); + setRegU32(addCtx, 6, 0u); + setRegU32(addCtx, 7, 0u); + t.IsTrue(callSyscall(0x10u, rdram, &addCtx, &env.runtime), "AddIntcHandler syscall should dispatch"); + t.IsTrue(getRegS32(addCtx, 2) > 0, "AddIntcHandler for cause 5 should return handler id"); + + R5900Context enableCtx{}; + setRegU32(enableCtx, 4, kCause); + t.IsTrue(callSyscall(0x14u, rdram, &enableCtx, &env.runtime), "EnableIntc syscall should dispatch"); + + stopInterruptWorker(); + + drainPendingIntc(rdram, &env.runtime); + + t.Equals(g_pendingIntcHits.load(std::memory_order_relaxed), 0u, + "a persistent cause-5 handler must not be invoked by an ordinary VIF1 packet with no I bit"); + + cleanupRuntime(env); + }); + tc.Run("MMIO VIF1 chain completion dispatches DMAC handler after CHCR store", [](TestCase &t) { notifyRuntimeStop(); @@ -953,7 +1069,7 @@ void register_ps2_runtime_interrupt_tests() cleanupRuntime(env); }); - tc.Run("undelivered pending cause survives the age window then drops", [](TestCase &t) + tc.Run("re-raise at the age-out boundary is not dropped", [](TestCase &t) { notifyRuntimeStop(); TestEnv env; @@ -964,31 +1080,70 @@ void register_ps2_runtime_interrupt_tests() // resets the process-global INTC handler table between tests, so the // handler table starts empty here by construction (order-independent). // Every drain therefore finds no registered+enabled handler for - // cause 5 and is a no-op until the age-out threshold fires. + // cause 5 and is a no-op until the wall-clock expiry fires. constexpr uint32_t kCause = 5u; + auto fakeNow = std::chrono::steady_clock::now(); + setPendingIntcClockForTest([&fakeNow]() { return fakeNow; }); + raisePendingIntc(kCause); t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), "pending bit should be set after raise"); - for (uint32_t i = 0; i < interrupt_state::kPendingIntcMaxAgeTicks; ++i) - { - drainPendingIntc(env.rdram.data(), &env.runtime); - } + // Advance to just under the expiry window measured from the raise. + fakeNow += std::chrono::nanoseconds(interrupt_state::kPendingIntcMaxAgeNs - 1); + drainPendingIntc(env.rdram.data(), &env.runtime); t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), - "pending bit should survive exactly kPendingIntcMaxAgeTicks undelivered drain ticks"); + "pending bit should survive a drain just under the expiry window"); + // Re-raise while still pending -- this refreshes the timestamp. + raisePendingIntc(kCause); + + // Advance again by just-under-the-window: measured from the + // ORIGINAL raise this is now well past expiry, but measured from + // the re-raise it is still just under. A comment-3 regression + // (only stamping the timestamp on the 0->1 edge) would drop the + // bit here. + fakeNow += std::chrono::nanoseconds(interrupt_state::kPendingIntcMaxAgeNs - 1); + drainPendingIntc(env.rdram.data(), &env.runtime); + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), + "pending bit should survive when re-raised -- freshness is measured from the most recent raise"); + + // Advance past the window measured from the re-raise. + fakeNow += std::chrono::nanoseconds(interrupt_state::kPendingIntcMaxAgeNs + 1); drainPendingIntc(env.rdram.data(), &env.runtime); t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), 0u, - "pending bit should age out and drop on the 121st undelivered drain"); + "pending bit should drop once the window has elapsed since the most recent raise"); + + cleanupRuntime(env); + }); + + tc.Run("a raise before worker startup cannot survive beyond the expiry window", [](TestCase &t) + { + notifyRuntimeStop(); + TestEnv env; + stopInterruptWorker(); + interrupt_state::g_pending_intc_causes.store(0u); + constexpr uint32_t kCause = 5u; + + auto fakeNow = std::chrono::steady_clock::now(); + setPendingIntcClockForTest([&fakeNow]() { return fakeNow; }); + + // No worker started and no drain yet -- this models a cause raised + // synchronously before the game ever reaches a VSync/AddIntc path + // that would start the interrupt worker. raisePendingIntc(kCause); t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), - "re-raise after drop should set the pending bit again"); + "pending bit should be set after raise"); + fakeNow += std::chrono::nanoseconds(interrupt_state::kPendingIntcMaxAgeNs + 1); + + // The worker's first tick after a late start. drainPendingIntc(env.rdram.data(), &env.runtime); - t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), (1u << kCause), - "pending bit should survive a single drain after re-raise since its age restarted"); + t.Equals(interrupt_state::g_pending_intc_causes.load() & (1u << kCause), 0u, + "a raise that predates worker startup by more than the expiry window must not survive " + "the worker's first drain"); cleanupRuntime(env); }); From 8d324ed6bca6e974ac72dce139e21e2ddd109897 Mon Sep 17 00:00:00 2001 From: Shane Michael Mathews Date: Sun, 26 Jul 2026 08:40:08 -0400 Subject: [PATCH 7/7] =?UTF-8?q?docs(runtime):=20address=20cold-review=20?= =?UTF-8?q?=E2=80=94=20genericize=20test=20comment;=20drop=20internal=20do?= =?UTF-8?q?c=20ref;=20trim=20level-triggered=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp | 7 +------ ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h | 2 +- ps2xTest/src/ps2_runtime_interrupt_tests.cpp | 3 ++- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp b/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp index 84fff50d3..39cdaf5d5 100644 --- a/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp +++ b/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.cpp @@ -284,12 +284,7 @@ namespace ps2_syscalls const int ran = dispatchAndCountIntcHandlersForCause(rdram, runtime, cause); if (ran > 0) { - // Level-triggered by design: delivery clears the single pending - // bit. If another raise of this same cause lands mid-drain - // (between the dispatch above and this fetch_and), the two raises - // collapse into one delivery. Accepted under the - // level-triggered design -- a set bit means "at least one pending", - // not a count -- a deliberate tradeoff, not a lost-wakeup bug. + // Level-triggered: concurrent same-cause raises collapse into one delivery (intentional). g_pending_intc_causes.fetch_and(~bit, std::memory_order_acq_rel); PS2_IF_AGRESSIVE_LOGS({ static std::atomic s_deliverLogCount{0u}; diff --git a/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h b/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h index e0c42b6c8..6695af19d 100644 --- a/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h +++ b/ps2xRuntime/src/lib/Kernel/Syscalls/Interrupt.h @@ -29,7 +29,7 @@ namespace ps2_syscalls extern uint64_t g_vsync_tick_counter; extern VSyncFlagRegistration g_vsync_registration; extern std::atomic g_pending_intc_causes; - constexpr int64_t kPendingIntcMaxAgeNs = 2'000'000'000; // ~2 s wall-clock (see Q1) + constexpr int64_t kPendingIntcMaxAgeNs = 2'000'000'000; // ~2 s wall-clock } void dispatchDmacHandlersForCause(uint8_t *rdram, PS2Runtime *runtime, uint32_t cause); diff --git a/ps2xTest/src/ps2_runtime_interrupt_tests.cpp b/ps2xTest/src/ps2_runtime_interrupt_tests.cpp index 3feb4bd75..103f519d9 100644 --- a/ps2xTest/src/ps2_runtime_interrupt_tests.cpp +++ b/ps2xTest/src/ps2_runtime_interrupt_tests.cpp @@ -560,7 +560,8 @@ void register_ps2_runtime_interrupt_tests() g_pendingIntcLastCause.store(0xFFFFFFFFu, std::memory_order_relaxed); // Kick before registering: sce libdma only registers the cause-5 - // handler after the kick returns, so this is the DQ8 boot order. + // handler after the kick returns -- the typical sce-libdma + // kick-before-register order. R5900Context sendCtx{}; setRegU32(sendCtx, 4, kVif1Ch); setRegU32(sendCtx, 5, kTag0);