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.ra...@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.
