From: Mikhail Malyshev <[email protected]> Hi Moeko,
Thanks for the review. > Just wondering if this aimed to solve some kind of "CPU pipe A FIFO > underrun" error. If possible, could you please paste some log? The i915 > driver seems to only read POWER_STATE in gen9_dbuf_slice_set(), and a > warning should be printed if there is an inconsisitency. Worth clarifying up front: the guest here is Windows (Intel KMD), not Linux/i915, so there is no i915 dmesg or gen9_dbuf_slice_set() WARN to paste. The visible symptom is corrupted scanout (~128px vertical stripes) from a plane FIFO underrun, not a logged kernel error. I have not run a Linux/i915 guest on this platform, so I can't show it hitting or not hitting the bug -- and per your point about the WARN, i915 may well flag or avoid it, which is consistent with us only observing this on Windows. The trigger is a single inconsistent read on the first-boot modeset. Captured with x-no-mmap=on so BAR0 is trapped and every access is visible, the DBUF_CTL programming is a read-modify-write that never sets POWER_REQUEST: vfio_region_read index=0x0 addr=0x45008 data=0x4040c000 # STATE(30)=1, REQUEST(31)=0 (stale POST leftover) vfio_region_write index=0x0 addr=0x45008 data=0x4043c000 # RMW: tracker bits only; REQUEST still 0 vfio_region_read index=0x0 addr=0x45008 data=0x0043c000 # DBUF now powered down (STATE=0) -> underrun The driver sees STATE=1, assumes the slice is already powered, and leaves REQUEST clear. Only after a display sleep/wake does it actually request power, which is exactly why sleep/wake "repairs" the display: vfio_region_write index=0x0 addr=0x45008 data=0x8043c000 # REQUEST=1 vfio_region_read index=0x0 addr=0x45008 data=0xc043c000 # STATE follows -> powered, scanout clean Same pattern on 0x44fe8/0x44300/0x44304. This was originally reproduced, and the fix hardware-validated, on QEMU 9.1; this patch is the forward-port to current master. I'm happy to share the full trace event log (~400 MB) -- where would you prefer it? I can drop it on Google Drive and link it here if that's acceptable, or attach a trimmed excerpt inline. > Here GVT means the GVT-g (kvmgt) driver under drivers/gpu/drm/i915/gvt, > right? Please correct me if I am wrong. Yes, GVT-g (kvmgt). It emulates STATE-follows-REQUEST for this class of inconsistency; this quirk is the read-side equivalent for full passthrough. > IGDDbufCtlQuirk *dq = g_malloc0(sizeof(*dq)); > dquirk->data = dq; > would have better readability. > > A `vfio_quirk_alloc(nslices)` might also be better here? Both make sense -- I'll split the allocation and use a single vfio_quirk_alloc(nslices) with one mem[] entry per slice. Let me know if the patch looks acceptable in general and I'll proceed with a v2 folding in the above. Thanks, Mike
