[PATCH] vfio/pci: update igd matching conditions

[PATCH] vfio/pci: update igd matching conditions

[Tomita Moeko]

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@gmail.com>
---
 drivers/vfio/pci/vfio_pci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index e727941f589d..051ef4ad3f43 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) &&
+	if (IS_ENABLED(CONFIG_VFIO_PCI_IGD) &&
 	    pdev->vendor == PCI_VENDOR_ID_INTEL &&
-	    IS_ENABLED(CONFIG_VFIO_PCI_IGD)) {
+	    ((pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA ||
+	     (pdev->class >> 8) == PCI_CLASS_DISPLAY_OTHER) &&
+	    pdev == pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(2, 0))) {
 		ret = vfio_pci_igd_init(vdev);
 		if (ret && ret != -ENODEV) {
 			pci_warn(pdev, "Failed to setup Intel IGD regions\n");
-- 
2.45.2

Re: [PATCH] vfio/pci: update igd matching conditions

[Alex Williamson]

On Sun, 29 Dec 2024 23:51:40 +0800
Tomita Moeko <tomitamoeko@gmail.com> 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@gmail.com>
> ---
>  drivers/vfio/pci/vfio_pci.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index e727941f589d..051ef4ad3f43 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) &&
> +	if (IS_ENABLED(CONFIG_VFIO_PCI_IGD) &&
>  	    pdev->vendor == PCI_VENDOR_ID_INTEL &&
> -	    IS_ENABLED(CONFIG_VFIO_PCI_IGD)) {
> +	    ((pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA ||

The above is vfio_pci_is_vga(pdev), maybe below should have a similar
helper.

> +	     (pdev->class >> 8) == PCI_CLASS_DISPLAY_OTHER) &&
> +	    pdev == pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(2, 0))) {

This increments the reference count on the device:

 * Given a PCI domain, bus, and slot/function number, the desired PCI
 * device is located in the list of PCI devices. If the device is
 * found, its reference count is increased and this function returns a
 * pointer to its data structure.  The caller must decrement the
 * reference count by calling pci_dev_put().

>  		ret = vfio_pci_igd_init(vdev);
>  		if (ret && ret != -ENODEV) {
>  			pci_warn(pdev, "Failed to setup Intel IGD regions\n");

Re: [PATCH] vfio/pci: update igd matching conditions

[Tomita Moeko]

On 12/30/24 00:26, Alex Williamson wrote:
> On Sun, 29 Dec 2024 23:51:40 +0800
> Tomita Moeko <tomitamoeko@gmail.com> 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@gmail.com>
>> ---
>>  drivers/vfio/pci/vfio_pci.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>> index e727941f589d..051ef4ad3f43 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) &&
>> +	if (IS_ENABLED(CONFIG_VFIO_PCI_IGD) &&
>>  	    pdev->vendor == PCI_VENDOR_ID_INTEL &&
>> -	    IS_ENABLED(CONFIG_VFIO_PCI_IGD)) {
>> +	    ((pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA ||
> 
> The above is vfio_pci_is_vga(pdev), maybe below should have a similar
> helper.

There isn't, shall I create a new helper function? or just keep it as
the match only happens here.
 
>> +	     (pdev->class >> 8) == PCI_CLASS_DISPLAY_OTHER) &&
>> +	    pdev == pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(2, 0))) {
> 
> This increments the reference count on the device:
> 
>  * Given a PCI domain, bus, and slot/function number, the desired PCI
>  * device is located in the list of PCI devices. If the device is
>  * found, its reference count is increased and this function returns a
>  * pointer to its data structure.  The caller must decrement the
>  * reference count by calling pci_dev_put().

Sorry I missed that, will fix in v2.

>>  		ret = vfio_pci_igd_init(vdev);
>>  		if (ret && ret != -ENODEV) {
>>  			pci_warn(pdev, "Failed to setup Intel IGD regions\n");
>