[PATCH] vfio/igd: sanitize DBUF_CTL POWER_STATE for iGPU passthrough

mike.malyshev@gmail.com posted 1 patch 1 week, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260717084942.1555442-1-mike.malyshev@gmail.com
Maintainers: Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>, Tomita Moeko <tomitamoeko@gmail.com>
There is a newer version of this series
hw/vfio/igd.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 116 insertions(+)
[PATCH] vfio/igd: sanitize DBUF_CTL POWER_STATE for iGPU passthrough
Posted by mike.malyshev@gmail.com 1 week, 1 day ago
From: Mikhail Malyshev <mike.malyshev@gmail.com>

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, which emulates the register so POWER_STATE
follows POWER_REQUEST.

A guest display driver samples POWER_STATE to determine which DBUF
slices are already enabled; seeing the stale "powered" bit it never
issues POWER_REQUEST, so DBUF actually 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.

Present a consistent view like GVT: 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 <mike.malyshev@gmail.com>
---
 hw/vfio/igd.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index e091f21b6a..ebe957f69d 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 (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: 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,32 @@ 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);
+
+        for (i = 0; i < nslices; i++) {
+            VFIOQuirk *dquirk = vfio_quirk_alloc(1);
+            IGDDbufCtlQuirk *dq = dquirk->data = g_malloc0(sizeof(*dq));
+
+            dq->vdev = vdev;
+            dq->bar = nr;
+            dq->bar_offset = igd_dbuf_ctl_offsets[i];
+            memory_region_init_io(&dquirk->mem[0], OBJECT(vdev),
+                                  &igd_dbuf_ctl_ops, dq,
+                                  "vfio-igd-dbuf-ctl-quirk", 4);
+            memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
+                                                igd_dbuf_ctl_offsets[i],
+                                                &dquirk->mem[0], 1);
+            QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, dquirk, next);
+        }
+    }
 }
 
 static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
-- 
2.43.0
Re: [PATCH] vfio/igd: sanitize DBUF_CTL POWER_STATE for iGPU passthrough
Posted by Tomita Moeko 2 days, 1 hour ago
Hi, Mike

On 2026-07-17 16:49, mike.malyshev@gmail.com wrote:
> From: Mikhail Malyshev <mike.malyshev@gmail.com>
> 
> 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, which emulates the register so POWER_STATE
> follows POWER_REQUEST.
> 
> A guest display driver samples POWER_STATE to determine which DBUF
> slices are already enabled; seeing the stale "powered" bit it never
> issues POWER_REQUEST, so DBUF actually 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.

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.

> 
> Present a consistent view like GVT: 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 <mike.malyshev@gmail.com>
> ---
>  hw/vfio/igd.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 116 insertions(+)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index e091f21b6a..ebe957f69d 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 (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: intercept DBUF_CTL reads and clear

Here GVT means the GVT-g (kvmgt) driver under drivers/gpu/drm/i915/gvt, right?
Please correct me if I am wrong.

> + * 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,32 @@ 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);
> +
> +        for (i = 0; i < nslices; i++) {
> +            VFIOQuirk *dquirk = vfio_quirk_alloc(1);
> +            IGDDbufCtlQuirk *dq = dquirk->data = g_malloc0(sizeof(*dq));

IGDDbufCtlQuirk *dq = g_malloc0(sizeof(*dq));
dquirk->data = dq;

would have better readability.

A `vfio_quirk_alloc(nslices)` might also be better here?

> +
> +            dq->vdev = vdev;
> +            dq->bar = nr;
> +            dq->bar_offset = igd_dbuf_ctl_offsets[i];
> +            memory_region_init_io(&dquirk->mem[0], OBJECT(vdev),
> +                                  &igd_dbuf_ctl_ops, dq,
> +                                  "vfio-igd-dbuf-ctl-quirk", 4);
> +            memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
> +                                                igd_dbuf_ctl_offsets[i],
> +                                                &dquirk->mem[0], 1);
> +            QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, dquirk, next);
> +        }
> +    }
>  }
>  
>  static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)

Thanks,
Moeko
Re: [PATCH] vfio/igd: sanitize DBUF_CTL POWER_STATE for iGPU passthrough
Posted by mike.malyshev@gmail.com 1 day, 20 hours ago
From: Mikhail Malyshev <mike.malyshev@gmail.com>

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
Re: [PATCH] vfio/igd: sanitize DBUF_CTL POWER_STATE for iGPU passthrough
Posted by Tomita Moeko 1 day, 6 hours ago
Hi, Mike

On 2026-07-24 01:41, mike.malyshev@gmail.com wrote:
> From: Mikhail Malyshev <mike.malyshev@gmail.com>
> 
> 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.


Got it! I think the lines above here are enough to pinpoint what actually
went wrong. It would be helpful if you could put the above explaination in
the commit message of v2.

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

This definitely looks great! Thank you for your contribution!

Moeko
[PATCH v2] vfio/igd: sanitize DBUF_CTL POWER_STATE for iGPU passthrough
Posted by mike.malyshev@gmail.com 1 day, 3 hours ago
From: Mikhail Malyshev <mike.malyshev@gmail.com>

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 <mike.malyshev@gmail.com>
---

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
Re: [PATCH v2] vfio/igd: sanitize DBUF_CTL POWER_STATE for iGPU passthrough
Posted by Tomita Moeko 1 day, 1 hour ago
Reviewed-by: Tomita Moeko <tomitamoeko@gmail.com>

Thank you for your contribution!

On 2026-07-24 18:29, mike.malyshev@gmail.com wrote:
> From: Mikhail Malyshev <mike.malyshev@gmail.com>
> 
> 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 <mike.malyshev@gmail.com>
> ---
> 
> 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)