[PATCH 00/11] vfio/igd: Detect IGD by OpRegion and enable OpRegion automatically

[PATCH 00/11] 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/T/
[2] https://edc.intel.com/content/www/us/en/design/publications/14th-generation-core-processors-cfg-and-mem-registers/d2-f0-processor-graphics-registers/

Tomita Moeko (11):
  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: Remove vfio_pci_igd_setup_opregion
  vfio/igd: Check vendor and device ID on GVT-g mdev
  vfio/igd: Enable OpRegion by default
  vfio/igd: Allow hotplugging with OpRegion enabled
  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
  vfio/igd: Update IGD passthrough documentation

 docs/igd-assign.txt |  11 +-
 hw/vfio/igd.c       | 257 ++++++++++++++++++++++++--------------------
 hw/vfio/pci.c       |   2 +-
 3 files changed, 153 insertions(+), 117 deletions(-)

-- 
2.47.2

[PATCH 01/11] 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>
---
 hw/vfio/igd.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 6678e0e5cd..01826acf10 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 doesn't support legacy mode",
+                   vdev->device_id);
         goto error;
     }
 
-- 
2.47.2

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

[Alex Williamson]

On Tue, 22 Apr 2025 00:31:01 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:

> 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>
> ---
>  hw/vfio/igd.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index 6678e0e5cd..01826acf10 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 doesn't support legacy mode",
> +                   vdev->device_id);
>          goto error;
>      }
>  

It seems more useful to me to print the generation than the device ID.
A bug report where the generation value is obviously bogus or outside
the supported ranges is more actionable than a PCI device ID.  Thanks,

Alex

[PATCH 02/11] 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 01826acf10..36316e50ea 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 03/11] 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 expose
OpRegion. Use the presence of VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION
to identify IGD devices.

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

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 36316e50ea..7a7c7735c1 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -479,6 +479,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 +487,20 @@ 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 */
+    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);
+    if (ret) {
+        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
@@ -570,9 +575,14 @@ 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)) {
-        goto error;
+    if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION)) {
+        if (vdev->pdev.qdev.hotplugged) {
+            error_setg(errp, "OpRegion is not supported on hotplugged device");
+            goto error;
+        }
+        if (!vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
+            goto error;
+        }
     }
 
     /* Setup LPC bridge / Host bridge PCI IDs */
-- 
2.47.2

Re: [PATCH 03/11] vfio/igd: Detect IGD device by OpRegion

[Corvin Köhne]

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

On Tue, 2025-04-22 at 00:31 +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 expose
> OpRegion. Use the presence of VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION
> to identify IGD devices.
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/igd.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index 36316e50ea..7a7c7735c1 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -479,6 +479,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 +487,20 @@ 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.
> -     */

Why do you remove this comment? Yes, the comment is not correct. Some OS driver
and the UEFI GOP depend on address 00:02.0 too. Wouldn't it be better to improve
the comment instead of removing it? This restriction looks a bit odd and IMO a
comment would help future reader to understand it easier. 

