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 10:56:10PM +0200, Michal Wajdeczko wrote:
> 
> 
> On 03.04.2025 22:28, Matthew Brost wrote:
> > 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.
> 
> why not? from commit 459777724d30 ("drm/xe/vf: Don't try to trigger a
> full GT reset if VF") we just use H2G action VF_RESET(0x5507) to
> exercise the reset flow as applicable method for the VF
>

Ah, yes. The above commit will ensure a VF can't clobber other VFs.
Sorry the noise.

Matt