On 3/31/2026 8:17 AM, 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
> 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 <[email protected]>
> Cc: Michal Wajdeczko <[email protected]>
> Cc: Rodrigo Vivi <[email protected]>
> Cc: Piotr Piórkowski <[email protected]>
> Cc: Matthew Brost <[email protected]>
> Cc: Thomas Hellström <[email protected]>
> Cc: Michał Winiarski <[email protected]>
> Cc: Dunajski Bartosz <[email protected]>
> Cc: Ashutosh Dixit <[email protected]>
> Cc: [email protected]
> Acked-by: Rodrigo Vivi <[email protected]>
> 
> ---
> 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 | 61 +++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/xe/xe_device.h |  1 +
>  drivers/gpu/drm/xe/xe_query.c  |  6 ++++
>  3 files changed, 64 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index cbce1d0ffe48..910a0aa4c3d0 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,40 @@ 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,
> +
> +     .gem_prime_import = xe_gem_prime_import,

not a expert here, but do we need this?
comment says "This defaults to drm_gem_prime_import() if not set."

> +
> +     .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,
> +#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)

after introducing this function, to avoid confusion, we should revisit: 
 - xe_sriov_pf_admin_only
 - xe->sriov.pf.admin_only

> +{
> +     return xe->drm.driver == &admin_only_driver;
> +}
> +
>  static void xe_device_destroy(struct drm_device *dev, void *dummy)
>  {
>       struct xe_device *xe = to_xe_device(dev);
> @@ -439,16 +479,24 @@ 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);
> +     /*
> +      * 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;
> +
> +     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 +756,11 @@ int xe_device_probe_early(struct xe_device *xe)
>  
>       xe_sriov_probe_early(xe);
>  
> +     if (xe_configfs_admin_only_pf(to_pci_dev(xe->drm.dev)) && 
> !IS_SRIOV_PF(xe)) {

we should be able to use xe_device_is_admin_only() now
no need to check configfs again

> +             drm_err(&xe->drm, "Admin-only PF mode is enabled in non PF 
> mode\n");

nit: we ca use xe_err() here
nit: maybe the message should be:

        "Can't run Admin-only mode without SR-IOV PF mode!\n"
or
        "Admin-only mode without SR-IOV PF is unsupported!\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_query.c b/drivers/gpu/drm/xe/xe_query.c
> index d84d6a422c45..40c7ab9fecf8 100644
> --- a/drivers/gpu/drm/xe/xe_query.c
> +++ b/drivers/gpu/drm/xe/xe_query.c
> @@ -217,6 +217,9 @@ static int query_engines(struct xe_device *xe,
>  
>       engines->num_engines = i;
>  
> +     if (xe_device_is_admin_only(xe))
> +             memset(engines, 0, size);

that looks broken - we will return some number of engines, but all of
them will be class=0, instance=0, gtid=0

why can't we put is_admin_only case in xe_hw_engine_is_reserved() as
previously suggested? this should filter-out all engines as expected
so returned query data will be:

        engines->num_engines = 0;
        engines->engines[] = { }

> +
>       if (copy_to_user(query_ptr, engines, size)) {
>               kfree(engines);
>               return -EFAULT;
> @@ -297,6 +300,9 @@ static int query_mem_regions(struct xe_device *xe,
>               }
>       }
>  
> +     if (xe_device_is_admin_only(xe))
> +             memset(mem_regions, 0, size);

similar problem here

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

Reply via email to