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 <[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]> Acked-by: Ashutosh Dixit <[email protected]> --- 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.
CI.Hooks reported compilation error for some configuration. So, need to used compilation flag here.+ 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 = ®ular_driver; struct xe_device *xe; int err;- xe_display_driver_set_hooks(&driver);+#ifdef CONFIG_PCI_IOVmaybe use if (IS_ENABLED()) to avoid complains about unused static in PCI_IOV=n ?
+ /* + * 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; +#endifnit: 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.hindex 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.cindex 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); +... hereif (!copy_to_user(query_ptr, mem_regions, size)) ret = 0; else
