Re: [PATCH] vfio/igd: Check host PCI address when probing

[Alex Williamson <alex.williamson@redhat.com>]

On Wed, 26 Mar 2025 01:22:39 +0800
Tomita Moeko <tomitamoeko@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 <tomitamoeko@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.

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.

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

> +
>  /*
>   * 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;
>      }
>