fix(runtime): make GS CSR atomic to fix vsync-worker/guest data race

[smmathews]

fix(runtime): make GS CSR atomic to fix vsync-worker/guest data race

Problem

updateGsCsrFieldForVSync (Interrupt.cpp) runs on the detached vsync/interrupt worker thread and updates the FIELD bit with a non-atomic read-modify-write of the shared GSRegisters::csr:

uint64_t &csr = runtime->memory().gs().csr;
csr = (csr & ~kGsCsrFieldMask) | ((tickValue & 1ull) ? kGsCsrFieldMask : 0ull);

Concurrently, guest EE threads read and write the same 64-bit word through the MMIO paths (ps2_memory.cpp read32/read64, and the write-1-to-clear handling in write32/write64), and the GIF path sets SIGNAL/FINISH (ps2_gs_gpu.cpp: csr |= 0x1 / csr |= 0x2) on the guest thread driving the GIF. All of these are plain, unsynchronized accesses to one uint64_t.

Although the vsync worker and the guest touch disjoint bits, a non-atomic RMW of the whole word still loses the other side's update. Concrete failure modes:

• Lost SIGNAL/FINISH: the GIF sets SIGNAL between the vsync worker's read and write-back → the bit is clobbered → a game synchronizing on GS SIGNAL/FINISH hangs.
• Lost W1C clear: the guest clears SIGNAL/FINISH; the vsync worker's stale write-back re-asserts it → spurious re-processing.
• Lost FIELD toggle: a guest CSR write clobbers the worker's FIELD update → an interlace field-polling loop stalls.

ThreadSanitizer confirms the race (report below).

Hardware background

GS CSR (0x12001000) is a status/control register that is legitimately written from two directions on real hardware: the GS side updates read-only status (FIELD bit 13 toggles every vblank in interlaced modes, NFIELD, FIFO state), while the EE side performs write-1-to-clear on the interrupt status bits (SIGNAL/FINISH/HSINT/VSINT/EDWINT, bits 0–4) and writes the FLUSH/RESET control bits. The emulator mirrors that split across threads — the vsync worker plays the GS role for FIELD, guest threads play the EE role — so the shared word needs atomic bit-level updates for the same word-level interleavings the silicon handles natively.

Fix

Make the field std::atomic<uint64_t> and convert every RMW to a single atomic RMW instruction:

• ps2_memory.h: std::atomic<uint64_t> csr; with a comment stating the concurrency contract. std::atomic<uint64_t> is lock-free on every supported target (static_assert added), same size/alignment (existing static_asserts unchanged), and GSRegisters is never copied or memcpy'd as a value.
• Vsync FIELD toggle → fetch_or(kGsCsrFieldMask) / fetch_and(~kGsCsrFieldMask).
• MMIO W1C write path → fetch_and(~clearMask) (a load+store pair would still race).
• GIF SIGNAL/FINISH → fetch_or(0x1) / fetch_or(0x2).
• Reads (read32/read64, debug panel, tests) → .load().
• Drive-by within the same scope: a 32-bit store to the CSR's upper dword (address 0x12001004) previously fell through to the plain non-atomic merge branch even though the low dword had W1C special-casing; both halves now go through the same atomic RMW helper with unchanged guest-visible semantics.

Default (seq_cst) memory order everywhere: these operations are rare (vblank ticks, GIF signals, guest MMIO on the CSR), so there is nothing to micro-optimize and the code stays trivially reviewable.

Testing

• New regression test: "Disjoint-bit GS CSR writers (SIGNAL vs FINISH vs vsync FIELD) never lose word-level updates" (ps2_runtime_interrupt_tests.cpp). Two racer threads each own one status bit (SIGNAL / FINISH) and loop 80k GIF-set + MMIO-W1C-clear cycles, each verifying only its own bit after every half-op, while the real vsync worker toggles FIELD. Post-fix each racer is the sole writer of its bit, so the assertions are exact and deterministic; pre-fix the whole-word RMWs clobber the other racer's bit — fails 20/20 runs against current main, passes 50/50 with the fix, and costs ~350 ms. No sanitizer required, so it guards the invariant on the existing CI matrix. (The racer-vs-racer design is deliberate: the vsync writer's once-per-16.7 ms RMW window is too narrow to hit deterministically in a bounded test, but the corrupting mechanism — a non-atomic whole-word RMW clobbering a concurrently-written disjoint bit — is identical.)
• Existing "VSync worker updates GS CSR FIELD bit" test (which asserts status bits survive FIELD updates) still passes.
• Full ps2x_tests suite passes on Linux (gcc); MSVC and clang via CI.
• ThreadSanitizer (Linux): the updateGsCsrFieldForVSync vs CSR-reader race is reported on main and gone with this patch.

TSan report on main (excerpt)

WARNING: ThreadSanitizer: data race
  Write of size 8 by thread T21:
    #0 ps2_syscalls::updateGsCsrFieldForVSync(PS2Runtime*, unsigned long)
    #1 ps2_syscalls::signalVSyncFlag(unsigned char*, PS2Runtime*)
    #2 ps2_syscalls::interruptWorkerMain(unsigned char*, PS2Runtime*)
  Previous write of size 8 by thread T22:
    #0 PS2Memory::write64(unsigned int, unsigned long)
    ...
SUMMARY: ThreadSanitizer: data race in ps2_syscalls::updateGsCsrFieldForVSync

(also reported: the same CSR word racing GS::writeRegister on the GIF SIGNAL path)

Scope

Single-word atomicity fix; no guest-visible behavior change on single-threaded paths, no new dependencies, ~10 edit sites across 5 files + 1 test.

[ran-j]

Hey thnak for your work.

I will review later

[ran-j]

Are you on discord ? @tealalchemist is refactoring the GS so its good to ask his review as well

[smmathews]

Are you on discord ? @tealalchemist is refactoring the GS so its good to ask his review as well

yeah, I'm shanemmathews in the discord