diff --git a/Makefile b/Makefile index 3e623be8..8a5cf963 100644 --- a/Makefile +++ b/Makefile @@ -1787,6 +1787,18 @@ test: test/test_dram_timing test/test_cheat test/test_event_queue test/test_jlin else \ bash scripts/test-skip.sh record "Hover Strike CD (HLE savestate determinism, #787)" "no disc matching 'Hover Strike - Unconquered Lands (USA).cue' in the private corpus"; \ fi + @# Issue #804: the same disc on the REAL-BIOS path. The boot-stub-injected + @# flag (jagcd_bios.c) and the BUTCH FIFO refill accumulator (cdrom.c) lived + @# outside the blob, so a rollback across the stub injection never injected + @# it and the replay diverged ~40 frames in (failed video_replay_identical + @# and state_reconverges pre-fix). The explicit boot-mode option is what + @# selects the BIOS path -- `--bios` only toggles the cart boot ROM. + @rom=$$(bash scripts/find-rom.sh 'Hover Strike - Unconquered Lands (USA).cue'); \ + if [ -n "$$rom" ]; then \ + ./test/tools/test_runahead_determinism ./$(TARGET) "$$rom" --option virtualjaguar_cd_boot_mode=bios --warmup 400 --frames 120 --quiet; \ + else \ + bash scripts/test-skip.sh record "Hover Strike CD (real-BIOS savestate determinism, #804)" "no disc matching 'Hover Strike - Unconquered Lands (USA).cue' in the private corpus"; \ + fi ./test/test_butch_cd ./test/test_cd_hle_idempotent ./test/test_cd_pregap diff --git a/docs/savestate-compat.md b/docs/savestate-compat.md index 84ae15a2..73373c60 100644 --- a/docs/savestate-compat.md +++ b/docs/savestate-compat.md @@ -9,7 +9,7 @@ Defined in `src/core/state.h`: | Constant | Value | Meaning | |---|---|---| | `STATE_MAGIC` | `0x564A5353` (`"VJSS"`) | Header magic | -| `STATE_VERSION` | `15` | Version this build **writes** (v3.7.0) | +| `STATE_VERSION` | `16` | Version this build **writes** (v3.7.1 cycle; v3.7.0 wrote 15) | | `STATE_MIN_VERSION` | `1` | Oldest version this build will **load** | `retro_unserialize()` refuses anything outside `STATE_MIN_VERSION … STATE_VERSION` @@ -51,6 +51,37 @@ not match the disc currently mounted in CD mode (`retro_unserialize()`, A state saved by an older build while an HLE CD read was in flight cannot resume that read: the transfer was never saved. +## v16: real-BIOS CD chunks (#804, v3.7.1 cycle) + +Two trailing chunks, appended strictly after the HLE CD streaming chunk, each +behind its own magic word. `STATE_SIZE` is unchanged (`0x280000`); the chunks +are 5 and 9 bytes. + +| Chunk | Magic | Fields | Owner | +|---|---|---|---| +| Real-BIOS boot | `"CDB1"` | `cdBootStubInjected` | `src/cd/jagcd_bios.c` | +| CD drive timing | `"CDX1"` | `fifoRefillAccum`, `cdPrevShouldIRQ` | `src/cd/cdrom.c` | + +Both lived outside the blob. A rollback (run-ahead, netplay) to a state taken +before the boot stub was injected kept the flag set by the replay that had +already injected it, so the replay never injected and the game never started; +the refill accumulator picks the next FIFO interval (2 or 3 ticks), so a stale +value shifted every later refill by one tick. Measured on Hover Strike at +warmup 400, real-BIOS path: video diverged 44 frames after the rollback before +the fix, all four `test_runahead_determinism` checks pass after. + +Loading: `retro_unserialize()` reads both chunks only for `version >= 16` +(`STATE_VERSION_BIOS_CD_BOOT`). A v15 or older state loads normally and the +chunks' values are left as the live session holds them, exactly as v3.7.0 did. +That is deliberate: defaulting `cdBootStubInjected` to false would re-arm the +`$005E40` GPU-magic stomp and the `$050176` boot-stub re-injection over a +mid-game v15 state's RAM, a regression for every existing real-BIOS state. (A +v15 state loaded into a fresh core still starts with the flag clear, as before.) +A damaged accumulator (outside 0..99) loads as 0. Verified with a genuine v15 +file written by a v3.7.0 build (`274fa74`), Hover Strike real-BIOS. Regression: +`test_state_compat` (`v16_*`, `v15_*` rows) and the ROM-gated real-BIOS Hover +Strike run in `make test`. + ## v8: Jaguar GameDrive chunk v8 (one shared bump per release policy — all in-flight changes since v3.1.0 diff --git a/libretro.c b/libretro.c index 127d8e6d..61d7fd93 100644 --- a/libretro.c +++ b/libretro.c @@ -4661,6 +4661,23 @@ bool retro_serialize(void *data, size_t size) return false; buf += JaguarCDHLEStateSave(buf); + /* v16 (#804): real-BIOS CD path. Two trailing chunks, each behind its + * own magic word: the boot-stub-injected flag (jagcd_bios.c) and the + * CD drive-timing statics (cdrom.c: FIFO refill accumulator, IRQ edge + * detector). Outside the blob, run-ahead replayed a real-BIOS load + * against a flag/phase the replay had already advanced (Hover Strike, + * warmup 400: video diverged 44 frames after the rollback). + * + * STRICTLY LAST, after the HLE chunk: appending keeps every v15 and + * older blob loadable. Room is checked BEFORE writing, as above. */ + if ((size_t)(buf - start) + JaguarCDBiosStateSize() > STATE_SIZE) + return false; + buf += JaguarCDBiosStateSave(buf); + + if ((size_t)(buf - start) + CDROMStateExtSize() > STATE_SIZE) + return false; + buf += CDROMStateExtSave(buf); + written = (size_t)(buf - start); if (written > STATE_SIZE) return false; @@ -4903,6 +4920,19 @@ bool retro_unserialize(const void *data, size_t size) else JaguarCDHLEStateReset(); + /* v16 (#804): real-BIOS CD chunks -- see the matching save comment. + * v15 and older end before them. For those the live session's values + * are LEFT ALONE, exactly as v3.7.0 did: users save mid-game, after the + * boot stub is in, so forcing "not injected" would re-arm the $005E40 + * GPU-magic stomp (and the $050176 re-injection) over the game's own + * RAM -- corrupting a state that loads fine today. A fresh core holds + * the defaults anyway. */ + if (version >= STATE_VERSION_BIOS_CD_BOOT) + { + buf += JaguarCDBiosStateLoad(buf); + buf += CDROMStateExtLoad(buf); + } + /* tomRam8 was restored raw above; recompute the DRAM/refresh timing * that bus_arbiter derives from MEMCON1/MEMCON2 so it matches the * loaded state (dram_row_miss/rom_clocks/dram_refresh_clks from diff --git a/src/cd/cdrom.c b/src/cd/cdrom.c index c7a93d00..698293ef 100644 --- a/src/cd/cdrom.c +++ b/src/cd/cdrom.c @@ -3015,3 +3015,66 @@ size_t CDROMStateLoad(const uint8_t *buf, uint32_t stateVersion) return (size_t)(buf - start); } + +/* ------------------------------------------------------------------ */ +/* Trailing CDROM drive-timing chunk (#804) */ +/* */ +/* Two statics the main CDROM chunk never carried. fifoRefillAccum is */ +/* the error-diffusion remainder behind CDROMNextRefillDelay(): it */ +/* decides whether the NEXT FIFO refill interval is 2 or 3 ticks, so */ +/* leaving it at the replay's value shifted every later refill by one */ +/* tick (Hover Strike real-BIOS load: state_reconverges differed in */ +/* fifoFillDelay 41 frames after the rollback). cdPrevShouldIRQ is the */ +/* edge detector for the 68K/GPU CD interrupt: the same class of hidden */ +/* cross-tick state, saved alongside (no divergence was observed from */ +/* it on its own). */ +/* ------------------------------------------------------------------ */ + +#define CDROM_EXT_MAGIC 0x31584443u /* "CDX1" little-endian */ + +size_t CDROMStateExtSize(void) +{ + return 4u + 4u + 1u; /* magic, fifoRefillAccum, cdPrevShouldIRQ */ +} + +size_t CDROMStateExtSave(uint8_t *buf) +{ + uint8_t *start = buf; + uint32_t magic = CDROM_EXT_MAGIC; + uint8_t b = cdPrevShouldIRQ ? 1 : 0; + + STATE_SAVE_VAR(buf, magic); + STATE_SAVE_VAR(buf, fifoRefillAccum); + STATE_SAVE_VAR(buf, b); + + return (size_t)(buf - start); +} + +void CDROMStateExtReset(void) +{ + fifoRefillAccum = 0; + cdPrevShouldIRQ = false; +} + +size_t CDROMStateExtLoad(const uint8_t *buf) +{ + const uint8_t *start = buf; + uint32_t magic; + uint8_t b; + + STATE_LOAD_VAR(buf, magic); + if (magic != CDROM_EXT_MAGIC) + { + CDROMStateExtReset(); + return (size_t)(buf - start); + } + STATE_LOAD_VAR(buf, fifoRefillAccum); + STATE_LOAD_VAR(buf, b); + cdPrevShouldIRQ = (b != 0); + /* CDROMNextRefillDelay does fifoRefillAccum / 100 and % 100: only + * 0..99 is ever produced, so anything else came from a damaged file. */ + if (fifoRefillAccum < 0 || fifoRefillAccum >= 100) + fifoRefillAccum = 0; + + return (size_t)(buf - start); +} diff --git a/src/cd/jagcd_bios.c b/src/cd/jagcd_bios.c index d25c040f..f39ca096 100644 --- a/src/cd/jagcd_bios.c +++ b/src/cd/jagcd_bios.c @@ -17,6 +17,7 @@ #include "settings.h" #include "vjag_memory.h" #include "m68000/m68kinterface.h" +#include "state.h" #include @@ -34,6 +35,59 @@ static void bios_reset(void) cdBootStubInjected = false; } +/* ------------------------------------------------------------------ */ +/* Savestate chunk (#804) */ +/* */ +/* cdBootStubInjected decides whether the 68K hooks below still inject */ +/* the boot stub ($050176) and still stomp the GPU auth magic ($005E40).*/ +/* It lives outside the machine state, so a rollback to a state taken */ +/* BEFORE the injection kept the flag set by the pass that had already */ +/* injected: the replay never injected, the game never started, and */ +/* run-ahead diverged mid-load. The reverse direction hurts too: a */ +/* state taken AFTER injection, loaded into a fresh core, re-armed the */ +/* $005E40 stomp over the game's own RAM (Myst's GPU IRQ0 stub). */ +/* ------------------------------------------------------------------ */ + +#define BIOS_STATE_MAGIC 0x31424443u /* "CDB1" little-endian */ + +size_t JaguarCDBiosStateSize(void) +{ + return 4u + 1u; /* magic, cdBootStubInjected */ +} + +size_t JaguarCDBiosStateSave(uint8_t *buf) +{ + uint8_t *start = buf; + uint32_t magic = BIOS_STATE_MAGIC; + uint8_t b = cdBootStubInjected ? 1 : 0; + + STATE_SAVE_VAR(buf, magic); + STATE_SAVE_VAR(buf, b); + return (size_t)(buf - start); +} + +void JaguarCDBiosStateReset(void) +{ + cdBootStubInjected = false; +} + +size_t JaguarCDBiosStateLoad(const uint8_t *buf) +{ + const uint8_t *start = buf; + uint32_t magic; + uint8_t b; + + STATE_LOAD_VAR(buf, magic); + if (magic != BIOS_STATE_MAGIC) + { + JaguarCDBiosStateReset(); + return (size_t)(buf - start); + } + STATE_LOAD_VAR(buf, b); + cdBootStubInjected = (b != 0); + return (size_t)(buf - start); +} + static bool bios_instruction_hook(uint32_t m68kPC) { /* GPU auth magic — boot ROM checks this to verify GPU ran auth code. diff --git a/src/cd/jagcd_boot.h b/src/cd/jagcd_boot.h index ab41e8d0..b5e79ddb 100644 --- a/src/cd/jagcd_boot.h +++ b/src/cd/jagcd_boot.h @@ -2,6 +2,7 @@ #define __JAGCD_BOOT_H__ #include +#include #include #ifdef __cplusplus @@ -33,6 +34,18 @@ extern const CDBootStrategy cd_boot_strategy_cart; * by ResolveBootConfig(), which only knows about cart/CD content. */ extern const CDBootStrategy cd_boot_strategy_none; +/* Savestate chunk for the real-BIOS boot path (#804): whether the boot stub + * has been injected yet. Appended strictly last to the state blob behind a + * magic word. retro_unserialize() only calls Load for v16+ states and + * leaves the live value alone for older ones (forcing "not injected" would + * re-arm the BIOS hooks over a mid-game state's RAM); a chunk whose magic + * is wrong takes JaguarCDBiosStateReset(). Save/Load return the bytes + * consumed. */ +size_t JaguarCDBiosStateSize(void); +size_t JaguarCDBiosStateSave(uint8_t *buf); +size_t JaguarCDBiosStateLoad(const uint8_t *buf); +void JaguarCDBiosStateReset(void); + #ifdef __cplusplus } #endif diff --git a/src/core/state.h b/src/core/state.h index fd626400..609b9a3d 100644 --- a/src/core/state.h +++ b/src/core/state.h @@ -81,14 +81,16 @@ extern "C" { * exactly what a pre-#552 core was — see UARTWireSpeedupStateLoad(). * v14: mounted-disc identity + image index (#651). Shipped in v3.6.0. * v15: CDROM chunk gained the last $03nn/$14nn TOC word handed out - * (dsaLastMultiWord). develop only — first bump of the post-v3.6.1 - * cycle, so any other in-flight state change this cycle extends v15 - * in place rather than bumping again. + * (dsaLastMultiWord). Shipped in v3.7.0. * EXTENDED IN PLACE (still v15): #787 appended the HLE CD streaming * chunk strictly last, behind a magic word, so a v15 blob written - * before it (zero-filled tail) loads with the transfer dropped. */ + * before it (zero-filled tail) loads with the transfer dropped. + * v16: real-BIOS CD boot chunk (#804), appended strictly last behind a + * magic word: whether the boot stub has been injected. First bump of + * the v3.7.1 cycle, so any other in-flight state change this cycle + * extends v16 in place rather than bumping again. */ #define STATE_MAGIC 0x564A5353 /* "VJSS" */ -#define STATE_VERSION 15 +#define STATE_VERSION 16 /* Oldest layout retro_unserialize still accepts. States between * STATE_MIN_VERSION and STATE_VERSION load by reading each chunk in the * layout the header version names (see DACStateLoad, CDROMStateLoad); @@ -200,6 +202,15 @@ extern "C" { /* First version that may carry the trailing HLE CD streaming chunk * (#787). "May": the chunk is tagged, see JaguarCDHLEStateLoad. */ #define STATE_VERSION_HLE_CD_STREAM 15 +/* v16: trailing real-BIOS CD boot chunk (#804): jagcd_bios.c's + * cdBootStubInjected flag, which gates the 68K boot-stub injection hook. + * Outside the blob, a rollback to a pre-injection state kept the flag set + * by the replay that had already injected, so the replay never injected + * the stub and the game never started (run-ahead diverged mid-load); and + * a post-injection state loaded into a fresh core re-armed the $005E40 + * GPU-magic stomp over the game's own RAM. Older states do not carry it: + * the live session's value is left alone (v3.7.0 behaviour). */ +#define STATE_VERSION_BIOS_CD_BOOT 16 /* Header flags */ #define STATE_FLAG_MEMTRACK 0x01 @@ -255,6 +266,14 @@ size_t TOMStateLoad(const uint8_t *buf); size_t CDROMStateSave(uint8_t *buf); size_t CDROMStateLoad(const uint8_t *buf, uint32_t stateVersion); +/* Trailing drive-timing chunk (#804, STATE_VERSION_BIOS_CD_BOOT): the FIFO + * refill error-diffusion accumulator and the CD IRQ edge detector. Tagged + * with a magic word; Reset is what a state without the chunk gets. */ +size_t CDROMStateExtSize(void); +size_t CDROMStateExtSave(uint8_t *buf); +size_t CDROMStateExtLoad(const uint8_t *buf); +void CDROMStateExtReset(void); + size_t JoystickStateSave(uint8_t *buf); size_t JoystickStateLoad(const uint8_t *buf); diff --git a/test/test_state_compat.c b/test/test_state_compat.c index 3147ef9e..5313d13c 100644 --- a/test/test_state_compat.c +++ b/test/test_state_compat.c @@ -81,7 +81,7 @@ const char *__lsan_default_suppressions(void) { } #endif -#define MAX_RESULTS 24 +#define MAX_RESULTS 32 #define DEFAULT_ROM "test/roms/yarc.j64" #define DEFAULT_FRAMES 120 /* Frames run between capturing the v3 state and loading the v2 fixture, @@ -340,6 +340,7 @@ int main(int argc, char **argv) harness_config cfg = HARNESS_CONFIG_DEFAULT; dac_state_save_fn dac_save; dac_state_save_fn cdrom_save, joy_save, mt_save, nvm_save; + dac_state_save_fn bios_chunk_save, cdext_chunk_save; serialize_size_fn ser_size_fn; serialize_fn ser; unserialize_fn unser; @@ -398,6 +399,8 @@ int main(int argc, char **argv) joy_save = (dac_state_save_fn)harness_dlsym(&cfg, "JoystickStateSave"); mt_save = (dac_state_save_fn)harness_dlsym(&cfg, "MTStateSave"); nvm_save = (dac_state_save_fn)harness_dlsym(&cfg, "NVMBiosStateSave"); + bios_chunk_save = (dac_state_save_fn)harness_dlsym(&cfg, "JaguarCDBiosStateSave"); + cdext_chunk_save = (dac_state_save_fn)harness_dlsym(&cfg, "CDROMStateExtSave"); ser_size_fn = (serialize_size_fn)harness_dlsym(&cfg, "retro_serialize_size"); ser = (serialize_fn)harness_dlsym(&cfg, "retro_serialize"); unser = (unserialize_fn)harness_dlsym(&cfg, "retro_unserialize"); @@ -406,6 +409,7 @@ int main(int argc, char **argv) height_ptr = (int *)harness_dlsym(&cfg, "game_height"); if (!dac_save || !cdrom_save || !joy_save || !mt_save + || !bios_chunk_save || !cdext_chunk_save || !ser_size_fn || !ser || !unser || !fb_ptr || !width_ptr || !height_ptr) { fprintf(stderr, @@ -700,6 +704,93 @@ int main(int argc, char **argv) "post-load DAC block is byte-identical to the saved one -- proves " "every chunk before it was read in the v3 layout, not the v7 one"); + /* ---- 8b. v16 real-BIOS CD chunks (#804) -------------------------- */ + /* Two trailing chunks -- JaguarCDBiosStateSave ("CDB1": boot stub + * injected) then CDROMStateExtSave ("CDX1": FIFO refill accumulator, + * IRQ edge) -- sit back to back at the end of the payload. A cart + * has no disc, so drive them through the state blob itself: patch the + * values into a freshly written state, load it, and read them back + * through the module's own save function. + * + * Negative control (verified): with the chunks' load calls removed + * from retro_unserialize, v16_chunks_restored goes red. */ + { + static uint8_t bios_a[64], bios_b[64], cdx_a[64], cdx_b[64]; + size_t bios_n, cdx_n, tail_off = 0; + unsigned tail_matches; + uint8_t pair[128]; + + if (!ser(state_v3, state_size)) { + check(0, "v16_chunks_located", "retro_serialize() returned false"); + } else { + bios_n = bios_chunk_save(bios_a); + cdx_n = cdext_chunk_save(cdx_a); + memcpy(pair, bios_a, bios_n); + memcpy(pair + bios_n, cdx_a, cdx_n); + tail_matches = find_pattern(state_v3, state_size, pair, + bios_n + cdx_n, &tail_off); + check(tail_matches == 1 && bios_n == 5 && cdx_n == 9, + "v16_chunks_located", + "CDB1 (%lu B) + CDX1 (%lu B) occur %u time(s) in the state " + "(expect 1; sizes 5 + 9) -- chunk layout changed? re-derive " + "the patch offsets below", + (unsigned long)bios_n, (unsigned long)cdx_n, tail_matches); + if (tail_matches == 1 && bios_n == 5 && cdx_n == 9) { + /* CDB1: magic(4) injected(1). CDX1: magic(4) accum(4) irq(1). */ + memcpy(scratch, state_v3, state_size); + scratch[tail_off + 4] = 1; + put_u32(scratch, tail_off + 5 + 4, 37u); + scratch[tail_off + 5 + 8] = 1; + + check(unser(scratch, state_size), "v16_chunks_load", + "retro_unserialize() of a v%d state carrying non-default " + "CD BIOS chunks", STATE_VERSION); + bios_chunk_save(bios_b); + cdext_chunk_save(cdx_b); + check(bios_b[4] == 1 && get_u32(cdx_b, 4) == 37u + && cdx_b[8] == 1, + "v16_chunks_restored", + "after load: stub-injected=%u refill-accum=%u irq-edge=%u " + "(expect 1 / 37 / 1) -- the rollback must restore " + "them, not inherit the replay's values", + (unsigned)bios_b[4], (unsigned)get_u32(cdx_b, 4), + (unsigned)cdx_b[8]); + + /* A v15 state ends before the chunks: whatever bytes sit + * there are NOT ours. The load must leave the live + * session's values alone (v3.7.0 behaviour) rather than + * force "boot stub not injected": a state saved mid-game + * would then re-arm the $005E40 GPU-magic stomp and the + * $050176 re-injection over the game's own RAM. */ + (void)unser(scratch, state_size); /* live values = 1/37/1 */ + put_u32(scratch, STATE_OFF_VERSION, 15u); + scratch[tail_off + 4] = 0; /* chunk bytes must be ignored */ + put_u32(scratch, tail_off + 5 + 4, 5u); + check(unser(scratch, state_size), "v15_state_loads", + "retro_unserialize() of a v15 state (shipped in v3.7.0)"); + bios_chunk_save(bios_b); + cdext_chunk_save(cdx_b); + check(bios_b[4] == 1 && get_u32(cdx_b, 4) == 37u + && cdx_b[8] == 1, + "v15_chunks_inherited", + "after a v15 load: stub-injected=%u refill-accum=%u " + "irq-edge=%u (expect the live 1 / 37 / 1, unchanged)", + (unsigned)bios_b[4], (unsigned)get_u32(cdx_b, 4), + (unsigned)cdx_b[8]); + + /* A damaged accumulator must not reach + * CDROMNextRefillDelay's divide/modulo unchecked. */ + memcpy(scratch, state_v3, state_size); + put_u32(scratch, tail_off + 5 + 4, 100000u); + (void)unser(scratch, state_size); + cdext_chunk_save(cdx_b); + check(get_u32(cdx_b, 4) < 100u, "v16_accum_bounded", + "out-of-range refill accumulator loaded as %u (expect " + "0..99)", (unsigned)get_u32(cdx_b, 4)); + } + } + } + /* ---- 9. Rejections -------------------------------------------- */ check(try_load_patched(unser, state_v3, state_size, STATE_OFF_VERSION, 0u, scratch) == false, diff --git a/test/tools/test_runahead_determinism.c b/test/tools/test_runahead_determinism.c index f8bf09f8..d318ef35 100644 --- a/test/tools/test_runahead_determinism.c +++ b/test/tools/test_runahead_determinism.c @@ -589,9 +589,14 @@ int main(int argc, char **argv) free(blob_b); free(blob_c); - remove(state_a); - remove(state_b); - remove(state_c); + /* VJ_RUNAHEAD_KEEP=1 keeps the three state files so a failing + * state_reconverges can be diffed offline (b = end of pass 1, + * c = end of pass 2; first differing offset names the subsystem). */ + if (!getenv("VJ_RUNAHEAD_KEEP")) { + remove(state_a); + remove(state_b); + remove(state_c); + } harness_shutdown(&cfg); return failed;