Re: [PATCH V18 09/14] drm/xe/configfs: Add bad_page_reservation attribute

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

On 8/26/2026 3:51 PM, Tejas Upadhyay wrote:
> Add a new configfs attribute 'bad_page_reservation' to control how bad
> VRAM pages are handled:
>   0 - Logging only (report in dmesg, no offlining)
>   1 - Offlining (default)
> 
> The attribute can only be set before binding to the device and defaults
> to true (offlining enabled). This gives administrators control over
> whether corrupted VRAM pages detected by hardware (e.g., ECC errors)
> are actively offlined or only logged.
> 
> Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_configfs.c | 67 +++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/xe/xe_configfs.h |  2 +
>  2 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/xe_configfs.c
> index 052cce962161..80c4c5f66e4c 100644
> --- a/drivers/gpu/drm/xe/xe_configfs.c
> +++ b/drivers/gpu/drm/xe/xe_configfs.c
> @@ -61,7 +61,8 @@
>   *	    ├── survivability_mode
>   *	    ├── gt_types_allowed
>   *	    ├── engines_allowed
> - *	    └── enable_psmi
> + *          ├── enable_psmi
> + *          └── bad_page_reservation

please double check alignment - look for \t

>   *
>   * After configuring the attributes as per next section, the device can be
>   * probed with::
> @@ -159,6 +160,19 @@
>   *
>   * This attribute can only be set before binding to the device.
>   *
> + * Bad pages reservation:
> + * ---------------------
> + *
> + * Controls how bad VRAM pages are handled:
> + *  0 - Logging only (report in dmesg, no offlining)
> + *  1 - Offlining (default)

this is bool, so Y/N/y/n will also work

> + *
> + *  Example to disable offlining::
> + *
> + *      # echo 0 > /sys/kernel/config/xe/0000:03:00.0/bad_page_reservation

if this is about VRAM, why attribute name does not say that?

maybe
	"enable_vram_offline"
or
	"disable_vram_offline"

to benefit from default '0' ?

> + *
> + * This attribute can only be set before binding to the device.

shouldn't we say that it is "available only on CRI+" or something?

