From 24d347436a4daec9f8d7fa3625c1bbbc542c45cd Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Fri, 1 May 2026 17:13:31 +0200 Subject: [PATCH] fix(cpu): PPCBUG-130 PPCBUG-150 add invalidate_for_write to byte/halfword/doubleword stores Continuation of the PPCBUG-107 cascade sweep (batch 1: word stores landed in 4538fa9). Plain stb/stbu/stbx/stbux, sth/sthu/sthx/sthux/sthbrx, and std/stdu/stdx/stdux/stdbrx now invalidate the reservation table before writing, so cross-thread lwarx/stwcx. atomicity holds when these widths are written by another host thread. Affected: PPCBUG-130 9 byte/halfword stores missing invalidate_for_write stb, stbu, stbx, stbux, sth, sthu, sthx, sthux, sthbrx PPCBUG-150 5 doubleword stores missing invalidate_for_write std, stdu, stdx, stdux, stdbrx Tests: lwarx_then_plain_stb_invalidates_reservation, lwarx_then_plain_std_invalidates_reservation Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/xenia-cpu/src/interpreter.rs | 132 ++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/crates/xenia-cpu/src/interpreter.rs b/crates/xenia-cpu/src/interpreter.rs index 3eb9309..f76bd0f 100644 --- a/crates/xenia-cpu/src/interpreter.rs +++ b/crates/xenia-cpu/src/interpreter.rs @@ -1219,11 +1219,17 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::stb => { 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_u8(ea, ctx.gpr[instr.rs()] as u8); ctx.pc += 4; } PpcOpcode::stbu => { 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_u8(ea, ctx.gpr[instr.rs()] as u8); ctx.gpr[instr.ra()] = ea as u64; ctx.pc += 4; @@ -1231,11 +1237,17 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::stbx => { 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_u8(ea, ctx.gpr[instr.rs()] as u8); ctx.pc += 4; } PpcOpcode::stbux => { 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_u8(ea, ctx.gpr[instr.rs()] as u8); ctx.gpr[instr.ra()] = ea as u64; ctx.pc += 4; @@ -1243,11 +1255,17 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::sth => { 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_u16(ea, ctx.gpr[instr.rs()] as u16); ctx.pc += 4; } PpcOpcode::sthu => { 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_u16(ea, ctx.gpr[instr.rs()] as u16); ctx.gpr[instr.ra()] = ea as u64; ctx.pc += 4; @@ -1255,11 +1273,17 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::sthx => { 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_u16(ea, ctx.gpr[instr.rs()] as u16); ctx.pc += 4; } PpcOpcode::sthux => { 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_u16(ea, ctx.gpr[instr.rs()] as u16); ctx.gpr[instr.ra()] = ea as u64; ctx.pc += 4; @@ -1267,23 +1291,35 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::std => { let ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] }; let ea = ea.wrapping_add(instr.ds() 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_u64(ea, ctx.gpr[instr.rs()]); ctx.pc += 4; } PpcOpcode::stdx => { 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_u64(ea, ctx.gpr[instr.rs()]); ctx.pc += 4; } PpcOpcode::stdu => { let ea = ctx.gpr[instr.ra()].wrapping_add(instr.ds() 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_u64(ea, ctx.gpr[instr.rs()]); ctx.gpr[instr.ra()] = ea as u64; ctx.pc += 4; } PpcOpcode::stdux => { 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_u64(ea, ctx.gpr[instr.rs()]); ctx.gpr[instr.ra()] = ea as u64; ctx.pc += 4; @@ -1589,6 +1625,9 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::sthbrx => { 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_u16(ea, (ctx.gpr[instr.rs()] as u16).swap_bytes()); ctx.pc += 4; } @@ -4175,6 +4214,9 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - } PpcOpcode::stdbrx => { let ea = ea_indexed(ctx, instr); + 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_u64(ea, ctx.gpr[instr.rs()].swap_bytes()); ctx.pc += 4; } @@ -5300,6 +5342,96 @@ mod tests { assert_eq!(mem.read_u32(0x1000), 0xDEAD_BEEF, "stwcx. must write on success"); } + // ---------- PPCBUG-130: invalidate_for_write via plain stb ---------- + + /// PPCBUG-130: A plain `stb` to a reserved line must invalidate the + /// reservation so that a subsequent `stwcx.` fails (CR0.EQ=0). + #[test] + fn lwarx_then_plain_stb_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(); + + // r4=0x1000 (target addr), r5=0 (index), r6=byte store val, r7=stwcx val. + ctx.gpr[4] = 0x1000; + ctx.gpr[5] = 0; + ctx.gpr[6] = 0xAB; + 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: stb r6, 0(r4) (opcode 38, D-form) + let stb_plain = (38u32 << 26) | (6 << 21) | (4 << 16) | 0; + write_instr(&mut mem, 4, stb_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 stb — must call invalidate_for_write and clear the reservation. + step(&mut ctx, &mut mem); + assert_eq!(mem.read_u8(0x1000), 0xAB, "plain stb 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 stb"); + assert_eq!(mem.read_u8(0x1000), 0xAB, "stwcx. must not overwrite on failure"); + } + + // ---------- PPCBUG-150: invalidate_for_write via plain std ---------- + + /// PPCBUG-150: A plain `std` to a reserved line must invalidate the + /// reservation so that a subsequent `stwcx.` fails (CR0.EQ=0). + #[test] + fn lwarx_then_plain_std_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(); + + // r4=0x1000 (target addr), r5=0 (index), r6=doubleword store val, r7=stwcx val. + ctx.gpr[4] = 0x1000; + ctx.gpr[5] = 0; + ctx.gpr[6] = 0xDEADBEEF_CAFEBABEu64; + 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: std r6, 0(r4) (opcode 62, DS-form, XO=0b00) + let std_plain = (62u32 << 26) | (6 << 21) | (4 << 16) | 0; + write_instr(&mut mem, 4, std_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 std — must call invalidate_for_write and clear the reservation. + step(&mut ctx, &mut mem); + assert_eq!(mem.read_u64(0x1000), 0xDEADBEEF_CAFEBABEu64, "plain std 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 std"); + assert_eq!(mem.read_u64(0x1000), 0xDEADBEEF_CAFEBABEu64, "stwcx. must not overwrite on failure"); + } + // ---------- Phase 2m: SPR DEC + TBL/TBU write ---------- #[test]