Re: [PATCH v10 1/3] drm/xe/pf: Restrict device query responses in admin-only PF mode

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

On 4/8/2026 6:05 PM, Satyanarayana K V P wrote:
> When a PF is configured in admin-only mode, it is intended for management
> only and must not expose workload-facing capabilities to userspace.
> 
> Limit the exposed ioctl set in admin-only PF mode to XE_DEVICE_QUERY and
> XE_OBSERVATION, and suppress capability-bearing query payloads so that
> the userspace cannot discover execution-related device details in this
> mode.
> 
> 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: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Acked-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> 
> ---
> V9 -> V10:
> - Moved some parts of admin_only_pf mode under CONFIG_PCI_IOV config
> option (Michal).
> - Updated commit message.
> 
> V8 -> V9:
> - Memory regions are skipped in case of admin_only_pf mode (Michal)
> - removed .dumb_create, .dumb_map_offset and .show_fdinfo device specific
> operations in admin-only mode (Michal).
> 
> V7 -> V8:
> - Fixed issues reported by CI.Hooks
> - Updated commit message (Ashutosh)
> - Removed gem_prime_import from admin_only_driver structure (Michal)
> 
> V6 -> V7:
> - Allowed xe_observation_ioctl as well with admin-only PF (Ashutosh,
> Michal).
> - Updated commit message with steps to enable admin-only mode (Rodrigo).
> 
> V5 -> V6:
> - Updated commit message.
> - Return number of engines and memory regions as zero instead of
> returning query size as zero (Michal Wajdeczko).
> - Allow all other query IOCTLs excepts query_engines and
> query_mem_regions (Michal Wajdeczko).
> 
> V4 -> V5:
> - Updated commit message (Matt B).
> - Introduced new driver_admin_only_pf structure (Michal Wajdeczko).
> - Updated all query configs (Michal Wajdeczko).
> - Renamed xe_device_is_admin_only() to xe_device_is_admin_only_pf()
> - Fixed other review comments (Michal Wajdeczko).
> 
> 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    | 62 +++++++++++++++++++++++++++++--
>  drivers/gpu/drm/xe/xe_device.h    |  1 +
>  drivers/gpu/drm/xe/xe_hw_engine.c |  3 ++
>  drivers/gpu/drm/xe/xe_query.c     |  8 +++-
>  4 files changed, 69 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index cbce1d0ffe48..b0bbb079ca8e 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"
> @@ -390,7 +391,7 @@ bool xe_is_xe_file(const struct file *file)
>  	return file->f_op == &xe_driver_fops;
>  }
>  
> -static struct drm_driver driver = {
> +static struct drm_driver regular_driver = {
>  	.driver_features =
>  	    DRIVER_GEM |
>  	    DRIVER_RENDER | DRIVER_SYNCOBJ |
> @@ -415,6 +416,42 @@ static struct drm_driver driver = {
>  	.patchlevel = DRIVER_PATCHLEVEL,
>  };
>  
> +#ifdef CONFIG_PCI_IOV
> +static const struct drm_ioctl_desc xe_ioctls_admin_only[] = {
> +	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),
> +};
> +
> +static struct drm_driver admin_only_driver = {
> +	.driver_features =
> +	    DRIVER_GEM | DRIVER_RENDER | DRIVER_GEM_GPUVA,
> +	.open = xe_file_open,
> +	.postclose = xe_file_close,
> +	.ioctls = xe_ioctls_admin_only,
> +	.num_ioctls = ARRAY_SIZE(xe_ioctls_admin_only),
> +	.fops = &xe_driver_fops,
> +	.name = DRIVER_NAME,
> +	.desc = DRIVER_DESC,
> +	.major = DRIVER_MAJOR,
> +	.minor = DRIVER_MINOR,
> +	.patchlevel = DRIVER_PATCHLEVEL,
> +};
> +#endif
> +
> +/**
> + * xe_device_is_admin_only() - Check whether device is admin only or not.
> + * @xe: the &xe_device to check
> + *
> + * Return: true if the device is admin only, false otherwise.
> + */
> +bool xe_device_is_admin_only(const struct xe_device *xe)
> +{
> +#ifdef CONFIG_PCI_IOV
> +	return xe->drm.driver == &admin_only_driver;
> +#endif
> +	return false;

nit: maybe put this under #else ?

or better, provide an inline with false in .h 

#ifdef CONFIG_PCI_IOV
bool xe_device_is_admin_only(const struct xe_device *xe);
#else
static inline bool xe_device_is_admin_only(const struct xe_device *xe)
{
	return false;
}
#endif

> +}
> +
>  static void xe_device_destroy(struct drm_device *dev, void *dummy)
>  {
>  	struct xe_device *xe = to_xe_device(dev);
> @@ -439,16 +476,26 @@ static void xe_device_destroy(struct drm_device *dev, void *dummy)
>  struct xe_device *xe_device_create(struct pci_dev *pdev,
>  				   const struct pci_device_id *ent)
>  {
> +	struct drm_driver *driver = &regular_driver;
>  	struct xe_device *xe;
>  	int err;
>  
> -	xe_display_driver_set_hooks(&driver);
> +#ifdef CONFIG_PCI_IOV
> +	/*
> +	 * Since XE device is not initialized yet, read from configfs
> +	 * directly to decide whether we are in admin-only PF mode or not.
> +	 */
> +	if (xe_configfs_admin_only_pf(pdev))
> +		driver = &admin_only_driver;
> +#endif
> +
> +	xe_display_driver_set_hooks(driver);
>  
> -	err = aperture_remove_conflicting_pci_devices(pdev, driver.name);
> +	err = aperture_remove_conflicting_pci_devices(pdev, driver->name);
>  	if (err)
>  		return ERR_PTR(err);
>  
> -	xe = devm_drm_dev_alloc(&pdev->dev, &driver, struct xe_device, drm);
> +	xe = devm_drm_dev_alloc(&pdev->dev, driver, struct xe_device, drm);
>  	if (IS_ERR(xe))
>  		return xe;
>  
> @@ -708,6 +755,13 @@ int xe_device_probe_early(struct xe_device *xe)
>  
>  	xe_sriov_probe_early(xe);
>  
> +#ifdef CONFIG_PCI_IOV

nit: not sure if we need this #ifdef here as for !PCI_IOV below code should be
optimized-out by the compiler anyway, since is_admin_only will be always false

> +	if (xe_device_is_admin_only(xe) && !IS_SRIOV_PF(xe)) {
> +		xe_err(xe, "Can't run Admin-only mode without SR-IOV PF mode!\n");
> +		return -ENODEV;
> +	}
> +#endif
> +
>  	if (IS_SRIOV_VF(xe))
>  		vf_update_device_info(xe);
>  
> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> index e4b9de8d8e95..c220f2f1352f 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -43,6 +43,7 @@ static inline struct xe_device *ttm_to_xe_device(struct ttm_device *ttm)
>  	return container_of(ttm, struct xe_device, ttm);
>  }
>  
> +bool xe_device_is_admin_only(const struct xe_device *xe);

nit: can we move it little down, maybe at closer to xe_device_wmb?

this is not the most important xe_device function, and prototypes here
are just for high-level create/probe/remove/shutdown functions

and if we go with the inline in .h then it could be placed near the eof

>  struct xe_device *xe_device_create(struct pci_dev *pdev,
>  				   const struct pci_device_id *ent);
>  int xe_device_probe_early(struct xe_device *xe);
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c
> index 6dd05fac6595..2f9c1c063f16 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -1026,6 +1026,9 @@ bool xe_hw_engine_is_reserved(struct xe_hw_engine *hwe)
>  	struct xe_gt *gt = hwe->gt;
>  	struct xe_device *xe = gt_to_xe(gt);
>  
> +	if (xe_device_is_admin_only(xe))
> +		return true;
> +
>  	if (hwe->class == XE_ENGINE_CLASS_OTHER)
>  		return true;
>  
> diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> index d84d6a422c45..0dc5a9bb481a 100644
> --- a/drivers/gpu/drm/xe/xe_query.c
> +++ b/drivers/gpu/drm/xe/xe_query.c
> @@ -231,6 +231,9 @@ static size_t calc_mem_regions_size(struct xe_device *xe)
>  	u32 num_managers = 1;
>  	int i;
>  
> +	if (xe_device_is_admin_only(xe))
> +		return sizeof(struct drm_xe_query_mem_regions);
> +
>  	for (i = XE_PL_VRAM0; i <= XE_PL_VRAM1; ++i)
>  		if (ttm_manager_type(&xe->ttm, i))
>  			num_managers++;
> @@ -259,6 +262,9 @@ static int query_mem_regions(struct xe_device *xe,
>  	if (XE_IOCTL_DBG(xe, !mem_regions))
>  		return -ENOMEM;
>  
> +	if (xe_device_is_admin_only(xe))
> +		goto out;
> +
>  	man = ttm_manager_type(&xe->ttm, XE_PL_TT);
>  	mem_regions->mem_regions[0].mem_class = DRM_XE_MEM_REGION_CLASS_SYSMEM;
>  	/*
> @@ -296,7 +302,7 @@ static int query_mem_regions(struct xe_device *xe,
>  			mem_regions->num_mem_regions++;
>  		}
>  	}
> -
> +out:

nit: "Choose label names which say what the goto does" - see [1]
so maybe as this is not pure 'out' but we're also doing copying:

		goto user_copy;
	...
	user_copy:

[1] https://docs.kernel.org/process/coding-style.html#centralized-exiting-of-functions

>  	if (!copy_to_user(query_ptr, mem_regions, size))
>  		ret = 0;
>  	else