fix(cpu): PPCBUG-107 PPCBUG-140-144 add invalidate_for_write to word stores
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:
@@ -1183,11 +1183,17 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) -
|
|||||||
PpcOpcode::stw => {
|
PpcOpcode::stw => {
|
||||||
let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] };
|
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;
|
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);
|
mem.write_u32(ea, ctx.gpr[instr.rs()] as u32);
|
||||||
ctx.pc += 4;
|
ctx.pc += 4;
|
||||||
}
|
}
|
||||||
PpcOpcode::stwu => {
|
PpcOpcode::stwu => {
|
||||||
let ea = ctx.gpr[instr.ra()].wrapping_add(instr.d() as i64 as u64) as u32;
|
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);
|
mem.write_u32(ea, ctx.gpr[instr.rs()] as u32);
|
||||||
ctx.gpr[instr.ra()] = ea as u64;
|
ctx.gpr[instr.ra()] = ea as u64;
|
||||||
ctx.pc += 4;
|
ctx.pc += 4;
|
||||||
@@ -1195,11 +1201,17 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) -
|
|||||||
PpcOpcode::stwx => {
|
PpcOpcode::stwx => {
|
||||||
let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] };
|
let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] };
|
||||||
let ea = ea.wrapping_add(ctx.gpr[instr.rb()]) as u32;
|
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);
|
mem.write_u32(ea, ctx.gpr[instr.rs()] as u32);
|
||||||
ctx.pc += 4;
|
ctx.pc += 4;
|
||||||
}
|
}
|
||||||
PpcOpcode::stwux => {
|
PpcOpcode::stwux => {
|
||||||
let ea = ctx.gpr[instr.ra()].wrapping_add(ctx.gpr[instr.rb()]) as u32;
|
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);
|
mem.write_u32(ea, ctx.gpr[instr.rs()] as u32);
|
||||||
ctx.gpr[instr.ra()] = ea as u64;
|
ctx.gpr[instr.ra()] = ea as u64;
|
||||||
ctx.pc += 4;
|
ctx.pc += 4;
|
||||||
@@ -1568,6 +1580,9 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) -
|
|||||||
PpcOpcode::stwbrx => {
|
PpcOpcode::stwbrx => {
|
||||||
let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] };
|
let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] };
|
||||||
let ea = ea.wrapping_add(ctx.gpr[instr.rb()]) as u32;
|
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());
|
mem.write_u32(ea, (ctx.gpr[instr.rs()] as u32).swap_bytes());
|
||||||
ctx.pc += 4;
|
ctx.pc += 4;
|
||||||
}
|
}
|
||||||
@@ -5207,6 +5222,84 @@ mod tests {
|
|||||||
assert_eq!(mem.read_u32(0x1080), 0, "memory not written on failure");
|
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 ----------
|
// ---------- Phase 2m: SPR DEC + TBL/TBU write ----------
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
Reference in New Issue
Block a user