[PATCH 0/9] vfio/igd: Detect IGD by OpRegion and enable OpRegion automatically

[PATCH 0/9] vfio/igd: Detect IGD by OpRegion and enable OpRegion automatically

[Tomita Moeko]

As proposed in a previous discussion [1], detect IGD devices based on
whether it has VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION exposed by kernel
and enables OpRegion access by default. This enables out-of-the-box
display output support for IGD passthrough without having to manually
set x-igd-opregion=on, which probably saves effort for libvirt users.

This patchset also limits legacy mode to Gen6-9 devices, as Intel no
longer provides CSM support and VBIOS on newer IGD, and Seabios does not
support setting up the 64 bit BDSM register on Gen 11+ devices. Exposing
VGA ranges to guests by default on platforms without VGA mode support is
undesirable.

Additionally, as we enables OpRegion on IGD devices by default, and
Intel removes the BDSM register from Meteor Lake [2]. There seems to be
no extra register quirks rather than OpRegion required on newer devices.
To support them (and probably future devices), the generation limit is
removed, with BDSM quirk only applied to known Gen 6-12 devices. 

Note: I have not been able to test this on Meteor Lake or newer
platforms due to lack of hardware.


[1] https://lore.kernel.org/qemu-devel/20250325172239.27926-1-tomitamoeko@gmail.com
[2] https://edc.intel.com/content/www/us/en/design/publications/14th-generation-core-processors-cfg-and-mem-registers/d2-f0-processor-graphics-registers/

Changelog:
v2:
* Removed "Allow hotplugging with OpRegion enabled", hotplugging is
  always forbidden.
* Test device is not hotplugged and get opregion in a single function.
* Update documentation along with code changes.
* Minor code style fixes.
Link: https://lore.kernel.org/qemu-devel/20250421163112.21316-1-tomitamoeko@gmail.coms

Tomita Moeko (9):
  vfio/igd: Restrict legacy mode to Gen6-9 devices
  vfio/igd: Always emulate ASLS (OpRegion) register
  vfio/igd: Detect IGD device by OpRegion
  vfio/igd: Check vendor and device ID on GVT-g mdev
  vfio/igd: Check OpRegion support on GVT-g mdev
  vfio/igd: Enable OpRegion by default
  vfio/igd: Allow overriding GMS with 0xf0 to 0xfe on Gen9+
  vfio/igd: Only emulate GGC register when x-igd-gms is set
  vfio/igd: Remove generation limitation for IGD passthrough

 docs/igd-assign.txt |  11 ++-
 hw/vfio/igd.c       | 218 ++++++++++++++++++++++++++------------------
 hw/vfio/pci.c       |   2 +-
 3 files changed, 137 insertions(+), 94 deletions(-)

-- 
2.47.2

[PATCH 1/9] vfio/igd: Restrict legacy mode to Gen6-9 devices

[Tomita Moeko]

Intel only provides legacy VBIOS for IGD up to Gen9, and there is no
CSM support on later devices. Additionally, Seabios can only handle
32-bit BDSM register used until Gen9. Since legacy mode requires VGA
capability, restrict it to Gen6 through Gen9 devices.

Link: https://lore.kernel.org/qemu-devel/20250325172239.27926-1-tomitamoeko@gmail.com/T/
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 docs/igd-assign.txt | 1 +
 hw/vfio/igd.c       | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
index 3aed7956d5..95beb76812 100644
--- a/docs/igd-assign.txt
+++ b/docs/igd-assign.txt
@@ -47,6 +47,7 @@ Intel document [1] shows how to dump VBIOS to file. For UEFI Option ROM, see
 
 QEMU also provides a "Legacy" mode that implicitly enables full functionality
 on IGD, it is automatically enabled when
+* IGD generation is 6 to 9 (Sandy Bridge to Comet Lake)
 * Machine type is i440fx
 * IGD is assigned to guest BDF 00:02.0
 * ROM BAR or romfile is present
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 6678e0e5cd..cecc3245b7 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -516,11 +516,13 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 
     /*
      * For backward compatibility, enable legacy mode when
+     * - Device geneation is 6 to 9 (including both)
      * - Machine type is i440fx (pc_piix)
      * - IGD device is at guest BDF 00:02.0
      * - Not manually disabled by x-igd-legacy-mode=off
      */
     if ((vdev->igd_legacy_mode != ON_OFF_AUTO_OFF) &&
+        (gen >= 6 && gen <= 9) &&
         !strcmp(MACHINE_GET_CLASS(qdev_get_machine())->family, "pc_piix") &&
         (&vdev->pdev == pci_find_device(pci_device_root_bus(&vdev->pdev),
         0, PCI_DEVFN(0x2, 0)))) {
@@ -565,7 +567,9 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
         vdev->features |= VFIO_FEATURE_ENABLE_IGD_LPC;
     } else if (vdev->igd_legacy_mode == ON_OFF_AUTO_ON) {
         error_setg(&err,
-                   "Machine is not i440fx or assigned BDF is not 00:02.0");
+                   "Machine is not i440fx, assigned BDF is not 00:02.0, "
+                   "or device %04x (gen %d) doesn't support legacy mode",
+                   vdev->device_id, gen);
         goto error;
     }
 
-- 
2.47.2

[PATCH 2/9] vfio/igd: Always emulate ASLS (OpRegion) register

[Tomita Moeko]

ASLS register represents the base address of OpRegion, and it is
programmed with HPA. In IGD passthrough scenario, it needs to be
reprogrammed with GPA by guest firmware. To prevent guest accessing
wrong memory range, ASLS should always be emulated and cleared.

In GVT-g scenario, emulating ASLS is unnecessary as access is handled
by kvmgt backend [1].

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/i915/gvt/cfg_space.c?h=v6.14#n295

Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/vfio/igd.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index cecc3245b7..ae19456457 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -182,10 +182,6 @@ static bool vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
 
     trace_vfio_pci_igd_opregion_enabled(vdev->vbasedev.name);
 
-    pci_set_long(vdev->pdev.config + IGD_ASLS, 0);
-    pci_set_long(vdev->pdev.wmask + IGD_ASLS, ~0);
-    pci_set_long(vdev->emulated_config_bits + IGD_ASLS, ~0);
-
     return true;
 }
 
@@ -583,7 +579,15 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
     if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_LPC) &&
         !vfio_pci_igd_setup_lpc_bridge(vdev, errp)) {
         goto error;
-     }
+    }
+
+    /*
+     * ASLS (OpRegion address) is read-only, emulated
+     * It contains HPA, guest firmware need to reprogram it with GPA.
+     */
+    pci_set_long(vdev->pdev.config + IGD_ASLS, 0);
+    pci_set_long(vdev->pdev.wmask + IGD_ASLS, ~0);
+    pci_set_long(vdev->emulated_config_bits + IGD_ASLS, ~0);
 
     /*
      * Allow user to override dsm size using x-igd-gms option, in multiples of
-- 
2.47.2

[PATCH 3/9] vfio/igd: Detect IGD device by OpRegion

[Tomita Moeko]

There is currently no straightforward way to distinguish if a Intel
graphics device is IGD or discrete GPU. However, only IGD devices have
OpRegion. Use the presence of VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION
to identify IGD devices. Still, OpRegion on hotplugged IGD device is
not supported.

Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/vfio/igd.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index ae19456457..d6880cbff7 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -185,9 +185,10 @@ static bool vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
     return true;
 }
 
-static bool vfio_pci_igd_setup_opregion(VFIOPCIDevice *vdev, Error **errp)
+static bool vfio_pci_igd_opregion_detect(VFIOPCIDevice *vdev,
+                                         struct vfio_region_info **opregion,
+                                         Error **errp)
 {
-    g_autofree struct vfio_region_info *opregion = NULL;
     int ret;
 
     /* Hotplugging is not supported for opregion access */
@@ -198,17 +199,13 @@ static bool vfio_pci_igd_setup_opregion(VFIOPCIDevice *vdev, Error **errp)
 
     ret = vfio_device_get_region_info_type(&vdev->vbasedev,
                     VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
-                    VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, &opregion);
+                    VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, opregion);
     if (ret) {
         error_setg_errno(errp, -ret,
                          "Device does not supports IGD OpRegion feature");
         return false;
     }
 
-    if (!vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
-        return false;
-    }
-
     return true;
 }
 
@@ -479,6 +476,7 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
 
 static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 {
+    g_autofree struct vfio_region_info *opregion = NULL;
     int ret, gen;
     uint64_t gms_size;
     uint64_t *bdsm_size;
@@ -486,16 +484,17 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
     bool legacy_mode_enabled = false;
     Error *err = NULL;
 
-    /*
-     * This must be an Intel VGA device at address 00:02.0 for us to even
-     * consider enabling legacy mode.  The vBIOS has dependencies on the
-     * PCI bus address.
-     */
     if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
         !vfio_is_vga(vdev)) {
         return true;
     }
 
+    /* IGD device always comes with OpRegion */
+    if (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp)) {
+        return true;
+    }
+    info_report("OpRegion detected on Intel display %x.", vdev->device_id);
+
     /*
      * IGD is not a standard, they like to change their specs often.  We
      * only attempt to support back to SandBridge and we hope that newer
@@ -571,7 +570,7 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 
     /* Setup OpRegion access */
     if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) &&
-        !vfio_pci_igd_setup_opregion(vdev, errp)) {
+        !vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
         goto error;
     }
 
@@ -671,8 +670,11 @@ error:
  */
 static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 {
+    g_autofree struct vfio_region_info *opregion = NULL;
+
     if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) &&
