Re: [PATCH v2 3/3] drm/xe/vf: Don't expose privileged GT debugfs files if VF

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Apr 03, 2025 at 04:26:35PM +0200, Michal Wajdeczko wrote:
> Some of the debugfs files require access to the registers that are
> not accessible to the VFs. Don't expose those files on VF drivers.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> v2: avoid "privileged" word, make it clear it's about VF/PF (Lucas)
>     add hint for developers where to add new files (Lucas)
> ---
>  drivers/gpu/drm/xe/xe_gt_debugfs.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> index 2d63a69cbfa3..a88076e9cc7d 100644
> --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> @@ -299,20 +299,20 @@ static int hwconfig(struct xe_gt *gt, struct drm_printer *p)
>  	return 0;
>  }
>  
> -static const struct drm_info_list debugfs_list[] = {
> -	{"hw_engines", .show = xe_gt_debugfs_simple_show, .data = hw_engines},
> +/*
> + * only for GT debugfs files which can be safely used on the VF as well:
> + * - without access to the GT privileged registers
> + * - without access to the PF specific data
> + */
> +static const struct drm_info_list vf_safe_debugfs_list[] = {
>  	{"force_reset", .show = xe_gt_debugfs_simple_show, .data = force_reset},
>  	{"force_reset_sync", .show = xe_gt_debugfs_simple_show, .data = force_reset_sync},

Probably don't expose the reset ones to VF either.

Matt

>  	{"sa_info", .show = xe_gt_debugfs_simple_show, .data = sa_info},
>  	{"topology", .show = xe_gt_debugfs_simple_show, .data = topology},
> -	{"steering", .show = xe_gt_debugfs_simple_show, .data = steering},
>  	{"ggtt", .show = xe_gt_debugfs_simple_show, .data = ggtt},
> -	{"powergate_info", .show = xe_gt_debugfs_simple_show, .data = powergate_info},
>  	{"register-save-restore", .show = xe_gt_debugfs_simple_show, .data = register_save_restore},
>  	{"workarounds", .show = xe_gt_debugfs_simple_show, .data = workarounds},
>  	{"tunings", .show = xe_gt_debugfs_simple_show, .data = tunings},
> -	{"pat", .show = xe_gt_debugfs_simple_show, .data = pat},
> -	{"mocs", .show = xe_gt_debugfs_simple_show, .data = mocs},
>  	{"default_lrc_rcs", .show = xe_gt_debugfs_simple_show, .data = rcs_default_lrc},
>  	{"default_lrc_ccs", .show = xe_gt_debugfs_simple_show, .data = ccs_default_lrc},
>  	{"default_lrc_bcs", .show = xe_gt_debugfs_simple_show, .data = bcs_default_lrc},
> @@ -322,6 +322,15 @@ static const struct drm_info_list debugfs_list[] = {
>  	{"hwconfig", .show = xe_gt_debugfs_simple_show, .data = hwconfig},
>  };
>  
> +/* everything else should be added here */
> +static const struct drm_info_list pf_only_debugfs_list[] = {
> +	{"hw_engines", .show = xe_gt_debugfs_simple_show, .data = hw_engines},
> +	{"mocs", .show = xe_gt_debugfs_simple_show, .data = mocs},
> +	{"pat", .show = xe_gt_debugfs_simple_show, .data = pat},
> +	{"powergate_info", .show = xe_gt_debugfs_simple_show, .data = powergate_info},
> +	{"steering", .show = xe_gt_debugfs_simple_show, .data = steering},
> +};
> +
>  void xe_gt_debugfs_register(struct xe_gt *gt)
>  {
>  	struct xe_device *xe = gt_to_xe(gt);
> @@ -345,10 +354,15 @@ void xe_gt_debugfs_register(struct xe_gt *gt)
>  	 */
>  	root->d_inode->i_private = gt;
>  
> -	drm_debugfs_create_files(debugfs_list,
> -				 ARRAY_SIZE(debugfs_list),
> +	drm_debugfs_create_files(vf_safe_debugfs_list,
> +				 ARRAY_SIZE(vf_safe_debugfs_list),
>  				 root, minor);
>  
> +	if (!IS_SRIOV_VF(xe))
> +		drm_debugfs_create_files(pf_only_debugfs_list,
> +					 ARRAY_SIZE(pf_only_debugfs_list),
> +					 root, minor);
> +
>  	xe_uc_debugfs_register(&gt->uc, root);
>  
>  	if (IS_SRIOV_PF(xe))
> -- 
> 2.47.1
>