From d75c4edf67c30d3341033e64b0f6146051ee30f9 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Fri, 1 May 2026 17:55:13 +0200 Subject: [PATCH] docs(cpu): PPCBUG-108 document legacy reservation path's strict-lockstep requirement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds doc comments above lwarx/ldarx/stwcx./stdcx. clarifying that the legacy per-ctx reservation path is only correct in strict lockstep (single host thread); under --parallel the M3 scheduler must enable the cross-thread ReservationTable before spawning a second host thread. A debug_assert fires in the legacy stwcx./stdcx. branch if a non-primary HW slot (hw_id != 0) takes that path — surfacing ReservationTable-disabled misconfiguration early in debug builds. Note: the primary slot (hw_id==0) racing other parallel slots is not caught by the assert; that case requires the table to be enabled. Affected: PPCBUG-108 legacy per-ctx reservation path cannot invalidate cross-thread; informational — no behavioral change Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/xenia-cpu/src/interpreter.rs | 41 ++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/crates/xenia-cpu/src/interpreter.rs b/crates/xenia-cpu/src/interpreter.rs index 63db6a4..9795ac8 100644 --- a/crates/xenia-cpu/src/interpreter.rs +++ b/crates/xenia-cpu/src/interpreter.rs @@ -1116,6 +1116,15 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - // per-`PpcContext` fields. Both paths leave the per-ctx fields // in a coherent state so a flag flip mid-run doesn't corrupt // outstanding reservations. + // + // PPCBUG-108: lwarx + stwcx. atomicity is provided by `ReservationTable` + // in the M3 multi-HW-thread runtime. The legacy per-ctx fallback (when + // `reservation_table` is None or the table is disabled) cannot observe + // stores from other host threads — a store by thread B cannot clear + // `ctx_A.has_reservation`. This path is only correct in strict lockstep + // (single-host-thread) mode. The M3 scheduler MUST enable the table + // before spawning a second host thread. See stwcx./stdcx. for the + // debug_assert that fires if a non-primary slot takes this path. PpcOpcode::lwarx => { let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] }; let ea = ea.wrapping_add(ctx.gpr[instr.rb()]) as u32; @@ -1132,6 +1141,8 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - } ctx.pc += 4; } + // PPCBUG-108: see lwarx comment above. stwcx. legacy path cannot observe + // cross-thread reservation invalidations; only safe in lockstep mode. PpcOpcode::stwcx => { let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] }; let ea = ea.wrapping_add(ctx.gpr[instr.rb()]) as u32; @@ -1154,7 +1165,19 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - && ctx.reserved_line == line && t.try_commit(ea, ctx.reserved_generation, ctx.hw_id) } else { - // Legacy per-ctx path (M2 default). + // Legacy per-ctx path (M2 default / lockstep). + // PPCBUG-108: fires on non-primary HW slots under misconfig — + // if the table is disabled while workers are active, slots + // 1..N will trip this assert, surfacing the misconfiguration + // early in debug builds. Note: hw_id==0 (primary slot) taking + // this path while other slots run in parallel would NOT be + // caught; that case requires the table to be enabled instead. + debug_assert!( + ctx.hw_id == 0, + "PPCBUG-108: legacy per-ctx stwcx. on non-primary HW slot \ + (hw_id={}) — ReservationTable must be enabled under --parallel", + ctx.hw_id + ); ctx.has_reservation && width_ok && ctx.reserved_line == line }; if success { @@ -4281,6 +4304,11 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - // §4k — Scalar reservation / byte-reverse (doubleword) // ═════════════════════════════════════════════════════════════════ // M3.7 — same table-vs-legacy split as lwarx/stwcx. + // PPCBUG-108: ldarx + stdcx. have the same cross-thread atomicity + // limitation as lwarx/stwcx. in the legacy per-ctx fallback path. + // See the lwarx block comment for the full explanation. The M3 + // scheduler must enable `ReservationTable` before spawning a second + // host thread. stdcx. carries the debug_assert (see below). PpcOpcode::ldarx => { let ea = ea_indexed(ctx, instr); let val = mem.read_u64(ea); @@ -4296,6 +4324,8 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - } ctx.pc += 4; } + // PPCBUG-108: see ldarx comment above. stdcx. legacy path cannot observe + // cross-thread reservation invalidations; only safe in lockstep mode. PpcOpcode::stdcx => { let ea = ea_indexed(ctx, instr); let line = ea & !RESERVATION_MASK; @@ -4313,6 +4343,15 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - && ctx.reserved_line == line && t.try_commit(ea, ctx.reserved_generation, ctx.hw_id) } else { + // Legacy per-ctx path (M2 default / lockstep). + // PPCBUG-108: same sentinel as stwcx. — fires on non-primary + // HW slots if the table is disabled under --parallel. + debug_assert!( + ctx.hw_id == 0, + "PPCBUG-108: legacy per-ctx stdcx. on non-primary HW slot \ + (hw_id={}) — ReservationTable must be enabled under --parallel", + ctx.hw_id + ); ctx.has_reservation && width_ok && ctx.reserved_line == line }; if success {