-        !vfio_pci_igd_setup_opregion(vdev, errp)) {
+        (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp) ||
+         !vfio_pci_igd_opregion_init(vdev, opregion, errp))) {
         return false;
     }
 
-- 
2.47.2

[PATCH 4/9] vfio/igd: Check vendor and device ID on GVT-g mdev

[Tomita Moeko]

Check the vendor and device ID on GVT-g mdev to ensure it is a supported
device [1]. This extra check is required for automatically enabling
OpRegion access later.

Note that Cherryview and Gemini Lake are marked as supported here since
current code cannot distinguish them with other Gen8 and Gen9 devices.
Since mdev cannot be created on these devices, this has no functional
impact.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/i915/intel_gvt.c?h=v6.14#n52

Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/vfio/igd.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index d6880cbff7..c6ecbefb35 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -671,6 +671,18 @@ error:
 static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 {
     g_autofree struct vfio_region_info *opregion = NULL;
+    int gen;
+
+    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
+        !vfio_is_vga(vdev)) {
+        return true;
+    }
+
+    /* FIXME: Cherryview is Gen8, but don't support GVT-g */
+    gen = igd_gen(vdev);
+    if (gen != 8 && gen != 9) {
+        return true;
+    }
 
     if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) &&
         (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp) ||
-- 
2.47.2

[PATCH 5/9] vfio/igd: Check OpRegion support on GVT-g mdev

[Tomita Moeko]

The Intel GVT-g backend `kvmgt` always emulates OpRegion for vGPU,
make sure the OpRegion is present for enabling access to it
automatically later.

Also, hotplugging GVT-g vGPU is now always disallowed regardless of
OpRegion to prevent potential issues. Intel has never claimed support
for GVT-g hotplugging.

Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/vfio/igd.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index c6ecbefb35..496d3df598 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -684,9 +684,13 @@ static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice *vdev, Error **errp)
         return true;
     }
 
+    if (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp)) {
+        /* Should never reach here, KVMGT always emulates OpRegion */
+        return false;
+    }
+
     if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) &&
-        (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp) ||
-         !vfio_pci_igd_opregion_init(vdev, opregion, errp))) {
+        !vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
         return false;
     }
 
-- 
2.47.2

[PATCH 6/9] vfio/igd: Enable OpRegion by default

[Tomita Moeko]

As the presence of OpRegion is used to detect IGD device now, and
guest driver usually depends on OpRegion to work. Enable OpRegion
on IGD devices by default for out-of-the-box passthrough experience
(except pre-boot display output), especially for libvirt users.

Example of IGD passthrough with libvirt:
<hostdev mode="subsystem" type="pci" managed="yes">
  <source>
    <address domain="0x0000" bus="0x00" slot="0x02" function="0x0"/>
  </source>
  <rom file="/path/to/igd/rom"/>
  <address type="pci" domain="0x0000" bus="0x00" slot="0x02" function="0x0"/>
 </hostdev>

Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 docs/igd-assign.txt | 4 ++--
 hw/vfio/pci.c       | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
index 95beb76812..fc444503ff 100644
--- a/docs/igd-assign.txt
+++ b/docs/igd-assign.txt
@@ -102,7 +102,7 @@ digital formats work well.
 
 Options
 =======
-* x-igd-opregion=[on|*off*]
+* x-igd-opregion=[*on*|off]
   Copy host IGD OpRegion and expose it to guest with fw_cfg
 
 * x-igd-lpc=[on|*off*]
@@ -124,7 +124,7 @@ Examples
 
 * Adding IGD with OpRegion and LPC ID hack, but without VGA ranges
   (For UEFI guests)
-  -device vfio-pci,host=00:02.0,id=hostdev0,addr=2.0,x-igd-legacy-mode=off,x-igd-opregion=on,x-igd-lpc=on,romfile=efi_oprom.rom
+  -device vfio-pci,host=00:02.0,id=hostdev0,addr=2.0,x-igd-legacy-mode=off,x-igd-lpc=on,romfile=efi_oprom.rom
 
 
 Guest firmware
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 05a7a62204..38ff231625 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3361,7 +3361,7 @@ static const Property vfio_pci_dev_properties[] = {
     DEFINE_PROP_BIT("x-req", VFIOPCIDevice, features,
                     VFIO_FEATURE_ENABLE_REQ_BIT, true),
     DEFINE_PROP_BIT("x-igd-opregion", VFIOPCIDevice, features,
-                    VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
+                    VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, true),
     DEFINE_PROP_BIT("x-igd-lpc", VFIOPCIDevice, features,
                     VFIO_FEATURE_ENABLE_IGD_LPC_BIT, false),
     DEFINE_PROP_ON_OFF_AUTO("x-igd-legacy-mode", VFIOPCIDevice,
-- 
2.47.2

[PATCH 7/9] vfio/igd: Allow overriding GMS with 0xf0 to 0xfe on Gen9+

[Tomita Moeko]

On Gen9 and later IGD devices, GMS 0xf0 to 0xfe represents 4MB to 60MB
pre-allocated memory size in 4MB increments. Allow users overriding
GMS with these values.

Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/vfio/igd.c | 59 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 18 deletions(-)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 496d3df598..7f289a62a3 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -411,6 +411,44 @@ static bool vfio_pci_igd_setup_lpc_bridge(VFIOPCIDevice *vdev, Error **errp)
     return true;
 }
 
+static bool vfio_pci_igd_override_gms(int gen, uint32_t gms, uint32_t *gmch)
+{
+    bool ret = false;
+
+    if (gen == -1) {
+        error_report("x-igd-gms is not supported on this device");
+    } else if (gen < 8) {
+        if (gms <= 0x10) {
+            *gmch &= ~(IGD_GMCH_GEN6_GMS_MASK << IGD_GMCH_GEN6_GMS_SHIFT);
+            *gmch |= gms << IGD_GMCH_GEN6_GMS_SHIFT;
+            ret = true;
+        } else {
+            error_report(QERR_INVALID_PARAMETER_VALUE, "x-igd-gms", "0~0x10");
+        }
+    } else if (gen == 8) {
+        if (gms <= 0x40) {
+            *gmch &= ~(IGD_GMCH_GEN8_GMS_MASK << IGD_GMCH_GEN8_GMS_SHIFT);
+            *gmch |= gms << IGD_GMCH_GEN8_GMS_SHIFT;
+            ret = true;
+        } else {
+            error_report(QERR_INVALID_PARAMETER_VALUE, "x-igd-gms", "0~0x40");
+        }
+    } else {
+        /* 0x0  to 0x40: 32MB increments starting at 0MB */
+        /* 0xf0 to 0xfe: 4MB increments starting at 4MB */
+        if ((gms <= 0x40) || (gms >= 0xf0 && gms <= 0xfe)) {
+            *gmch &= ~(IGD_GMCH_GEN8_GMS_MASK << IGD_GMCH_GEN8_GMS_SHIFT);
+            *gmch |= gms << IGD_GMCH_GEN8_GMS_SHIFT;
+            ret = true;
+        } else {
+            error_report(QERR_INVALID_PARAMETER_VALUE,
+                         "x-igd-gms", "0~0x40 or 0xf0~0xfe");
+        }
+    }
+
+    return ret;
+}
+
 #define IGD_GGC_MMIO_OFFSET     0x108040
 #define IGD_BDSM_MMIO_OFFSET    0x1080C0
 
@@ -593,24 +631,9 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
      * 32MiB. This option should only be used when the desired size cannot be
      * set from DVMT Pre-Allocated option in host BIOS.
      */
-    if (vdev->igd_gms) {
-        if (gen < 8) {
-            if (vdev->igd_gms <= 0x10) {
-                gmch &= ~(IGD_GMCH_GEN6_GMS_MASK << IGD_GMCH_GEN6_GMS_SHIFT);
-                gmch |= vdev->igd_gms << IGD_GMCH_GEN6_GMS_SHIFT;
-            } else {
-                error_report(QERR_INVALID_PARAMETER_VALUE,
-                             "x-igd-gms", "0~0x10");
-            }
-        } else {
-            if (vdev->igd_gms <= 0x40) {
-                gmch &= ~(IGD_GMCH_GEN8_GMS_MASK << IGD_GMCH_GEN8_GMS_SHIFT);
-                gmch |= vdev->igd_gms << IGD_GMCH_GEN8_GMS_SHIFT;
-            } else {
-                error_report(QERR_INVALID_PARAMETER_VALUE,
-                             "x-igd-gms", "0~0x40");
-            }
-        }
+    if (vdev->igd_gms &&
+        !vfio_pci_igd_override_gms(gen, vdev->igd_gms, &gmch)) {
+        return false;
     }
 
     gms_size = igd_stolen_memory_size(gen, gmch);
-- 
2.47.2

[PATCH 8/9] vfio/igd: Only emulate GGC register when x-igd-gms is set

[Tomita Moeko]

x-igd-gms is used for overriding DSM region size in GGC register in
both config space and MMIO BAR0, by default host value is used.
There is no need to emulate it in default case.

Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/vfio/igd.c | 49 ++++++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 7f289a62a3..5d12f753ab 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -477,22 +477,24 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
         return;
     }
 
-    ggc_quirk = vfio_quirk_alloc(1);
-    ggc_mirror = ggc_quirk->data = g_malloc0(sizeof(*ggc_mirror));
-    ggc_mirror->mem = ggc_quirk->mem;
-    ggc_mirror->vdev = vdev;
-    ggc_mirror->bar = nr;
-    ggc_mirror->offset = IGD_GGC_MMIO_OFFSET;
-    ggc_mirror->config_offset = IGD_GMCH;
-
-    memory_region_init_io(ggc_mirror->mem, OBJECT(vdev),
-                          &vfio_generic_mirror_quirk, ggc_mirror,
-                          "vfio-igd-ggc-quirk", 2);
-    memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
-                                        ggc_mirror->offset, ggc_mirror->mem,
-                                        1);
+    if (vdev->igd_gms) {
+        ggc_quirk = vfio_quirk_alloc(1);
+        ggc_mirror = ggc_quirk->data = g_malloc0(sizeof(*ggc_mirror));
+        ggc_mirror->mem = ggc_quirk->mem;
+        ggc_mirror->vdev = vdev;
+        ggc_mirror->bar = nr;
+        ggc_mirror->offset = IGD_GGC_MMIO_OFFSET;
+        ggc_mirror->config_offset = IGD_GMCH;
 
-    QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, ggc_quirk, next);
+        memory_region_init_io(ggc_mirror->mem, OBJECT(vdev),
+                              &vfio_generic_mirror_quirk, ggc_mirror,
+                              "vfio-igd-ggc-quirk", 2);
+        memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
+                                            ggc_mirror->offset, ggc_mirror->mem,
+                                            1);
+
+        QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, ggc_quirk, next);
+    }
 
     bdsm_quirk = vfio_quirk_alloc(1);
     bdsm_mirror = bdsm_quirk->data = g_malloc0(sizeof(*bdsm_mirror));
