diff --git a/crates/xenia-app/tests/golden/sylpheed_n50m.json b/crates/xenia-app/tests/golden/sylpheed_n50m.json index 552dc43..f3d5ba1 100644 --- a/crates/xenia-app/tests/golden/sylpheed_n50m.json +++ b/crates/xenia-app/tests/golden/sylpheed_n50m.json @@ -1,10 +1,10 @@ { - "instructions": 50000003, - "imports": 451508, + "instructions": 50000001, + "imports": 451499, "unimpl": 0, - "draws": 0, - "swaps": 2, - "unique_render_targets": 0, - "shader_blobs_live": 0, + "draws": 78, + "swaps": 3, + "unique_render_targets": 2, + "shader_blobs_live": 3, "texture_cache_entries": 0 } diff --git a/crates/xenia-gpu/src/gpu_system.rs b/crates/xenia-gpu/src/gpu_system.rs index bf27114..fbc5bd7 100644 --- a/crates/xenia-gpu/src/gpu_system.rs +++ b/crates/xenia-gpu/src/gpu_system.rs @@ -603,14 +603,21 @@ impl GpuSystem { /// Release. pub fn sync_with_mmio(&mut self) { let wptr_dwords = self.mmio.cp_rb_wptr.load(Ordering::Acquire); - if wptr_dwords != self.ring.write_offset_dwords && self.ring.size_dwords != 0 { - self.ring.write_offset_dwords = wptr_dwords % self.ring.size_dwords; + // CP_RB_WPTR governs ONLY the primary ring. While an indirect buffer + // is executing, the active `self.ring` is a fixed linear sub-stream + // and the primary ring is saved at the bottom of the IB stack — + // applying the (primary) write pointer to the IB would corrupt its + // extent (e.g. `wptr % ib_size`) and strand the GPU mid-buffer. + let primary = self.ib_stack.first_mut().unwrap_or(&mut self.ring); + if wptr_dwords != primary.write_offset_dwords && primary.size_dwords != 0 { + primary.write_offset_dwords = wptr_dwords % primary.size_dwords; } - // Mirror our read pointer (Release pairs with any guest-side + let primary_rptr = primary.read_offset_dwords; + // Mirror the *primary* read pointer (Release pairs with any guest-side // Acquire-load of CP_RB_RPTR for ring writeback bookkeeping). self.mmio .cp_rb_rptr - .store(self.ring.read_offset_dwords, Ordering::Release); + .store(primary_rptr, Ordering::Release); } /// True iff `execute_one` is expected to make progress without blocking. @@ -618,7 +625,11 @@ impl GpuSystem { if let Some(block) = &self.pending_block { return block.is_satisfied(mem, &self.register_file); } - self.ring.has_pending() + // Pending work may be in the active ring OR in a saved caller ring + // further down the IB stack (an exhausted IB still needs `execute_one` + // to pop back and resume the primary ring, whose WPTR may have since + // advanced). + self.ring.has_pending() || self.ib_stack.iter().any(|r| r.has_pending()) } /// Execute exactly one PM4 packet. Returns [`ExecOutcome::Idle`] when @@ -730,13 +741,21 @@ impl GpuSystem { /// Called by `VdInitializeRingBuffer` to give us the primary ring. pub fn initialize_ring_buffer(&mut self, base: u32, size_log2: u32) { - let size_bytes = 1u32 << size_log2.min(31); + // Canary `CommandProcessor::InitializeRingBuffer` (command_processor.cc: + // 436): `primary_buffer_size_ = 1 << (size_log2 + 3)` *bytes*. The + // `VdInitializeRingBuffer` `r4` argument is log2(size-in-quadwords), + // so the byte size is `1 << (size_log2 + 3)` (× 8 bytes/quadword), i.e. + // `1 << (size_log2 + 1)` dwords. (Sylpheed passes size_log2=12 → + // 32768 bytes / 8192 dwords; the previous `1 << size_log2` undersized + // the ring 8× and desynced WPTR wrap math from the guest.) + let size_bytes = 1u32 << size_log2.saturating_add(3).min(31); // The guest hands us a bare *physical* ring base; project it onto the // committed backing window so ring reads hit real PM4 packets (see // `physical_to_backing`). let base = physical_to_backing(base); self.ring.base = base; self.ring.size_dwords = size_bytes / 4; + self.ring.indirect = false; self.ring.read_offset_dwords = 0; // `write_offset` is driven by the guest — start at 0 so the ring // appears empty until MMIO writes advance it. @@ -935,6 +954,10 @@ impl GpuSystem { write_offset_dwords: ib_size, // IB is fully-written at jump time rptr_writeback_addr: 0, rptr_writeback_block_dwords: 0, + // Linear sub-stream: drain [0, ib_size) then pop. Never + // wraps, and `sync_with_mmio`'s CP_RB_WPTR must not touch + // it (canary executes IBs through a separate reader). + indirect: true, }; tracing::debug!( ib_ptr = format_args!("{ib_ptr:#010x}"), diff --git a/crates/xenia-gpu/src/ring_view.rs b/crates/xenia-gpu/src/ring_view.rs index a164d73..df8bf64 100644 --- a/crates/xenia-gpu/src/ring_view.rs +++ b/crates/xenia-gpu/src/ring_view.rs @@ -32,6 +32,16 @@ pub struct RingBufferView { /// `VdEnableRingBufferRPtrWriteBack`). We always write back eagerly, so /// we don't actually use this for scheduling — kept for observability. pub rptr_writeback_block_dwords: u32, + /// True for an indirect-buffer (`INDIRECT_BUFFER`) view. An IB is a fixed + /// *linear* sub-stream, not a circular ring: it is fully written when the + /// GPU jumps to it, so the read pointer advances monotonically from `0` to + /// `size_dwords` and then the buffer is exhausted (the caller ring is + /// popped). It must NOT wrap, and the primary `CP_RB_WPTR` must not be + /// applied to it. Mirrors canary `ExecuteIndirectBuffer`, which executes + /// the IB through a separate `RingBuffer reader_` and restores the primary + /// reader afterward (command_processor.cc). Circular (primary-ring) + /// semantics are used when this is `false`. + pub indirect: bool, } impl RingBufferView { @@ -46,7 +56,16 @@ impl RingBufferView { /// True if there is pending unread data to consume. pub fn has_pending(&self) -> bool { - self.is_initialized() && self.read_offset_dwords != self.write_offset_dwords + if !self.is_initialized() { + return false; + } + if self.indirect { + // Linear sub-stream: exhausted once the read pointer reaches the + // (fixed) write pointer. Never wraps. + self.read_offset_dwords < self.write_offset_dwords + } else { + self.read_offset_dwords != self.write_offset_dwords + } } /// Number of dwords we can consume without wrapping past the write ptr. @@ -54,7 +73,10 @@ impl RingBufferView { if !self.is_initialized() { return 0; } - if self.write_offset_dwords >= self.read_offset_dwords { + if self.indirect { + self.write_offset_dwords + .saturating_sub(self.read_offset_dwords) + } else if self.write_offset_dwords >= self.read_offset_dwords { self.write_offset_dwords - self.read_offset_dwords } else { // write has wrapped — we can read up to the end of the ring. @@ -62,13 +84,19 @@ impl RingBufferView { } } - /// Advance the read pointer by `dwords`, wrapping at `size_dwords`. + /// Advance the read pointer by `dwords`. Circular rings wrap at + /// `size_dwords`; an indirect buffer advances linearly (no wrap) so it + /// terminates exactly at its fixed write pointer. pub fn advance_read(&mut self, dwords: u32) { if self.size_dwords == 0 { return; } - self.read_offset_dwords = - (self.read_offset_dwords + dwords) % self.size_dwords; + if self.indirect { + self.read_offset_dwords = self.read_offset_dwords.saturating_add(dwords); + } else { + self.read_offset_dwords = + (self.read_offset_dwords + dwords) % self.size_dwords; + } } /// Guest address for the dword at relative offset `i` from the current @@ -77,7 +105,11 @@ impl RingBufferView { if !self.is_initialized() { return None; } - let off = (self.read_offset_dwords + offset_dwords) % self.size_dwords; + let off = if self.indirect { + self.read_offset_dwords.saturating_add(offset_dwords) + } else { + (self.read_offset_dwords + offset_dwords) % self.size_dwords + }; Some(self.base.wrapping_add(off.wrapping_mul(4))) } } @@ -120,4 +152,52 @@ mod tests { assert_eq!(v.addr_at_offset(1), Some(0x4000_0000)); assert_eq!(v.addr_at_offset(2), Some(0x4000_0004)); } + + #[test] + fn indirect_buffer_drains_linearly_and_terminates() { + // An indirect buffer is a fixed linear sub-stream: read advances from + // 0 to `size_dwords` and then is exhausted — it must NOT wrap back to + // 0 (which previously caused an infinite re-read of a system command + // buffer; iterate-2O). write_offset == size, exactly as the + // INDIRECT_BUFFER handler sets it. + let mut ib = RingBufferView { + base: 0x4adf_5080, + size_dwords: 11, + read_offset_dwords: 0, + write_offset_dwords: 11, + rptr_writeback_addr: 0, + rptr_writeback_block_dwords: 0, + indirect: true, + }; + assert!(ib.has_pending()); + // Drain the exact packet layout observed for Sylpheed's init IB: + // 2 + 3 + 6 dwords = 11. + ib.advance_read(2); + assert!(ib.has_pending()); + ib.advance_read(3); + assert!(ib.has_pending()); + ib.advance_read(6); // reaches 11 == write + assert_eq!(ib.read_offset_dwords, 11); + assert!( + !ib.has_pending(), + "indirect buffer must terminate at write ptr, not wrap to 0" + ); + // addr_at_offset must not modulo-wrap for an indirect buffer. + ib.read_offset_dwords = 9; + assert_eq!(ib.addr_at_offset(1), Some(0x4adf_5080 + 10 * 4)); + } + + #[test] + fn indirect_flag_does_not_affect_circular_ring() { + // Sanity: a circular (primary) ring still wraps as before. + let mut v = RingBufferView::new(); + v.base = 0x4adc_c000; + v.size_dwords = 8192; + v.read_offset_dwords = 8190; + v.write_offset_dwords = 2; + assert!(v.has_pending()); + v.advance_read(4); // (8190 + 4) % 8192 = 2 + assert_eq!(v.read_offset_dwords, 2); + assert!(!v.has_pending()); + } } diff --git a/crates/xenia-kernel/src/exports.rs b/crates/xenia-kernel/src/exports.rs index e118e57..7632cf1 100644 --- a/crates/xenia-kernel/src/exports.rs +++ b/crates/xenia-kernel/src/exports.rs @@ -2883,10 +2883,12 @@ fn vd_initialize_ring_buffer(ctx: &mut PpcContext, _mem: &GuestMemory, state: &m // packets directly into ring memory at the current WPTR (the GPU // backend lives on a worker thread under `--gpu-thread` so we can't // read its `ring.base` from the kernel side without a channel hop). - // Per canary: size_log2 is log2(size in BYTES), so size in dwords = - // 2^size_log2 / 4 = 1 << (size_log2 - 2). + // Per canary `CommandProcessor::InitializeRingBuffer`: the ring is + // `1 << (size_log2 + 3)` bytes = `1 << (size_log2 + 1)` dwords (`r4` is + // log2 of the size in quadwords). Kept in sync with + // `GpuSystem::initialize_ring_buffer`. (Currently bookkeeping-only.) state.ring_base = ptr; - state.ring_size_dwords = if size_log2 >= 2 { 1u32 << (size_log2 - 2) } else { 0 }; + state.ring_size_dwords = 1u32 << (size_log2 + 1); ctx.gpr[3] = 0; }