[PATCH v2] vfio/igd: Respect x-igd-opregion=off in IGD quirk handling

[PATCH v2] vfio/igd: Respect x-igd-opregion=off in IGD quirk handling

[edmund.raile via]

Restore SR-IOV Intel iGPU VF passthrough capability:
Check x-igd-opregion=off parameter in vfio_pci_igd_config_quirk and
vfio_pci_kvmgt_config_quirk to ensure x-igd-opregion=off is
respected despite subsequent attempt of automatic
IGD opregion detection.

Fixes: c0273e77f2d7 ("vfio/igd: Detect IGD device by OpRegion")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2968
Signed-off-by: Edmund Raile <edmund.raile@protonmail.com>
---
This patch fixes a regression in QEMU’s VFIO IGD quirk handling that
established automatic IGD opregion detection which ignores 
x-igd-opregion=off necessary for SR-IOV VF passthrough of
Intel iGPUs using i915-sriov-dkms.

Please review and provide feedback.
Let me know if additional testing or changes are needed.

Kind regards,
Edmund Raile.

 hw/vfio/igd.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index e7952d15a0..e54a2a2f00 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -523,6 +523,11 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
         return true;
     }
 
+    /* Respect x-igd-opregion=off by skipping OpRegion handling */
+    if (!vdev->igd_opregion) {
+        return true;
+    }
+
     /* IGD device always comes with OpRegion */
     if (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp)) {
         return true;
@@ -689,6 +694,11 @@ static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice *vdev, Error **errp)
         return true;
     }
 
+    /* Respect x-igd-opregion=off by skipping OpRegion handling */
+    if (!vdev->igd_opregion) {
+        return true;
+    }
+
     /* FIXME: Cherryview is Gen8, but don't support GVT-g */
     gen = igd_gen(vdev);
     if (gen != 8 && gen != 9) {
-- 
2.49.0

Re: [PATCH v2] vfio/igd: Respect x-igd-opregion=off in IGD quirk handling

[Tomita Moeko]

On 5/19/25 16:03, edmund.raile wrote:
> Restore SR-IOV Intel iGPU VF passthrough capability:
> Check x-igd-opregion=off parameter in vfio_pci_igd_config_quirk and
> vfio_pci_kvmgt_config_quirk to ensure x-igd-opregion=off is
> respected despite subsequent attempt of automatic
> IGD opregion detection.
> 
> Fixes: c0273e77f2d7 ("vfio/igd: Detect IGD device by OpRegion")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2968
> Signed-off-by: Edmund Raile <edmund.raile@protonmail.com>
> ---
> This patch fixes a regression in QEMU’s VFIO IGD quirk handling that
> established automatic IGD opregion detection which ignores 
> x-igd-opregion=off necessary for SR-IOV VF passthrough of
> Intel iGPUs using i915-sriov-dkms.
> 
> Please review and provide feedback.
> Let me know if additional testing or changes are needed.
> 
> Kind regards,
> Edmund Raile.

Hi, Edmund

I did a quick test with x-igd-opregion=off with c0273e77f2d7 included on
SRIOV PF, setting x-igd-opregion=off works as expected on my linux
guest. Per my understanding, SRIOV PF should not have OpRegion address
in its config space 0xfc, kernel returns -ENODEV when accessing, and
QEMU continues after vfio_pci_igd_opregion_detect() fails by returing
true. Could you please share more details about this issue?

[    0.655035] i915 0000:00:02.0: [drm:intel_opregion_setup [i915]] graphic opregion physical addr: 0x0
[    0.655490] i915 0000:00:02.0: [drm:intel_opregion_setup [i915]] ACPI OpRegion not supported!
[    0.656088] i915 0000:00:02.0: Invalid PCI ROM header signature: expecting 0xaa55, got 0x0000
[    0.656462] i915 0000:00:02.0: [drm] Failed to find VBIOS tables (VBT)

If you are mentioning the log "Device does not supports IGD OpRegion
feature", it is a false error and can be ignored I think. Maybe we can
improve this, making it more clear?

 
>  hw/vfio/igd.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index e7952d15a0..e54a2a2f00 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -523,6 +523,11 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>          return true;
>      }
>  
> +    /* Respect x-igd-opregion=off by skipping OpRegion handling */
> +    if (!vdev->igd_opregion) {
> +        return true;
> +    }
> +