@@ -631,9 +633,15 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
      * 32MiB. This option should only be used when the desired size cannot be
      * set from DVMT Pre-Allocated option in host BIOS.
      */
-    if (vdev->igd_gms &&
-        !vfio_pci_igd_override_gms(gen, vdev->igd_gms, &gmch)) {
-        return false;
+    if (vdev->igd_gms) {
+        if (vfio_pci_igd_override_gms(gen, vdev->igd_gms, &gmch)) {
+            /* GMCH is read-only, emulated */
+            pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
+            pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
+            pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
+        } else {
+            return false;
+        }
     }
 
     gms_size = igd_stolen_memory_size(gen, gmch);
@@ -651,11 +659,6 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
     fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
                     bdsm_size, sizeof(*bdsm_size));
 
-    /* GMCH is read-only, emulated */
-    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
-    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
-    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
-
     /* BDSM is read-write, emulated.  The BIOS needs to be able to write it */
     if (gen < 11) {
         pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
-- 
2.47.2

[PATCH 9/9] vfio/igd: Remove generation limitation for IGD passthrough

[Tomita Moeko]

Starting from Intel Core Ultra Series (Meteor Lake), Data Stolen Memory
has became a part of LMEMBAR (MMIO BAR2) [1][2], meaning that BDSM and
GGC register quirks are no longer needed on these platforms.

To support Meteor/Arrow/Lunar Lake and future IGD devices, remove the
generation limitation in IGD passthrough, and apply BDSM and GGC quirks
only to known Gen6-12 devices.

[1] https://edc.intel.com/content/www/us/en/design/publications/14th-generation-core-processors-cfg-and-mem-registers/d2-f0-processor-graphics-registers/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/i915/gem/i915_gem_stolen.c?h=v6.14#n142

Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 docs/igd-assign.txt |  6 +++++
 hw/vfio/igd.c       | 58 ++++++++++++++++-----------------------------
 2 files changed, 27 insertions(+), 37 deletions(-)

diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
index fc444503ff..af4e8391fc 100644
--- a/docs/igd-assign.txt
+++ b/docs/igd-assign.txt
@@ -157,6 +157,12 @@ fw_cfg requirements on the VM firmware:
    it's expected that this fw_cfg file is only relevant to a single PCI
    class VGA device with Intel vendor ID, appearing at PCI bus address 00:02.0.
 
+   Starting from Meteor Lake, IGD devices access stolen memory via its MMIO
+   BAR2 (LMEMBAR) and removed the BDSM register in config space. There is
+   no need for guest firmware to allocate data stolen memory in guest address
+   space and write it to BDSM register. Value of this fw_cfg file is 0 in
+   such case.
+
 Upstream Seabios has OpRegion and BDSM (pre-Gen11 device only) support.
 However, the support is not accepted by upstream EDK2/OVMF. A recommended
 solution is to create a virtual OpRom with following DXE drivers:
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 5d12f753ab..2584861ae6 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -103,6 +103,7 @@ static int igd_gen(VFIOPCIDevice *vdev)
     /*
      * Unfortunately, Intel changes it's specification quite often. This makes
      * it impossible to use a suitable default value for unknown devices.
+     * Return -1 for not applying any generation-specific quirks.
      */
     return -1;
 }
@@ -458,20 +459,12 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
     VFIOConfigMirrorQuirk *ggc_mirror, *bdsm_mirror;
     int gen;
 
-    /*
-     * This must be an Intel VGA device at address 00:02.0 for us to even
-     * consider enabling legacy mode. Some driver have dependencies on the PCI
-     * bus address.
-     */
     if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
         !vfio_is_vga(vdev) || nr != 0) {
         return;
     }
 
-    /*
-     * Only on IGD devices of gen 11 and above, the BDSM register is mirrored
-     * into MMIO space and read from MMIO space by the Windows driver.
-     */
+    /* Only on IGD Gen6-12 device needs quirks in BAR 0 */
     gen = igd_gen(vdev);
     if (gen < 6) {
         return;
@@ -518,7 +511,7 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 {
     g_autofree struct vfio_region_info *opregion = NULL;
     int ret, gen;
-    uint64_t gms_size;
+    uint64_t gms_size = 0;
     uint64_t *bdsm_size;
     uint32_t gmch;
     bool legacy_mode_enabled = false;
@@ -535,18 +528,7 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
     }
     info_report("OpRegion detected on Intel display %x.", vdev->device_id);
 
-    /*
-     * IGD is not a standard, they like to change their specs often.  We
-     * only attempt to support back to SandBridge and we hope that newer
-     * devices maintain compatibility with generation 8.
-     */
     gen = igd_gen(vdev);
-    if (gen == -1) {
-        error_report("IGD device %s is unsupported in legacy mode, "
-                     "try SandyBridge or newer", vdev->vbasedev.name);
-        return true;
-    }
-
     gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
 
     /*
@@ -644,32 +626,34 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
         }
     }
 
-    gms_size = igd_stolen_memory_size(gen, gmch);
+    if (gen > 0) {
+        gms_size = igd_stolen_memory_size(gen, gmch);
+
+        /* BDSM is read-write, emulated. BIOS needs to be able to write it */
+        if (gen < 11) {
+            pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
+            pci_set_long(vdev->pdev.wmask + IGD_BDSM, ~0);
+            pci_set_long(vdev->emulated_config_bits + IGD_BDSM, ~0);
+        } else {
+            pci_set_quad(vdev->pdev.config + IGD_BDSM_GEN11, 0);
+            pci_set_quad(vdev->pdev.wmask + IGD_BDSM_GEN11, ~0);
+            pci_set_quad(vdev->emulated_config_bits + IGD_BDSM_GEN11, ~0);
+        }
+    }
 
     /*
      * Request reserved memory for stolen memory via fw_cfg.  VM firmware
      * must allocate a 1MB aligned reserved memory region below 4GB with
-     * the requested size (in bytes) for use by the Intel PCI class VGA
-     * device at VM address 00:02.0.  The base address of this reserved
-     * memory region must be written to the device BDSM register at PCI
-     * config offset 0x5C.
+     * the requested size (in bytes) for use by the IGD device. The base
+     * address of this reserved memory region must be written to the
+     * device BDSM register.
+     * For newer device without BDSM register, this fw_cfg item is 0.
      */
     bdsm_size = g_malloc(sizeof(*bdsm_size));
     *bdsm_size = cpu_to_le64(gms_size);
     fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
                     bdsm_size, sizeof(*bdsm_size));
 
-    /* BDSM is read-write, emulated.  The BIOS needs to be able to write it */
-    if (gen < 11) {
-        pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
-        pci_set_long(vdev->pdev.wmask + IGD_BDSM, ~0);
-        pci_set_long(vdev->emulated_config_bits + IGD_BDSM, ~0);
-    } else {
-        pci_set_quad(vdev->pdev.config + IGD_BDSM_GEN11, 0);
-        pci_set_quad(vdev->pdev.wmask + IGD_BDSM_GEN11, ~0);
-        pci_set_quad(vdev->emulated_config_bits + IGD_BDSM_GEN11, ~0);
-    }
-
     trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name, (gms_size / MiB));
 
     return true;
-- 
2.47.2

Re: [PATCH 8/9] vfio/igd: Only emulate GGC register when x-igd-gms is set

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]

On Tue, 2025-04-29 at 00:10 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> x-igd-gms is used for overriding DSM region size in GGC register in
> both config space and MMIO BAR0, by default host value is used.
> There is no need to emulate it in default case.
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> 

Is the GGC register writeable after UEFI or is it locked? If it's writable, I'm
wondering what might happen if the guest writes to the GGC register to increase
DSM region size.


-- 
Kind regards,
Corvin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 8/9] vfio/igd: Only emulate GGC register when x-igd-gms is set

[Tomita Moeko]

On 2025/4/29 14:28, Corvin Köhne wrote:
> On Tue, 2025-04-29 at 00:10 +0800, Tomita Moeko wrote:
>> CAUTION: External Email!!
>> x-igd-gms is used for overriding DSM region size in GGC register in
>> both config space and MMIO BAR0, by default host value is used.
>> There is no need to emulate it in default case.
>>
>> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
>>
> 
> Is the GGC register writeable after UEFI or is it locked? If it's writable, I'm
> wondering what might happen if the guest writes to the GGC register to increase
> DSM region size.
> 

