Re: [PATCH v3 09/13] drm/xe/configfs: Block runtime attribute changes

[Riana Tauro <riana.tauro@intel.com>]

On 8/8/2025 10:59 PM, Lucas De Marchi wrote:
> Although it's possible to change the attributes in runtime, they have no
> effect after the driver is already bound to the device. Check for that
> and return -EBUSY in that case.
> 
> This should help users understand what's going on when the behavior is
> not changing even if the value from the configfs is "right", but it got
> to that state too late.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_configfs.c | 38 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/xe_configfs.c
> index 4c2d4ff6a70f5..489c5c67001dc 100644
> --- a/drivers/gpu/drm/xe/xe_configfs.c
> +++ b/drivers/gpu/drm/xe/xe_configfs.c
> @@ -54,6 +54,8 @@
>    *	# echo 1 > /sys/kernel/config/xe/0000:03:00.0/survivability_mode
>    *	# echo 0000:03:00.0 > /sys/bus/pci/drivers/xe/bind  (Enters survivability mode if supported)
>    *
> + * This attribute can only be set before binding to the device.
> + *
>    * Allowed engines:
>    * ----------------
>    *
> @@ -78,6 +80,8 @@
>    * available for migrations, but it's disabled. This is intended for debugging
>    * purposes only.
>    *
> + * This attribute can only be set before binding to the device.
> + *
>    * PSMI
>    * ----
>    *
> @@ -88,6 +92,8 @@
>    *
>    *	# echo 1 > /sys/kernel/config/xe/0000:03:00.0/enable_psmi
>    *
> + * This attribute can only be set before binding to the device.
> + *
>    * Remove devices
>    * ==============
>    *
> @@ -148,6 +154,29 @@ static struct xe_config_device *to_xe_config_device(struct config_item *item)
>   	return &to_xe_config_group_device(item)->config;
>   }
>   
> +static bool is_bound(struct xe_config_group_device *dev)
> +{
> +	unsigned int domain, bus, slot, function;
> +	struct pci_dev *pdev;
> +	const char *name;
> +	bool ret;
> +
> +	lockdep_assert_held(&dev->lock);
> +
> +	name = dev->group.cg_item.ci_name;
> +	if (sscanf(name, "%x:%x:%x.%x", &domain, &bus, &slot, &function) != 4)
> +		return false;
> +
> +	pdev = pci_get_domain_bus_and_slot(domain, bus, PCI_DEVFN(slot, function));
> +	if (!pdev)
> +		return false;
> +
> +	ret = pci_get_drvdata(pdev);
> +	pci_dev_put(pdev);

It would be useful to have a dbg log here to inform users about the 
expectation if device is bound

With that
Reviewed-by: Riana Tauro <riana.tauro@intel.com>

> +
> +	return ret;
> +}
> +
>   static ssize_t survivability_mode_show(struct config_item *item, char *page)
>   {
>   	struct xe_config_device *dev = to_xe_config_device(item);
> @@ -166,6 +195,9 @@ static ssize_t survivability_mode_store(struct config_item *item, const char *pa
>   		return ret;
>   
>   	guard(mutex)(&dev->lock);
> +	if (is_bound(dev))
> +		return -EBUSY;
> +
>   	dev->config.survivability_mode = survivability_mode;
>   
>   	return len;
> @@ -249,6 +281,9 @@ static ssize_t engines_allowed_store(struct config_item *item, const char *page,
>   	}
>   
>   	guard(mutex)(&dev->lock);
> +	if (is_bound(dev))
> +		return -EBUSY;
> +
>   	dev->config.engines_allowed = val;
>   
>   	return len;
> @@ -272,6 +307,9 @@ static ssize_t enable_psmi_store(struct config_item *item, const char *page, siz
>   		return ret;
>   
>   	guard(mutex)(&dev->lock);
> +	if (is_bound(dev))
> +		return -EBUSY;
> +
>   	dev->config.enable_psmi = val;
>   
>   	return len;
>