On 4/10/25 01:18, Alex Williamson wrote:
> On Wed, 26 Mar 2025 01:22:39 +0800
> Tomita Moeko <tomitamo...@gmail.com> wrote:
>
>> So far, all Intel VGA adapters, including discrete GPUs like A770 and
>> B580, were treated as IGD devices. While this had no functional impact,
>> a error about "unsupported IGD device" will be printed when passthrough
>> Intel discrete GPUs.
>>
>> Since IGD devices must be at "0000:00:02.0", let's check the host PCI
>> address when probing.
>>
>> Signed-off-by: Tomita Moeko <tomitamo...@gmail.com>
>> ---
>> hw/vfio/igd.c | 23 +++++++++--------------
>> 1 file changed, 9 insertions(+), 14 deletions(-)
>>
>> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
>> index 265fffc2aa..ff250017b0 100644
>> --- a/hw/vfio/igd.c
>> +++ b/hw/vfio/igd.c
>> @@ -53,6 +53,13 @@
>> * headless setup is desired, the OpRegion gets in the way of that.
>> */
>>
>> +static bool vfio_is_igd(VFIOPCIDevice *vdev)
>> +{
>> + return vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) &&
>> + vfio_is_vga(vdev) &&
>> + vfio_pci_host_match(&vdev->host, "0000:00:02.0");
>> +}
>
> vfio-pci devices can also be specified via sysfsdev= rather than host=,
> so at a minimum I think we'd need to test against vdev->vbasedev.name,
> as other callers of vfio_pci_host_match do. For example building a
> local PCIHostDeviceAddress and comparing it to name. This is also not
> foolproof though if we start taking advantage of devices passed by fd.
Sorry for my late reply. Yes `vfio_pci_host_match` does not work for
sysfsdev of fd, I will drop this change.
> Could we instead rely PCIe capabilities? A discrete GPU should
> identify as either an endpoint or legacy endpoint and IGD should
> identify as a root complex integrated endpoint, or maybe older versions
> would lack the PCIe capability altogether.
Older generations seems do not have PCIe capabilities in their config
space, like the sandy bridge spec [1] does not mention it. I don't have
a sandy bridge box for now to verify it :(
> Also I think the comments that were dropped below are still valid and
> useful to transfer to this new helper. I think those are actually
> referring to the guest address of 00:02.0 though, which should maybe be
> a test as well. Thanks,
>
> Alex
Guest address of 00:02.0 is not mandatory, newer drivers does not depend
on the address (probably thanks to the discrete GPU, they removed these
hardcode). Though putting it at guest 00:02.0 is still recommended.
I would prefer not making this a limit.
[1]
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/3rd-gen-core-desktop-vol-2-datasheet.pdf
Thanks,
Moeko
>> +
>> /*
>> * This presumes the device is already known to be an Intel VGA device, so
>> we
>> * take liberties in which device ID bits match which generation. This
>> should
>> @@ -427,13 +434,7 @@ 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) {
>> + if (nr != 0 || !vfio_is_igd(vdev)) {
>> return;
>> }
>>
>> @@ -490,13 +491,7 @@ 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)) {
>> + if (!vfio_is_igd(vdev)) {
>> return true;
>> }
>>
>
