mirror of
https://github.com/libretro/virtualjaguar-libretro.git
synced 2026-11-08 08:10:33 -10:00
fix(gpu): don't clobber IRQ dispatch raised in a branch delay slot
The CD streaming ISR epilogue idiom (retail CD BIOS engine and the game-uploaded engines alike) is `JUMP T,(Rret)` with `STORE Rflags,(G_FLAGS)` in the delay slot -- the store clears IMASK and acks INT_CLR1. gpu.c dispatches pending interrupts synchronously from that store (IMASKCleared -> GPUHandleIRQs), so with a FIFO interrupt already latched (BUTCHExec re-asserts every halfline while fifoDataReady; the latch survives IMASK) the dispatch ran INSIDE gpu_opcode_jump/jr's inline delay slot: it set IMASK, pushed a return address, pointed gpu_pc at the vector -- and the jump then overwrote gpu_pc with the branch target. Result: IMASK stuck set forever, the CD ISR never entered again, FIFO drains frozen, GPU foreground mailbox spin + 68K CD_read service loop starved -- the post-first-load CD transfer wedge (cd_seek_wedge / video_stall). Evidence (live GPU RAM dumps at the wedge, two independent engines): - Primal Rage (bios) 2nd transfer: G_FLAGS=$4038 (IMASK set) while gpu_pc spins in FOREGROUND $F03168-$F0316C; stale un-popped push [$F03368]=$00F03470 = its epilogue delay-slot address. - BrainDead 13 (bios) 1st game transfer (matches the user's iOS device trace): G_FLAGS=$40F9, foreground spin $F03270-$F0327A, stale push [$F03FFC]=$00F03BE2 = its epilogue delay-slot address. Fix: GPUHandleIRQs is delay-slot aware -- when the dispatch is triggered by the delay-slot instruction it pushes the BRANCH TARGET (the true interrupted-code address; the delay slot has completed) as the return address and flags the in-flight jump opcode to not overwrite gpu_pc. Transient state within one opcode; never live at a savestate boundary (not serialized); reset in GPUReset for iOS static-state hygiene. The DSP is not affected: dsp.c defers the IMASK-clear dispatch via its IMASKCleared flag to the top of the exec loop, after delayed_pc is applied. TDD: new test/test_cd_second_transfer.c (run-gated on VJ_FIFO_DISC, SKIPs in CI) asserts the liveness contract -- never 120 consecutive frames of (IMASK set + GPU running + FIFO drains frozen) while a CD transfer is in flight. RED at HEAD~ (wedges at frame ~727 with G_FLAGS=$4038), GREEN with this fix. test_cd_fifo_stream (first-load byte-exact floor) stays GREEN. make TEST_EXPORTS=1 test exit 0; audio pair verified (clipping self-test PASS, IS1 presence RMS 1175.7). Post-fix: Primal Rage (bios) completes transfer #2 (ISR signals completion by clearing BUTCH bit 0, IMASK clear, foreground advances to the same $F031F6 OP-wait state as the HLE oracle); BrainDead 13 (bios) completes the logo load AND issues+completes a second ~1 MB FMV streaming transfer (38915 drains, IMASK clear). Full diagnosis: .superpowers/sdd/task-8-second-wedge-report.md Co-Authored-By: Claude Fable 5 <[email protected]>
This commit is contained in:
Joseph MattielloandClaude Fable 5 committed
1 parent
d2cd04bfd8
commit
7c98e16d91
3 files changed
+284
-6
No files matched your search
@@ -735,7 +735,7 @@ clean:
|
||||
test/test_blitter_mmio test/test_eeprom_lifecycle \
|
||||
test/test_tom_visible_window test/test_framebuffer_integrity \
|
||||
test/test_butch_cd test/test_bios_config test/test_boot_config \
|
||||
test/test_cd_boot test/test_cd_hle_boot test/test_cd_bios_boot test/test_cd_toc_contract test/test_cd_fifo_stream test/test_cd_hle_idempotent \
|
||||
test/test_cd_boot test/test_cd_hle_boot test/test_cd_bios_boot test/test_cd_toc_contract test/test_cd_fifo_stream test/test_cd_second_transfer test/test_cd_hle_idempotent \
|
||||
test/test_audio_dac test/test_blitter \
|
||||
test/dump_pc test/heap_search \
|
||||
test/tools/test_memory_map test/tools/test_dsp_audio_diag \
|
||||
@@ -769,7 +769,7 @@ test: test/test_cheat test/test_event_queue test/test_blitter_simd test/test_dsp
|
||||
test/test_blitter_mmio test/test_eeprom_lifecycle test/test_tom_visible_window \
|
||||
test/test_framebuffer_integrity \
|
||||
test/test_butch_cd test/test_bios_config test/test_boot_config \
|
||||
test/test_cd_boot test/test_cd_hle_boot test/test_cd_bios_boot test/test_cd_toc_contract test/test_cd_fifo_stream test/test_cd_hle_idempotent \
|
||||
test/test_cd_boot test/test_cd_hle_boot test/test_cd_bios_boot test/test_cd_toc_contract test/test_cd_fifo_stream test/test_cd_second_transfer test/test_cd_hle_idempotent \
|
||||
test/test_audio_dac test/test_blitter \
|
||||
test/tools/test_memory_map
|
||||
./test/test_cheat
|
||||
@@ -839,7 +839,7 @@ test: test/test_cheat test/test_event_queue test/test_blitter_simd test/test_dsp
|
||||
@echo ""
|
||||
@echo "Note: test/test_cd_boot, test/test_cd_hle_boot, test/test_cd_bios_boot,"
|
||||
@echo "test/test_cd_toc_contract (needs VJ_TOC_DISC=<image>),"
|
||||
@echo "test/test_cd_fifo_stream (needs VJ_FIFO_DISC=<image>),"
|
||||
@echo "test/test_cd_fifo_stream and test/test_cd_second_transfer (need VJ_FIFO_DISC=<image>),"
|
||||
@echo "and test/test_blitter (register-readback) are built but not run from"
|
||||
@echo "'make test'. The CD sweeps walk every disc in test/roms/private/; the"
|
||||
@echo "blitter readback tests probe register read paths that the emulator"
|
||||
@@ -994,6 +994,10 @@ test/test_cd_fifo_stream: test/test_cd_fifo_stream.c test/test_framework.h
|
||||
$(CC) -O2 -Wall -Wno-unused-function -Wno-unused-variable -std=c99 $(INCFLAGS) \
|
||||
-o $@ test/test_cd_fifo_stream.c -ldl
|
||||
|
||||
test/test_cd_second_transfer: test/test_cd_second_transfer.c test/test_framework.h
|
||||
$(CC) -O2 -Wall -Wno-unused-function -Wno-unused-variable -std=c99 $(INCFLAGS) \
|
||||
-o $@ test/test_cd_second_transfer.c -ldl
|
||||
|
||||
test/test_audio_dac: test/test_audio_dac.c test/test_framework.h
|
||||
$(CC) -O2 -Wall -Wno-unused-function -Wno-unused-variable -std=c99 $(INCFLAGS) \
|
||||
-o $@ test/test_audio_dac.c -ldl -lm
|
||||
|
||||
+52
-3
@@ -206,6 +206,23 @@ static uint32_t gpu_instruction;
|
||||
static uint32_t gpu_opcode_first_parameter;
|
||||
static uint32_t gpu_opcode_second_parameter;
|
||||
|
||||
/* Branch delay-slot IRQ hazard state. gpu_opcode_jump/jr execute their
|
||||
* delay-slot instruction inline and then apply the branch target to gpu_pc.
|
||||
* If the delay-slot instruction's side effects dispatch a GPU interrupt
|
||||
* synchronously -- the canonical case is the CD BIOS / game streaming-ISR
|
||||
* epilogue `JUMP T,(Rret)` with `STORE Rflags,(G_FLAGS)` in the delay slot,
|
||||
* where the store clears IMASK while a FIFO interrupt is already latched --
|
||||
* then GPUHandleIRQs must push the BRANCH TARGET as the return address
|
||||
* (the delay slot has completed; the next instruction is the target), and
|
||||
* the jump must NOT overwrite gpu_pc afterwards, or the vector jump is
|
||||
* clobbered: IMASK stays set forever, no interrupt is ever delivered again,
|
||||
* and CD transfers wedge (video_stall / cd_seek_wedge with G_FLAGS IMASK
|
||||
* stuck). Transient within a single opcode's execution -- never live at a
|
||||
* savestate boundary, so deliberately not serialized. */
|
||||
static uint32_t gpu_ds_branch_target;
|
||||
static uint8_t gpu_in_delay_slot;
|
||||
static uint8_t gpu_ds_irq_dispatched;
|
||||
|
||||
#define GPU_RUNNING (gpu_control & 0x01)
|
||||
|
||||
#define RM gpu_reg[gpu_opcode_first_parameter]
|
||||
@@ -620,7 +637,19 @@ void GPUHandleIRQs(void)
|
||||
// move pc,r30 ; address of interrupted code
|
||||
// store r30,(r31) ; store return address
|
||||
gpu_reg[31] -= 4;
|
||||
GPUWriteLong(gpu_reg[31], gpu_pc - 2, GPU);
|
||||
if (gpu_in_delay_slot)
|
||||
{
|
||||
/* Dispatch triggered by the delay-slot instruction itself (e.g. an
|
||||
* IMASK-clearing G_FLAGS store in an ISR epilogue's delay slot).
|
||||
* The delay slot has already executed, so the interrupted-code
|
||||
* address is the pending BRANCH TARGET, not gpu_pc (which still
|
||||
* points just past the delay slot). Flag the in-flight jump opcode
|
||||
* so it does not overwrite gpu_pc (the vector) afterwards. */
|
||||
GPUWriteLong(gpu_reg[31], gpu_ds_branch_target - 2, GPU);
|
||||
gpu_ds_irq_dispatched = 1;
|
||||
}
|
||||
else
|
||||
GPUWriteLong(gpu_reg[31], gpu_pc - 2, GPU);
|
||||
|
||||
// movei #service_address,r30 ; pointer to ISR entry
|
||||
// jump (r30) ; jump to ISR
|
||||
@@ -681,6 +710,11 @@ void GPUReset(void)
|
||||
// GPU internal register
|
||||
gpu_acc = 0x00000000;
|
||||
|
||||
// Delay-slot IRQ hazard state (transient; reset for iOS static-state hygiene)
|
||||
gpu_ds_branch_target = 0x00000000;
|
||||
gpu_in_delay_slot = 0;
|
||||
gpu_ds_irq_dispatched = 0;
|
||||
|
||||
gpu_reg = gpu_reg_bank_0;
|
||||
gpu_alternate_reg = gpu_reg_bank_1;
|
||||
|
||||
@@ -1015,8 +1049,16 @@ INLINE static void gpu_opcode_jump(void)
|
||||
gpu_opcode_first_parameter = (ds_opcode >> 5) & 0x1F;
|
||||
gpu_opcode_second_parameter = ds_opcode & 0x1F;
|
||||
gpu_pc += 2;
|
||||
gpu_in_delay_slot = 1;
|
||||
gpu_ds_branch_target = delayed_pc;
|
||||
gpu_ds_irq_dispatched = 0;
|
||||
executeOpcode(ds_index);
|
||||
gpu_pc = delayed_pc;
|
||||
gpu_in_delay_slot = 0;
|
||||
/* If the delay-slot instruction dispatched an interrupt, gpu_pc is
|
||||
* the ISR vector and the branch target is on the ISR stack as the
|
||||
* return address -- do not clobber the vector. */
|
||||
if (!gpu_ds_irq_dispatched)
|
||||
gpu_pc = delayed_pc;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1045,8 +1087,15 @@ INLINE static void gpu_opcode_jr(void)
|
||||
gpu_opcode_first_parameter = (ds_opcode >> 5) & 0x1F;
|
||||
gpu_opcode_second_parameter = ds_opcode & 0x1F;
|
||||
gpu_pc += 2;
|
||||
gpu_in_delay_slot = 1;
|
||||
gpu_ds_branch_target = (uint32_t)delayed_pc;
|
||||
gpu_ds_irq_dispatched = 0;
|
||||
executeOpcode(ds_index);
|
||||
gpu_pc = delayed_pc;
|
||||
gpu_in_delay_slot = 0;
|
||||
/* See gpu_opcode_jump: don't clobber a vector jump dispatched by
|
||||
* the delay-slot instruction. */
|
||||
if (!gpu_ds_irq_dispatched)
|
||||
gpu_pc = delayed_pc;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,225 @@
|
||||
/*
|
||||
* test_cd_second_transfer.c -- CD second-transfer liveness contract (Task 8).
|
||||
*
|
||||
* test_cd_fifo_stream proves the FIRST boot-stub load delivers byte-exact
|
||||
* payload. This test covers the next failure class: the SECOND (and every
|
||||
* later) transfer wedging because a GPU interrupt dispatch was clobbered by
|
||||
* a branch delay slot.
|
||||
*
|
||||
* Mechanism (diagnosed on Primal Rage bios + BrainDead 13 bios, plus a
|
||||
* user-captured iOS device trace): the CD streaming ISR epilogue idiom is
|
||||
* `JUMP T,(Rret)` with `STORE Rflags,(G_FLAGS)` in the DELAY SLOT -- the
|
||||
* store clears IMASK and acks INT_CLR1. gpu.c dispatches pending IRQs
|
||||
* synchronously from that store (IMASKCleared -> GPUHandleIRQs); the
|
||||
* dispatch ran INSIDE gpu_opcode_jump's inline delay slot, and the jump
|
||||
* then overwrote gpu_pc with the branch target -- clobbering the vector.
|
||||
* IMASK stayed set forever, the CD ISR never ran again, FIFO drains froze,
|
||||
* and both the GPU foreground mailbox spin and the 68K CD_read service
|
||||
* loop starved (video_stall / cd_seek_wedge).
|
||||
*
|
||||
* Contract asserted here: while a CD transfer is in flight, the core never
|
||||
* enters the "IMASK stuck" state -- G_FLAGS.IMASK observed set on many
|
||||
* consecutive frame boundaries while the GPU is running and FIFO drain
|
||||
* progress is frozen. A real ISR lasts microseconds; IMASK visible at
|
||||
* WEDGE_WINDOW successive frame boundaries with zero drain progress means
|
||||
* interrupt delivery is dead.
|
||||
*
|
||||
* SKIPs cleanly when no disc / CD BIOS is available (mirrors the other
|
||||
* test_cd_* harnesses).
|
||||
*
|
||||
* Build:
|
||||
* make -j8 TEST_EXPORTS=1 && cc -O2 -Wall -std=c99 \
|
||||
* -I./libretro-common/include \
|
||||
* -o test/test_cd_second_transfer test/test_cd_second_transfer.c -ldl
|
||||
* Run (local, needs disc + CD BIOS under test/roms/private):
|
||||
* VJ_FIFO_DISC="test/roms/private/Primal Rage (USA)/Primal Rage (USA).cue" \
|
||||
* test/test_cd_second_transfer
|
||||
*/
|
||||
|
||||
#include "test_framework.h"
|
||||
#include "../libretro-common/include/libretro.h"
|
||||
|
||||
#include <dlfcn.h>
|
||||
|
||||
static struct vj_core C;
|
||||
static const char *g_system_dir = "test/roms/private";
|
||||
|
||||
/* ---- libretro environment: force real-BIOS CD boot mode ---------------- */
|
||||
static bool st_environment(unsigned cmd, void *data)
|
||||
{
|
||||
switch (cmd & 0xFF) {
|
||||
case RETRO_ENVIRONMENT_GET_LOG_INTERFACE:
|
||||
return false;
|
||||
case RETRO_ENVIRONMENT_SET_PIXEL_FORMAT:
|
||||
return true;
|
||||
case RETRO_ENVIRONMENT_GET_SYSTEM_DIRECTORY: {
|
||||
const char *root = getenv("VJ_TEST_CD_ROOT");
|
||||
*(const char **)data = (root && root[0]) ? root : g_system_dir;
|
||||
return true;
|
||||
}
|
||||
case RETRO_ENVIRONMENT_GET_SAVE_DIRECTORY:
|
||||
case RETRO_ENVIRONMENT_GET_CORE_ASSETS_DIRECTORY:
|
||||
*(const char **)data = ".";
|
||||
return true;
|
||||
case RETRO_ENVIRONMENT_SET_VARIABLES:
|
||||
case RETRO_ENVIRONMENT_SET_CORE_OPTIONS_V2:
|
||||
return true;
|
||||
case RETRO_ENVIRONMENT_GET_VARIABLE: {
|
||||
struct retro_variable *var = (struct retro_variable *)data;
|
||||
if (!var || !var->key) return false;
|
||||
if (strcmp(var->key, "virtualjaguar_bios") == 0) { var->value = "enabled"; return true; }
|
||||
if (strcmp(var->key, "virtualjaguar_usefastblitter") == 0) { var->value = "enabled"; return true; }
|
||||
if (strcmp(var->key, "virtualjaguar_cd_bios_type") == 0) { var->value = "retail"; return true; }
|
||||
if (strcmp(var->key, "virtualjaguar_cd_boot_mode") == 0) { var->value = "bios"; return true; }
|
||||
var->value = NULL;
|
||||
return false;
|
||||
}
|
||||
case RETRO_ENVIRONMENT_GET_VARIABLE_UPDATE:
|
||||
*(bool *)data = false;
|
||||
return true;
|
||||
default:
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
static void st_video(const void *d, unsigned w, unsigned h, size_t p)
|
||||
{ (void)d; (void)w; (void)h; (void)p; }
|
||||
static void st_audio(int16_t l, int16_t r) { (void)l; (void)r; }
|
||||
static size_t st_audio_batch(const int16_t *d, size_t f) { (void)d; return f; }
|
||||
static void st_input_poll(void) {}
|
||||
static int16_t st_input_state(unsigned p, unsigned d, unsigned i, unsigned id)
|
||||
{ (void)p; (void)d; (void)i; (void)id; return 0; }
|
||||
|
||||
/* ---- exported internals (TEST_EXPORTS builds) --------------------------- */
|
||||
static void (*p_CDROMDiagGetSeekWedgeState)(uint32_t *, uint32_t *, uint32_t *);
|
||||
static uint32_t (*p_GPUReadLong)(uint32_t, uint32_t);
|
||||
static bool (*p_GPUIsRunning)(void);
|
||||
|
||||
#define G_FLAGS_ADDR 0x00F02100u
|
||||
#define G_FLAGS_IMASK 0x00000008u
|
||||
|
||||
/* Frames of continuous (IMASK set + GPU running + drains frozen) that count
|
||||
* as a dead interrupt path. The real wedge holds this state forever; a
|
||||
* healthy ISR clears IMASK within microseconds, so IMASK is essentially
|
||||
* never visible at a frame boundary twice in a row -- 120 frames (~2 s) is
|
||||
* decisively past any legitimate pause between transfers. */
|
||||
#define WEDGE_WINDOW 120u
|
||||
|
||||
TEST(second_transfer_liveness)
|
||||
{
|
||||
const char *disc = getenv("VJ_FIFO_DISC");
|
||||
struct retro_game_info info;
|
||||
bool (*p_retro_load_game)(const struct retro_game_info *);
|
||||
void (*p_retro_run)(void);
|
||||
void (*p_retro_unload_game)(void);
|
||||
uint32_t f, frames;
|
||||
uint32_t starts, dones, drains;
|
||||
uint32_t lastDrains, stuckFrames, worstStuck;
|
||||
uint32_t stuckStarts, stuckDrains, stuckFlags;
|
||||
|
||||
if (!disc || !disc[0]) {
|
||||
SKIP_TEST(second_transfer_liveness, "set VJ_FIFO_DISC to a CD image path");
|
||||
return;
|
||||
}
|
||||
|
||||
p_retro_load_game = dlsym(C.handle, "retro_load_game");
|
||||
p_retro_run = dlsym(C.handle, "retro_run");
|
||||
p_retro_unload_game = dlsym(C.handle, "retro_unload_game");
|
||||
p_CDROMDiagGetSeekWedgeState = dlsym(C.handle, "CDROMDiagGetSeekWedgeState");
|
||||
p_GPUReadLong = dlsym(C.handle, "GPUReadLong");
|
||||
p_GPUIsRunning = dlsym(C.handle, "GPUIsRunning");
|
||||
if (!p_retro_load_game || !p_retro_run
|
||||
|| !p_CDROMDiagGetSeekWedgeState || !p_GPUReadLong || !p_GPUIsRunning) {
|
||||
SKIP_TEST(second_transfer_liveness,
|
||||
"core missing required exports (build with TEST_EXPORTS=1)");
|
||||
return;
|
||||
}
|
||||
|
||||
memset(&info, 0, sizeof(info));
|
||||
info.path = disc;
|
||||
if (!p_retro_load_game(&info)) {
|
||||
SKIP_TEST(second_transfer_liveness,
|
||||
"retro_load_game failed (CD BIOS missing or disc parse failed)");
|
||||
return;
|
||||
}
|
||||
|
||||
frames = 1500;
|
||||
{
|
||||
const char *fe = getenv("VJ_FIFO_FRAMES");
|
||||
if (fe && fe[0]) frames = (uint32_t)atoi(fe);
|
||||
}
|
||||
|
||||
starts = dones = drains = 0;
|
||||
lastDrains = 0;
|
||||
stuckFrames = 0;
|
||||
worstStuck = 0;
|
||||
stuckStarts = stuckDrains = stuckFlags = 0;
|
||||
|
||||
for (f = 0; f < frames; f++) {
|
||||
uint32_t flags;
|
||||
p_retro_run();
|
||||
p_CDROMDiagGetSeekWedgeState(&starts, &dones, &drains);
|
||||
flags = p_GPUReadLong(G_FLAGS_ADDR, 0);
|
||||
|
||||
if (starts > 0 && (flags & G_FLAGS_IMASK)
|
||||
&& p_GPUIsRunning() && drains == lastDrains) {
|
||||
stuckFrames++;
|
||||
if (stuckFrames > worstStuck) {
|
||||
worstStuck = stuckFrames;
|
||||
stuckStarts = starts;
|
||||
stuckDrains = drains;
|
||||
stuckFlags = flags;
|
||||
}
|
||||
} else {
|
||||
stuckFrames = 0;
|
||||
}
|
||||
lastDrains = drains;
|
||||
|
||||
if (worstStuck >= WEDGE_WINDOW)
|
||||
break; /* decisively wedged -- no need to run out the clock */
|
||||
}
|
||||
|
||||
fprintf(stderr,
|
||||
" frames=%u seek_starts=%u seek_dones=%u fifo_drains=%u "
|
||||
"worst IMASK-stuck window=%u frames\n",
|
||||
f, starts, dones, drains, worstStuck);
|
||||
|
||||
if (p_retro_unload_game)
|
||||
p_retro_unload_game();
|
||||
|
||||
if (worstStuck >= WEDGE_WINDOW)
|
||||
FAIL("GPU interrupt delivery died mid-transfer: IMASK stuck set for "
|
||||
"%u consecutive frames while the GPU ran and FIFO drains were "
|
||||
"frozen (seek_starts=%u drains=%u G_FLAGS=$%08X) -- the CD ISR "
|
||||
"dispatch was clobbered (delay-slot IRQ hazard)",
|
||||
worstStuck, stuckStarts, stuckDrains, stuckFlags);
|
||||
}
|
||||
|
||||
int main(int argc, char *argv[])
|
||||
{
|
||||
void (*p_retro_deinit)(void);
|
||||
(void)argc; (void)argv;
|
||||
|
||||
TEST_INIT("CD second-transfer liveness contract");
|
||||
|
||||
if (!vj_core_load(&C)) {
|
||||
fprintf(stderr, "FATAL: failed to load core\n");
|
||||
return 1;
|
||||
}
|
||||
|
||||
C.retro_set_environment(st_environment);
|
||||
C.retro_set_video_refresh(st_video);
|
||||
C.retro_set_audio_sample(st_audio);
|
||||
C.retro_set_audio_sample_batch(st_audio_batch);
|
||||
C.retro_set_input_poll(st_input_poll);
|
||||
C.retro_set_input_state(st_input_state);
|
||||
C.retro_init();
|
||||
|
||||
RUN_TEST(second_transfer_liveness);
|
||||
|
||||
p_retro_deinit = dlsym(C.handle, "retro_deinit");
|
||||
if (p_retro_deinit) p_retro_deinit();
|
||||
if (C.handle) dlclose(C.handle);
|
||||
|
||||
return TEST_REPORT();
|
||||
}
|
||||
Reference in new issue
Block a user