Merge pull request #642 from RetroPortingToolKit/fix/overlay-unit-irq-starvation

overlay: deliver IRQs inside call units (polling callee starvation; LoD memcard hang)

runtime/CMakeLists.txt

@@ -278,6 +278,8 @@ if(BUILD_TESTING)
             "${CMAKE_CURRENT_SOURCE_DIR}/tests/test_mirror_pc_preserved_guards.py")
         add_test(NAME segment_miss_wiring_test COMMAND "${Python3_EXECUTABLE}"
             "${CMAKE_CURRENT_SOURCE_DIR}/tests/test_segment_miss_wiring.py")
+        add_test(NAME overlay_unit_defer_wiring_test COMMAND "${Python3_EXECUTABLE}"
+            "${CMAKE_CURRENT_SOURCE_DIR}/tests/test_overlay_unit_defer_wiring.py")
         if(CMAKE_C_COMPILER_ID MATCHES "GNU|Clang")
             add_test(NAME ws_screen_tags_exec_test COMMAND "${Python3_EXECUTABLE}"
                 "${CMAKE_CURRENT_SOURCE_DIR}/tests/test_ws_screen_tags_exec.py"
@@ -1877,6 +1879,18 @@ if(BUILD_TESTING)
         add_test(NAME netplay_input_hist_test COMMAND netplay_input_hist_test)
     endif()
 
+    # IRQ policy for nested overlay call units: IRQs are delivered inside a
+    # unit (a callee that polls for an IRQ-set flag, libcard MemCardSync, must
+    # not starve: LoD "Checking MEMORY CARD" hang). Header-only, no runtime link.
+    add_executable(overlay_unit_defer_test tests/test_overlay_unit_defer.c)
+    target_include_directories(overlay_unit_defer_test PRIVATE src)
+    if(MSVC)
+        target_compile_options(overlay_unit_defer_test PRIVATE /UNDEBUG)
+    else()
+        target_compile_options(overlay_unit_defer_test PRIVATE -UNDEBUG)
+    endif()
+    add_test(NAME overlay_unit_defer_test COMMAND overlay_unit_defer_test)
+
     add_executable(overlay_store_cycle_barrier_test
         tests/test_overlay_store_cycle_barrier.c)
     target_include_directories(overlay_store_cycle_barrier_test PRIVATE include)

runtime/src/debug_server.c

@@ -14317,6 +14317,19 @@ static void handle_overlay_loader_status(int id, const char *json)
             (unsigned long long)nd, (unsigned long long)ni,
             (unsigned long long)sn, (unsigned long long)ss,
             (unsigned long long)sc, (unsigned long long)sx);
