Re: [PATCH 3/5] drm/xe/pf: Expose basic info about VFs in debugfs

["Piotr Piórkowski" <piotr.piorkowski@intel.com>]

Michal Wajdeczko <michal.wajdeczko@intel.com> wrote on wto [2025-cze-24 22:09:21 +0200]:
> We already have function to print summary about VFs, but we missed
> to add debugfs attribute to make it visible. Do it now.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_debugfs.c  |  4 +++
>  drivers/gpu/drm/xe/xe_sriov_pf.c | 46 ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_sriov_pf.h |  6 +++++
>  3 files changed, 56 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index d83cd6ed3fa8..24a5f99ed52f 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -20,6 +20,7 @@
>  #include "xe_pm.h"
>  #include "xe_pxp_debugfs.h"
>  #include "xe_sriov.h"
> +#include "xe_sriov_pf.h"
>  #include "xe_step.h"
>  
>  #ifdef CONFIG_DRM_XE_DEBUG
> @@ -273,4 +274,7 @@ void xe_debugfs_register(struct xe_device *xe)
>  	xe_pxp_debugfs_register(xe->pxp);
>  
>  	fault_create_debugfs_attr("fail_gt_reset", root, &gt_reset_failure);
> +
> +	if (IS_SRIOV_PF(xe))
> +		xe_sriov_pf_debugfs_register(xe, root);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_sriov_pf.c b/drivers/gpu/drm/xe/xe_sriov_pf.c
> index 0f721ae17b26..32788b83b8bd 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_pf.c
> +++ b/drivers/gpu/drm/xe/xe_sriov_pf.c
> @@ -3,6 +3,8 @@
>   * Copyright © 2023-2024 Intel Corporation
>   */
>  
> +#include <linux/debugfs.h>
> +#include <drm/drm_debugfs.h>
>  #include <drm/drm_managed.h>
>  
>  #include "xe_assert.h"
> @@ -102,3 +104,47 @@ void xe_sriov_pf_print_vfs_summary(struct xe_device *xe, struct drm_printer *p)
>  	drm_printf(p, "supported: %u\n", xe->sriov.pf.driver_max_vfs);
>  	drm_printf(p, "enabled: %u\n", pci_num_vf(pdev));
>  }
> +
> +static int simple_show(struct seq_file *m, void *data)
> +{
> +	struct drm_printer p = drm_seq_file_printer(m);
> +	struct drm_info_node *node = m->private;
> +	struct dentry *parent = node->dent->d_parent;
> +	struct xe_device *xe = parent->d_inode->i_private;
> +	void (*print)(struct xe_device *, struct drm_printer *) = node->info_ent->data;
> +
> +	if (WARN_ON(!print))
> +		return -EINVAL;

NIT:
In my opinion, checking for null pointer dereference is unnecessary, and returning the error
to userspace can be omitted in this case.

> +
> +	print(xe, &p);
> +	return 0;
> +}
> +
> +static const struct drm_info_list debugfs_list[] = {
> +	{ .name = "vfs", .show = simple_show, .data = xe_sriov_pf_print_vfs_summary },
> +};
> +
> +/**
> + * xe_sriov_pf_debugfs_register - Register PF debugfs attributes.
> + * @xe: the &xe_device
> + * @root: the root &dentry
> + *
> + * Prepare debugfs attributes exposed by the PF.
> + */
> +void xe_sriov_pf_debugfs_register(struct xe_device *xe, struct dentry *root)
> +{
> +	struct drm_minor *minor = xe->drm.primary;
> +	struct dentry *parent;
> +
> +	/*
> +	 *      /sys/kernel/debug/dri/0/
> +	 *      ├── pf
> +	 *      │   ├── ...
> +	 */
> +	parent = debugfs_create_dir("pf", root);
> +	if (IS_ERR(parent))
> +		return;
> +	parent->d_inode->i_private = xe;
> +
> +	drm_debugfs_create_files(debugfs_list, ARRAY_SIZE(debugfs_list), parent, minor);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_sriov_pf.h b/drivers/gpu/drm/xe/xe_sriov_pf.h
> index d1220e70e1c0..c392c3fcf085 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_pf.h
> +++ b/drivers/gpu/drm/xe/xe_sriov_pf.h
> @@ -8,12 +8,14 @@
>  
>  #include <linux/types.h>
>  
> +struct dentry;
>  struct drm_printer;
>  struct xe_device;
>  
>  #ifdef CONFIG_PCI_IOV
>  bool xe_sriov_pf_readiness(struct xe_device *xe);
>  int xe_sriov_pf_init_early(struct xe_device *xe);
> +void xe_sriov_pf_debugfs_register(struct xe_device *xe, struct dentry *root);
>  void xe_sriov_pf_print_vfs_summary(struct xe_device *xe, struct drm_printer *p);
>  #else
>  static inline bool xe_sriov_pf_readiness(struct xe_device *xe)
> @@ -25,6 +27,10 @@ static inline int xe_sriov_pf_init_early(struct xe_device *xe)
>  {
>  	return 0;
>  }
> +
> +static inline void xe_sriov_pf_debugfs_register(struct xe_device *xe, struct dentry *root)
> +{
> +}

One minor comment above. Besides that, LGTM
Reviewed-by: Piotr Piórkowski <piotr.piorkowski@intel.com>

>  #endif
>  
>  #endif
> -- 
> 2.47.1
> 

--