Here (vdev->igd_opregion == NULL) is always true, the pointer is zero-
initialized and only get assigned in vfio_pci_igd_opregion_init().
Enabling OpRegion or not is by VFIO_FEATURE_ENABLE_IGD_OPREGION bit.

>      /* IGD device always comes with OpRegion */
>      if (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp)) {
>          return true;
> @@ -689,6 +694,11 @@ static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>          return true;
>      }
>  
> +    /* Respect x-igd-opregion=off by skipping OpRegion handling */
> +    if (!vdev->igd_opregion) {
> +        return true;
> +    }
> +

As I mentioned in a comment below, GVT-g vGPU always emulate OpRegion,
so let QEMU fail immediately if OpRegion is not avaliable here.

Thanks,
Moeko

>      /* FIXME: Cherryview is Gen8, but don't support GVT-g */
>      gen = igd_gen(vdev);
>      if (gen != 8 && gen != 9) {

Re: [PATCH v2] vfio/igd: Respect x-igd-opregion=off in IGD quirk handling

[edmund.raile via]

Hi Moeko,
>On 5/19/25 16:03, edmund.raile wrote:
>> Restore SR-IOV Intel iGPU VF passthrough capability:
>> Check x-igd-opregion=off parameter in vfio_pci_igd_config_quirk and
>> vfio_pci_kvmgt_config_quirk to ensure x-igd-opregion=off is
>> respected despite subsequent attempt of automatic
>> IGD opregion detection.
>>
>> Fixes: c0273e77f2d7 ("vfio/igd: Detect IGD device by OpRegion")
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2968
>> Signed-off-by: Edmund Raile <edmund.raile@protonmail.com>
>> ---
>> This patch fixes a regression in QEMU’s VFIO IGD quirk handling that
>> established automatic IGD opregion detection which ignores
>> x-igd-opregion=off necessary for SR-IOV VF passthrough of
>> Intel iGPUs using i915-sriov-dkms.
>>
>> Please review and provide feedback.
>> Let me know if additional testing or changes are needed.
>>
>> Kind regards,
>> Edmund Raile.
>
>Hi, Edmund
>
>I did a quick test with x-igd-opregion=off with c0273e77f2d7 included on
>SRIOV PF, setting x-igd-opregion=off works as expected on my linux
>guest. Per my understanding, SRIOV PF should not have OpRegion address
>in its config space 0xfc, kernel returns -ENODEV when accessing, and
>QEMU continues after vfio_pci_igd_opregion_detect() fails by returing
>true. Could you please share more details about this issue?

This is with the i915-sriov-dkms module creating a virtual function
from my iGPU:
00:02.0 VGA compatible controller: Intel Corporation AlderLake-S GT1 (rev 0c)
00:02.1 VGA compatible controller: Intel Corporation AlderLake-S GT1 (rev 0c)
Which I pass through to the VM, allowing both the host and the guest
to use the same iGPU.

>
>[    0.655035] i915 0000:00:02.0: [drm:intel_opregion_setup [i915]] graphic opregion physical addr: 0x0
>[    0.655490] i915 0000:00:02.0: [drm:intel_opregion_setup [i915]] ACPI OpRegion not supported!
>[    0.656088] i915 0000:00:02.0: Invalid PCI ROM header signature: expecting 0xaa55, got 0x0000
>[    0.656462] i915 0000:00:02.0: [drm] Failed to find VBIOS tables (VBT)

Where are you getting these logs from?
These aren't kernel messages, are they?

>If you are mentioning the log "Device does not supports IGD OpRegion
>feature", it is a false error and can be ignored I think. Maybe we can
>improve this, making it more clear?

The exact error I am getting without my patch is this:
qemu-system-x86_64: -device vfio-pci,host=0000:00:02.1,romfile=/path/to/intelgopdriver_desktop.bin,x-vga=on,x-igd-opregion=off: Device does not supports IGD OpRegion feature: No such device
after which QEMU immediately exits.
Maybe the issue is that the parrent device (the iGPU) DOES support
OpRegion but the virtual function child does not.
The intent is to give the option of disabling it back to the user.

>>  hw/vfio/igd.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
>> index e7952d15a0..e54a2a2f00 100644
>> --- a/hw/vfio/igd.c
>> +++ b/hw/vfio/igd.c
>> @@ -523,6 +523,11 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>>          return true;
>>      }
>>
>> +    /* Respect x-igd-opregion=off by skipping OpRegion handling */
>> +    if (!vdev->igd_opregion) {
>> +        return true;
>> +    }
>> +
>
>Here (vdev->igd_opregion == NULL) is always true, the pointer is zero-
>initialized and only get assigned in vfio_pci_igd_opregion_init().
>Enabling OpRegion or not is by VFIO_FEATURE_ENABLE_IGD_OPREGION bit.
>