+        {
+            /* Call-unit IRQ starvation signature: call_unit_depth stuck > 0 with
+             * ci_skip_unit climbing while an unmasked IRQ stays pending. */
+            uint64_t cu=0, cs=0, cn=0, cr=0, cd=0, ce=0;
+            overlay_loader_get_ci_skip_diag(&cu, &cs, &cn, &cr, &cd, &ce);
+            n += snprintf(buf + n, sizeof(buf) - n,
+                ",\"ci_skip_unit\":%llu,\"ci_skip_supp\":%llu,\"ci_skip_none\":%llu,"
+                "\"ci_skip_sr\":%llu,\"ci_skip_deliv\":%llu,\"ci_enter\":%llu,"
+                "\"call_unit_depth\":%d",
+                (unsigned long long)cu, (unsigned long long)cs, (unsigned long long)cn,
+                (unsigned long long)cr, (unsigned long long)cd, (unsigned long long)ce,
+                overlay_loader_call_unit_depth());
+        }
         extern volatile long g_overlay_image_warm_loaded;
         extern volatile long g_overlay_image_warm_pending;
         n += snprintf(buf + n, sizeof(buf) - n,

runtime/src/overlay_loader.c

@@ -8,6 +8,7 @@
 #include "interrupts.h"
 #include "debug_server.h"
 #include "psx_cycles.h"
+#include "overlay_unit_defer.h"
 
 /* Out-of-line copy of the header-inline CPU charge for the overlay callback
  * table: overlay DLLs publish their accumulated CPU charge here, into the
@@ -2283,12 +2284,17 @@ extern uint32_t i_mask;
  * nested unit suspended the interrupted thread with an INCONSISTENT snapshot
  * (resume PC at the caller's post-call point, sp still mid-callee) -> a leaked
  * stack frame that compounded across cooperative cycles into a smeared jumptable
- * index (the Ape "Checking MEMORY CARD" softlock/fatal). While this depth is >0
- * both backends defer the IRQ check; the callee runs to completion, then the
- * enclosing top-level dirty pump / outermost dispatch return delivers the IRQ at
- * a consistent (pc, sp) boundary. The TOP-LEVEL flow (depth 0) still pumps IRQs,
- * so the consumer's poll loop and long overlay game-loops keep their
- * responsiveness. Incremented only when the A/B toggle is on (below). */
+ * index (the Ape "Checking MEMORY CARD" softlock/fatal). The depth is what
+ * psx_check_interrupts keys its deferred cooperative-switch path on (a switch
+ * is honored only at depth 0, the outermost dispatch boundary, where the
+ * outgoing thread's state is fully materialized).
+ *
+ * The overlay CI wrappers used to ALSO skip every interrupt check while depth
+ * > 0. That starves a callee that polls for an IRQ-set flag (libcard
+ * MemCardSync: LoD "Checking MEMORY CARD" hang), so IRQs are now delivered
+ * inside units; the blanket skip survives only behind
+ * PSX_OVERLAY_UNIT_IRQ_DEFER=1 for A/B bisection (overlay_unit_defer.h).
+ * Incremented only when the A/B toggle is on (below). */
 int g_call_unit_depth = 0;
 
 /* A/B toggle for the nested-unit IRQ deferral (PSX_OVERLAY_UNIT_DEFER, default
@@ -2304,6 +2310,18 @@ int overlay_unit_defer_enabled(void) {
     return cached;
 }
 
+/* Legacy blanket IRQ deferral inside call units (overlay_unit_defer.h): default
+ * OFF; PSX_OVERLAY_UNIT_IRQ_DEFER=1 restores it for A/B bisection. */
+static int overlay_unit_irq_defer_enabled(void) {
+    static int cached = -1;
+    if (cached < 0) {
+        const char *e = getenv("PSX_OVERLAY_UNIT_IRQ_DEFER");
+        cached = (e && e[0]) ? (e[0] == '1' || e[0] == 'y' || e[0] == 'Y')
+                             : OVERLAY_UNIT_IRQ_DEFER_DEFAULT;
+    }
+    return cached;
+}
+
 void overlay_loader_set_irq_suppress(int mode, uint32_t ratelimit) {
     s_suppress_irq  = mode ? 1 : 0;
     s_irq_ratelimit = ratelimit;
@@ -2383,9 +2401,12 @@ static int overlay_irq_suppressed_now(void) {
 }
 
 static void overlay_ci_wrapper(CPUState *cpu) {
-    /* Defer while inside a nested call unit — a callee must not interrupt
-     * mid-call (static-call atomicity). See g_call_unit_depth. */
-    if (g_call_unit_depth > 0) { s_ci_skip_unit++; return; }
+    /* Legacy blanket deferral inside a nested call unit (A/B knob only; default
+     * is to deliver — see g_call_unit_depth / overlay_unit_defer.h). */
+    if (overlay_unit_irq_must_defer(g_call_unit_depth, overlay_unit_irq_defer_enabled())) {
+        s_ci_skip_unit++;
+        return;
+    }
     if (overlay_irq_suppressed_now()) { s_ci_skip_supp++; return; }
     /* psx_advance_cycles() has already raised every device edge due at this
      * block. Avoid entering the full scheduler/diagnostic path when COP0 could
@@ -2426,10 +2447,13 @@ static int overlay_idle_note_is_internal_or_return(const CPUState *cpu,
 }
 
 static void overlay_ci_at_wrapper(CPUState *cpu, uint32_t resume_pc) {
-    /* Defer while inside a nested call unit (see g_call_unit_depth): suspending
-     * here would save resume_pc at the callee's block leader while the enclosing
-     * dirty caller expects an atomic unit — the resume-desync bug. */
-    if (g_call_unit_depth > 0) { s_ci_skip_unit++; return; }
+    /* Legacy blanket deferral inside a nested call unit (A/B knob only; default
+     * is to deliver — see g_call_unit_depth / overlay_unit_defer.h). A thread
+     * switch landing here is deferred by psx_check_interrupts, not by skipping. */
+    if (overlay_unit_irq_must_defer(g_call_unit_depth, overlay_unit_irq_defer_enabled())) {
+        s_ci_skip_unit++;
+        return;
+    }
     if (overlay_irq_suppressed_now()) { s_ci_skip_supp++; return; }
     if ((i_stat & i_mask) == 0) { s_ci_skip_none++; return; }
     if ((cpu->cop0[12] & ((1u << 10) | 1u)) != ((1u << 10) | 1u)) {

runtime/src/overlay_unit_defer.h

@@ -0,0 +1,41 @@
+/* overlay_unit_defer.h - IRQ policy for nested overlay call units.
+ *
+ * A callee run as a UNIT (overlay_loader_call_native: the dirty interpreter
+ * jal's into an overlay-native shard) used to defer EVERY interrupt check while
+ * g_call_unit_depth > 0 (5478d83, an early Ape memcard "partial fix": a
+ * cooperative ChangeThread landing inside a nested unit saved an inconsistent
+ * (resume_pc, sp) snapshot).
+ *
+ * That blanket deferral is a liveness hole. A callee that busy-waits on a flag
+ * only an interrupt handler can set (libcard MemCardSync polling 0x800BF178,
+ * set by the HwCARD event callback; the card protocol itself is SIO-IRQ driven
+ * per byte) never sees an IRQ, spins forever natively, and the game hangs on
+ * "Checking MEMORY CARD" (Legend of Dragoon Continue / Save). It needs an
+ * INTERPRETED caller with a NATIVE callee, so it appears or vanishes with the
+ * overlay cache contents: cold cache = both interpreted, fully warm = both
+ * native, partially warm = wedge. Bounding the deferral by age does not fix it
+ * (a one-frame bound still delivers one IRQ per frame, far below the per-byte
+ * SIO rate; ~5k guest cycles is needed) - the deferral itself is the wrong tool.
+ *
+ * Hardware delivers the IRQ immediately, and so does the interpreter at every
+ * entry. What a unit really needs is protection from a cooperative thread
+ * SWITCH landing inside it, and psx_check_interrupts already provides that:
+ * the "Fix #2" path keys on g_call_unit_depth (at_outermost) and defers the
+ * switch to the outermost dispatch boundary. So the default is: deliver.
+ * The legacy blanket deferral stays available as an A/B knob
+ * (PSX_OVERLAY_UNIT_IRQ_DEFER=1) to bisect any title that regressed with it
+ * gone. Header-only so the policy is unit-testable
+ * (runtime/tests/test_overlay_unit_defer.c). */
+#ifndef OVERLAY_UNIT_DEFER_H
+#define OVERLAY_UNIT_DEFER_H
+
+/* Default for the legacy blanket deferral: OFF (deliver IRQs inside units). */
+#define OVERLAY_UNIT_IRQ_DEFER_DEFAULT 0
+
+/* 1 = a block-leader interrupt check made at `unit_depth` must be skipped. */
+static inline int overlay_unit_irq_must_defer(int unit_depth, int legacy_blanket_defer)
+{
+    return unit_depth > 0 && legacy_blanket_defer;
+}
+
+#endif /* OVERLAY_UNIT_DEFER_H */

runtime/tests/test_overlay_unit_defer.c

@@ -0,0 +1,31 @@
+/* IRQ policy for nested overlay call units (overlay_unit_defer.h).
+ *
+ * Regression for the Legend of Dragoon "Checking MEMORY CARD" hang: libcard's
+ * MemCardSync busy-waits on a flag set by an IRQ-driven event callback. Called
+ * from an interpreted caller into a native shard it ran as an atomic call unit
+ * and the overlay CI wrappers skipped every interrupt check for the whole call,
+ * so the flag was never set. The default must be to DELIVER inside units; the
+ * blanket skip exists only as an explicit A/B knob.
+ *
+ * Run: `ctest -R overlay_unit_defer_test`. */
+#include <assert.h>
+#include <stdio.h>
+
+#include "../src/overlay_unit_defer.h"
+
+int main(void)
+{
+    /* Default policy: never defer, at any nesting depth. */
+    assert(OVERLAY_UNIT_IRQ_DEFER_DEFAULT == 0);
+    for (int depth = -1; depth <= 8; depth++)
+        assert(overlay_unit_irq_must_defer(depth, OVERLAY_UNIT_IRQ_DEFER_DEFAULT) == 0);
+
+    /* Legacy A/B knob: defers exactly while inside a unit. */
+    assert(overlay_unit_irq_must_defer(0, 1) == 0);
+    assert(overlay_unit_irq_must_defer(1, 1) == 1);
+    assert(overlay_unit_irq_must_defer(3, 1) == 1);
+    assert(overlay_unit_irq_must_defer(-1, 1) == 0);
+
+    printf("overlay_unit_defer_test: ok\n");
+    return 0;
+}

runtime/tests/test_overlay_unit_defer_wiring.py

@@ -0,0 +1,48 @@
+#!/usr/bin/env python3
+"""Wiring guard for the call-unit IRQ policy (overlay_unit_defer.h).
+
+Both overlay CI wrappers must route the call-unit deferral through
+overlay_unit_irq_must_defer(depth, overlay_unit_irq_defer_enabled()), and the
+env knob must default to OVERLAY_UNIT_IRQ_DEFER_DEFAULT. A raw
+`g_call_unit_depth > 0 -> return` in either wrapper restores the unconditional
+deferral that wedged Legend of Dragoon on "Checking MEMORY CARD" (libcard
+MemCardSync polling for an IRQ-set flag from a native unit entered by an
+interpreted caller; call_unit_depth stuck at 1, ci_skip_unit in the tens of
+millions).
+
+Usage: python runtime/tests/test_overlay_unit_defer_wiring.py   Exit 0 = PASS.
+"""
+from pathlib import Path
+import re
+import sys
+
+ROOT = Path(__file__).resolve().parents[2]
+SRC = (ROOT / "runtime" / "src" / "overlay_loader.c").read_text(encoding="utf-8")
+
+
+def body(name: str) -> str:
+    m = re.search(rf"\bstatic\s+(?:int|void)\s+{re.escape(name)}\s*\([^;]*?\)\s*\{{", SRC, re.S)
+    assert m, f"missing {name}"
+    depth, i = 1, m.end()
+    while depth:
+        c = SRC[i]
+        depth += (c == "{") - (c == "}")
+        i += 1
+    return SRC[m.end():i]
+
+
+def main() -> int:
+    for fn in ("overlay_ci_wrapper", "overlay_ci_at_wrapper"):
+        b = body(fn)
+        assert "overlay_unit_irq_must_defer(g_call_unit_depth, overlay_unit_irq_defer_enabled())" in b, \
+            f"{fn} must gate the unit deferral through the policy helper"
+        assert not re.search(r"if\s*\(\s*g_call_unit_depth\s*>\s*0", b), \
+            f"{fn} must not skip interrupt checks on g_call_unit_depth directly"
+    knob = body("overlay_unit_irq_defer_enabled")
+    assert "PSX_OVERLAY_UNIT_IRQ_DEFER" in knob and "OVERLAY_UNIT_IRQ_DEFER_DEFAULT" in knob
+    print("overlay_unit_defer_wiring: ok")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())