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

["K V P, Satyanarayana" <satyanarayana.k.v.p@intel.com>]

On 02-Apr-26 7:39 PM, Michal Wajdeczko wrote:
>
> On 4/2/2026 3:19 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.
>>
>> Enable admin-only mode with:
>> echo <B:D:F> | sudo tee /sys/bus/pci/drivers/xe/unbind
>> sudo mkdir /sys/kernel/config/xe/<B:D:F>
>> echo yes | sudo tee /sys/kernel/config/xe/<B:D:F>/sriov/admin_only_pf
>> echo <B:D:F> | sudo tee /sys/bus/pci/drivers/xe/bind
>>
>> 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>
>>
>> ---
>> 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    | 60 ++++++++++++++++++++++++++++---
>>   drivers/gpu/drm/xe/xe_device.h    |  1 +
>>   drivers/gpu/drm/xe/xe_hw_engine.c |  3 ++
>>   drivers/gpu/drm/xe/xe_query.c     | 10 +++++-
>>   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..eba2fa6dc7d3 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"
>> @@ -216,6 +217,11 @@ static const struct drm_ioctl_desc xe_ioctls[] = {
>>   			  DRM_RENDER_ALLOW),
>>   };
>>   
>> +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 long xe_drm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>>   {
>>   	struct drm_file *file_priv = file->private_data;
>> @@ -390,7 +396,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 +421,38 @@ static struct drm_driver driver = {
>>   	.patchlevel = DRIVER_PATCHLEVEL,
>>   };
>>   
>> +static struct drm_driver admin_only_driver = {
>> +	.driver_features =
>> +	    DRIVER_GEM | DRIVER_RENDER | DRIVER_GEM_GPUVA,
>> +	.open = xe_file_open,
>> +	.postclose = xe_file_close,
>> +
>> +	.dumb_create = xe_bo_dumb_create,
>> +	.dumb_map_offset = drm_gem_ttm_dumb_map_offset,
>> +#ifdef CONFIG_PROC_FS
>> +	.show_fdinfo = xe_drm_client_fdinfo,
> do we want to expose memory or engines details here?
Fixed in the new revision.
>> +#endif
>> +	.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,
>> +};
>> +
>> +/**
>> + * 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)
>> +{
>> +	return xe->drm.driver == &admin_only_driver;
>> +}
> I'm still looking for patch #2 which would update xe_sriov_pf_admin_only()
Sent in the new revision.
>> +
>>   static void xe_device_destroy(struct drm_device *dev, void *dummy)
>>   {
>>   	struct xe_device *xe = to_xe_device(dev);
>> @@ -439,16 +477,25 @@ 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
> maybe use if (IS_ENABLED()) to avoid complains about unused static in PCI_IOV=n ?
CI.Hooks reported compilation error for some configuration. So, need to 
used compilation flag here.
>> +	/*
>> +	 * 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
> nit: add empty line here
Fixed in new revision.
>> +	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,11 @@ int xe_device_probe_early(struct xe_device *xe)
>>   
>>   	xe_sriov_probe_early(xe);
>>   
>> +	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;
>> +	}
>> +
>>   	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);
>>   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 337baf0a6e87..2c324acb1dd0 100644
>> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
>> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
>> @@ -1027,6 +1027,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..b10a281c6ae0 100644
>> --- a/drivers/gpu/drm/xe/xe_query.c
>> +++ b/drivers/gpu/drm/xe/xe_query.c
>> @@ -231,10 +231,13 @@ static size_t calc_mem_regions_size(struct xe_device *xe)
>>   	u32 num_managers = 1;
>>   	int i;
>>   
>> +	if (xe_device_is_admin_only(xe))
>> +		goto out;
> or maybe just:
> 		return sizeof(drm_xe_query_mem_regions);
Fixed in new revision.
>> +
>>   	for (i = XE_PL_VRAM0; i <= XE_PL_VRAM1; ++i)
>>   		if (ttm_manager_type(&xe->ttm, i))
>>   			num_managers++;
>> -
>> +out:
>>   	return offsetof(struct drm_xe_query_mem_regions, mem_regions[num_managers]);
>>   }
>>   
>> @@ -273,6 +276,8 @@ static int query_mem_regions(struct xe_device *xe,
>>   	mem_regions->num_mem_regions = 1;
> IMO we shouldn't attempt to fill region0 here, and then memset it
> but just jump to ...
Fixed in new revision.
>>   
>>   	for (i = XE_PL_VRAM0; i <= XE_PL_VRAM1; ++i) {
>> +		if (xe_device_is_admin_only(xe))
>> +			break;
>>   		man = ttm_manager_type(&xe->ttm, i);
>>   		if (man) {
>>   			mem_regions->mem_regions[mem_regions->num_mem_regions].mem_class =
>> @@ -297,6 +302,9 @@ static int query_mem_regions(struct xe_device *xe,
>>   		}
>>   	}
>>   
>> +	if (xe_device_is_admin_only(xe))
>> +		memset(mem_regions, 0, size);
>> +
> ... here
>
>>   	if (!copy_to_user(query_ptr, mem_regions, size))
>>   		ret = 0;
>>   	else