Re: [PATCH 11/11] drm/xe: Add debugfs support for page reclamation

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

On 11/18/2025 10:05 AM, Brian Nguyen wrote:
> Allow for runtime modification to page reclamation feature through
> debugfs configuration. This parameter will only take effect if the
> platform supports the page reclamation feature by default.
> 
> Move xe_match_desc to common header for debugfs access to read default
> device values of xe driver for current platform.

this seems to be unnecessary, see below

> 
> Signed-off-by: Brian Nguyen <brian3.nguyen@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_configfs.c | 11 +-------
>  drivers/gpu/drm/xe/xe_debugfs.c  | 47 ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_device.c   | 10 +++++++
>  drivers/gpu/drm/xe/xe_device.h   |  2 ++
>  4 files changed, 60 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/xe_configfs.c
> index 9f6251b1008b..efc6d0690b27 100644
> --- a/drivers/gpu/drm/xe/xe_configfs.c
> +++ b/drivers/gpu/drm/xe/xe_configfs.c
> @@ -15,6 +15,7 @@
>  
>  #include "instructions/xe_mi_commands.h"
>  #include "xe_configfs.h"
> +#include "xe_device.h"
>  #include "xe_gt_types.h"
>  #include "xe_hw_engine_types.h"
>  #include "xe_module.h"
> @@ -925,16 +926,6 @@ static const struct config_item_type xe_config_sriov_type = {
>  	.ct_attrs	= xe_config_sriov_attrs,
>  };
>  
> -static const struct xe_device_desc *xe_match_desc(struct pci_dev *pdev)
> -{
> -	struct device_driver *driver = driver_find("xe", &pci_bus_type);
> -	struct pci_driver *drv = to_pci_driver(driver);
> -	const struct pci_device_id *ids = drv ? drv->id_table : NULL;
> -	const struct pci_device_id *found = pci_match_id(ids, pdev);
> -
> -	return found ? (const void *)found->driver_data : NULL;
> -}
> -
>  static struct pci_dev *get_physfn_instead(struct pci_dev *virtfn)
>  {
>  	struct pci_dev *physfn = pci_physfn(virtfn);
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index e91da9589c5f..572c61ee1e29 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -19,6 +19,7 @@
>  #include "xe_gt_printk.h"
>  #include "xe_guc_ads.h"
>  #include "xe_mmio.h"
> +#include "xe_pci_types.h"
>  #include "xe_pm.h"
>  #include "xe_psmi.h"
>  #include "xe_pxp_debugfs.h"
> @@ -297,6 +298,49 @@ static const struct file_operations wedged_mode_fops = {
>  	.write = wedged_mode_set,
>  };
>  
> +static ssize_t page_reclaim_hw_assist_show(struct file *f, char __user *ubuf,
> +					   size_t size, loff_t *pos)
> +{
> +	struct xe_device *xe = file_inode(f)->i_private;
> +	char buf[8];
> +	int len;
> +
> +	len = scnprintf(buf, sizeof(buf), "%d\n", xe->info.has_page_reclaim_hw_assist);
> +	return simple_read_from_buffer(ubuf, size, pos, buf, len);
> +}
> +
> +static ssize_t page_reclaim_hw_assist_set(struct file *f, const char __user *ubuf,
> +					  size_t size, loff_t *pos)
> +{
> +	struct xe_device *xe = file_inode(f)->i_private;
> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> +	const struct xe_device_desc *desc = xe_match_desc(pdev);
> +	unsigned int val;
> +	ssize_t ret;
> +
> +	ret = kstrtouint_from_user(ubuf, size, 0, &val);

kstrtobool_from_user

> +	if (ret)
> +		return ret;
> +
> +	/**
> +	 * Don't modify if page reclamation support isn't normally
> +	 * supported by the HW.
> +	 */
> +
> +	if (!desc || !desc->has_page_reclaim_hw_assist)
> +		return -ENODEV;

instead of checking desc->has_page_reclaim_hw_assist capability here

> +
> +	xe->info.has_page_reclaim_hw_assist = !!val;
> +
> +	return size;
> +}
> +
> +static const struct file_operations page_reclaim_hw_assist_fops = {
> +	.owner = THIS_MODULE,
> +	.read = page_reclaim_hw_assist_show,
> +	.write = page_reclaim_hw_assist_set,
> +};
> +
>  static ssize_t atomic_svm_timeslice_ms_show(struct file *f, char __user *ubuf,
>  					    size_t size, loff_t *pos)
>  {
> @@ -403,6 +447,9 @@ void xe_debugfs_register(struct xe_device *xe)
>  	debugfs_create_file("disable_late_binding", 0600, root, xe,
>  			    &disable_late_binding_fops);
>  

better to expose "page_reclaim_hw_assist" file *only* if required
capability is present and we can get that flag directly from the xe:

	if (xe->info.has_page_reclaim_hw_assist)

> +	debugfs_create_file("page_reclaim_hw_assist", 0600, root, xe,
> +			    &page_reclaim_hw_assist_fops);
> +
>  	for (mem_type = XE_PL_VRAM0; mem_type <= XE_PL_VRAM1; ++mem_type) {
>  		man = ttm_manager_type(bdev, mem_type);
>  
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index c7d373c70f0f..16afddc5e35e 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -1295,3 +1295,13 @@ void xe_device_declare_wedged(struct xe_device *xe)
>  		drm_dev_wedged_event(&xe->drm, xe->wedged.method, NULL);
>  	}
>  }
> +
> +const struct xe_device_desc *xe_match_desc(struct pci_dev *pdev)

note that this function was specific for configfs case where might not
have the xe device, hence the manual lookup was needed

if in the future for some reasons we would like to get access to the desc
from the xe, then we should rather consider adding a const pointer to it

> +{
> +	struct device_driver *driver = driver_find("xe", &pci_bus_type);
> +	struct pci_driver *drv = to_pci_driver(driver);
> +	const struct pci_device_id *ids = drv ? drv->id_table : NULL;
> +	const struct pci_device_id *found = pci_match_id(ids, pdev);
> +
> +	return found ? (const void *)found->driver_data : NULL;
> +}
> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> index 32cc6323b7f6..a66e8e4b3e01 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -193,6 +193,8 @@ void xe_device_declare_wedged(struct xe_device *xe);
>  struct xe_file *xe_file_get(struct xe_file *xef);
>  void xe_file_put(struct xe_file *xef);
>  
> +const struct xe_device_desc *xe_match_desc(struct pci_dev *pdev);
> +
>  int xe_is_injection_active(void);
>  
>  /*