> + *
>   * Context restore BB
>   * ------------------
>   *
> @@ -275,6 +289,7 @@ struct xe_config_group_device {
>  		bool survivability_mode;
>  		bool enable_psmi;
>  		bool enable_multi_queue;
> +		bool bad_page_reservation;
>  		struct {
>  			unsigned int max_vfs;
>  			bool admin_only_pf;
> @@ -295,6 +310,7 @@ static const struct xe_config_device device_defaults = {
>  	.survivability_mode = false,
>  	.enable_psmi = false,
>  	.enable_multi_queue = true,
> +	.bad_page_reservation = true,
>  	.sriov = {
>  		.max_vfs = XE_DEFAULT_MAX_VFS,
>  		.admin_only_pf = XE_DEFAULT_ADMIN_ONLY_PF,
> @@ -616,6 +632,32 @@ static ssize_t enable_multi_queue_store(struct config_item *item, const char *pa
>  	return len;
>  }
>  
> +static ssize_t bad_page_reservation_show(struct config_item *item, char *page)
> +{
> +	struct xe_config_device *dev = to_xe_config_device(item);
> +
> +	return sprintf(page, "%d\n", dev->bad_page_reservation);

	%s and str_yes_no() ?> +}
> +
> +static ssize_t bad_page_reservation_store(struct config_item *item, const char *page, size_t len)
> +{
> +	struct xe_config_group_device *dev = to_xe_config_group_device(item);
> +	bool val;
> +	int ret;
> +
> +	ret = kstrtobool(page, &val);
> +	if (ret)
> +		return ret;
> +
> +	guard(mutex)(&dev->lock);
> +	if (is_bound(dev))
> +		return -EBUSY;
> +
> +	dev->config.bad_page_reservation = val;
> +
> +	return len;
> +}
> +
>  static bool wa_bb_read_advance(bool dereference, char **p,
>  			       const char *append, size_t len,
>  			       size_t *max_size)
> @@ -855,6 +897,7 @@ CONFIGFS_ATTR(, ctx_restore_mid_bb);
>  CONFIGFS_ATTR(, ctx_restore_post_bb);
>  CONFIGFS_ATTR(, enable_multi_queue);
>  CONFIGFS_ATTR(, enable_psmi);
> +CONFIGFS_ATTR(, bad_page_reservation);
>  CONFIGFS_ATTR(, engines_allowed);
>  CONFIGFS_ATTR(, gt_types_allowed);
>  CONFIGFS_ATTR(, survivability_mode);

shouldn't you also update xe_config_device_is_visible() to make it CRI+ only?
> @@ -864,6 +907,7 @@ static struct configfs_attribute *xe_config_device_attrs[] = {
>  	&attr_ctx_restore_post_bb,
>  	&attr_enable_multi_queue,
>  	&attr_enable_psmi,
> +	&attr_bad_page_reservation,
>  	&attr_engines_allowed,
>  	&attr_gt_types_allowed,
>  	&attr_survivability_mode,
> @@ -1142,6 +1186,7 @@ static void dump_custom_dev_config(struct pci_dev *pdev,
>  	PRI_CUSTOM_ATTR("%llx", engines_allowed);
>  	PRI_CUSTOM_ATTR("%d", enable_multi_queue);
>  	PRI_CUSTOM_ATTR("%d", enable_psmi);
> +	PRI_CUSTOM_ATTR("%d", bad_page_reservation);
>  	PRI_CUSTOM_ATTR("%d", survivability_mode);
>  	PRI_CUSTOM_ATTR("%u", sriov.admin_only_pf);
>  
> @@ -1290,6 +1335,26 @@ bool xe_configfs_get_enable_multi_queue(struct pci_dev *pdev)
>  	return ret;
>  }
>  
> +/**
> + * xe_configfs_get_bad_page_reservation - get configfs bad_page_reservation setting
> + * @pdev: pci device
> + *
> + * Return: bad_page_reservation setting in configfs
> + */
> +bool xe_configfs_get_bad_page_reservation(struct pci_dev *pdev)
> +{
> +	struct xe_config_group_device *dev = find_xe_config_group_device(pdev);
> +	bool ret;
> +
> +	if (!dev)
> +		return device_defaults.bad_page_reservation;
> +
> +	ret = dev->config.bad_page_reservation;
> +	config_group_put(&dev->group);
> +
> +	return ret;
> +}
> +
>  /**
>   * xe_configfs_get_ctx_restore_mid_bb - get configfs ctx_restore_mid_bb setting
>   * @pdev: pci device
> diff --git a/drivers/gpu/drm/xe/xe_configfs.h b/drivers/gpu/drm/xe/xe_configfs.h
> index 4fbbeafba473..7405cc5f3207 100644
> --- a/drivers/gpu/drm/xe/xe_configfs.h
> +++ b/drivers/gpu/drm/xe/xe_configfs.h
> @@ -24,6 +24,7 @@ bool xe_configfs_media_gt_allowed(struct pci_dev *pdev);
>  u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev);
>  bool xe_configfs_get_psmi_enabled(struct pci_dev *pdev);
>  bool xe_configfs_get_enable_multi_queue(struct pci_dev *pdev);
> +bool xe_configfs_get_bad_page_reservation(struct pci_dev *pdev);
>  u32 xe_configfs_get_ctx_restore_mid_bb(struct pci_dev *pdev,
>  				       enum xe_engine_class class,
>  				       const u32 **cs);
> @@ -44,6 +45,7 @@ static inline bool xe_configfs_media_gt_allowed(struct pci_dev *pdev) { return t
>  static inline u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev) { return U64_MAX; }
>  static inline bool xe_configfs_get_psmi_enabled(struct pci_dev *pdev) { return false; }
>  static inline bool xe_configfs_get_enable_multi_queue(struct pci_dev *pdev) { return true; }
> +static inline bool xe_configfs_get_bad_page_reservation(struct pci_dev *pdev) { return true; }
>  static inline u32 xe_configfs_get_ctx_restore_mid_bb(struct pci_dev *pdev,
>  						     enum xe_engine_class class,
>  						     const u32 **cs) { return 0; }