On Tue, 31 Dec 2024 00:10:54 +0800 Tomita Moeko <tomitamoeko@xxxxxxxxx> wrote: > igd device can either expose as a VGA controller or display controller > depending on whether it is configured as the primary display device in > BIOS. In both cases, the OpRegion may be present. Also checks if the > device is at bdf 00:02.0 to avoid setting up igd-specific regions on > Intel discrete GPUs. > > Signed-off-by: Tomita Moeko <tomitamoeko@xxxxxxxxx> > --- > Changelog: > v2: > Fix misuse of pci_get_domain_bus_and_slot(), now only compares bdf > without touching device reference count. > Link: https://lore.kernel.org/all/20241229155140.7434-1-tomitamoeko@xxxxxxxxx/ > > drivers/vfio/pci/vfio_pci.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c > index e727941f589d..906a1db46d15 100644 > --- a/drivers/vfio/pci/vfio_pci.c > +++ b/drivers/vfio/pci/vfio_pci.c > @@ -111,9 +111,11 @@ static int vfio_pci_open_device(struct vfio_device *core_vdev) > if (ret) > return ret; > > - if (vfio_pci_is_vga(pdev) && > - pdev->vendor == PCI_VENDOR_ID_INTEL && > - IS_ENABLED(CONFIG_VFIO_PCI_IGD)) { > + if (IS_ENABLED(CONFIG_VFIO_PCI_IGD) && > + (pdev->vendor == PCI_VENDOR_ID_INTEL) && > + (((pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA) || > + ((pdev->class >> 8) == PCI_CLASS_DISPLAY_OTHER)) && > + (pci_dev_id(pdev) == PCI_DEVID(0, PCI_DEVFN(2, 0)))) { Sorry I wasn't available to reply on previous thread before v2 was posted, but given that we have vfio_pci_is_vga() we should use it rather than duplicate the contents. I think that suggests we should create a similar helper for display_other. Alternatively we should maybe consider if it's sufficient to use just the base class. The DEVID of course does not include the domain, which make it a rather suspect check already. What do the discrete cards report at 0xfc in config space? If it's zero or -1 or points to something that we can't memremap() or points to contents that doesn't include the opregion signature, then we'll already exit out of vfio_pci_igd_init(). Is there actually a case that we're actually configuring IGD specific regions for a discrete card? Thanks, Alex > ret = vfio_pci_igd_init(vdev); > if (ret && ret != -ENODEV) { > pci_warn(pdev, "Failed to setup Intel IGD regions\n");
