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

[Lucas De Marchi]

On Thu, Jul 17, 2025 at 10:51:36PM +0200, Michal Wajdeczko wrote:
>
>
>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 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;
>>
>> 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

that should be ok.

>
>>
>> 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?

when I mentioned that they "would not stick" I meant something along the
lines:

sudo mkdir 0000:0e:00.0 /sys/kernel/config/...

		 ^ typo here on a system that actually has the card as
		 03:00.0

The user would expect that configuration would be set on the device, but
it's not actually applying anything due to the typo.

My main concern is about maintaining these kind of checks in the long
run, when we enable new HW, having to adjust multiple places in the
different parts of the driver.

>
>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!

fair enough, it can be orthogonal to the check here. I'm ok adding this
and if we end up needing to adjust it later, we can decide if we drop it
or not.

thanks
Lucas De Marchi

>
>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
>>>
>