The document says its is read-only mirror to the Graphics Control register
in Host bridge (00:00.0). In guest, the register being mirrored does not
exist, and GCC seems to be a scratch register that only used by software.
GOP driver setup DSM according to the value emulated by QEMU, this can be
confirmed with intel_gtt tool in intel-gpu-tools. 

Setting GMS to 128MB do solves the strange issue that screen flickers on
4k60Hz display when host value is 64MB. Since the GOP driver is closed-
source, I am unable to explain this, but it works. 

Memory view of IGD passthrough looks like below. The emulated GGC register
is responsible for the size of Guest DSM region, there is no impact to the
host side.

       IGD Addr Space                 Host Addr Space         Guest Addr Space
       +-------------+                +-------------+         +-------------+
       |             |                |             |         |             |
       |             |                |             |         |             |
       |             |                +-------------+         +-------------+
       |             |                | Data Stolen |         | Data Stolen |
       |             |                |   (Guest)   |         |   (Guest)   |
       |             |  +------------>+-------------+<------->+-------------+<--Guest BDSM
       |             |  | Passthrough |             | EPT     |             |   Emulated by QEMU
DSMSIZE+-------------+  | with IOMMU  |             | Mapping |             |   Programmed by guest FW
       |             |  |             |             |         |             |
       |             |  |             |             |         |             |
      0+-------------+--+             |             |         |             |
                        |             +-------------+         |             |
                        |             | Data Stolen |         +-------------+
                        |             |   (Host)    |
                        +------------>+-------------+<--Host BDSM
                          Non-        |             |   "real" one in HW
                          Passthrough |             |   Programmed by host FW
                                      +-------------+
Thanks,
Moeko

Re: [PATCH 0/9] vfio/igd: Detect IGD by OpRegion and enable OpRegion automatically

[Tomita Moeko]

I forgot adding v2 in subject, sorry for my mistake.

Moeko

On 4/29/25 00:09, Tomita Moeko wrote:
> As proposed in a previous discussion [1], detect IGD devices based on
> whether it has VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION exposed by kernel
> and enables OpRegion access by default. This enables out-of-the-box
> display output support for IGD passthrough without having to manually
> set x-igd-opregion=on, which probably saves effort for libvirt users.
> 
> This patchset also limits legacy mode to Gen6-9 devices, as Intel no
> longer provides CSM support and VBIOS on newer IGD, and Seabios does not
> support setting up the 64 bit BDSM register on Gen 11+ devices. Exposing
> VGA ranges to guests by default on platforms without VGA mode support is
> undesirable.
> 
> Additionally, as we enables OpRegion on IGD devices by default, and
> Intel removes the BDSM register from Meteor Lake [2]. There seems to be
> no extra register quirks rather than OpRegion required on newer devices.
> To support them (and probably future devices), the generation limit is
> removed, with BDSM quirk only applied to known Gen 6-12 devices. 
> 
> Note: I have not been able to test this on Meteor Lake or newer
> platforms due to lack of hardware.
> 
> 
> [1] https://lore.kernel.org/qemu-devel/20250325172239.27926-1-tomitamoeko@gmail.com
> [2] https://edc.intel.com/content/www/us/en/design/publications/14th-generation-core-processors-cfg-and-mem-registers/d2-f0-processor-graphics-registers/
> 
> Changelog:
> v2:
> * Removed "Allow hotplugging with OpRegion enabled", hotplugging is
>   always forbidden.
> * Test device is not hotplugged and get opregion in a single function.
> * Update documentation along with code changes.
> * Minor code style fixes.
> Link: https://lore.kernel.org/qemu-devel/20250421163112.21316-1-tomitamoeko@gmail.coms
> 
> Tomita Moeko (9):
>   vfio/igd: Restrict legacy mode to Gen6-9 devices
>   vfio/igd: Always emulate ASLS (OpRegion) register
>   vfio/igd: Detect IGD device by OpRegion
>   vfio/igd: Check vendor and device ID on GVT-g mdev
>   vfio/igd: Check OpRegion support on GVT-g mdev
>   vfio/igd: Enable OpRegion by default
>   vfio/igd: Allow overriding GMS with 0xf0 to 0xfe on Gen9+
>   vfio/igd: Only emulate GGC register when x-igd-gms is set
>   vfio/igd: Remove generation limitation for IGD passthrough
> 
>  docs/igd-assign.txt |  11 ++-
>  hw/vfio/igd.c       | 218 ++++++++++++++++++++++++++------------------
>  hw/vfio/pci.c       |   2 +-
>  3 files changed, 137 insertions(+), 94 deletions(-)
> 

Re: [PATCH 1/9] vfio/igd: Restrict legacy mode to Gen6-9 devices

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 3358 bytes --]

On Tue, 2025-04-29 at 00:09 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> Intel only provides legacy VBIOS for IGD up to Gen9, and there is no
> CSM support on later devices. Additionally, Seabios can only handle
> 32-bit BDSM register used until Gen9. Since legacy mode requires VGA
> capability, restrict it to Gen6 through Gen9 devices.
> 
> Link:
> https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAADAsHNi0zEFiakAAACORk589NFmMEL0IRdt7DRVpnoi_6WKuXqQANgyXom23JAMNMr3UlEVSZvZdOtespZyqyGqsgLV2z0x8MkoDwNY9VFWuoC1hAOa_kpJihDYFTkqJw9sxp1uw6X9Oml3OGmOnrY8bVlHo2n7h5uiu73mh7RdgB0cFnBy59ExBLmCowsh6nXK5LA_Udnf-WbH0pzMWtCwjhPF453hx1Rm93_KYzUhuyswu_sg0
>  
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  docs/igd-assign.txt | 1 +
>  hw/vfio/igd.c       | 6 +++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
> index 3aed7956d5..95beb76812 100644
> --- a/docs/igd-assign.txt
> +++ b/docs/igd-assign.txt
> @@ -47,6 +47,7 @@ Intel document [1] shows how to dump VBIOS to file. For UEFI
> Option ROM, see
>  
>  QEMU also provides a "Legacy" mode that implicitly enables full functionality
>  on IGD, it is automatically enabled when
> +* IGD generation is 6 to 9 (Sandy Bridge to Comet Lake)
>  * Machine type is i440fx
>  * IGD is assigned to guest BDF 00:02.0
>  * ROM BAR or romfile is present
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index 6678e0e5cd..cecc3245b7 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -516,11 +516,13 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
>  
>      /*
>       * For backward compatibility, enable legacy mode when
> +     * - Device geneation is 6 to 9 (including both)
>       * - Machine type is i440fx (pc_piix)
>       * - IGD device is at guest BDF 00:02.0
>       * - Not manually disabled by x-igd-legacy-mode=off
>       */
>      if ((vdev->igd_legacy_mode != ON_OFF_AUTO_OFF) &&
> +        (gen >= 6 && gen <= 9) &&
>          !strcmp(MACHINE_GET_CLASS(qdev_get_machine())->family, "pc_piix") &&
>          (&vdev->pdev == pci_find_device(pci_device_root_bus(&vdev->pdev),
>          0, PCI_DEVFN(0x2, 0)))) {
> @@ -565,7 +567,9 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev,
> Error **errp)
>          vdev->features |= VFIO_FEATURE_ENABLE_IGD_LPC;
>      } else if (vdev->igd_legacy_mode == ON_OFF_AUTO_ON) {
>          error_setg(&err,
> -                   "Machine is not i440fx or assigned BDF is not 00:02.0");
> +                   "Machine is not i440fx, assigned BDF is not 00:02.0, "
> +                   "or device %04x (gen %d) doesn't support legacy mode",
> +                   vdev->device_id, gen);
>          goto error;
>      }
>  

Afaik, at least the LPC quirk is required for Gen 10 and above for the GOP to
work properly. On the other hand, Intel hasn't published any GOP yet, so it
might be unlikely that user use it. So, not sure if this is a breaking change or
not.

Besides that:

Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>


-- 
Kind regards,
Corvin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/9] vfio/igd: Always emulate ASLS (OpRegion) register

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 2602 bytes --]

