fix(cd): serialize the HLE CD streaming state (#803)

* fix(cd): serialize the HLE CD streaming state (#787)

The HLE BIOS streams CD_read data into RAM at drive rate from statics in
jagcd_hle.c (hleStream, the CD_read/CD_poll bookkeeping, the #766 align
phase) that were never in the state blob. Measured with
test_runahead_determinism: Hover Strike (HLE) failed all four checks at
warmup 200/400/700/1000; Myst (HLE) failed while its reads were in
flight (200/400) and passed after. Cart (yarc) and Hover Strike with the
real BIOS at 1000 pass, so the leak was the HLE path.

Append an HLE chunk strictly last, behind a magic word ("HLE1"),
extending v15 in place (v15 is still develop-only). Field by field, never
the raw struct (padding, sizeof(bool)). hle_active is boot-strategy
configuration and is not saved. A damaged bufOff/written drops the
transfer rather than reading past the sector buffer.

Backward compatibility, measured by saving at frame 400 (mid-stream) and
loading into this build:
- v14 states from v3.6.1 load and run byte-identically (per-frame hash
  CSV) to v3.6.1 loading its own state, for Hover Strike and Myst.
- v15 states written by develop before this change (zero-filled tail)
  load with the in-flight transfer dropped, as before.
- New states resume the transfer (Hover Strike: 200 distinct frames
  after load vs a frozen black screen without the chunk).
A pre-#787 state saved mid-read still cannot resume that read on any
build -- that data was never saved.

After: all eight runs above pass all four checks. make test gains a
ROM-gated Hover Strike HLE run-ahead check at warmup 400 (passes here,
exits 1 on the pre-#787 build).

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* fix(cd): validate a restored HLE stream; synthetic mid-stream round trip

Review on #803:
- Copilot: a damaged or crafted state could set dest near $FFFFFFFF,
  wrapping HLEStreamFinish's dest+total bound check into an out-of-bounds
  write, or point the GPU data base where CD_poll's +0/+8 reads land
  outside RAM. Load now requires dest/total/reqTotal/written and the
  CD_poll range to fit main RAM, and both data bases to be 0, main RAM or
  GPU local RAM; otherwise it drops the transfer and those pointers.
- Copilot: CI has no private discs, so nothing exercised an active
  stream through save/load. test_cd_synth_read gains
  inflight_stream_survives_savestate_round_trip on its synthetic disc:
  save mid-stream, finish, restore, and the same transfer must be in
  flight and complete byte-exact. Red with the chunk load disabled.
- Kimi: check room for the ~2.4 KB chunk before writing it, not after;
  JaguarCDHLEStateSize() is pinned to what Save writes by the same test.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

---------

Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Joe MattielloandClaude Opus 5.5 authored and GitHub committed 2026-10-05 23:40:42 -04:00
1 parent b89b1ccf67
commit 0e3e504c8a
6 files changed
+321 -1

No files matched your search

+10
View File
@@ -1777,6 +1777,16 @@ test: test/test_dram_timing test/test_cheat test/test_event_queue test/test_jlin
else \
bash scripts/test-skip.sh record "Doom (savestate determinism, enhancement path)" "no ROM matching 'Doom*' in the private corpus"; \
fi
@# Issue #787: the HLE CD BIOS streams CD_read data at drive rate from
@# state that used to live outside the blob, so every HLE disc diverged
@# under run-ahead while a read was in flight. Warmup 400 lands
@# mid-stream on Hover Strike's intro (failed all four checks pre-fix).
@rom=$$(bash scripts/find-rom.sh 'Hover Strike - Unconquered Lands (USA).cue'); \
if [ -n "$$rom" ]; then \
./test/tools/test_runahead_determinism ./$(TARGET) "$$rom" --warmup 400 --frames 120 --quiet; \
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
./test/test_butch_cd
./test/test_cd_hle_idempotent
./test/test_cd_pregap
+25
View File
@@ -4646,6 +4646,21 @@ bool retro_serialize(void *data, size_t size)
STATE_SAVE_VAR(buf, disc_index);
}
/* v15 (extended in place, #787): HLE CD streaming state -- the
* in-flight CD_read the HLE BIOS delivers at drive rate, plus its
* CD_poll bookkeeping. Outside the blob, a rollback replayed frames
* against a transfer that had already advanced (run-ahead divergence
* on every HLE CD title measured mid-stream).
*
* STRICTLY LAST, after the disc identity: appending keeps every older
* blob loadable, and the chunk's own magic word tells a pre-#787 v15
* blob (zero-filled tail) from one that carries it. */
/* Check room BEFORE writing (Kimi review on #803): the chunk is ~2.4 KB
* and the size check below only runs after the bytes are already out. */
if ((size_t)(buf - start) + JaguarCDHLEStateSize() > STATE_SIZE)
return false;
buf += JaguarCDHLEStateSave(buf);
written = (size_t)(buf - start);
if (written > STATE_SIZE)
return false;
@@ -4878,6 +4893,16 @@ bool retro_unserialize(const void *data, size_t size)
disk_index = disc_index;
}
/* v15 (extended in place, #787): HLE CD streaming state -- see the
* matching save comment. Older layouts end before it; a v15 blob
* written before #787 ends with a zero-filled tail, which the chunk's
* magic word rejects. Either way the in-flight transfer is dropped
* rather than inherited from whatever this session was doing. */
if (version >= STATE_VERSION_HLE_CD_STREAM)
buf += JaguarCDHLEStateLoad(buf);
else
JaguarCDHLEStateReset();
/* 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
+170
View File
@@ -26,6 +26,7 @@
#include "dsp.h"
#include "jaguar.h"
#include "m68000/m68kinterface.h"
#include "state.h"
/* There is NO DSP-RAM "CD transfer done" flag. An earlier HLE wrote
* $00000000 / $FFFFFFFF to $F1B4C8 around every CD_read, citing a
@@ -1414,6 +1415,175 @@ uint32_t JaguarCDHLEStreamArmCount(void)
return hle_stream_arm_count;
}
/* ------------------------------------------------------------------ */
/* Savestate chunk (#787) */
/* */
/* Field by field, never the raw struct: padding bytes and the size of */
/* `bool` are compiler properties, and a rollback-netplay peer may run */
/* a different build. hle_active is NOT saved -- it is set by the boot */
/* strategy (HLE vs real BIOS), which is session configuration like */
/* NTSC/PAL. hle_stream_arm_count is a probe counter, not machine */
/* state. */
/* ------------------------------------------------------------------ */
#define HLE_STATE_MAGIC 0x31454C48u /* "HLE1" little-endian */
#define HLE_STATE_RAM_SIZE 0x200000u /* 2 MB main RAM */
/* A GPU data-area base is either unset (0) or a range that holds its
* +0/+4/+8 longs: main RAM, or GPU local RAM ($F03000-$F03FFF, where the
* BIOS default $F03B00 and Battle Morph's $F03158 live). */
static bool hle_state_base_ok(uint32_t base)
{
if (base == 0)
return true;
if ((base & 3u) != 0)
return false;
if (base <= HLE_STATE_RAM_SIZE - 12u)
return true;
return base >= 0xF03000u && base <= 0xF04000u - 12u;
}
/* Bytes JaguarCDHLEStateSave writes, so the caller can check room first. */
size_t JaguarCDHLEStateSize(void)
{
return 4u + 1u + 7u * 4u /* magic, pending, 7 bookkeeping words */
+ 2u + 15u * 4u /* active, bufValid, 15 stream words */
+ sizeof(hleStream.buf);
}
size_t JaguarCDHLEStateSave(uint8_t *buf)
{
uint8_t *start = buf;
uint32_t magic = HLE_STATE_MAGIC;
uint8_t b;
STATE_SAVE_VAR(buf, magic);
b = hle_read_pending ? 1 : 0;
STATE_SAVE_VAR(buf, b);
STATE_SAVE_VAR(buf, hle_read_dest);
STATE_SAVE_VAR(buf, hle_read_end_addr);
STATE_SAVE_VAR(buf, hle_read_progress);
STATE_SAVE_VAR(buf, hle_gpu_data_base);
STATE_SAVE_VAR(buf, hle_post_read_lba);
STATE_SAVE_VAR(buf, hle_align_phase_track);
STATE_SAVE_VAR(buf, hle_align_phase);
b = hleStream.active ? 1 : 0;
STATE_SAVE_VAR(buf, b);
b = hleStream.bufValid ? 1 : 0;
STATE_SAVE_VAR(buf, b);
STATE_SAVE_VAR(buf, hleStream.lba);
STATE_SAVE_VAR(buf, hleStream.bufOff);
STATE_SAVE_VAR(buf, hleStream.dest);
STATE_SAVE_VAR(buf, hleStream.total);
STATE_SAVE_VAR(buf, hleStream.reqTotal);
STATE_SAVE_VAR(buf, hleStream.written);
STATE_SAVE_VAR(buf, hleStream.accFrac);
STATE_SAVE_VAR(buf, hleStream.d1);
STATE_SAVE_VAR(buf, hleStream.statusBase);
STATE_SAVE_VAR(buf, hleStream.sigD0);
STATE_SAVE_VAR(buf, hleStream.sigD1);
STATE_SAVE_VAR(buf, hleStream.sigA0);
STATE_SAVE_VAR(buf, hleStream.sigA1);
STATE_SAVE_VAR(buf, hleStream.speedMult);
STATE_SAVE_VAR(buf, hleStream.startDelay);
/* The sector buffer holds the un-swapped bytes of the sector being
* delivered; re-reading it from the disc on load would work too, but
* only for a valid buffer, and this keeps the load path trivial. */
STATE_SAVE_BUF(buf, hleStream.buf, sizeof(hleStream.buf));
return (size_t)(buf - start);
}
void JaguarCDHLEStateReset(void)
{
hleStream.active = false;
hleStream.bufValid = false;
hle_read_pending = false;
hle_read_progress = 0;
}
size_t JaguarCDHLEStateLoad(const uint8_t *buf)
{
const uint8_t *start = buf;
uint32_t magic;
uint8_t pending, active, bufValid;
STATE_LOAD_VAR(buf, magic);
if (magic != HLE_STATE_MAGIC)
{
/* No chunk: a v15 blob written before #787 (its zero-filled tail
* reads as magic 0). Nothing past this point is ours. */
JaguarCDHLEStateReset();
return (size_t)(buf - start);
}
STATE_LOAD_VAR(buf, pending);
STATE_LOAD_VAR(buf, hle_read_dest);
STATE_LOAD_VAR(buf, hle_read_end_addr);
STATE_LOAD_VAR(buf, hle_read_progress);
STATE_LOAD_VAR(buf, hle_gpu_data_base);
STATE_LOAD_VAR(buf, hle_post_read_lba);
STATE_LOAD_VAR(buf, hle_align_phase_track);
STATE_LOAD_VAR(buf, hle_align_phase);
hle_read_pending = pending != 0;
STATE_LOAD_VAR(buf, active);
STATE_LOAD_VAR(buf, bufValid);
STATE_LOAD_VAR(buf, hleStream.lba);
STATE_LOAD_VAR(buf, hleStream.bufOff);
STATE_LOAD_VAR(buf, hleStream.dest);
STATE_LOAD_VAR(buf, hleStream.total);
STATE_LOAD_VAR(buf, hleStream.reqTotal);
STATE_LOAD_VAR(buf, hleStream.written);
STATE_LOAD_VAR(buf, hleStream.accFrac);
STATE_LOAD_VAR(buf, hleStream.d1);
STATE_LOAD_VAR(buf, hleStream.statusBase);
STATE_LOAD_VAR(buf, hleStream.sigD0);
STATE_LOAD_VAR(buf, hleStream.sigD1);
STATE_LOAD_VAR(buf, hleStream.sigA0);
STATE_LOAD_VAR(buf, hleStream.sigA1);
STATE_LOAD_VAR(buf, hleStream.speedMult);
STATE_LOAD_VAR(buf, hleStream.startDelay);
STATE_LOAD_BUF(buf, hleStream.buf, sizeof(hleStream.buf));
hleStream.active = active != 0;
hleStream.bufValid = bufValid != 0;
/* Everything below came from a file. The arm path only ever builds
* these from a bounded CD_read, so a state that a real session wrote
* always passes; a damaged or crafted one must not turn into an
* out-of-bounds access (Copilot review on #803):
* - StreamTick indexes buf[bufOff..2351] and copies total - written;
* - HLEStreamFinish writes the pad/ATRI block at dest + total, with a
* bound check that a dest near $FFFFFFFF would wrap past;
* - CD_poll and the status writes touch base +0/+4/+8.
* On any failure drop the transfer AND the pointers it would use. */
if (!hle_state_base_ok(hle_gpu_data_base)
|| !hle_state_base_ok(hleStream.statusBase))
{
hle_gpu_data_base = 0;
hleStream.statusBase = 0;
JaguarCDHLEStateReset();
}
if (hleStream.bufOff > sizeof(hleStream.buf)
|| hleStream.written > hleStream.total
|| hleStream.reqTotal > hleStream.total
|| hleStream.dest >= HLE_STATE_RAM_SIZE
|| hleStream.total > HLE_STATE_RAM_SIZE - hleStream.dest)
JaguarCDHLEStateReset();
if (hle_read_dest >= HLE_STATE_RAM_SIZE
|| hle_read_end_addr < hle_read_dest
|| hle_read_end_addr > HLE_STATE_RAM_SIZE)
{
hle_read_dest = 0;
hle_read_end_addr = 0;
JaguarCDHLEStateReset();
}
return (size_t)(buf - start);
}
void JaguarCDHLEStreamTick(void)
{
uint32_t budget;
+17
View File
@@ -2,6 +2,7 @@
#define __JAGCD_HLE_H__
#include <stdint.h>
#include <stddef.h>
#include <boolean.h>
#ifdef __cplusplus
@@ -58,6 +59,22 @@ uint32_t JaguarCDHLEStreamArmCount(void);
/* Force HLE active state (for unit testing without a disc image). */
void JaguarCDHLESetActive(bool active);
/* Savestate chunk (#787): the in-flight streamed CD_read and the CD_read /
* CD_poll bookkeeping, which used to live outside the state blob -- so a
* rollback (run-ahead, netplay) or a load into a fresh core lost or
* leaked the transfer. Appended strictly last to the blob and tagged
* with a magic word, so any blob without it (v14 and older, and v15
* written before this chunk existed) is detected and takes
* JaguarCDHLEStateReset() instead. Save/Load return the bytes consumed. */
size_t JaguarCDHLEStateSave(uint8_t *buf);
size_t JaguarCDHLEStateSize(void);
size_t JaguarCDHLEStateLoad(const uint8_t *buf);
/* For a state that carries no HLE chunk: drop any in-flight transfer
* (the state's RAM and CPU registers describe a machine that is not
* mid-read as far as we can know) and leave the session-stable setup --
* GPU data area base, align phase, post-read LBA -- as it is. */
void JaguarCDHLEStateReset(void);
#ifdef __cplusplus
}
#endif
+7 -1
View File
@@ -83,7 +83,10 @@ extern "C" {
* 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. */
* in place rather than bumping again.
* 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. */
#define STATE_MAGIC 0x564A5353 /* "VJSS" */
#define STATE_VERSION 15
/* Oldest layout retro_unserialize still accepts. States between
@@ -194,6 +197,9 @@ extern "C" {
* loaded into a fresh core would otherwise read 0, and run-ahead/netplay
* would diverge (the #400 class). Older states load it as 0. */
#define STATE_VERSION_CDROM_DSA_LASTWORD 15
/* 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
/* Header flags */
#define STATE_FLAG_MEMTRACK 0x01
+92
View File
@@ -82,6 +82,11 @@ static uint32_t (*p_cd_total)(void);
static bool (*p_load_game)(const struct retro_game_info *);
static void (*p_unload_game)(void);
static void (*p_run)(void);
static size_t (*p_ser_size)(void);
static bool (*p_serialize)(void *, size_t);
static bool (*p_unserialize)(const void *, size_t);
static size_t (*p_hle_state_save)(uint8_t *);
static size_t (*p_hle_state_size)(void);
static uint8_t *ram;
@@ -577,6 +582,85 @@ TEST(delivery_is_streamed_not_instant)
bad, ram[DEST_A + bad], expect_ram(bad));
}
/* ------------------------------------------------------------------ */
/* 3b. An in-flight stream survives a savestate round trip (#787) */
/* ------------------------------------------------------------------ */
/* The HLE stream lives in jagcd_hle.c statics. Before #787 none of it was
* in the state blob, so restoring a state saved mid-transfer left the
* stream wherever the live session had taken it -- here, already finished
* -- and the bytes still owed after the save point never arrived: exactly
* what run-ahead's rollback does every frame. Save part-way through,
* finish the transfer, restore, and the SAME transfer must be in flight
* again and complete byte-exact.
*
* Negative control (verified): on the pre-#787 core the stream is inactive
* right after the restore and this test goes red. */
TEST(inflight_stream_survives_savestate_round_trip)
{
uint32_t size = 0x8000u;
uint32_t totalAtSave, ticks, bad, i;
size_t ssz;
uint8_t *state;
/* The room check in retro_serialize trusts JaguarCDHLEStateSize(). */
{
static uint8_t chunk[8192];
ASSERT_EQ_U32((uint32_t)p_hle_state_save(chunk),
(uint32_t)p_hle_state_size());
}
ssz = p_ser_size();
state = (uint8_t *)malloc(ssz);
ASSERT_TRUE(state != NULL);
fill_dest(DEST_A, size + 128u);
arm_read(payloadLBA, DEST_A, size);
for (i = 0; i < 20u; i++)
p_hle_tick();
ASSERT_TRUE(p_hle_active());
if (ram[DEST_A + size - 1u] != FILLER)
FAIL("stream finished before the save point; shorten the warmup");
totalAtSave = p_hle_bytes();
if (!p_serialize(state, ssz))
{
free(state);
FAIL("retro_serialize failed mid-stream");
}
/* Let the live session finish the transfer. */
drive_stream(200000u);
if (p_hle_active())
{
free(state);
FAIL("stream never completed before the restore");
}
/* Roll back to the save point. */
if (!p_unserialize(state, ssz))
{
free(state);
FAIL("retro_unserialize failed");
}
free(state);
if (!p_hle_active())
FAIL("restored state has no stream in flight -- the transfer was lost");
ASSERT_EQ_U32(p_hle_dest(), DEST_A);
ASSERT_EQ_U32(p_hle_bytes(), totalAtSave);
if (ram[DEST_A + size - 1u] != FILLER)
FAIL("restored RAM already holds the tail -- RAM was not rolled back");
ticks = drive_stream(200000u);
if (p_hle_active())
FAIL("restored stream never completed (%u ticks)", ticks);
bad = first_bad(DEST_A, size);
if (bad != size)
FAIL("after restore, byte %u: got $%02X, expected $%02X",
bad, ram[DEST_A + bad], expect_ram(bad));
}
/* ------------------------------------------------------------------ */
/* 4. Arming over an in-flight stream */
/* ------------------------------------------------------------------ */
@@ -725,10 +809,17 @@ static bool resolve_symbols(void)
p_load_game = (bool (*)(const struct retro_game_info *))dlsym(C.handle, "retro_load_game");
p_unload_game = (void (*)(void))dlsym(C.handle, "retro_unload_game");
p_run = (void (*)(void))dlsym(C.handle, "retro_run");
p_ser_size = (size_t (*)(void))dlsym(C.handle, "retro_serialize_size");
p_serialize = (bool (*)(void *, size_t))dlsym(C.handle, "retro_serialize");
p_unserialize = (bool (*)(const void *, size_t))dlsym(C.handle, "retro_unserialize");
p_hle_state_save = (size_t (*)(uint8_t *))dlsym(C.handle, "JaguarCDHLEStateSave");
p_hle_state_size = (size_t (*)(void))dlsym(C.handle, "JaguarCDHLEStateSize");
return p_hle_hook && p_hle_tick && p_hle_active && p_hle_dest &&
p_hle_bytes && p_hle_arms && p_cd_read_block && p_cd_s2_first &&
p_cd_total && p_load_game && p_unload_game && p_run &&
p_ser_size && p_serialize && p_unserialize &&
p_hle_state_save && p_hle_state_size &&
C.m68k_set_reg && C.m68k_get_reg && C.GetRamPtr;
}
@@ -830,6 +921,7 @@ int main(int argc, char *argv[])
RUN_TEST(synth_disc_lba_addressing);
RUN_TEST(long_rounded_tail_holds_real_disc_bytes);
RUN_TEST(delivery_is_streamed_not_instant);
RUN_TEST(inflight_stream_survives_savestate_round_trip);
RUN_TEST(arming_over_inflight_drops_tail_and_completion);
/* Fresh boot for the end-to-end run so the 68K starts at the stub. */