From: Mikhail Malyshev <[email protected]>

On some hosts the firmware POST modeset leaves the display data buffer
(DBUF) powered, so the passed-through iGPU's DBUF_CTL registers read
back POWER_STATE=1 while POWER_REQUEST=0 -- an inconsistent leftover that
never occurs under GVT-g, which emulates the register so POWER_STATE
follows POWER_REQUEST.

This was observed with a Windows guest (Intel KMD) on QEMU 9.1; a
Linux/i915 guest was not tested.  On the first-boot modeset the guest
driver samples POWER_STATE to decide which DBUF slices are already
enabled, sees the stale "powered" bit, and therefore never issues
POWER_REQUEST.  DBUF then powers down, the plane FIFO underruns, and
scanout is corrupted (vertical stripes) until a full modeset (e.g. a
display sleep/wake) re-requests power.

The programming is a single inconsistent read-modify-write.  Traced with
x-no-mmap=on so every BAR0 access is visible:

  read  0x45008 = 0x4040c000   STATE=1, REQUEST=0 (stale leftover)
  write 0x45008 = 0x4043c000   RMW: tracker bits only; REQUEST still 0
  read  0x45008 = 0x0043c000   STATE=0: DBUF powered down -> underrun

The driver sees STATE=1, assumes the slice is already powered, and leaves
REQUEST clear.  Only after a display sleep/wake does it set REQUEST, which
is why sleep/wake repairs the display:

  write 0x45008 = 0x8043c000   REQUEST=1
  read  0x45008 = 0xc043c000   STATE follows -> powered, scanout clean

The same pattern occurs on the other slice registers (0x44fe8, 0x44300,
0x44304).

Present a consistent view like GVT-g: trap the DBUF_CTL slice registers
(S1..S4, only as many as the generation exposes) in BAR0 and clear
POWER_STATE on read whenever POWER_REQUEST is not set.  Writes pass
straight through to the device.

Signed-off-by: Mikhail Malyshev <[email protected]>
---

Changes in v2:
- Expand the commit message with the traced read-modify-write sequence
  that shows POWER_REQUEST is never set on first boot (Tomita Moeko).
- Collapse the per-slice quirk allocations into a single
  vfio_quirk_alloc(nslices) with one mem[] entry per slice, backed by a
  single IGDDbufCtlQuirk array (Tomita Moeko).
- Split the quirk allocation onto its own line for readability (Tomita
  Moeko).
 hw/vfio/igd.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 413a49aae9..5e6160763c 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -455,6 +455,96 @@ static bool vfio_pci_igd_override_gms(int gen, uint32_t 
gms, uint32_t *gmch)
 #define IGD_GGC_MMIO_OFFSET     0x108040
 #define IGD_BDSM_MMIO_OFFSET    0x1080C0
 