>      if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
>          !vfio_is_vga(vdev)) {
>          return true;
>      }
>  
> +    /* IGD device always comes with OpRegion */
> +    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);
> +    if (ret) {
> +        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
> @@ -570,9 +575,14 @@ 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)) {
> -        goto error;
> +    if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION)) {
> +        if (vdev->pdev.qdev.hotplugged) {
> +            error_setg(errp, "OpRegion is not supported on hotplugged
> device");
> +            goto error;
> +        }
> +        if (!vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
> +            goto error;
> +        }

How is this part related to "Detect IGD device by OpRegion"?

>      }
>  
>      /* Setup LPC bridge / Host bridge PCI IDs */

-- 
Kind regards,
Corvin

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

Re: [PATCH 03/11] vfio/igd: Detect IGD device by OpRegion

[Tomita Moeko]

On 4/23/25 14:54, Corvin Köhne wrote:
> On Tue, 2025-04-22 at 00:31 +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 expose
>> OpRegion. Use the presence of VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION
>> to identify IGD devices.
>>
>> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
>> ---
>>  hw/vfio/igd.c | 26 ++++++++++++++++++--------
>>  1 file changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
>> index 36316e50ea..7a7c7735c1 100644
>> --- a/hw/vfio/igd.c
>> +++ b/hw/vfio/igd.c
>> @@ -479,6 +479,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 +487,20 @@ 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.
>> -     */
> 
> Why do you remove this comment? Yes, the comment is not correct. Some OS driver
> and the UEFI GOP depend on address 00:02.0 too. Wouldn't it be better to improve
> the comment instead of removing it? This restriction looks a bit odd and IMO a
> comment would help future reader to understand it easier. 

The restriction is documented in doc/igd-assign.txt, keeping the comment here
seems misleading IMO. That's why I decided to remove it here.


>>      if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
>>          !vfio_is_vga(vdev)) {
>>          return true;
>>      }
>>  
>> +    /* IGD device always comes with OpRegion */
>> +    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);
>> +    if (ret) {
>> +        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
>> @@ -570,9 +575,14 @@ 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)) {
>> -        goto error;
>> +    if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION)) {
>> +        if (vdev->pdev.qdev.hotplugged) {
>> +            error_setg(errp, "OpRegion is not supported on hotplugged
>> device");
>> +            goto error;
>> +        }
>> +        if (!vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
>> +            goto error;
>> +        }
> 
> How is this part related to "Detect IGD device by OpRegion"?

As Alex suggested, this part will be refactored in v2.

Thanks,
Moeko

>>      }
>>  
>>      /* Setup LPC bridge / Host bridge PCI IDs */
> 

Re: [PATCH 03/11] vfio/igd: Detect IGD device by OpRegion

[Alex Williamson]

On Tue, 22 Apr 2025 00:31:03 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:

> There is currently no straightforward way to distinguish if a Intel
> graphics device is IGD or discrete GPU. However, only IGD devices expose
> OpRegion. Use the presence of VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION
> to identify IGD devices.
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/igd.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index 36316e50ea..7a7c7735c1 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -479,6 +479,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 +487,20 @@ 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 */
> +    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);
> +    if (ret) {
> +        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
> @@ -570,9 +575,14 @@ 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)) {
> -        goto error;
> +    if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION)) {
> +        if (vdev->pdev.qdev.hotplugged) {
> +            error_setg(errp, "OpRegion is not supported on hotplugged device");
> +            goto error;
> +        }
> +        if (!vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
> +            goto error;
> +        }
>      }
>  
>      /* Setup LPC bridge / Host bridge PCI IDs */

I think this still needs some refactoring work.  The setup function
currently does:
 a) test hotplugged
 b) get opregion
 c) call init

We implemented b) above and therefore do a) and c) here, duplicating
them from the setup function.  It would be valid to test a) as we're
getting the opregion, so wouldn't it make sense to turn setup into a
function that does a) and b), returning an opregion, called by both
this path and GVT-g path, and each would call the init function
themselves?  The latter is already implemented in the next patch, but
a) and b) are still duplicated in GVT-g specific code.  Thanks,

Alex

[PATCH 04/11] vfio/igd: Remove vfio_pci_igd_setup_opregion

[Tomita Moeko]

Since the last commit, vfio_pci_igd_setup_opregion is only called
in GVT-g mode. Move its functionality into vfio_pci_kvmgt_config_quirk
for more GVT-g-specific error logging.

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 | 50 +++++++++++++++++++++-----------------------------
 1 file changed, 21 insertions(+), 29 deletions(-)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 7a7c7735c1..cc397f8829 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -185,33 +185,6 @@ static bool vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
     return true;
 }
 
-static bool vfio_pci_igd_setup_opregion(VFIOPCIDevice *vdev, Error **errp)
-{
-    g_autofree struct vfio_region_info *opregion = NULL;
-    int ret;
-
-    /* Hotplugging is not supported for opregion access */
-    if (vdev->pdev.qdev.hotplugged) {
-        error_setg(errp, "IGD OpRegion is not supported on hotplugged device");
-        return false;
-    }
-
-    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);
-    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;
-}
-
 /*
  * The rather short list of registers that we copy from the host devices.
  * The LPC/ISA bridge values are definitely needed to support the vBIOS, the
@@ -681,11 +654,30 @@ error:
  */
 static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 {
-    if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) &&
-        !vfio_pci_igd_setup_opregion(vdev, errp)) {
+    /* Hotplugging is not supported for opregion access */
+    if (vdev->pdev.qdev.hotplugged) {
+        error_setg(errp, "Hotplugging Intel GVT-g vGPU is not supported.");
         return false;
     }
 
+    if (vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) {
+        g_autofree struct vfio_region_info *opregion = NULL;
+        int ret;
+
+        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);
+        if (ret) {
+            /* Should never reach here, KVMGT always emulates OpRegion */
+            error_setg(errp, "No OpRegion on Intel GVT-g vGPU.");
+            return false;
+        }
+
+        if (!vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
+            return false;
+        }
+    }
+
     return true;
 }
 
-- 
2.47.2

[PATCH 05/11] 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 | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index cc397f8829..e94ed7029a 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -654,6 +654,19 @@ error:
  */
 static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 {
+    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;
+    }
+
     /* Hotplugging is not supported for opregion access */
     if (vdev->pdev.qdev.hotplugged) {
         error_setg(errp, "Hotplugging Intel GVT-g vGPU is not supported.");
-- 
2.47.2

[PATCH 06/11] 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>
---
 hw/vfio/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 07/11] vfio/igd: Allow hotplugging with OpRegion enabled

