Re: [RFC v4 1/1] drm/xe/pf: Restrict device query responses in admin-only PF mode

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

On Thu, Feb 26, 2026 at 10:47:58AM -0800, Matthew Brost wrote:

One more thing...

> On Thu, Feb 26, 2026 at 04:31:06PM +0000, Satyanarayana K V P wrote:
> > When the PF is configured for admin‑only mode, it is restricted to
> > management functions and should not allow users to run workloads.
> > Suppress device capabilities to userspace in admin-only PF mode.
> > 
> 
> I'd mention which IOCTLs we expose in PF admin only and why we expose
> each one of these IOCTLs.
> 
> > Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Piotr Piórkowski <piotr.piorkowski@intel.com>
> > Cc: Matthew Brost <matthew.brost@intel.com>
> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > Cc: Dunajski Bartosz <bartosz.dunajski@intel.com>
> > Cc: dri-devel@lists.freedesktop.org
> > 
> > ---
> > V3 -> V4:
> > - Suppressed device capabilities in admin-only PF mode. (Wajdeczko)
> > 
> > V2 -> V3:
> > - Introduced new helper function xe_debugfs_create_files() to create
> > debugfs entries based on admin_only_pf mode or normal mode.
> > 
> > V1 -> V2:
> > - Rebased to latest drm-tip.
> > - Update update_minor_dev() to debugfs_minor_dev().
> > ---
> >  drivers/gpu/drm/xe/xe_device.c | 14 ++++++++++++++
> >  drivers/gpu/drm/xe/xe_query.c  | 11 ++++++++++-
> >  drivers/gpu/drm/xe/xe_sriov.h  |  8 ++++++++
> >  3 files changed, 32 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > index 3462645ca13c..7bf462eed917 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -25,6 +25,7 @@
> >  #include "regs/xe_regs.h"
> >  #include "xe_bo.h"
> >  #include "xe_bo_evict.h"
> > +#include "xe_configfs.h"
> >  #include "xe_debugfs.h"
> >  #include "xe_defaults.h"
> >  #include "xe_devcoredump.h"
> > @@ -213,6 +214,11 @@ static const struct drm_ioctl_desc xe_ioctls[] = {
> >  			  DRM_RENDER_ALLOW),
> >  };
> >  
> > +static const struct drm_ioctl_desc xe_pf_admin_only_ioctls[] = {
> > +	DRM_IOCTL_DEF_DRV(XE_DEVICE_QUERY, xe_query_ioctl, DRM_RENDER_ALLOW),
> > +	DRM_IOCTL_DEF_DRV(XE_OBSERVATION, xe_observation_ioctl, DRM_RENDER_ALLOW),
> 
> XE_OBSERVATION can create buffer objects and exec queues? Is desired to
> expose things like this?
> 
> I like this approach of only exposing a subset of IOCTLs though, pretty slick.
> 
> > +};
> > +
> >  static long xe_drm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> >  {
> >  	struct drm_file *file_priv = file->private_data;
> > @@ -442,6 +448,14 @@ struct xe_device *xe_device_create(struct pci_dev *pdev,
> >  	struct xe_device *xe;
> >  	int err;
> >  
> > +	if (xe_configfs_admin_only_pf(pdev)) {
> > +		driver.ioctls = xe_pf_admin_only_ioctls;
> > +		driver.num_ioctls = ARRAY_SIZE(xe_pf_admin_only_ioctls);
> > +	} else {

While harmless, you don't need the else clause as these are default values.

Matt

>
 > +		driver.ioctls = xe_ioctls;
> > +		driver.num_ioctls = ARRAY_SIZE(xe_ioctls);
> > +	}
> > +
> >  	xe_display_driver_set_hooks(&driver);
> >  
> >  	err = aperture_remove_conflicting_pci_devices(pdev, driver.name);
> > diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> > index 34db266b723f..1416ab7be809 100644
> > --- a/drivers/gpu/drm/xe/xe_query.c
> > +++ b/drivers/gpu/drm/xe/xe_query.c
> > @@ -215,7 +215,10 @@ static int query_engines(struct xe_device *xe,
> >  			i++;
> >  		}
> >  
> > -	engines->num_engines = i;
> > +	if (xe_device_is_admin_only(xe))
> > +		engines->num_engines = 0;
> > +	else
> > +		engines->num_engines = i;
> >  
> >  	if (copy_to_user(query_ptr, engines, size)) {
> >  		kfree(engines);
> > @@ -297,6 +300,9 @@ static int query_mem_regions(struct xe_device *xe,
> >  		}
> >  	}
> >  
> > +	if (xe_device_is_admin_only(xe))
> > +		mem_regions->num_mem_regions = 0;
> > +
> >  	if (!copy_to_user(query_ptr, mem_regions, size))
> >  		ret = 0;
> >  	else
> > @@ -419,6 +425,9 @@ static int query_gt_list(struct xe_device *xe, struct drm_xe_device_query *query
> >  		iter++;
> >  	}
> >  
> > +	if (xe_device_is_admin_only(xe))
> > +		gt_list->num_gt = 0;
> > +
> >  	if (copy_to_user(query_ptr, gt_list, size)) {
> >  		kfree(gt_list);
> >  		return -EFAULT;
> > diff --git a/drivers/gpu/drm/xe/xe_sriov.h b/drivers/gpu/drm/xe/xe_sriov.h
> > index 72e55543c30e..be426afa90b1 100644
> > --- a/drivers/gpu/drm/xe/xe_sriov.h
> > +++ b/drivers/gpu/drm/xe/xe_sriov.h
> > @@ -37,6 +37,14 @@ static inline bool xe_device_is_sriov_vf(const struct xe_device *xe)
> >  	return xe_device_sriov_mode(xe) == XE_SRIOV_MODE_VF;
> >  }
> >  
> > +static inline bool xe_device_is_admin_only(const struct xe_device *xe)
> 
> Kernel doc.
> 
> Matt
> 
> > +{
> > +	if (xe_device_is_sriov_pf(xe) && xe->sriov.pf.admin_only)
> > +		return true;
> > +	else
> > +		return false;
> > +}
> > +
> >  #define IS_SRIOV_PF(xe) xe_device_is_sriov_pf(xe)
> >  #define IS_SRIOV_VF(xe) xe_device_is_sriov_vf(xe)
> >  
> > -- 
> > 2.43.0
> >