diff --git a/ps2xRuntime/src/lib/Kernel/Stubs/Helpers/Support.h b/ps2xRuntime/src/lib/Kernel/Stubs/Helpers/Support.h index a98e548d4..5acedd7f1 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,43 @@ namespace ps2_syscalls::dispatchDmacHandlersForCause(rdram, runtime, completedCause); } + // VIF1 (channel 0x10009000) completes with INTC cause 5, and sce libdma + // registers that cause-5 handler AFTER the kick returns, so delivery must + // be deferred (drained next tick), not synchronous here. + // + // This raises cause 5 unconditionally on every VIF1 kick, not only when an + // interrupt was actually requested. That is a deliberate over-approximation. + // Honest fidelity gap: when a cause-5 handler is registered, this + // unconditional raise delivers on every VIF1 kick regardless of whether real + // hardware would have raised the interrupt. + // + // Why not gate it? The runtime DOES decode both relevant interrupt sources, + // but neither is usable as a clean per-kick gate at this call site: + // - The VIFcode "i" bit IS decoded, into vif*_regs.stat bit 11 (INT), at + // ps2_vif1_interpreter.cpp:311-312 (VIF1) and :79-80 (VIF0). But that + // flag is STICKY -- cleared only by FBRST (RST memset at + // ps2_memory.cpp:1018, or STC at :1024) or a CPU write -- and currently + // inert: nothing in this runtime consumes the INT bit (the only stat + // reads, ps2_vif1_interpreter.cpp:380/398, test bit 7 / DBF; the guest + // MMIO read path does not expose vif1_regs.stat). With no per-kick clear + // point and no consumer, it is not a clean edge signal; gating on it + // would need new snapshot-and-clear semantics, and getting that wrong + // reintroduces the very interrupt-drop this raise exists to avoid. + // - The per-tag DMAtag IRQ bit IS decoded, at ps2_memory.cpp:1216, but in + // the register-path DMA chain walker; this sce-libdma stub kick path + // parses only the HEAD tag (for logging). + // Over-raising is chosen because it is benign when unused: the pending latch + // is level-triggered, so an unconsumed cause 5 ages out in + // kPendingIntcMaxAgeTicks (~2 s) drain ticks with no side effect, while DQ8's + // per-frame VIF1 display-list path consumes cause 5 every frame. A mis-gated + // raise, by contrast, would silently DROP legitimate interrupts. If the + // runtime ever makes the INT stat bit a clean per-kick edge (clear-on-read or + // a consumer with defined clear semantics), gate here instead. + 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..bddfd29a1 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 dispatchIntcHandlersForCause(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 = dispatchIntcHandlersForCause(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) @@ -358,6 +459,7 @@ namespace ps2_syscalls dispatchIntcHandlersForCause(rdram, runtime, kIntcVblankStart); std::this_thread::sleep_for(std::chrono::microseconds(500)); dispatchIntcHandlersForCause(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..cb42cf105 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,18 @@ 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: restore all INTC/DMAC handler bookkeeping to process-start + // defaults so regression tests are order-independent. Must live in + // Interrupt.cpp: the handler tables are static per-TU and the pending-age + // array is not header-exposed, so only that TU can reset the live state. + 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..0d765c7e7 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,14 @@ namespace { env.runtime.requestStop(); notifyRuntimeStop(); + // Belt-and-suspenders isolation. The suite's actual order-independence comes + // from main.cpp's BeforeEach(reset_ps2_test_function_table), which wipes the + // recompiled-function table between tests; disabling the reset below still + // yields a green suite. This reset additionally clears the INTC-specific + // global state (handler tables + enable masks + pending latch are global, not + // per-TestEnv) as forward-defensive cleanup so each test starts from a known + // INTC state. + resetInterruptHandlerState(); } void testIntcHandler(uint8_t *rdram, R5900Context *ctx, PS2Runtime *runtime) @@ -177,6 +188,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 +847,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); + }); }); }