On Tue, 2025-04-29 at 00:09 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> ASLS register represents the base address of OpRegion, and it is
> programmed with HPA. In IGD passthrough scenario, it needs to be
> reprogrammed with GPA by guest firmware. To prevent guest accessing
> wrong memory range, ASLS should always be emulated and cleared.
> 
> In GVT-g scenario, emulating ASLS is unnecessary as access is handled
> by kvmgt backend [1].
> 
> [1]:
> https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAABJA62ZTEhCO9AAAABrY_qcw0M6GbfmA-k7rdTnC1V9Xvjw6ICEYmTbOt6IWZMsfh2X-PfG-_1bXLcfyp_Hlcdbset-kq2osW-E9lPT94hoZN2NjovxXaqp2dueSyklxM-bP7ox1AHZkQJVv5lgR-iU3PuZeJMms5zmOuYd9fmL0ePbCC-bv9lN8VEaMAQWUFVb0pGt2t9WWfuMxqO6p0pmItn7Qhonu9rHCbBSelFWN9wby1aKHKufBkSBgBiuN2lSz9nZ8WTeKQ8ifh8cfOhPiAsVRWSeVpNeuoyV0
>  
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/igd.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index cecc3245b7..ae19456457 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -182,10 +182,6 @@ static bool vfio_pci_igd_opregion_init(VFIOPCIDevice
> *vdev,
>  
>      trace_vfio_pci_igd_opregion_enabled(vdev-
> >https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAABJA62ZTEhCO2wAAADrv
> lj4V-SebC4SGxyCHAuiWIj2uPV-Sufmo150PAYs9nDhYSQLRd1-
> LAyUjMsdhaDDdO6n0SKd9uzKZMhYr5A8uAV__w527iEN3jptdWlJNyitq2eScjUl3HLAVTzheEDnkr
> yTDojPqVjnFFk1 );
>  
> -    pci_set_long(vdev->pdev.config + IGD_ASLS, 0);
> -    pci_set_long(vdev->pdev.wmask + IGD_ASLS, ~0);
> -    pci_set_long(vdev->emulated_config_bits + IGD_ASLS, ~0);
> -
>      return true;
>  }
>  
> @@ -583,7 +579,15 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
>      if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_LPC) &&
>          !vfio_pci_igd_setup_lpc_bridge(vdev, errp)) {
>          goto error;
> -     }
> +    }
> +
> +    /*
> +     * ASLS (OpRegion address) is read-only, emulated
> +     * It contains HPA, guest firmware need to reprogram it with GPA.
> +     */
> +    pci_set_long(vdev->pdev.config + IGD_ASLS, 0);
> +    pci_set_long(vdev->pdev.wmask + IGD_ASLS, ~0);
> +    pci_set_long(vdev->emulated_config_bits + IGD_ASLS, ~0);
>  
>      /*
>       * Allow user to override dsm size using x-igd-gms option, in multiples
> of

Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>


-- 
Kind regards,
Corvin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/9] vfio/igd: Detect IGD device by OpRegion

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 4470 bytes --]

On Tue, 2025-04-29 at 00:09 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> There is currently no straightforward way to distinguish if a Intel
> graphics device is IGD or discrete GPU. However, only IGD devices have
> OpRegion. Use the presence of VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION
> to identify IGD devices. Still, OpRegion on hotplugged IGD device is
> not supported.
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/igd.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index ae19456457..d6880cbff7 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -185,9 +185,10 @@ static bool vfio_pci_igd_opregion_init(VFIOPCIDevice
> *vdev,
>      return true;
>  }
>  
> -static bool vfio_pci_igd_setup_opregion(VFIOPCIDevice *vdev, Error **errp)
> +static bool vfio_pci_igd_opregion_detect(VFIOPCIDevice *vdev,
> +                                         struct vfio_region_info **opregion,
> +                                         Error **errp)
>  {
> -    g_autofree struct vfio_region_info *opregion = NULL;
>      int ret;
>  
>      /* Hotplugging is not supported for opregion access */
> @@ -198,17 +199,13 @@ static bool vfio_pci_igd_setup_opregion(VFIOPCIDevice
> *vdev, Error **errp)
>  
>      ret = vfio_device_get_region_info_type(&vdev->vbasedev,
>                      VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
> -                    VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, &opregion);
> +                    VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, opregion);
>      if (ret) {
>          error_setg_errno(errp, -ret,
>                           "Device does not supports IGD OpRegion feature");
>          return false;
>      }
>  
> -    if (!vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
> -        return false;
> -    }
> -
>      return true;
>  }
>  
> @@ -479,6 +476,7 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int
> nr)
>  
>  static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>  {
> +    g_autofree struct vfio_region_info *opregion = NULL;
>      int ret, gen;
>      uint64_t gms_size;
>      uint64_t *bdsm_size;
> @@ -486,16 +484,17 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
>      bool legacy_mode_enabled = false;
>      Error *err = NULL;
>  
> -    /*
> -     * This must be an Intel VGA device at address 00:02.0 for us to even
> -     * consider enabling legacy mode.  The vBIOS has dependencies on the
> -     * PCI bus address.
> -     */
>      if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
>          !vfio_is_vga(vdev)) {
>          return true;
>      }
>  
> +    /* IGD device always comes with OpRegion */
> +    if (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp)) {
> +        return true;
> +    }
> +    info_report("OpRegion detected on Intel display %x.", vdev->device_id);
> +
>      /*
>       * IGD is not a standard, they like to change their specs often.  We
>       * only attempt to support back to SandBridge and we hope that newer
> @@ -571,7 +570,7 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev,
> Error **errp)
>  
>      /* Setup OpRegion access */
>      if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) &&
> -        !vfio_pci_igd_setup_opregion(vdev, errp)) {
> +        !vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
>          goto error;
>      }
>  
> @@ -671,8 +670,11 @@ error:
>   */
>  static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>  {
> +    g_autofree struct vfio_region_info *opregion = NULL;
> +
>      if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) &&
> -        !vfio_pci_igd_setup_opregion(vdev, errp)) {
> +        (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp) ||
> +         !vfio_pci_igd_opregion_init(vdev, opregion, errp))) {
>          return false;
>      }
>  

Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>


-- 
Kind regards,
Corvin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/9] vfio/igd: Check vendor and device ID on GVT-g mdev

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 1935 bytes --]

On Tue, 2025-04-29 at 00:09 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> Check the vendor and device ID on GVT-g mdev to ensure it is a supported
> device [1]. This extra check is required for automatically enabling
> OpRegion access later.
> 
> Note that Cherryview and Gemini Lake are marked as supported here since
> current code cannot distinguish them with other Gen8 and Gen9 devices.
> Since mdev cannot be created on these devices, this has no functional
> impact.
> 
> [1]
> https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAAC7d6njJfw46ssAAADF-nyovaDPsQJyKOV5T7sLGnzTfabj4BLcrjbmZT8baVh1nRPWc5X1Mlcgmchoiq4Ame7F9pv8Dm3p32EVUqXHVZUZ4ydwKGK8058NOSzZdk88Xvq87l3akIni9zsivM8SufPB80Ps4QQhkcFR2xjFsyp1doPFX7-vdSDEuv_KSFgq4SZ6UXxq6JDwaLRbsVEkIog7tW-TROEpexsHJm7QzPVbqSCuoHky-Rb7fvkEimsF6AGSvCpAEejUDe78EDkE1k4RSw53IT6Ohg2
>  
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/igd.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index d6880cbff7..c6ecbefb35 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -671,6 +671,18 @@ error:
>  static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>  {
>      g_autofree struct vfio_region_info *opregion = NULL;
> +    int gen;
> +
> +    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
> +        !vfio_is_vga(vdev)) {
> +        return true;
> +    }
> +
> +    /* FIXME: Cherryview is Gen8, but don't support GVT-g */
> +    gen = igd_gen(vdev);
> +    if (gen != 8 && gen != 9) {
> +        return true;
> +    }
>  
>      if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) &&
>          (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp) ||

Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>


-- 
Kind regards,
Corvin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/9] vfio/igd: Check OpRegion support on GVT-g mdev

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 1497 bytes --]

On Tue, 2025-04-29 at 00:10 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> The Intel GVT-g backend `kvmgt` always emulates OpRegion for vGPU,
> make sure the OpRegion is present for enabling access to it
> automatically later.
> 
> Also, hotplugging GVT-g vGPU is now always disallowed regardless of
> OpRegion to prevent potential issues. Intel has never claimed support
> for GVT-g hotplugging.
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/igd.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index c6ecbefb35..496d3df598 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -684,9 +684,13 @@ static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
>          return true;
>      }
>  
> +    if (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp)) {
> +        /* Should never reach here, KVMGT always emulates OpRegion */
> +        return false;
> +    }
> +
>      if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) &&
> -        (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp) ||
> -         !vfio_pci_igd_opregion_init(vdev, opregion, errp))) {
> +        !vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
>          return false;
>      }
>  

Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>


-- 
Kind regards,
Corvin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 6/9] vfio/igd: Enable OpRegion by default

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 2636 bytes --]

On Tue, 2025-04-29 at 00:10 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> As the presence of OpRegion is used to detect IGD device now, and
> guest driver usually depends on OpRegion to work. Enable OpRegion
> on IGD devices by default for out-of-the-box passthrough experience
> (except pre-boot display output), especially for libvirt users.
> 
> Example of IGD passthrough with libvirt:
> <hostdev mode="subsystem" type="pci" managed="yes">
>   <source>
>     <address domain="0x0000" bus="0x00" slot="0x02" function="0x0"/>
>   </source>
>   <rom file="/path/to/igd/rom"/>
>   <address type="pci" domain="0x0000" bus="0x00" slot="0x02" function="0x0"/>
>  </hostdev>
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  docs/igd-assign.txt | 4 ++--
>  hw/vfio/pci.c       | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
> index 95beb76812..fc444503ff 100644
> --- a/docs/igd-assign.txt
> +++ b/docs/igd-assign.txt
> @@ -102,7 +102,7 @@ digital formats work well.
>  
>  Options
>  =======
> -* x-igd-opregion=[on|*off*]
> +* x-igd-opregion=[*on*|off]
>    Copy host IGD OpRegion and expose it to guest with fw_cfg
>  
>  * x-igd-lpc=[on|*off*]
> @@ -124,7 +124,7 @@ Examples
>  
>  * Adding IGD with OpRegion and LPC ID hack, but without VGA ranges
>    (For UEFI guests)
> -  -device vfio-pci,host=00:02.0,id=hostdev0,addr=2.0,x-igd-legacy-mode=off,x-
> igd-opregion=on,x-igd-lpc=on,romfile=efi_oprom.rom
> +  -device vfio-pci,host=00:02.0,id=hostdev0,addr=2.0,x-igd-legacy-mode=off,x-
> igd-lpc=on,romfile=efi_oprom.rom
>  
>  
>  Guest firmware
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 05a7a62204..38ff231625 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3361,7 +3361,7 @@ static const Property vfio_pci_dev_properties[] = {
>      DEFINE_PROP_BIT("x-req", VFIOPCIDevice, features,
>                      VFIO_FEATURE_ENABLE_REQ_BIT, true),
>      DEFINE_PROP_BIT("x-igd-opregion", VFIOPCIDevice, features,
> -                    VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
> +                    VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, true),
>      DEFINE_PROP_BIT("x-igd-lpc", VFIOPCIDevice, features,
>                      VFIO_FEATURE_ENABLE_IGD_LPC_BIT, false),
>      DEFINE_PROP_ON_OFF_AUTO("x-igd-legacy-mode", VFIOPCIDevice,

Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>


-- 
Kind regards,
Corvin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 7/9] vfio/igd: Allow overriding GMS with 0xf0 to 0xfe on Gen9+

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 4108 bytes --]

