From b1fd460350e8bd06ddc8d82bddba00a5343fcdd1 Mon Sep 17 00:00:00 2001 From: Alexandros Mandravillis Date: Sat, 17 Oct 2026 18:01:31 +0300 Subject: [PATCH] interrupts: retain handler k0 on qualified same-thread RFE --- docs/internal/upstream/same-thread-rfe-k0.md | 29 +++++ runtime/CMakeLists.txt | 12 ++ runtime/src/interrupts.c | 37 +++++-- runtime/tests/test_same_thread_rfe_k0.c | 110 +++++++++++++++++++ 4 files changed, 177 insertions(+), 11 deletions(-) create mode 100644 docs/internal/upstream/same-thread-rfe-k0.md create mode 100644 runtime/tests/test_same_thread_rfe_k0.c diff --git a/docs/internal/upstream/same-thread-rfe-k0.md b/docs/internal/upstream/same-thread-rfe-k0.md new file mode 100644 index 00000000..53980c4c --- /dev/null +++ b/docs/internal/upstream/same-thread-rfe-k0.md @@ -0,0 +1,29 @@ +# Primary same-thread RFE register restoration + +This is a new authored correction to the +[primary register restore at upstream 7cddab8237e1](https://github.com/RetroPortingToolKit/psxrecomp/blob/7cddab8237e1f73b7067b8401af602d90a96b80d/runtime/src/interrupts.c), +by Alexandros Mandravillis (Alexbeav). A real handler's final jump register is +architectural state; restoring the interrupted snapshot must not overwrite that +actual result on the qualified same-thread RFE return. + +The added local captures the original delivery EPC before handler dispatch. +The primary copy omits only GPR26 when mode 1, real RFE, outermost delivery, +stable nonzero TCB and both original-EPC comparisons qualify. It does not +manufacture a register value or alter the return PC. Other restore modes, +legacy/syscall policy, other GPRs, HI/LO and the secondary deferred copy retain +their existing behavior. RestoreState is not separately disqualified: a redirect +that ultimately returns to the original EPC with the same TCB can qualify. + +The regression uses the real current interrupts.c and traps.c, the existing +upstream inert link fixture, and a new synthetic handler with arbitrary register +values. It contains no ROM-derived instructions, retail addresses/captures, generated +BIOS/game code, reference-emulator body or passive observer. Source-specific +scheduler/redirect policies, drive configuration and recording infrastructure +are deliberately excluded. + +The current base fails 64 qualifying K0 checks. The fixed source is checked in +modes 0 through 3, with real-RFE/syscall/legacy reasons, null/stable/changed TCB, +KSEG aliases, changed targets/global EPC, actual sentinel fallback and arbitrary +live K0 values. The existing restore-edge IRQ regression is also run. +Nested delivery and RestoreState execution are not exercised by these fixtures. +No new retail replay or whole-title accuracy claim is made. diff --git a/runtime/CMakeLists.txt b/runtime/CMakeLists.txt index 76f588f6..7215dddb 100644 --- a/runtime/CMakeLists.txt +++ b/runtime/CMakeLists.txt @@ -1108,6 +1108,18 @@ if(BUILD_TESTING) # the real scheduler (traps.c) and IRQ delivery (interrupts.c). Compiled # game, BIOS ROM, CPS continuation and interpreted resumes; no pending # IRQ; I_MASK/SR gating; same-edge dedup; null-PC recovery. + add_executable(same_thread_rfe_k0_test + tests/test_same_thread_rfe_k0.c + tests/restore_edge_irq_stubs.c + src/traps.c + src/interrupts.c) + target_include_directories(same_thread_rfe_k0_test PRIVATE include src) + foreach(_rfe_mode RANGE 0 3) + add_test(NAME same_thread_rfe_k0_mode${_rfe_mode} + COMMAND same_thread_rfe_k0_test ${_rfe_mode}) + set_tests_properties(same_thread_rfe_k0_mode${_rfe_mode} PROPERTIES + ENVIRONMENT "PSX_SAME_THREAD_RESTORE=${_rfe_mode}" TIMEOUT 30) + endforeach() add_executable(restore_edge_irq_replay_test tests/test_restore_edge_irq_replay.c tests/restore_edge_irq_stubs.c diff --git a/runtime/src/interrupts.c b/runtime/src/interrupts.c index 9e2b00e4..275c9a02 100644 --- a/runtime/src/interrupts.c +++ b/runtime/src/interrupts.c @@ -1686,12 +1686,15 @@ irq_deliver_eval: } } + /* Keep this delivery's EPC stable across nested handler dispatches. */ + const uint32_t delivered_epc = g_exception_real_epc; + /* Save the interrupted code's full register state. * - * On real hardware, the exception handler saves all GPRs to the - * TCB save area at entry, and ReturnFromException restores them - * before jumping back to EPC. The interrupted code always gets - * its exact pre-exception register values back. + * The BIOS handler saves the interrupted context to the TCB and restores + * it before returning to EPC. Kernel scratch registers may instead retain + * the handler's architectural result: the qualified real-RFE path below + * keeps its post-handler k0. Other host-restored values use this snapshot. * * In our model, the recompiled handler runs as a C function and * its `return;` goes back here — not to EPC. The handler's @@ -1699,9 +1702,9 @@ irq_deliver_eval: * kernel jmpbuf (intended for longjmp to WaitEvent's caller), * which corrupts the interrupted code's registers. * - * We save all GPRs/HI/LO before the handler and restore them - * after, so the interrupted code resumes with its original state - * — matching real hardware behaviour. */ + * Save all GPRs/HI/LO before the handler for the host restore policy + * below. Its qualified RFE path preserves the actual handler k0 result + * rather than overwriting it with the interrupted snapshot. */ uint32_t saved_gpr[32]; uint32_t saved_hi, saved_lo; for (int i = 0; i < 32; i++) saved_gpr[i] = cpu->gpr[i]; @@ -1907,9 +1910,9 @@ irq_deliver_eval: * interrupted — either the LEGACY sentinel exit, OR a real-EPC RFE that * resumes at exactly the PC we installed as EPC (no ChangeThread happened). * - * Real hardware saves and restores ALL GPRs across every exception, so a - * transparent same-thread interrupt must give the interrupted code its exact - * pre-exception registers back. The original Fix B skipped this for all + * The host restore preserves the interrupted context across a transparent + * same-thread interrupt, except for architectural post-handler k0 on the + * qualified real-RFE path below. The original Fix B skipped this for all * real-EPC resumes, trusting the recompiled BIOS handler's TCB restore — but * for a same-thread compiled resume that restore is incomplete, so a live * value held in a register across a block-leader IRQ check gets clobbered by @@ -2040,8 +2043,20 @@ irq_deliver_eval: } } } + /* A real same-thread RFE leaves the handler's jump register live. + * Preserve its actual result only for the primary mode-1 return; legacy, + * syscall, nested, changed-target/global-EPC and other restore modes keep + * the old copy. A RestoreState redirect that returns to the original EPC + * with the same TCB can still qualify. */ + const int keep_handler_k0 = do_restore && s_str_mode == 1 && + same_thread_resume && g_exc_escape_reason == PSX_EXC_ESCAPE_RFE_RETURN && + prev_in_exception == 0 && entry_tcb != 0u && entry_tcb == exit_tcb && + delivered_epc != 0u && delivered_epc != (uint32_t)PSX_EXC_SENTINEL_PC && + same_guest_pc(cpu->pc, delivered_epc) && + same_guest_pc(g_exception_real_epc, delivered_epc); if (do_restore) { - for (int i = 0; i < 32; i++) cpu->gpr[i] = saved_gpr[i]; + for (int i = 0; i < 32; i++) + if (i != 26 || !keep_handler_k0) cpu->gpr[i] = saved_gpr[i]; cpu->hi = saved_hi; cpu->lo = saved_lo; } diff --git a/runtime/tests/test_same_thread_rfe_k0.c b/runtime/tests/test_same_thread_rfe_k0.c new file mode 100644 index 00000000..ea5c676f --- /dev/null +++ b/runtime/tests/test_same_thread_rfe_k0.c @@ -0,0 +1,110 @@ +/* Synthetic handler over the real IRQ/scheduler implementation. + * No ROM instructions, retail addresses, captures or reference-core bodies. */ +#include "cpu_state.h" +#include "interrupts.h" +#include "psx_scheduler.h" +#include +#include +#include +#include +extern uint8_t g_test_ram[0x200000]; +extern uint64_t psx_cycle_count; +extern uint32_t i_stat, i_mask; +extern uint32_t psx_read_word(uint32_t); +extern void psx_host_write_word(uint32_t, uint32_t); +#define HANDLER 0x80001000u +#define PCB 0x80000200u +#define TCB 0x80000300u +#define RESUME 0x80010000u +static int reason, alias_pc, scenario, deliveries, failures; +static uint32_t live_k0; +void psx_dispatch(CPUState *cpu, uint32_t pc) { + if (pc != HANDLER) { fprintf(stderr, "unexpected dispatch %08X\n", pc); exit(2); } + ++deliveries; + i_stat = 0; + for (unsigned i = 1; i < 32; ++i) cpu->gpr[i] = 0xD0000000u + i; + cpu->gpr[26] = live_k0; + cpu->hi = 0x11111111u; + cpu->lo = 0x22222222u; + uint32_t sr = cpu->cop0[12]; + cpu->cop0[12] = (sr & ~0x0Fu) | ((sr >> 2) & 0x0Fu); + cpu->pc = cpu->cop0[14] ^ (alias_pc ? 0x20000000u : 0u); + if (scenario == 1) psx_host_write_word(PCB, TCB + 0x100u); + if (scenario == 2) cpu->pc = RESUME + 4u; + if (scenario == 3) { + g_exception_real_epc = RESUME + 4u; + cpu->pc = RESUME + 4u; + } + if (scenario == 5) g_exception_real_epc = RESUME + 4u; + g_exc_escape_reason = reason; + psx_exception_longjmp(); +} +static void check(const char *field, uint32_t want, uint32_t got) { + if (want != got) { + fprintf(stderr, "%s: expected %08X, got %08X\n", field, want, got); + ++failures; + } +} +static void run_case(int mode, int selected_reason, int tcb, int alias, unsigned value) { + CPUState cpu; + uint32_t saved[32]; + memset(&cpu, 0, sizeof(cpu)); + memset(g_test_ram, 0, sizeof(g_test_ram)); + cpu.read_word = psx_read_word; + cpu.write_word = psx_host_write_word; + psx_host_write_word(0x80u, 0x3C1A8000u); + psx_host_write_word(0x84u, 0x275A1000u); + psx_host_write_word(0x108u, tcb ? PCB : 0u); + psx_host_write_word(PCB, TCB); + for (unsigned i = 0; i < 32; ++i) + cpu.gpr[i] = saved[i] = i ? 0x50000000u + i : 0u; + cpu.hi = 0x33333333u; + cpu.lo = 0x44444444u; + cpu.cop0[12] = 0x401u; + cpu.pc = RESUME; + reason = selected_reason; alias_pc = alias; + live_k0 = 0xE0000000u + value * 7919u; + psx_cycle_count = 1000; + i_stat = i_mask = 1; + interrupts_resync_after_restore(); + psx_scheduler_top_level_resume_clear(); + if (scenario == 4) cpu.pc = 0; + psx_check_interrupts_at(&cpu, scenario == 4 ? 0 : RESUME); + const int pc_matches = scenario != 2 && scenario != 5; + const int stable_tcb = tcb && scenario != 1; + int restored = reason == PSX_EXC_ESCAPE_LEGACY_SENTINEL || + (mode == 1 && pc_matches) || + (mode == 2 && stable_tcb && pc_matches) || + (mode == 3 && stable_tcb && + (reason == PSX_EXC_ESCAPE_RFE_RETURN || + reason == PSX_EXC_ESCAPE_SYSCALL_RETURN)) || scenario == 4; + int keep = mode == 1 && stable_tcb && scenario == 0 && + reason == PSX_EXC_ESCAPE_RFE_RETURN; + for (unsigned i = 1; i < 32; ++i) + check("GPR", keep && i == 26 ? live_k0 : + (restored ? saved[i] : (i == 26 ? live_k0 : 0xD0000000u + i)), + cpu.gpr[i]); + check("zero register", 0, cpu.gpr[0]); + check("HI", restored ? 0x33333333u : 0x11111111u, cpu.hi); + check("LO", restored ? 0x44444444u : 0x22222222u, cpu.lo); + check("raw PC", scenario == 4 ? 0u : + (scenario == 2 || scenario == 3 ? RESUME + 4u : + RESUME ^ (alias ? 0x20000000u : 0u)), cpu.pc); +} +int main(int argc, char **argv) { + if (argc != 2) return 2; + int mode = atoi(argv[1]); + for (unsigned value = 0; value < 32; ++value) + for (int alias = 0; alias < 2; ++alias) + for (int tcb = 0; tcb < 2; ++tcb) { + run_case(mode, PSX_EXC_ESCAPE_RFE_RETURN, tcb, alias, value); + run_case(mode, PSX_EXC_ESCAPE_SYSCALL_RETURN, tcb, alias, value); + run_case(mode, PSX_EXC_ESCAPE_LEGACY_SENTINEL, tcb, alias, value); + } + for (scenario = 1; scenario <= 5; ++scenario) + run_case(mode, PSX_EXC_ESCAPE_RFE_RETURN, 1, 0, 123); + check("deliveries", 389, (uint32_t)deliveries); + printf("same-thread RFE mode %d: %d deliveries, %d failures\n", + mode, deliveries, failures); + return failures ? 1 : 0; +}