From d96986a10e7a37a32883f323e7b7e0fd39a830ba Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 2 May 2026 13:42:50 +0200 Subject: [PATCH 1/5] fix(cpu): PPCBUG-063/064/065 trap PC + sc LEV + twi typed-trap logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 6 batch 1 — trap/sc semantics. - PPCBUG-063 trap PC: previously ctx.pc was incremented to CIA+4 BEFORE StepResult::Trap returned, forcing handlers to .wrapping_sub(4) to recover the faulting instruction address. Now ctx.pc stays at CIA on trap, matching SRR0 semantics on real hardware. Critical for any future SEH/exception-delivery path (e.g. the Sylpheed C++ throw work). - PPCBUG-065 typed-trap logging: `twi 31, r0, IMM` is the Xbox 360 CRT/kernel typed-trap convention encoding C++ exception class via SIMM. The trace now logs the SIMM type code when this pattern fires. Routing the type code via a StepResult payload requires an enum extension (multiple consumer sites) that's deferred. - PPCBUG-064 sc LEV logging: `sc 2` is the Xbox 360 hypervisor-call convention; canary dispatches it to a different handler than `sc 0`. Now logs a warning when LEV != 0. Routing LEV=2 to a HypervisorCall variant also requires a StepResult enum extension; deferred. The two enum-extension follow-ups can land as a structural sub-batch once a clear consumer (SEH dispatch, hypervisor-call HLE) is in place. Co-Authored-By: Claude Sonnet 4.6 --- crates/xenia-cpu/src/interpreter.rs | 31 ++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/crates/xenia-cpu/src/interpreter.rs b/crates/xenia-cpu/src/interpreter.rs index 31d54f5..4f0b8e6 100644 --- a/crates/xenia-cpu/src/interpreter.rs +++ b/crates/xenia-cpu/src/interpreter.rs @@ -982,6 +982,16 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - // ===== System call ===== PpcOpcode::sc => { + // PPCBUG-064: log non-zero LEV (`sc 2` is the Xbox 360 hypervisor-call + // convention; canary dispatches it to a different handler than `sc 0`). + // Routing LEV=2 requires a StepResult variant extension; deferred. + let lev = (instr.raw >> 5) & 0x7F; + if lev != 0 { + tracing::warn!( + "sc with LEV={} at {:#010x}: dispatched as plain SystemCall (HVcall routing not implemented)", + lev, ctx.pc + ); + } ctx.pc += 4; return StepResult::SystemCall; } @@ -1733,6 +1743,14 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - // ===== Trap ===== PpcOpcode::tw | PpcOpcode::twi | PpcOpcode::td | PpcOpcode::tdi => { + // PPCBUG-063: save CIA before incrementing so a trap handler reads + // the faulting instruction address, not CIA+4. + // PPCBUG-065: log the SIMM type code on `twi 31, r0, IMM` (Xbox 360 + // typed-trap convention used by the CRT/kernel for C++ exception + // class dispatch). The audit notes this is relevant to the Sylpheed + // throw investigation; routing the type code via a payload requires + // a StepResult enum extension that's deferred for now. + let trap_pc = ctx.pc; let a = ctx.gpr[instr.ra()]; let b = match instr.opcode { PpcOpcode::twi | PpcOpcode::tdi => instr.simm16() as i64 as u64, @@ -1743,14 +1761,21 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - _ => trap::TrapWidth::Doubleword, }; let fired = trap::evaluate(instr.to(), a, b, width); - ctx.pc += 4; if fired { + let typed_trap_simm = if matches!(instr.opcode, PpcOpcode::twi) + && instr.to() == 31 && instr.ra() == 0 { + Some(instr.simm16() as u16) + } else { None }; tracing::warn!( - "Trap fired at {:#010x}: {:?} TO={} a={:#x} b={:#x}", - ctx.pc.wrapping_sub(4), instr.opcode, instr.to(), a, b + "Trap fired at {:#010x}: {:?} TO={} a={:#x} b={:#x}{}", + trap_pc, instr.opcode, instr.to(), a, b, + typed_trap_simm.map_or(String::new(), |t| format!(" typed_trap_simm={:#06x}", t)) ); + // Leave ctx.pc at CIA (NOT NIA) so trap handlers / SEH delivery + // can read the faulting instruction address from ctx.pc. return StepResult::Trap; } + ctx.pc += 4; } // ===== Byte-reverse loads ===== From 68c0ee55ce415bccad933ca2b848daa475e02252 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 2 May 2026 13:48:03 +0200 Subject: [PATCH 2/5] fix(cpu): PPCBUG-123/124/125/126/161/162/566 XER TBC + lswi/stswi/lmw MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 6 batch 2 — XER TBC enabling + load/store-multiple cleanups. - PPCBUG-123/124/161/566 (coupled): XER TBC field was unmodelled — `ctx.xer()` always returned 0 in bits 0-6, and `ctx.set_xer()` silently discarded any TBC writes. Result: `lswx` and `stswx` were permanent no-ops (the `while bytes_left > 0` loop never executed). Fix: add `pub xer_tbc: u8` to `PpcContext`; wire into `xer()` and `set_xer()`. Initialize to 0 in `PpcContext::new()`. lswx/stswx bodies are correct as-is once the infrastructure is wired. - PPCBUG-125 lmw: PowerISA marks `lmw rT, D(rA)` invalid when rA is in [rT..31]; canary skips the write to rA to preserve the EA base. Now matches canary. - PPCBUG-126/162 lswi/stswi: replaced `instr.rb()` with `instr.nb()` for the NB field. Both accessors return identical values today (bits 16-20), but the maintenance hazard from the misnomer is now removed. A future `rb()` type-system refactor would have broken lswi/stswi silently. Co-Authored-By: Claude Sonnet 4.6 --- crates/xenia-cpu/src/context.rs | 11 ++++++++++- crates/xenia-cpu/src/interpreter.rs | 10 ++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/crates/xenia-cpu/src/context.rs b/crates/xenia-cpu/src/context.rs index 5826120..60e658f 100644 --- a/crates/xenia-cpu/src/context.rs +++ b/crates/xenia-cpu/src/context.rs @@ -85,6 +85,10 @@ pub struct PpcContext { pub xer_ca: u8, pub xer_ov: u8, pub xer_so: u8, + /// XER[25:31] string-byte count (`TBC`). Read/written by `mtspr XER`, + /// consumed by `lswx`/`stswx`. Per PPCBUG-123/124/161: was previously + /// unmodelled, making `lswx`/`stswx` a permanent no-op. + pub xer_tbc: u8, // Altivec VSCR. Only bits 16 (NJ) and 31 (SAT) of word 3 are meaningful. pub vscr: Vec128, // VRSAVE (SPR 256). Bitmask of which VRs need saving across context switches. @@ -157,6 +161,7 @@ impl PpcContext { xer_ca: 0, xer_ov: 0, xer_so: 0, + xer_tbc: 0, // VSCR starts with NJ bit set (denormals flushed) — matches canary // thread_state.cc initialization. vscr: Vec128::from_u32x4(0, 0, 0, VSCR_NJ_MASK), @@ -240,7 +245,10 @@ impl PpcContext { /// Get the full XER register value. pub fn xer(&self) -> u32 { - ((self.xer_so as u32) << 31) | ((self.xer_ov as u32) << 30) | ((self.xer_ca as u32) << 29) + ((self.xer_so as u32) << 31) + | ((self.xer_ov as u32) << 30) + | ((self.xer_ca as u32) << 29) + | (self.xer_tbc as u32) // PPCBUG-123/566: bits 0-6 (TBC). } /// Set XER from a full 32-bit value. @@ -248,6 +256,7 @@ impl PpcContext { self.xer_so = ((val >> 31) & 1) as u8; self.xer_ov = ((val >> 30) & 1) as u8; self.xer_ca = ((val >> 29) & 1) as u8; + self.xer_tbc = (val & 0x7F) as u8; // PPCBUG-124. } /// Read the VSCR SAT (sticky saturation) bit. diff --git a/crates/xenia-cpu/src/interpreter.rs b/crates/xenia-cpu/src/interpreter.rs index 4f0b8e6..8143d7d 100644 --- a/crates/xenia-cpu/src/interpreter.rs +++ b/crates/xenia-cpu/src/interpreter.rs @@ -1520,7 +1520,7 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - // String load/store PpcOpcode::lswi => { let mut ea = if instr.ra() == 0 { 0u32 } else { ctx.gpr[instr.ra()] as u32 }; - let nb = if instr.rb() == 0 { 32 } else { instr.rb() as u32 }; + let nb = if instr.nb() == 0 { 32 } else { instr.nb() }; let mut rd = instr.rd(); let mut bytes_left = nb; while bytes_left > 0 { @@ -1539,7 +1539,7 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - } PpcOpcode::stswi => { let mut ea = if instr.ra() == 0 { 0u32 } else { ctx.gpr[instr.ra()] as u32 }; - let nb = if instr.rb() == 0 { 32 } else { instr.rb() as u32 }; + let nb = if instr.nb() == 0 { 32 } else { instr.nb() }; let mut rs = instr.rs(); let mut bytes_left = nb; if let Some(t) = ctx.reservation_table.as_ref().filter(|t| t.is_enabled()) { @@ -1707,9 +1707,15 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - // ===== Load multiple ===== PpcOpcode::lmw => { + // PPCBUG-125: PowerISA marks `lmw` invalid when rA is in [rT..31]; + // canary skips the write to rA in that case to preserve the EA base. let mut ea = if instr.ra() == 0 { 0u64 } else { ctx.gpr[instr.ra()] }; ea = ea.wrapping_add(instr.d() as i64 as u64); for r in instr.rd()..32 { + if r == instr.ra() { + ea = ea.wrapping_add(4); + continue; + } ctx.gpr[r] = mem.read_u32(ea as u32) as u64; ea = ea.wrapping_add(4); } From 0f2a26c460d7810332cdabbdd8a00a67c0998574 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 2 May 2026 13:50:10 +0200 Subject: [PATCH 3/5] fix(cpu): PPCBUG-068/078/080 mcrfs VX recompute + mtmsrd L=1 + mfvscr zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 6 batch 3 — SPR/MSR/VSCR semantics. - PPCBUG-078 mtmsrd L=1: PowerISA requires partial-MSR-write — only MSR[EE] (u64 bit 15) and MSR[RI] (u64 bit 0) modified, all other MSR bits preserved. Used by kernel code to toggle external interrupts. Previously merged with mtmsr (full overwrite), silently corrupting MSR for any L=1 caller. - PPCBUG-080 mfvscr: ISA places VSCR in the rightmost word of VD with bytes 0-11 zeroed. Previously copied the full 128-bit ctx.vscr, leaking stale upper data to guest. Now zero-extends per canary. - PPCBUG-068 mcrfs VX summary: when mcrfs clears VX* exception bits, the VX summary bit at FPSCR[2] must be recomputed (clears if all contributors are 0; remains 1 otherwise). Previously left stale, causing subsequent CR-test sequences to misread the FPU state. Co-Authored-By: Claude Sonnet 4.6 --- crates/xenia-cpu/src/interpreter.rs | 32 +++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/crates/xenia-cpu/src/interpreter.rs b/crates/xenia-cpu/src/interpreter.rs index 8143d7d..a606b4d 100644 --- a/crates/xenia-cpu/src/interpreter.rs +++ b/crates/xenia-cpu/src/interpreter.rs @@ -1647,7 +1647,18 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - ctx.pc += 4; } PpcOpcode::mtmsr | PpcOpcode::mtmsrd => { - ctx.msr = ctx.gpr[instr.rs()]; + // PPCBUG-078: mtmsrd L=1 is a partial-MSR-write — only MSR[EE] + // (u64 bit 15) and MSR[RI] (u64 bit 0) are modified; all other + // MSR bits preserved. Used by kernel code to re-enable external + // interrupts without disturbing the rest of the MSR. + let l = (instr.raw >> (31 - 15)) & 1; + let rs = ctx.gpr[instr.rs()]; + if matches!(instr.opcode, PpcOpcode::mtmsrd) && l == 1 { + let mask: u64 = (1u64 << 15) | 1u64; + ctx.msr = (ctx.msr & !mask) | (rs & mask); + } else { + ctx.msr = rs; + } ctx.pc += 4; } PpcOpcode::mftb => { @@ -2493,7 +2504,11 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - // SAT (bit 31) are defined. Canary stores the full Vec128 so we do // the same: mfvscr copies the register, mtvscr overwrites it. PpcOpcode::mfvscr => { - ctx.vr[instr.rd()] = ctx.vscr; + // PPCBUG-080: ISA places VSCR in the rightmost word of VD with + // bytes 0-11 zeroed. Previously the full 128-bit ctx.vscr was + // copied (leaking stale upper data to guest). + let vscr_word = ctx.vscr.as_u32x4()[3]; + ctx.vr[instr.rd()] = xenia_types::Vec128::from_u32x4_array([0, 0, 0, vscr_word]); ctx.pc += 4; } PpcOpcode::mtvscr => { @@ -4717,6 +4732,19 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - (1 << (31 - 21)) | (1 << (31 - 22)) | (1 << (31 - 23)); let nibble_mask = 0xFu32 << shift; ctx.fpscr &= !(nibble_mask & CLEARABLE_MASK); + // PPCBUG-068: recompute the VX summary bit. If any VX* exception + // bit remains set, VX must remain set; if all are cleared, VX + // must clear. (FEX recomputation omitted — xenia doesn't model + // enabled-exception dispatch.) + const VX_ALL_MASK: u32 = + fpscr::VXSNAN | fpscr::VXISI | fpscr::VXIDI | + fpscr::VXZDZ | fpscr::VXIMZ | fpscr::VXVC | + fpscr::VXSOFT | fpscr::VXSQRT | fpscr::VXCVI; + if ctx.fpscr & VX_ALL_MASK != 0 { + ctx.fpscr |= fpscr::VX; + } else { + ctx.fpscr &= !fpscr::VX; + } ctx.pc += 4; } From 99e781483676c8e1cb4d056f9d1453159a0dc5c6 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 2 May 2026 13:51:43 +0200 Subject: [PATCH 4/5] test(cpu): PPCBUG-022 verify mulld_ov INT_MIN*-1 + auto-resolved markers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 6 batch 4 — overflow/cleanup verification. - PPCBUG-022 mulld_ov INT_MIN * -1: the audit-claimed missing edge case is actually handled by `i64::checked_mul()` (returns None when the result would be -i64::MIN = i64::MAX+1, which doesn't fit). New regression tests in overflow.rs confirm: INT_MIN * -1 overflows; INT_MIN * 1 doesn't; (INT_MIN+1) * -1 = INT_MAX, no overflow. Audit's claim was incorrect; documented by the new tests. - PPCBUG-021 (overflow.rs OE checks at bit 63): largely auto-resolved by P4 batch 6 (16993bb), which switched all 32-bit ABI ops to inline `true_sum != (result32 as i32) as i128`. Helpers like add_ov_64 are now only called from 64-bit ABI ops where bit-63 is correct. - PPCBUG-027 (rlwimix upper-32 zeros): auto-resolved by P4 (rlwimix now writes via `as u32 as u64` truncation). - PPCBUG-039 (cntlzdx 32-bit-ABI): wontfix per audit — only matters if a 32-bit-ABI binary emits cntlzd, which compilers don't. Remaining low-impact items (PPCBUG-642 ISA-undefined fmt_bcctr decr, PPCBUG-643/644 SIMM/D-form hex display, PPCBUG-367/368 vupkhpx/vpkpx channel ordering, PPCBUG-487/495 vsum operand naming, PPCBUG-515/516 lvebx/lvsr documentation, PPCBUG-601 decode_op6 invariant doc) are left for a P9 or follow-up batch — they're cosmetic/test-coverage items rather than correctness bugs. Co-Authored-By: Claude Sonnet 4.6 --- crates/xenia-cpu/src/overflow.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/xenia-cpu/src/overflow.rs b/crates/xenia-cpu/src/overflow.rs index a55f505..79bba9c 100644 --- a/crates/xenia-cpu/src/overflow.rs +++ b/crates/xenia-cpu/src/overflow.rs @@ -162,6 +162,11 @@ mod tests { fn mulld_overflows() { assert!(mulld_ov(i64::MAX, 2)); assert!(!mulld_ov(i64::MAX, 1)); + // PPCBUG-022: INT_MIN * -1 overflows (=-INT_MIN > INT_MAX). + // checked_mul correctly returns None for this case. + assert!(mulld_ov(i64::MIN, -1), "INT_MIN * -1 overflows i64"); + assert!(!mulld_ov(i64::MIN, 1)); + assert!(!mulld_ov(i64::MIN + 1, -1), "INT_MIN+1 * -1 = INT_MAX, no overflow"); } #[test] From 5ece5e315fe3d0ebc1786621e017bc242e0a6e1f Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 2 May 2026 13:56:34 +0200 Subject: [PATCH 5/5] refactor(cpu): mcrfs uses fpscr::VX_ALL constant per reviewer nit P6 review nit: replace the inline `const VX_ALL_MASK` in the mcrfs arm with the existing `fpscr::VX_ALL` constant (single source of truth). Behaviorally identical. Co-Authored-By: Claude Sonnet 4.6 --- crates/xenia-cpu/src/interpreter.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/xenia-cpu/src/interpreter.rs b/crates/xenia-cpu/src/interpreter.rs index a606b4d..00a6108 100644 --- a/crates/xenia-cpu/src/interpreter.rs +++ b/crates/xenia-cpu/src/interpreter.rs @@ -4736,11 +4736,7 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - // bit remains set, VX must remain set; if all are cleared, VX // must clear. (FEX recomputation omitted — xenia doesn't model // enabled-exception dispatch.) - const VX_ALL_MASK: u32 = - fpscr::VXSNAN | fpscr::VXISI | fpscr::VXIDI | - fpscr::VXZDZ | fpscr::VXIMZ | fpscr::VXVC | - fpscr::VXSOFT | fpscr::VXSQRT | fpscr::VXCVI; - if ctx.fpscr & VX_ALL_MASK != 0 { + if ctx.fpscr & fpscr::VX_ALL != 0 { ctx.fpscr |= fpscr::VX; } else { ctx.fpscr &= !fpscr::VX;