I thought `vdev->igd_opregion` would already contain the
`x-igd-opregion=` option state of the `-device vfio-pci`
parameter but you're right, it's not assigned here yet,
so is this the correct way to access the state of the
parameter option:
(vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION)
Which would then make it
+    if (!(vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION)) {
+        return true;
+    }

>>      /* IGD device always comes with OpRegion */
>>      if (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp)) {
>>          return true;
>> @@ -689,6 +694,11 @@ static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>>          return true;
>>      }
>>
>> +    /* Respect x-igd-opregion=off by skipping OpRegion handling */
>> +    if (!vdev->igd_opregion) {
>> +        return true;
>> +    }
>> +
>
>As I mentioned in a comment below, GVT-g vGPU always emulate OpRegion,
>so let QEMU fail immediately if OpRegion is not avaliable here.

Agreed, you are right, it shouldn't be necessary here,
I'll remove it in v3.

>
>Thanks,
>Moeko

Thank you for taking the time to review (and contribute in the
first place).

Kind regards,
Edmund.

Re: [PATCH v2] vfio/igd: Respect x-igd-opregion=off in IGD quirk handling

[Tomita Moeko]

On 5/21/25 04:46, edmund.raile wrote:
> Hi Moeko,
>> On 5/19/25 16:03, edmund.raile wrote:
>>> Restore SR-IOV Intel iGPU VF passthrough capability:
>>> Check x-igd-opregion=off parameter in vfio_pci_igd_config_quirk and
>>> vfio_pci_kvmgt_config_quirk to ensure x-igd-opregion=off is
>>> respected despite subsequent attempt of automatic
>>> IGD opregion detection.
>>>
>>> Fixes: c0273e77f2d7 ("vfio/igd: Detect IGD device by OpRegion")
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2968
>>> Signed-off-by: Edmund Raile <edmund.raile@protonmail.com>
>>> ---
>>> This patch fixes a regression in QEMU’s VFIO IGD quirk handling that
>>> established automatic IGD opregion detection which ignores
>>> x-igd-opregion=off necessary for SR-IOV VF passthrough of
>>> Intel iGPUs using i915-sriov-dkms.
>>>
>>> Please review and provide feedback.
>>> Let me know if additional testing or changes are needed.
>>>
>>> Kind regards,
>>> Edmund Raile.
>>
>> Hi, Edmund
>>
>> I did a quick test with x-igd-opregion=off with c0273e77f2d7 included on
>> SRIOV PF, setting x-igd-opregion=off works as expected on my linux
>> guest. Per my understanding, SRIOV PF should not have OpRegion address
>> in its config space 0xfc, kernel returns -ENODEV when accessing, and
>> QEMU continues after vfio_pci_igd_opregion_detect() fails by returing
>> true. Could you please share more details about this issue?
> 
> This is with the i915-sriov-dkms module creating a virtual function
> from my iGPU:
> 00:02.0 VGA compatible controller: Intel Corporation AlderLake-S GT1 (rev 0c)
> 00:02.1 VGA compatible controller: Intel Corporation AlderLake-S GT1 (rev 0c)
> Which I pass through to the VM, allowing both the host and the guest
> to use the same iGPU.
> 
>>
>> [    0.655035] i915 0000:00:02.0: [drm:intel_opregion_setup [i915]] graphic opregion physical addr: 0x0
>> [    0.655490] i915 0000:00:02.0: [drm:intel_opregion_setup [i915]] ACPI OpRegion not supported!
>> [    0.656088] i915 0000:00:02.0: Invalid PCI ROM header signature: expecting 0xaa55, got 0x0000
>> [    0.656462] i915 0000:00:02.0: [drm] Failed to find VBIOS tables (VBT)
> 
> Where are you getting these logs from?
> These aren't kernel messages, are they?