On Tue, 2025-04-29 at 00:10 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> On Gen9 and later IGD devices, GMS 0xf0 to 0xfe represents 4MB to 60MB
> pre-allocated memory size in 4MB increments. Allow users overriding
> GMS with these values.
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/igd.c | 59 +++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 41 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index 496d3df598..7f289a62a3 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -411,6 +411,44 @@ static bool vfio_pci_igd_setup_lpc_bridge(VFIOPCIDevice
> *vdev, Error **errp)
>      return true;
>  }
>  
> +static bool vfio_pci_igd_override_gms(int gen, uint32_t gms, uint32_t *gmch)
> +{
> +    bool ret = false;
> +
> +    if (gen == -1) {
> +        error_report("x-igd-gms is not supported on this device");
> +    } else if (gen < 8) {
> +        if (gms <= 0x10) {
> +            *gmch &= ~(IGD_GMCH_GEN6_GMS_MASK << IGD_GMCH_GEN6_GMS_SHIFT);
> +            *gmch |= gms << IGD_GMCH_GEN6_GMS_SHIFT;
> +            ret = true;
> +        } else {
> +            error_report(QERR_INVALID_PARAMETER_VALUE, "x-igd-gms",
> "0~0x10");
> +        }
> +    } else if (gen == 8) {
> +        if (gms <= 0x40) {
> +            *gmch &= ~(IGD_GMCH_GEN8_GMS_MASK << IGD_GMCH_GEN8_GMS_SHIFT);
> +            *gmch |= gms << IGD_GMCH_GEN8_GMS_SHIFT;
> +            ret = true;
> +        } else {
> +            error_report(QERR_INVALID_PARAMETER_VALUE, "x-igd-gms",
> "0~0x40");
> +        }
> +    } else {
> +        /* 0x0  to 0x40: 32MB increments starting at 0MB */
> +        /* 0xf0 to 0xfe: 4MB increments starting at 4MB */
> +        if ((gms <= 0x40) || (gms >= 0xf0 && gms <= 0xfe)) {
> +            *gmch &= ~(IGD_GMCH_GEN8_GMS_MASK << IGD_GMCH_GEN8_GMS_SHIFT);
> +            *gmch |= gms << IGD_GMCH_GEN8_GMS_SHIFT;
> +            ret = true;
> +        } else {
> +            error_report(QERR_INVALID_PARAMETER_VALUE,
> +                         "x-igd-gms", "0~0x40 or 0xf0~0xfe");
> +        }
> +    }
> +
> +    return ret;
> +}
> +
>  #define IGD_GGC_MMIO_OFFSET     0x108040
>  #define IGD_BDSM_MMIO_OFFSET    0x1080C0
>  
> @@ -593,24 +631,9 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
>       * 32MiB. This option should only be used when the desired size cannot be
>       * set from DVMT Pre-Allocated option in host BIOS.
>       */
> -    if (vdev->igd_gms) {
> -        if (gen < 8) {
> -            if (vdev->igd_gms <= 0x10) {
> -                gmch &= ~(IGD_GMCH_GEN6_GMS_MASK << IGD_GMCH_GEN6_GMS_SHIFT);
> -                gmch |= vdev->igd_gms << IGD_GMCH_GEN6_GMS_SHIFT;
> -            } else {
> -                error_report(QERR_INVALID_PARAMETER_VALUE,
> -                             "x-igd-gms", "0~0x10");
> -            }
> -        } else {
> -            if (vdev->igd_gms <= 0x40) {
> -                gmch &= ~(IGD_GMCH_GEN8_GMS_MASK << IGD_GMCH_GEN8_GMS_SHIFT);
> -                gmch |= vdev->igd_gms << IGD_GMCH_GEN8_GMS_SHIFT;
> -            } else {
> -                error_report(QERR_INVALID_PARAMETER_VALUE,
> -                             "x-igd-gms", "0~0x40");
> -            }
> -        }
> +    if (vdev->igd_gms &&
> +        !vfio_pci_igd_override_gms(gen, vdev->igd_gms, &gmch)) {
> +        return false;
>      }
>  
>      gms_size = igd_stolen_memory_size(gen, gmch);

Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>


-- 
Kind regards,
Corvin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 8/9] vfio/igd: Only emulate GGC register when x-igd-gms is set

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 4733 bytes --]

On Tue, 2025-04-29 at 00:10 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> x-igd-gms is used for overriding DSM region size in GGC register in
> both config space and MMIO BAR0, by default host value is used.
> There is no need to emulate it in default case.
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/igd.c | 49 ++++++++++++++++++++++++++-----------------------
>  1 file changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index 7f289a62a3..5d12f753ab 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -477,22 +477,24 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int
> nr)
>          return;
>      }
>  
> -    ggc_quirk = vfio_quirk_alloc(1);
> -    ggc_mirror = ggc_quirk->data = g_malloc0(sizeof(*ggc_mirror));
> -    ggc_mirror->mem = ggc_quirk->mem;
> -    ggc_mirror->vdev = vdev;
> -    ggc_mirror->bar = nr;
> -    ggc_mirror->offset = IGD_GGC_MMIO_OFFSET;
> -    ggc_mirror->config_offset = IGD_GMCH;
> -
> -    memory_region_init_io(ggc_mirror->mem, OBJECT(vdev),
> -                          &vfio_generic_mirror_quirk, ggc_mirror,
> -                          "vfio-igd-ggc-quirk", 2);
> -    memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
> -                                        ggc_mirror->offset, ggc_mirror->mem,
> -                                        1);
> +    if (vdev->igd_gms) {
> +        ggc_quirk = vfio_quirk_alloc(1);
> +        ggc_mirror = ggc_quirk->data = g_malloc0(sizeof(*ggc_mirror));
> +        ggc_mirror->mem = ggc_quirk->mem;
> +        ggc_mirror->vdev = vdev;
> +        ggc_mirror->bar = nr;
> +        ggc_mirror->offset = IGD_GGC_MMIO_OFFSET;
> +        ggc_mirror->config_offset = IGD_GMCH;
>  
> -    QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, ggc_quirk, next);
> +        memory_region_init_io(ggc_mirror->mem, OBJECT(vdev),
> +                              &vfio_generic_mirror_quirk, ggc_mirror,
> +                              "vfio-igd-ggc-quirk", 2);
> +        memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
> +                                            ggc_mirror->offset, ggc_mirror-
> >mem,
> +                                            1);
> +
> +        QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, ggc_quirk, next);
> +    }
>  
>      bdsm_quirk = vfio_quirk_alloc(1);
>      bdsm_mirror = bdsm_quirk->data = g_malloc0(sizeof(*bdsm_mirror));
> @@ -631,9 +633,15 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
>       * 32MiB. This option should only be used when the desired size cannot be
>       * set from DVMT Pre-Allocated option in host BIOS.
>       */
> -    if (vdev->igd_gms &&
> -        !vfio_pci_igd_override_gms(gen, vdev->igd_gms, &gmch)) {
> -        return false;
> +    if (vdev->igd_gms) {
> +        if (vfio_pci_igd_override_gms(gen, vdev->igd_gms, &gmch)) {
> +            /* GMCH is read-only, emulated */
> +            pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
> +            pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
> +            pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
> +        } else {
> +            return false;
> +        }

nit suggestion: You could avoid a level of indentation by using this style:

if (!vfio_pci_igd_override_gms(...)) {
    return false;
}

/* GMCH is read-only, emulated */
pci_set_long(...)

>      }
>  
>      gms_size = igd_stolen_memory_size(gen, gmch);
> @@ -651,11 +659,6 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
>      fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
>                      bdsm_size, sizeof(*bdsm_size));
>  
> -    /* GMCH is read-only, emulated */
> -    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
> -    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
> -    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
> -
>      /* BDSM is read-write, emulated.  The BIOS needs to be able to write it
> */
>      if (gen < 11) {
>          pci_set_long(vdev->pdev.config + IGD_BDSM, 0);

Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>


-- 
Kind regards,
Corvin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/9] vfio/igd: Detect IGD by OpRegion and enable OpRegion automatically

[Cédric Le Goater]

On 5/5/25 09:33, Tomita Moeko wrote:
> I forgot adding v2 in subject, sorry for my mistake.

That's ok. Just use a v3 in the next spin, if needed.

Only one patch, the last, is not reviewed yet. Will there be a v3 ?

Thanks,

C.