[Tomita Moeko]

OpRegion is exposed to guest as a read-only fw_cfg item, so hotplugging
with it wouldn't cause issues. Since OpRegion needs to be set up by
guest firmware, a guest reboot is typically required. For linux guests,
i915 driver is able to mock VBT [1] when OpRegion is not present, the
reboot may not required.

Still, hotplugging IGD devices is highly discouraged.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bb1d132935c2f87cd261eb559759fe49d5e5dc43

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

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index e94ed7029a..e3ff86d0e6 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -549,10 +549,6 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 
     /* Setup OpRegion access */
     if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION)) {
-        if (vdev->pdev.qdev.hotplugged) {
-            error_setg(errp, "OpRegion is not supported on hotplugged device");
-            goto error;
-        }
         if (!vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
             goto error;
         }
-- 
2.47.2

Re: [PATCH 07/11] vfio/igd: Allow hotplugging with OpRegion enabled

[Alex Williamson]

On Tue, 22 Apr 2025 00:31:07 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:

> OpRegion is exposed to guest as a read-only fw_cfg item, so hotplugging
> with it wouldn't cause issues. Since OpRegion needs to be set up by
> guest firmware, a guest reboot is typically required. For linux guests,
> i915 driver is able to mock VBT [1] when OpRegion is not present, the
> reboot may not required.
> 
> Still, hotplugging IGD devices is highly discouraged.

So why exactly are we doing this... ?

Thanks,
Alex
 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bb1d132935c2f87cd261eb559759fe49d5e5dc43
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/igd.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index e94ed7029a..e3ff86d0e6 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -549,10 +549,6 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>  
>      /* Setup OpRegion access */
>      if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION)) {
> -        if (vdev->pdev.qdev.hotplugged) {
> -            error_setg(errp, "OpRegion is not supported on hotplugged device");
> -            goto error;
> -        }
>          if (!vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
>              goto error;
>          }

Re: [PATCH 07/11] vfio/igd: Allow hotplugging with OpRegion enabled

[Tomita Moeko]

On 4/25/25 06:57, Alex Williamson wrote:
> On Tue, 22 Apr 2025 00:31:07 +0800
> Tomita Moeko <tomitamoeko@gmail.com> wrote:
> 
>> OpRegion is exposed to guest as a read-only fw_cfg item, so hotplugging
>> with it wouldn't cause issues. Since OpRegion needs to be set up by
>> guest firmware, a guest reboot is typically required. For linux guests,
>> i915 driver is able to mock VBT [1] when OpRegion is not present, the
>> reboot may not required.
>>
>> Still, hotplugging IGD devices is highly discouraged.
> 
> So why exactly are we doing this... ?
> 
> Thanks,
> Alex

Just out of curiosity :) will remove it in v2.

Thanks,
Moeko

>  
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bb1d132935c2f87cd261eb559759fe49d5e5dc43
>>
>> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
>> ---
>>  hw/vfio/igd.c | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
>> index e94ed7029a..e3ff86d0e6 100644
>> --- a/hw/vfio/igd.c
>> +++ b/hw/vfio/igd.c
>> @@ -549,10 +549,6 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>>  
>>      /* Setup OpRegion access */
>>      if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION)) {
>> -        if (vdev->pdev.qdev.hotplugged) {
>> -            error_setg(errp, "OpRegion is not supported on hotplugged device");
>> -            goto error;
>> -        }
>>          if (!vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
>>              goto error;
>>          }
> 

[PATCH 08/11] 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 e3ff86d0e6..b747fdfaeb 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -387,6 +387,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
 
@@ -573,24 +611,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

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

[Corvin Köhne]

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

On Tue, 2025-04-22 at 00:31 +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 e3ff86d0e6..b747fdfaeb 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -387,6 +387,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)) {

nit: you can remove a parantheses pair.

> +            *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
>  
> @@ -573,24 +611,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);

-- 
Kind regards,
Corvin

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

[PATCH 09/11] 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 b747fdfaeb..bc4c79837d 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -453,22 +453,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));
@@ -611,9 +613,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);
@@ -631,11 +639,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 10/11] 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>
---
 hw/vfio/igd.c | 58 +++++++++++++++++++--------------------------------
 1 file changed, 21 insertions(+), 37 deletions(-)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index bc4c79837d..5b888616f0 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;
 }