+/*
+ * IGD BAR0 DBUF_CTL sanitize quirk.
+ *
+ * On hosts where the firmware POST modeset left the display engine powered,
+ * DBUF_CTL reads back POWER_STATE=1 while POWER_REQUEST=0 -- an inconsistent
+ * leftover that never occurs under GVT-g (which emulates the register so
+ * STATE follows REQUEST).  A passed-through guest driver samples POWER_STATE
+ * to decide which DBUF slices are already enabled, sees this stale "powered"
+ * bit, and therefore never issues POWER_REQUEST.  DBUF then powers down, the
+ * plane FIFO underruns, and scanout is corrupted until a full modeset (e.g.
+ * a display sleep/wake) re-requests power.
+ *
+ * Present a consistent view like GVT-g: intercept DBUF_CTL reads and clear
+ * POWER_STATE whenever POWER_REQUEST is not set.  Writes pass straight
+ * through to the device.
+ */
+#define IGD_DBUF_POWER_REQUEST  (1u << 31)
+#define IGD_DBUF_POWER_STATE    (1u << 30)
+
+/*
+ * DBUF_CTL slice registers within BAR0, in slice order (i915 numbers these
+ * S1..S4).  How many slices exist is generation-dependent, so only the first
+ * igd_dbuf_ctl_nslices(gen) entries are real DBUF_CTL registers on a given
+ * part; the rest are unrelated registers and must not be trapped.
+ */
+static const uint32_t igd_dbuf_ctl_offsets[] = {
+    0x45008, /* S1 */ 0x44FE8, /* S2 */ 0x44300, /* S3 */ 0x44304, /* S4 */
+};
+
+/*
+ * DBUF slice count by generation, matching i915 dbuf.slice_mask:
+ * gen9/10 = 1 (S1), gen11 = 2 (S1-S2), gen12+ = 4 (S1-S4).
+ *
+ * Within gen12 the slice count is actually per-platform, not per-gen: ADL-P /
+ * RPL-P / DG2 expose 4 slices, but TGL / RKL / ADL-S have only 2.  We return 4
+ * for all gen12+ (igd_gen() can't distinguish them), so on a <4-slice gen12
+ * part S3/S4 (0x44300/0x44304) are over-trapped.  This is harmless in 
practice:
+ * the read handler only mutates a value when POWER_REQUEST=0 && POWER_STATE=1,
+ * which whatever register lives at those offsets is very unlikely to present.
+ * A fully-correct count would have to key off the PCI device ID.
+ */
+static int igd_dbuf_ctl_nslices(int gen)
+{
+    if (gen <= 10) {
+        return 1;
+    }
+    if (gen == 11) {
+        return 2;
+    }
+    return 4;
+}
+
+typedef struct IGDDbufCtlQuirk {
+    VFIOPCIDevice *vdev;
+    uint32_t bar_offset;    /* offset within BAR0 MMIO */
+    uint8_t bar;
+} IGDDbufCtlQuirk;
+
+static uint64_t igd_dbuf_ctl_read(void *opaque, hwaddr addr, unsigned size)
+{
+    IGDDbufCtlQuirk *q = opaque;
+    VFIOPCIDevice *vdev = q->vdev;
+    uint64_t val = vfio_region_read(&vdev->bars[q->bar].region,
+                                    addr + q->bar_offset, size);
+
+    if (size == 4 && !(val & IGD_DBUF_POWER_REQUEST) &&
+        (val & IGD_DBUF_POWER_STATE)) {
+        val &= ~(uint64_t)IGD_DBUF_POWER_STATE;
+        error_report_once("IGD quirk: DBUF_CTL@0x%x cleared stale "
+                          "POWER_STATE (POWER_REQUEST=0)", q->bar_offset);
+    }
+    return val;
+}
+
+static void igd_dbuf_ctl_write(void *opaque, hwaddr addr,
+                               uint64_t data, unsigned size)
+{
+    IGDDbufCtlQuirk *q = opaque;
+    VFIOPCIDevice *vdev = q->vdev;
+
+    vfio_region_write(&vdev->bars[q->bar].region,
+                      addr + q->bar_offset, data, size);
+}
+
+static const MemoryRegionOps igd_dbuf_ctl_ops = {
+    .read = igd_dbuf_ctl_read,
+    .write = igd_dbuf_ctl_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
 void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
 {
     VFIOQuirk *ggc_quirk, *bdsm_quirk;
@@ -507,6 +597,35 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
                                         1);
 
     QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, bdsm_quirk, next);
+
+    /*
+     * DBUF_CTL sanitize quirk (gen9+): trap the DBUF_CTL slice registers so
+     * POWER_STATE is reported consistently with POWER_REQUEST (see
+     * igd_dbuf_ctl_read()).  Only trap slices that actually exist on this
+     * generation -- the remaining offsets are unrelated registers.
+     */
+    if (gen >= 9) {
+        int i, nslices = igd_dbuf_ctl_nslices(gen);
+        VFIOQuirk *dbuf_quirk = vfio_quirk_alloc(nslices);
+        IGDDbufCtlQuirk *dq;
+
+        dq = g_new0(IGDDbufCtlQuirk, nslices);
+        dbuf_quirk->data = dq;
+
+        for (i = 0; i < nslices; i++) {
+            dq[i].vdev = vdev;
+            dq[i].bar = nr;
+            dq[i].bar_offset = igd_dbuf_ctl_offsets[i];
+            memory_region_init_io(&dbuf_quirk->mem[i], OBJECT(vdev),
+                                  &igd_dbuf_ctl_ops, &dq[i],
+                                  "vfio-igd-dbuf-ctl-quirk", 4);
+            memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
+                                                igd_dbuf_ctl_offsets[i],
+                                                &dbuf_quirk->mem[i], 1);
+        }
+
+        QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, dbuf_quirk, next);
+    }
 }
 
 static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
-- 
2.43.0


Reply via email to