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

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 17.07.2025 21:52, Lucas De Marchi wrote:
> On Thu, Jul 17, 2025 at 08:48:24PM +0200, Michal Wajdeczko wrote:
>> 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@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@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;
> 
> although not officially supported by xe, we had devices in the past with
> class 0x0380 and (I think) 0x302.

I can look directly at pdev->class to check for all legacy classes

> 
> is the goal here to error early on typos that cause the config to not
> "stick" and now error/warning to the user? 

they would stick, but what's the point to allow user adding config for
the PCI devices that we will never be able to use?

IMO since we error early if given PCI device address is not present on
current system, we should also try to error early if someone enters data
using PCI address of not our device

> Maybe we should rather have a
> positive indication during bind that the user can grep for in the log?

that could be added independently as a confirmation that driver noticed
specific configuration was prepared for given device:

 [ ] xe 0000:00:02.0: [drm] found custom settings in configfs!

I guess individual params, if different than default, should be logged
on the go, like something like this one from engines_allowed:

 [ ] xe 0000:00:02.0: [drm] GT1: vcs0 disabled via configfs

> 
> Lucas De Marchi
> 
>> +
>>     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
>>