@@ -434,20 +435,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;
@@ -494,7 +487,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;
@@ -514,18 +507,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);
 
     /*
@@ -624,32 +606,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 10/11] vfio/igd: Remove generation limitation for IGD passthrough

[Corvin Köhne]

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

On Tue, 2025-04-22 at 00:31 +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=BAgAAABHhgBhwX01GO0AAACNi-GgHiUI4sV0qRTHkaBvP5PrY5DqqTz9YhuTNO_iXQUZlCVu442aEFUtFJIN9H4QuJJwNJh5ry3XO19WptfEahlpGX9F7Nq8Xpv-GQ9_LZd6niU0I-kMcs9DxxlKk8E802ILvUeF86QyZP4D7A7AEaoUDo15UvzlwHlTOoUj4fzKJYvykniL_MlVr8H8FS2ySMcVqfE6IFb5Aiwhkq7hrpL53XHUYCVoK_y89PCzSWC-Y0fS5-Xo-SxRDOtI140aTd7KpmA39tf447LtBR1YQP8ccM8PjukqvroMpZtV3cDQcgya3EsmP0Fgrmc1
>  
> [2]
> https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAABHhgBhwX01GNYAAAAtMYPOda9hw4SLBezXYKaYHKRl05pN8tzXcVl9njzCJlE1HkXUG-mljVszTJQKhcQYQTJNcuCekCys_GWgxsVTyOaS_NHFmHSUCZG_K4lK67xtRF6gmeuH6VTzSH0evGV976t_N2r5ADGfofxEA3bNt0iJmL1IzjhRilkRiNe-EmvdQ7DLMn1nAV9ZQCkPM5bCjIyd7MIjXMAlwhK4CraFMv1xw33w-ZnR0YrP4VIGHwKE7vUKp7-BjGZzyrgtwQ5m2SKD0zkEOs3dls5l351dSvNfqp740
>  
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/igd.c | 58 +++++++++++++++++++--------------------------------
>  1 file changed, 21 insertions(+), 37 deletions(-)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index bc4c79837d..5b888616f0 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;
>  }
> @@ -434,20 +435,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.
> -     */

Same goes for this comment. It's not really correct as UEFI GOP and OS driver
depend on address 0:2.0 too but I'd keep and improve it to make it more clear
for future reader.

>      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;
> @@ -494,7 +487,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;
> @@ -514,18 +507,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=BAgAAABHhgBhwX01GGwAAADNE
> nn7fAHvEAN3EJV6Ng4dOzKc7MR7f3e317FXOTfRW-ZxuzGFRyquEjUjSeP-
> 6ByDI2TtwvI3jwlJBjQdXL-V9t40AFbX3wSV_K-
> M0j4dDCcdYxjOhLKhEroDvVJ0XE20BM2QEteRk67iEFo1 );
> -        return true;
> -    }
> -
>      gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
>  
>      /*
> @@ -624,32 +606,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=BAgAAABHhgBhwX01GGwAAADNE
> nn7fAHvEAN3EJV6Ng4dOzKc7MR7f3e317FXOTfRW-ZxuzGFRyquEjUjSeP-
> 6ByDI2TtwvI3jwlJBjQdXL-V9t40AFbX3wSV_K-
> M0j4dDCcdYxjOhLKhEroDvVJ0XE20BM2QEteRk67iEFo1 , (gms_size / MiB));
>  
>      return true;

-- 
Kind regards,
Corvin

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

[PATCH 11/11] vfio/igd: Update IGD passthrough documentation

[Tomita Moeko]

In previous commits, several changes were made to IGD passthrough:
* Legacy mode now requires the IGD to be Gen6–Gen9.
* OpRegion quirk is enabled by default.
* "etc/igd-bdsm-size" is set to 0 when guest firmware does not need to
  allocate Data Stolen Memory and write BDSM register.
Update the documentation to reflect these changes.

Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 docs/igd-assign.txt | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
index 3aed7956d5..eac31ea3dd 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
@@ -101,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*]
@@ -123,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
@@ -156,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:
-- 
2.47.2

Re: [PATCH 11/11] vfio/igd: Update IGD passthrough documentation

[Corvin Köhne]

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

On Tue, 2025-04-22 at 00:31 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> In previous commits, several changes were made to IGD passthrough:
> * Legacy mode now requires the IGD to be Gen6–Gen9.
> * OpRegion quirk is enabled by default.
> * "etc/igd-bdsm-size" is set to 0 when guest firmware does not need to
>   allocate Data Stolen Memory and write BDSM register.
> Update the documentation to reflect these changes.
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  docs/igd-assign.txt | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
> index 3aed7956d5..eac31ea3dd 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
> @@ -101,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*]
> @@ -123,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
> @@ -156,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:

Not sure how upstream prefers handling documentation. Those are only small
changes, so I'd add them to the corresponding commits.


-- 
Kind regards,
Corvin

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