[PATCH 4/5] drm/xe/configfs: Allow configurations only for Intel VGA devices

[Cavitt, Jonathan]

-----Original Message-----
> From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Michal Wajdeczko
> Sent: Thursday, July 17, 2025 11:48 AM
> To: intel-xe at lists.freedesktop.org
> Cc: Wajdeczko, Michal <Michal.Wajdeczko at intel.com>; De Marchi, Lucas <lucas.demarchi at intel.com>
> Subject: [PATCH 4/5] drm/xe/configfs: Allow configurations only for Intel VGA devices
> 
> The Xe driver supports only Intel GPUs devices that all are PCI
> VGA class devices. Reject creation of configuration directories
> for PCI device addresses that are not Intel or VGA.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_configfs.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/xe_configfs.c
> index 00bb4e412c12..1df8cce78f13 100644
> --- a/drivers/gpu/drm/xe/xe_configfs.c
> +++ b/drivers/gpu/drm/xe/xe_configfs.c
> @@ -260,6 +260,7 @@ static struct config_group *xe_config_make_device_group(struct config_group *gro
>  	struct xe_config_device *dev;
>  	struct pci_dev *pdev;
>  	char canonical[16];
> +	bool match;
>  	int ret;
>  
>  	ret = sscanf(name, "%x:%x:%x.%d", &domain, &bus, &slot, &function);
> @@ -275,8 +276,14 @@ static struct config_group *xe_config_make_device_group(struct config_group *gro
>  	pdev = pci_get_domain_bus_and_slot(domain, bus, PCI_DEVFN(slot, function));
>  	if (!pdev)
>  		return ERR_PTR(-ENODEV);
> +
> +	match = pci_is_vga(pdev) && pdev->vendor == PCI_VENDOR_ID_INTEL;

This match check is relatively short.  Debatably, we could just do the comparison
directly in the if-statement instead of assigning the result to a new variable.

I won't block on it, though, since clearly labeling these comparisons is important.  Though,
perhaps we should call it "op_supported" instead of "match"?  I understand why it's called
"match" here (it's more in line with how it's used in patch 5), but maybe this check and the
check in patch 5 should be separated out?

Just a suggestion.

Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
-Jonathan Cavitt

> +
>  	pci_dev_put(pdev);
>  
> +	if (!match)
> +		return ERR_PTR(-ENODEV);
> +
>  	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>  	if (!dev)
>  		return ERR_PTR(-ENOMEM);
> -- 
> 2.47.1
> 
>