Hi, Edmund

This is get printed by guest kernel, with `drm.debug=0x2` in cmdline.
 
>> If you are mentioning the log "Device does not supports IGD OpRegion
>> feature", it is a false error and can be ignored I think. Maybe we can
>> improve this, making it more clear?
> 
> The exact error I am getting without my patch is this:
> qemu-system-x86_64: -device vfio-pci,host=0000:00:02.1,romfile=/path/to/intelgopdriver_desktop.bin,x-vga=on,x-igd-opregion=off: Device does not supports IGD OpRegion feature: No such device
> after which QEMU immediately exits.
> Maybe the issue is that the parrent device (the iGPU) DOES support
> OpRegion but the virtual function child does not.
> The intent is to give the option of disabling it back to the user.

I did some further debugging, and found it was caused by a mistake in
error handling. In vfio_pci_igd_opregion_detect(), `errp` is set when
OpRegion is not found. However, in pci_qdev_realize(), the caller of
vfio_realize(), it checks if the errp is set for failure, rather than
the return value.

hw/pci/pci.c:2268
>    if (pc->realize) {
>        pc->realize(pci_dev, &local_err);
>        if (local_err) {
>            error_propagate(errp, local_err);
>            do_pci_unregister_device(pci_dev);
>            return;
>        }
>    }

I will submit a patch later to fix error handling.


>>>  hw/vfio/igd.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
>>> index e7952d15a0..e54a2a2f00 100644
>>> --- a/hw/vfio/igd.c
>>> +++ b/hw/vfio/igd.c
>>> @@ -523,6 +523,11 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>>>          return true;
>>>      }
>>>
>>> +    /* Respect x-igd-opregion=off by skipping OpRegion handling */
>>> +    if (!vdev->igd_opregion) {
>>> +        return true;
>>> +    }
>>> +
>>
>> Here (vdev->igd_opregion == NULL) is always true, the pointer is zero-
>> initialized and only get assigned in vfio_pci_igd_opregion_init().
>> Enabling OpRegion or not is by VFIO_FEATURE_ENABLE_IGD_OPREGION bit.
>>
> 
> I thought `vdev->igd_opregion` would already contain the
> `x-igd-opregion=` option state of the `-device vfio-pci`
> parameter but you're right, it's not assigned here yet,
> so is this the correct way to access the state of the
> parameter option:
> (vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION)
> Which would then make it
> +    if (!(vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION)) {
> +        return true;
> +    }

Yes that's right.

Thanks,
Moeko

>>>      /* IGD device always comes with OpRegion */
>>>      if (!vfio_pci_igd_opregion_detect(vdev, &opregion, errp)) {
>>>          return true;
>>> @@ -689,6 +694,11 @@ static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>>>          return true;
>>>      }
>>>
>>> +    /* Respect x-igd-opregion=off by skipping OpRegion handling */
>>> +    if (!vdev->igd_opregion) {
>>> +        return true;
>>> +    }
>>> +
>>
>> As I mentioned in a comment below, GVT-g vGPU always emulate OpRegion,
>> so let QEMU fail immediately if OpRegion is not avaliable here.
> 
> Agreed, you are right, it shouldn't be necessary here,
> I'll remove it in v3.
> 
>>
>> Thanks,
>> Moeko
> 
> Thank you for taking the time to review (and contribute in the
> first place).
> 
> Kind regards,
> Edmund.
>