From 2810aefeee7cbef1ba46428a4f684603ea5a9d9e Mon Sep 17 00:00:00 2001 From: Matt Stanley <1379tech@gmail.com> Date: Mon, 19 Oct 2026 12:27:36 -0700 Subject: [PATCH] Integrate PR #642 for combined review regression validation --- runtime/CMakeLists.txt | 9 ++++ runtime/src/debug_server.c | 13 +++++ runtime/src/overlay_loader.c | 50 ++++++++++++++----- runtime/src/overlay_unit_defer.h | 41 +++++++++++++++ runtime/tests/test_overlay_unit_defer.c | 31 ++++++++++++ .../tests/test_overlay_unit_defer_wiring.py | 48 ++++++++++++++++++ 6 files changed, 179 insertions(+), 13 deletions(-) create mode 100644 runtime/src/overlay_unit_defer.h create mode 100644 runtime/tests/test_overlay_unit_defer.c create mode 100644 runtime/tests/test_overlay_unit_defer_wiring.py diff --git a/runtime/CMakeLists.txt b/runtime/CMakeLists.txt index 6c1b165c..d06a90f0 100644 --- a/runtime/CMakeLists.txt +++ b/runtime/CMakeLists.txt @@ -265,6 +265,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" @@ -1850,6 +1852,13 @@ 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) + 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) diff --git a/runtime/src/debug_server.c b/runtime/src/debug_server.c index b1b99c12..a50ea680 100644 --- a/runtime/src/debug_server.c +++ b/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, diff --git a/runtime/src/overlay_loader.c b/runtime/src/overlay_loader.c index a831bf9f..bceb61d9 100644 --- a/runtime/src/overlay_loader.c +++ b/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)) { diff --git a/runtime/src/overlay_unit_defer.h b/runtime/src/overlay_unit_defer.h new file mode 100644 index 00000000..5b286117 --- /dev/null +++ b/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 */ diff --git a/runtime/tests/test_overlay_unit_defer.c b/runtime/tests/test_overlay_unit_defer.c new file mode 100644 index 00000000..149baeb4 --- /dev/null +++ b/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 +#include + +#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; +} diff --git a/runtime/tests/test_overlay_unit_defer_wiring.py b/runtime/tests/test_overlay_unit_defer_wiring.py new file mode 100644 index 00000000..901a639e --- /dev/null +++ b/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())