> 
> Moeko
> 
> On 4/29/25 00:09, Tomita Moeko wrote:
>> As proposed in a previous discussion [1], detect IGD devices based on
>> whether it has VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION exposed by kernel
>> and enables OpRegion access by default. This enables out-of-the-box
>> display output support for IGD passthrough without having to manually
>> set x-igd-opregion=on, which probably saves effort for libvirt users.
>>
>> This patchset also limits legacy mode to Gen6-9 devices, as Intel no
>> longer provides CSM support and VBIOS on newer IGD, and Seabios does not
>> support setting up the 64 bit BDSM register on Gen 11+ devices. Exposing
>> VGA ranges to guests by default on platforms without VGA mode support is
>> undesirable.
>>
>> Additionally, as we enables OpRegion on IGD devices by default, and
>> Intel removes the BDSM register from Meteor Lake [2]. There seems to be
>> no extra register quirks rather than OpRegion required on newer devices.
>> To support them (and probably future devices), the generation limit is
>> removed, with BDSM quirk only applied to known Gen 6-12 devices.
>>
>> Note: I have not been able to test this on Meteor Lake or newer
>> platforms due to lack of hardware.
>>
>>
>> [1] https://lore.kernel.org/qemu-devel/20250325172239.27926-1-tomitamoeko@gmail.com
>> [2] https://edc.intel.com/content/www/us/en/design/publications/14th-generation-core-processors-cfg-and-mem-registers/d2-f0-processor-graphics-registers/
>>
>> Changelog:
>> v2:
>> * Removed "Allow hotplugging with OpRegion enabled", hotplugging is
>>    always forbidden.
>> * Test device is not hotplugged and get opregion in a single function.
>> * Update documentation along with code changes.
>> * Minor code style fixes.
>> Link: https://lore.kernel.org/qemu-devel/20250421163112.21316-1-tomitamoeko@gmail.coms
>>
>> Tomita Moeko (9):
>>    vfio/igd: Restrict legacy mode to Gen6-9 devices
>>    vfio/igd: Always emulate ASLS (OpRegion) register
>>    vfio/igd: Detect IGD device by OpRegion
>>    vfio/igd: Check vendor and device ID on GVT-g mdev
>>    vfio/igd: Check OpRegion support on GVT-g mdev
>>    vfio/igd: Enable OpRegion by default
>>    vfio/igd: Allow overriding GMS with 0xf0 to 0xfe on Gen9+
>>    vfio/igd: Only emulate GGC register when x-igd-gms is set
>>    vfio/igd: Remove generation limitation for IGD passthrough
>>
>>   docs/igd-assign.txt |  11 ++-
>>   hw/vfio/igd.c       | 218 ++++++++++++++++++++++++++------------------
>>   hw/vfio/pci.c       |   2 +-
>>   3 files changed, 137 insertions(+), 94 deletions(-)
>>
> 

Re: [PATCH 9/9] vfio/igd: Remove generation limitation for IGD passthrough

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 8560 bytes --]

On Tue, 2025-04-29 at 00:10 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> Starting from Intel Core Ultra Series (Meteor Lake), Data Stolen Memory
> has became a part of LMEMBAR (MMIO BAR2) [1][2], meaning that BDSM and
> GGC register quirks are no longer needed on these platforms.
> 
> To support Meteor/Arrow/Lunar Lake and future IGD devices, remove the
> generation limitation in IGD passthrough, and apply BDSM and GGC quirks
> only to known Gen6-12 devices.
> 
> [1]
> https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAACrZmEBKRIGeu0AAADvIwGqJzMNfJw1SGDaz4T4NOe0pQjgcXIl8aa6EBISsE_mEsZ1x35XrJZk1uUAVgzAGUkZWFMG--4B2zOo1pBHv9oVATF-lJkWnY1dOUOHqYlAt4T5EfdMCovIk0M0ZxBIgFBnJEE3wNG6NOh1mPjge5-M1OW80X-Dp9n6iSKirvdFiYnh9VLEHlff9BdoT5IJ8JjnKnoVVAT7iuWwkFDayl2MoMIuAKFMrDxfrXsbkPQYuHMP0b_bdAgRcors5TKTBFPsQ1IKC7wICpETvUXKQnqex7TU1gzMwYVj2iEC9PKyiY8RBfLgXlCWhE81
>  
> [2]
> https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAACrZmEBKRIGetYAAACNfuvceS_GGt4XbczrWqbmNztfDneI6ldPTQC3yZpB9711r9-sHzWe45iikAQRMr99rOo7ZX_vc6L9JWHxUovjxtqg88ymadBor_RtfcJUH6gTjN4bCIsufZ84hsdPPZ4VCkMbFxROFqERsVxQpR_kPhvdqbni1CwWW3rGeBkifKTUC4rH-OmGNSww_6COlh2arRPZR899bXdYf1SFGjDc6zbSFE36nMBDW9jc3tBgb2VaMcERYIL-TVSLQoYnRxeybcAqQAE41ZyIoFEH8FHyGaikRWLl0
>  
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  docs/igd-assign.txt |  6 +++++
>  hw/vfio/igd.c       | 58 ++++++++++++++++-----------------------------
>  2 files changed, 27 insertions(+), 37 deletions(-)
> 
> diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
> index fc444503ff..af4e8391fc 100644
> --- a/docs/igd-assign.txt
> +++ b/docs/igd-assign.txt
> @@ -157,6 +157,12 @@ fw_cfg requirements on the VM firmware:
>     it's expected that this fw_cfg file is only relevant to a single PCI
>     class VGA device with Intel vendor ID, appearing at PCI bus address
> 00:02.0.
>  
> +   Starting from Meteor Lake, IGD devices access stolen memory via its MMIO
> +   BAR2 (LMEMBAR) and removed the BDSM register in config space. There is
> +   no need for guest firmware to allocate data stolen memory in guest address
> +   space and write it to BDSM register. Value of this fw_cfg file is 0 in
> +   such case.
> +
>  Upstream Seabios has OpRegion and BDSM (pre-Gen11 device only) support.
>  However, the support is not accepted by upstream EDK2/OVMF. A recommended
>  solution is to create a virtual OpRom with following DXE drivers:
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index 5d12f753ab..2584861ae6 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -103,6 +103,7 @@ static int igd_gen(VFIOPCIDevice *vdev)
>      /*
>       * Unfortunately, Intel changes it's specification quite often. This
> makes
>       * it impossible to use a suitable default value for unknown devices.
> +     * Return -1 for not applying any generation-specific quirks.
>       */
>      return -1;
>  }
> @@ -458,20 +459,12 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int
> nr)
>      VFIOConfigMirrorQuirk *ggc_mirror, *bdsm_mirror;
>      int gen;
>  
> -    /*
> -     * This must be an Intel VGA device at address 00:02.0 for us to even
> -     * consider enabling legacy mode. Some driver have dependencies on the
> PCI
> -     * bus address.
> -     */
>      if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
>          !vfio_is_vga(vdev) || nr != 0) {
>          return;
>      }
>  
> -    /*
> -     * Only on IGD devices of gen 11 and above, the BDSM register is mirrored
> -     * into MMIO space and read from MMIO space by the Windows driver.
> -     */
> +    /* Only on IGD Gen6-12 device needs quirks in BAR 0 */
>      gen = igd_gen(vdev);
>      if (gen < 6) {
>          return;
> @@ -518,7 +511,7 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev,
> Error **errp)
>  {
>      g_autofree struct vfio_region_info *opregion = NULL;
>      int ret, gen;
> -    uint64_t gms_size;
> +    uint64_t gms_size = 0;
>      uint64_t *bdsm_size;
>      uint32_t gmch;
>      bool legacy_mode_enabled = false;
> @@ -535,18 +528,7 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
>      }
>      info_report("OpRegion detected on Intel display %x.", vdev->device_id);
>  
> -    /*
> -     * IGD is not a standard, they like to change their specs often.  We
> -     * only attempt to support back to SandBridge and we hope that newer
> -     * devices maintain compatibility with generation 8.
> -     */
>      gen = igd_gen(vdev);
> -    if (gen == -1) {
> -        error_report("IGD device %s is unsupported in legacy mode, "
> -                     "try SandyBridge or newer", vdev-
> >https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAACrZmEBKRIGemwAAAAwg
> gXVbgmULncwUjdD3WRW6m3nKghz9vudEZhl9xzeICl7FUK5O-
> hjEdzY8nxw3ASLDUPNCoEiymJZadffJUCslCcwoArfPIlRFLV9huLvwU-
> 6mMTuTItplXGJHszjVRgrc7pHIkf98_n1wyM1 );
> -        return true;
> -    }
> -
>      gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
>  
>      /*
> @@ -644,32 +626,34 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
>          }
>      }
>  
> -    gms_size = igd_stolen_memory_size(gen, gmch);
> +    if (gen > 0) {
> +        gms_size = igd_stolen_memory_size(gen, gmch);
> +
> +        /* BDSM is read-write, emulated. BIOS needs to be able to write it */
> +        if (gen < 11) {
> +            pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
> +            pci_set_long(vdev->pdev.wmask + IGD_BDSM, ~0);
> +            pci_set_long(vdev->emulated_config_bits + IGD_BDSM, ~0);
> +        } else {
> +            pci_set_quad(vdev->pdev.config + IGD_BDSM_GEN11, 0);
> +            pci_set_quad(vdev->pdev.wmask + IGD_BDSM_GEN11, ~0);
> +            pci_set_quad(vdev->emulated_config_bits + IGD_BDSM_GEN11, ~0);
> +        }
> +    }
>  
>      /*
>       * Request reserved memory for stolen memory via fw_cfg.  VM firmware
>       * must allocate a 1MB aligned reserved memory region below 4GB with
> -     * the requested size (in bytes) for use by the Intel PCI class VGA
> -     * device at VM address 00:02.0.  The base address of this reserved
> -     * memory region must be written to the device BDSM register at PCI
> -     * config offset 0x5C.
> +     * the requested size (in bytes) for use by the IGD device. The base
> +     * address of this reserved memory region must be written to the
> +     * device BDSM register.
> +     * For newer device without BDSM register, this fw_cfg item is 0.
>       */
>      bdsm_size = g_malloc(sizeof(*bdsm_size));
>      *bdsm_size = cpu_to_le64(gms_size);
>      fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
>                      bdsm_size, sizeof(*bdsm_size));
>  
> -    /* BDSM is read-write, emulated.  The BIOS needs to be able to write it
> */
> -    if (gen < 11) {
> -        pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
> -        pci_set_long(vdev->pdev.wmask + IGD_BDSM, ~0);
> -        pci_set_long(vdev->emulated_config_bits + IGD_BDSM, ~0);
> -    } else {
> -        pci_set_quad(vdev->pdev.config + IGD_BDSM_GEN11, 0);
> -        pci_set_quad(vdev->pdev.wmask + IGD_BDSM_GEN11, ~0);
> -        pci_set_quad(vdev->emulated_config_bits + IGD_BDSM_GEN11, ~0);
> -    }
> -
>      trace_vfio_pci_igd_bdsm_enabled(vdev-
> >https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAACrZmEBKRIGemwAAAAwg
> gXVbgmULncwUjdD3WRW6m3nKghz9vudEZhl9xzeICl7FUK5O-
> hjEdzY8nxw3ASLDUPNCoEiymJZadffJUCslCcwoArfPIlRFLV9huLvwU-
> 6mMTuTItplXGJHszjVRgrc7pHIkf98_n1wyM1 , (gms_size / MiB));
>  
>      return true;

