[iterate-2O] GPU: drain indirect buffers correctly — Sylpheed renders splash (draws 0→78)

Ours' GPU never drained the D3D driver's system command buffer past the first
11-dword indirect buffer, so DRAW_INDX / reg-0x57C-arm packets never executed
and draws stayed 0 (the long-hunted render gate; see UPDATE-18). Runtime tracing
(temporary, removed) showed the guest submits 6 INDIRECT_BUFFER packets at boot
(CP_RB_WPTR 22→37) but ours executed exactly ONE IB and then spun 15.7M packets
inside it. Three coupled command-processor bugs, all corrected to match canary:

1. `sync_with_mmio` applied the primary CP_RB_WPTR to whichever ring was active,
   including an executing indirect buffer — `37 % 11 = 3` clobbered the IB's
   write pointer so its read pointer looped 0→2→5→0 forever and never popped
   back to the primary ring. CP_RB_WPTR governs ONLY the primary ring; while an
   IB executes, the primary is the bottom of the IB stack. Canary executes each
   IB through a separate `RingBuffer reader_` (command_processor.cc), so the
   primary write pointer is structurally inapplicable to an IB.

2. Indirect buffers were treated as circular rings: read wrapped at `size_dwords`
   (`11 % 11 = 0`) and never reached the fixed write pointer, so even without the
   clobber the IB could not terminate. An IB is a fixed *linear* sub-stream; add
   `RingBufferView.indirect` and drain `[0, ib_size)` monotonically, then pop.

3. `is_ready` only checked the active ring, so an IB that now correctly exhausts
   would never get `execute_one` called again to pop back to the primary ring
   (whose WPTR may have advanced). Check the whole IB stack.

Also: the ring was sized `1 << size_log2` bytes (1024 dwords) vs canary's
`1 << (size_log2 + 3)` (8192 dwords) — an 8× undersize that desynced WPTR-wrap
math from the guest. Fixed in `GpuSystem::initialize_ring_buffer` (and the
dead bookkeeping copy in `vd_initialize_ring_buffer`).

Cascade (deterministic; threaded-default backend, byte-identical across runs):
reg 0x57C now written, IB jumps 1→12, packets 15.7M→9,825, and the splash
renders — draws 0→78, shaders 0→3, render_targets 0→2, swaps 2→3 — stable at
50M / 200M / 1B. Boot then reaches a new downstream gate (draws plateau at 78,
interrupts keep climbing → engine alive, not deadlocked).

golden `sylpheed_n50m.json` re-baselined (draws 78). `cargo test --workspace`
green (674; +2 ring_view regression tests). vd_swap's synthetic-swap
short-circuit is now redundant but left untouched (cascade works without
changing it); cleaning it up is a separate follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-13 22:06:16 +02:00
parent 93f60a3ba0
commit 034ec8b47f
4 changed files with 126 additions and 21 deletions

View File

@@ -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}"),

View File

@@ -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());
}
}