fabi/xenia-rs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(cpu): PPCBUG-107 PPCBUG-140-144 add invalidate_for_write to word stores

Browse Source 
Word stores (stw, stwu, stwx, stwux, stwbrx) now invalidate the
reservation table for the target line before writing. Without this,
plain stores by other host threads silently fail to clear reservations
held by lwarx, causing stwcx. to spuriously succeed under --parallel.

Affected:
  PPCBUG-107  ReservationTable::invalidate_for_write never called from any store
  PPCBUG-140  stw missing invalidate_for_write   (interpreter.rs:1183)
  PPCBUG-141  stwu missing invalidate_for_write  (interpreter.rs:1189)
  PPCBUG-142  stwx missing invalidate_for_write  (interpreter.rs:1195)
  PPCBUG-143  stwux missing invalidate_for_write (interpreter.rs:1201)
  PPCBUG-144  stwbrx missing invalidate_for_write (interpreter.rs:1568)

Tests: lwarx_then_plain_stw_invalidates_reservation,
       lwarx_then_stwcx_succeeds_without_intervening_store

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-05-01 16:57:05 +02:00
parent bae9305982
commit 4538fa9e70
1 changed files with 93 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

93
crates/xenia-cpu/src/interpreter.rs
View File

@@ -1183,11 +1183,17 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) -
PpcOpcode::stw => {
let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] };
let ea = ea.wrapping_add(instr.d() as i64 as u64) as u32;
if let Some(t) = ctx.reservation_table.as_ref().filter(|t| t.is_enabled()) {
if t.has_active_reservers() { t.invalidate_for_write(ea); }
}
mem.write_u32(ea, ctx.gpr[instr.rs()] as u32);
ctx.pc += 4;
}
PpcOpcode::stwu => {
let ea = ctx.gpr[instr.ra()].wrapping_add(instr.d() as i64 as u64) as u32;
if let Some(t) = ctx.reservation_table.as_ref().filter(|t| t.is_enabled()) {
if t.has_active_reservers() { t.invalidate_for_write(ea); }
}
mem.write_u32(ea, ctx.gpr[instr.rs()] as u32);
ctx.gpr[instr.ra()] = ea as u64;
ctx.pc += 4;
@@ -1195,11 +1201,17 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) -
PpcOpcode::stwx => {
let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] };
let ea = ea.wrapping_add(ctx.gpr[instr.rb()]) as u32;
if let Some(t) = ctx.reservation_table.as_ref().filter(|t| t.is_enabled()) {
if t.has_active_reservers() { t.invalidate_for_write(ea); }
}
mem.write_u32(ea, ctx.gpr[instr.rs()] as u32);
ctx.pc += 4;
}
PpcOpcode::stwux => {
let ea = ctx.gpr[instr.ra()].wrapping_add(ctx.gpr[instr.rb()]) as u32;
if let Some(t) = ctx.reservation_table.as_ref().filter(|t| t.is_enabled()) {
if t.has_active_reservers() { t.invalidate_for_write(ea); }
}
mem.write_u32(ea, ctx.gpr[instr.rs()] as u32);
ctx.gpr[instr.ra()] = ea as u64;
ctx.pc += 4;
@@ -1568,6 +1580,9 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) -
PpcOpcode::stwbrx => {
let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] };
let ea = ea.wrapping_add(ctx.gpr[instr.rb()]) as u32;
if let Some(t) = ctx.reservation_table.as_ref().filter(|t| t.is_enabled()) {
if t.has_active_reservers() { t.invalidate_for_write(ea); }
}
mem.write_u32(ea, (ctx.gpr[instr.rs()] as u32).swap_bytes());
ctx.pc += 4;
}
@@ -5207,6 +5222,84 @@ mod tests {
assert_eq!(mem.read_u32(0x1080), 0, "memory not written on failure");
}
// ---------- PPCBUG-107/140: invalidate_for_write via plain stw ----------
/// PPCBUG-107/140: A plain `stw` to a reserved line must invalidate the
/// reservation so that a subsequent `stwcx.` fails (CR0.EQ=0).
#[test]
fn lwarx_then_plain_stw_invalidates_reservation() {
let table = std::sync::Arc::new(crate::ReservationTable::new());
table.enable();
let mut ctx = PpcContext::new();
ctx.reservation_table = Some(table.clone());
ctx.hw_id = 0;
let mut mem = TestMem::new();
// Set up registers: r4=0x1000 (target addr), r5=0 (index), r6=plain store val, r7=stwcx val.
ctx.gpr[4] = 0x1000;
ctx.gpr[5] = 0;
ctx.gpr[6] = 0xBBBB_BBBB;
ctx.gpr[7] = 0xCCCC_CCCC;
// Instr 0: lwarx r3, r4, r5 (opcode 31, XO 20)
let lwarx = (31u32 << 26) | (3 << 21) | (4 << 16) | (5 << 11) | (20 << 1);
write_instr(&mut mem, 0, lwarx);
// Instr 1: stw r6, 0(r4) (opcode 36, D-form)
let stw_plain = (36u32 << 26) | (6 << 21) | (4 << 16) | 0;
write_instr(&mut mem, 4, stw_plain);
// Instr 2: stwcx. r7, r4, r5 (opcode 31, XO 150, Rc=1)
let stwcx = (31u32 << 26) | (7 << 21) | (4 << 16) | (5 << 11) | (150 << 1) | 1;
write_instr(&mut mem, 8, stwcx);
// Execute lwarx — reserves 0x1000's cache line.
ctx.pc = 0;
step(&mut ctx, &mut mem);
assert!(ctx.has_reservation, "lwarx must set has_reservation");
// Execute plain stw — must call invalidate_for_write and clear the reservation.
step(&mut ctx, &mut mem);
assert_eq!(mem.read_u32(0x1000), 0xBBBB_BBBB, "plain stw must land");
// Execute stwcx. — reservation was invalidated; must fail (CR0.EQ=0).
step(&mut ctx, &mut mem);
assert!(!ctx.cr[0].eq, "stwcx. must fail after reservation was invalidated by plain stw");
// Memory must still hold the value from the plain stw, not from stwcx..
assert_eq!(mem.read_u32(0x1000), 0xBBBB_BBBB, "stwcx. must not overwrite on failure");
}
/// Regression: without any intervening store, `lwarx` + `stwcx.` must still
/// succeed (CR0.EQ=1). Ensures the fix didn't accidentally break the happy path.
#[test]
fn lwarx_then_stwcx_succeeds_without_intervening_store() {
let table = std::sync::Arc::new(crate::ReservationTable::new());
table.enable();
let mut ctx = PpcContext::new();
ctx.reservation_table = Some(table.clone());
ctx.hw_id = 0;
let mut mem = TestMem::new();
ctx.gpr[4] = 0x1000;
ctx.gpr[5] = 0;
ctx.gpr[7] = 0xDEAD_BEEF;
// Instr 0: lwarx r3, r4, r5
let lwarx = (31u32 << 26) | (3 << 21) | (4 << 16) | (5 << 11) | (20 << 1);
write_instr(&mut mem, 0, lwarx);
// Instr 1: stwcx. r7, r4, r5
let stwcx = (31u32 << 26) | (7 << 21) | (4 << 16) | (5 << 11) | (150 << 1) | 1;
write_instr(&mut mem, 4, stwcx);
ctx.pc = 0;
step(&mut ctx, &mut mem);
assert!(ctx.has_reservation, "lwarx must set has_reservation");
step(&mut ctx, &mut mem);
assert!(ctx.cr[0].eq, "stwcx. must succeed when reservation is intact");
assert_eq!(mem.read_u32(0x1000), 0xDEAD_BEEF, "stwcx. must write on success");
}
// ---------- Phase 2m: SPR DEC + TBL/TBU write ----------
#[test]