This commit looks good. However, you haven't tested it yet, right? I'm also
unable to test it. It would be good if someone could test this patch before
merging it.

Not sure what upstream thinks about it. From my side:

Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>


-- 
Kind regards,
Corvin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/9] vfio/igd: Detect IGD by OpRegion and enable OpRegion automatically

[Corvin Köhne]

[-- Attachment #1: Type: text/plain, Size: 446 bytes --]

On Mon, 2025-05-05 at 14:54 +0200, Cédric Le Goater wrote:
> CAUTION: External Email!!
> On 5/5/25 09:33, Tomita Moeko wrote:
> > I forgot adding v2 in subject, sorry for my mistake.
> 
> That's ok. Just use a v3 in the next spin, if needed.
> 
> Only one patch, the last, is not reviewed yet. Will there be a v3 ?
> 
> Thanks,
> 
> C.
> 

Sry, somehow missed the last patch. Have reviewed it now.


-- 
Kind regards,
Corvin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/9] vfio/igd: Restrict legacy mode to Gen6-9 devices

[Tomita Moeko]

On 5/5/25 16:42, Corvin Köhne wrote:
> On Tue, 2025-04-29 at 00:09 +0800, Tomita Moeko wrote:
>> CAUTION: External Email!!
>> Intel only provides legacy VBIOS for IGD up to Gen9, and there is no
>> CSM support on later devices. Additionally, Seabios can only handle
>> 32-bit BDSM register used until Gen9. Since legacy mode requires VGA
>> capability, restrict it to Gen6 through Gen9 devices.
>>
>> Link:
>> https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAADAsHNi0zEFiakAAACORk589NFmMEL0IRdt7DRVpnoi_6WKuXqQANgyXom23JAMNMr3UlEVSZvZdOtespZyqyGqsgLV2z0x8MkoDwNY9VFWuoC1hAOa_kpJihDYFTkqJw9sxp1uw6X9Oml3OGmOnrY8bVlHo2n7h5uiu73mh7RdgB0cFnBy59ExBLmCowsh6nXK5LA_Udnf-WbH0pzMWtCwjhPF453hx1Rm93_KYzUhuyswu_sg0
>>  
>> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
>> ---
>>  docs/igd-assign.txt | 1 +
>>  hw/vfio/igd.c       | 6 +++++-
>>  2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
>> index 3aed7956d5..95beb76812 100644
>> --- a/docs/igd-assign.txt
>> +++ b/docs/igd-assign.txt
>> @@ -47,6 +47,7 @@ Intel document [1] shows how to dump VBIOS to file. For UEFI
>> Option ROM, see
>>  
>>  QEMU also provides a "Legacy" mode that implicitly enables full functionality
>>  on IGD, it is automatically enabled when
>> +* IGD generation is 6 to 9 (Sandy Bridge to Comet Lake)
>>  * Machine type is i440fx
>>  * IGD is assigned to guest BDF 00:02.0
>>  * ROM BAR or romfile is present
>> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
>> index 6678e0e5cd..cecc3245b7 100644
>> --- a/hw/vfio/igd.c
>> +++ b/hw/vfio/igd.c
>> @@ -516,11 +516,13 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
>> *vdev, Error **errp)
>>  
>>      /*
>>       * For backward compatibility, enable legacy mode when
>> +     * - Device geneation is 6 to 9 (including both)
>>       * - Machine type is i440fx (pc_piix)
>>       * - IGD device is at guest BDF 00:02.0
>>       * - Not manually disabled by x-igd-legacy-mode=off
>>       */
>>      if ((vdev->igd_legacy_mode != ON_OFF_AUTO_OFF) &&
>> +        (gen >= 6 && gen <= 9) &&
>>          !strcmp(MACHINE_GET_CLASS(qdev_get_machine())->family, "pc_piix") &&
>>          (&vdev->pdev == pci_find_device(pci_device_root_bus(&vdev->pdev),
>>          0, PCI_DEVFN(0x2, 0)))) {
>> @@ -565,7 +567,9 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev,
>> Error **errp)
>>          vdev->features |= VFIO_FEATURE_ENABLE_IGD_LPC;
>>      } else if (vdev->igd_legacy_mode == ON_OFF_AUTO_ON) {
>>          error_setg(&err,
>> -                   "Machine is not i440fx or assigned BDF is not 00:02.0");
>> +                   "Machine is not i440fx, assigned BDF is not 00:02.0, "
>> +                   "or device %04x (gen %d) doesn't support legacy mode",
>> +                   vdev->device_id, gen);
>>          goto error;
>>      }
>>  
> 
> Afaik, at least the LPC quirk is required for Gen 10 and above for the GOP to
> work properly. On the other hand, Intel hasn't published any GOP yet, so it
> might be unlikely that user use it. So, not sure if this is a breaking change or
> not.
> 
> Besides that:
> 
> Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
> 

A previous change has added a new experimental option x-igd-lpc to allow
users manually enable the LPC bridge quirk, the documentation was also
updated with "GOP driver depends on LPC bridge present". In non-legacy
mode, user can manually enable lpc quirk with x-igd-lpc=on option.

Thanks,
Moeko

Re: [PATCH 0/9] vfio/igd: Detect IGD by OpRegion and enable OpRegion automatically

[Alex Williamson]

On Tue, 29 Apr 2025 00:09:55 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:

> As proposed in a previous discussion [1], detect IGD devices based on
> whether it has VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION exposed by kernel
> and enables OpRegion access by default. This enables out-of-the-box
> display output support for IGD passthrough without having to manually
> set x-igd-opregion=on, which probably saves effort for libvirt users.
> 
> This patchset also limits legacy mode to Gen6-9 devices, as Intel no
> longer provides CSM support and VBIOS on newer IGD, and Seabios does not
> support setting up the 64 bit BDSM register on Gen 11+ devices. Exposing
> VGA ranges to guests by default on platforms without VGA mode support is
> undesirable.
> 
> Additionally, as we enables OpRegion on IGD devices by default, and
> Intel removes the BDSM register from Meteor Lake [2]. There seems to be
> no extra register quirks rather than OpRegion required on newer devices.
> To support them (and probably future devices), the generation limit is
> removed, with BDSM quirk only applied to known Gen 6-12 devices. 
> 
> Note: I have not been able to test this on Meteor Lake or newer
> platforms due to lack of hardware.
> 
> 
> [1] https://lore.kernel.org/qemu-devel/20250325172239.27926-1-tomitamoeko@gmail.com
> [2] https://edc.intel.com/content/www/us/en/design/publications/14th-generation-core-processors-cfg-and-mem-registers/d2-f0-processor-graphics-registers/
> 
> Changelog:
> v2:
> * Removed "Allow hotplugging with OpRegion enabled", hotplugging is
>   always forbidden.
> * Test device is not hotplugged and get opregion in a single function.
> * Update documentation along with code changes.
> * Minor code style fixes.
> Link: https://lore.kernel.org/qemu-devel/20250421163112.21316-1-tomitamoeko@gmail.coms
> 
> Tomita Moeko (9):
>   vfio/igd: Restrict legacy mode to Gen6-9 devices
>   vfio/igd: Always emulate ASLS (OpRegion) register
>   vfio/igd: Detect IGD device by OpRegion
>   vfio/igd: Check vendor and device ID on GVT-g mdev
>   vfio/igd: Check OpRegion support on GVT-g mdev
>   vfio/igd: Enable OpRegion by default
>   vfio/igd: Allow overriding GMS with 0xf0 to 0xfe on Gen9+
>   vfio/igd: Only emulate GGC register when x-igd-gms is set
>   vfio/igd: Remove generation limitation for IGD passthrough
> 
>  docs/igd-assign.txt |  11 ++-
>  hw/vfio/igd.c       | 218 ++++++++++++++++++++++++++------------------
>  hw/vfio/pci.c       |   2 +-
>  3 files changed, 137 insertions(+), 94 deletions(-)
> 

Looks ok to me, my Kaby Lake GVT-g and GVT-d configs still work.

Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>