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

Merge branch 'ppc-audit-fix/p6-medium' — Phase 6 Other MEDIUM correctness

Browse Source 
Phase 6 of the PPC instruction audit fix application: misc MEDIUM
correctness items across trap/sc, XER TBC, MSR/VSCR/FPSCR semantics.
~13 PPCBUGs landed across 4 batches.

- d96986a: Batch 1 — trap PC fix, sc LEV logging, twi typed-trap logging
  (PPCBUG-063/064/065)
- 68c0ee5: Batch 2 — XER TBC infrastructure (enabling lswx/stswx) +
  lswi/stswi nb fix + lmw RA-skip (PPCBUG-123/124/125/126/161/162/566)
- 0f2a26c: Batch 3 — mcrfs VX recompute, mtmsrd L=1 partial, mfvscr zero
  (PPCBUG-068/078/080)
- 99e7814: Batch 4 — mulld_ov INT_MIN*-1 verification + auto-resolved
  markers for PPCBUG-021/022/027/039
- 5ece5e3: review-fix nit — mcrfs uses existing fpscr::VX_ALL constant

Independent reviewer verdict: all 4 commits LGTM, one cosmetic nit
(applied immediately in 5ece5e3). Audit fix-shapes match canary
prescriptions; trap-PC change verified against all StepResult::Trap
consumers; XER TBC field initialization verified through the single
PpcContext::new() construction path.

Two structural enum extensions deferred (not yet needed by any
consumer):
- StepResult::HypervisorCall variant (would enable PPCBUG-064 routing
  for sc 2)
- StepResult::Trap { type_code: u16 } payload (would enable PPCBUG-065
  routing for typed C++ traps; relevant if SEH dispatch is added)

Cosmetic / test-coverage items left for future cleanup batch:
PPCBUG-642 (cosmetic disasm), PPCBUG-643/644 (SIMM/D-form hex display),
PPCBUG-367/368 (vupkhpx/vpkpx channel ordering), PPCBUG-487/495 (vsum
naming), PPCBUG-515/516 (lvebx/lvsr docs), PPCBUG-601 (decode_op6 doc).

Verification at merge: cargo test --workspace --release reports 498
passed, 0 failed. Acid test deferred to end of all phases.
This commit is contained in:
MechaCat02
2026-05-02 13:57:00 +02:00
parent 9f88e275b8 5ece5e315f
commit 112202c2b9
3 changed files with 77 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
crates/xenia-cpu/src/context.rs
View File

@@ -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.

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

@@ -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;
}
@@ -1510,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 {
@@ -1529,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()) {
@@ -1637,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 => {
@@ -1697,9 +1718,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);
}
@@ -1733,6 +1760,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 +1778,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 =====
@@ -2462,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 => {
@@ -4686,6 +4732,15 @@ 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.)
if ctx.fpscr & fpscr::VX_ALL != 0 {
ctx.fpscr |= fpscr::VX;
} else {
ctx.fpscr &= !fpscr::VX;
}
ctx.pc += 4;
}

5
crates/xenia-cpu/src/overflow.rs
View File

@@ -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]