dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RFC v6 1/1] drm/xe/pf: Restrict device query responses in admin-only PF mode
       [not found] <20260316064100.2542412-3-satyanarayana.k.v.p@intel.com>
@ 2026-03-16  6:41 ` Satyanarayana K V P
  2026-03-23 22:03   ` Rodrigo Vivi
  2026-03-24 21:17   ` Dixit, Ashutosh
  0 siblings, 2 replies; 8+ messages in thread
From: Satyanarayana K V P @ 2026-03-16  6:41 UTC (permalink / raw)
  To: intel-xe
  Cc: Satyanarayana K V P, Michal Wajdeczko, Rodrigo Vivi,
	Piotr Piórkowski, Matthew Brost, Thomas Hellström,
	Michał Winiarski, Dunajski Bartosz, dri-devel

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 0 | sudo tee /sys/bus/pci/drivers_autoprobe
sudo modprobe xe
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: dri-devel@lists.freedesktop.org

---
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_query.c  |  6 ++++
 3 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index e77a3a3db73d..7decc6510681 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,10 @@ 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),
+};
+
 static long xe_drm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	struct drm_file *file_priv = file->private_data;
@@ -387,7 +392,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 = {
 	/* Don't use MTRRs here; the Xserver or userspace app should
 	 * deal with them for Intel hardware.
 	 */
@@ -415,6 +420,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,
+
+	.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)
+{
+	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 +478,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 +755,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)) {
+		drm_err(&xe->drm, "Admin-only PF mode is enabled in non 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 c4d267002661..4695761585e6 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 4852fdcb4b95..a42b6606a55b 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);
+
 	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);
+
 	if (!copy_to_user(query_ptr, mem_regions, size))
 		ret = 0;
 	else
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC v6 1/1] drm/xe/pf: Restrict device query responses in admin-only PF mode
  2026-03-16  6:41 ` [RFC v6 1/1] drm/xe/pf: Restrict device query responses in admin-only PF mode Satyanarayana K V P
@ 2026-03-23 22:03   ` Rodrigo Vivi
  2026-03-24 21:17   ` Dixit, Ashutosh
  1 sibling, 0 replies; 8+ messages in thread
From: Rodrigo Vivi @ 2026-03-23 22:03 UTC (permalink / raw)
  To: Satyanarayana K V P
  Cc: intel-xe, Michal Wajdeczko, Piotr Piórkowski, Matthew Brost,
	Thomas Hellström, Michał Winiarski, Dunajski Bartosz,
	dri-devel

On Mon, Mar 16, 2026 at 06:41:02AM +0000, 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 0 | sudo tee /sys/bus/pci/drivers_autoprobe
> sudo modprobe xe

instead of playing with autoprobe/modprobe we can assume it is already
probed and then we can simply put the
echo $BDF | 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: dri-devel@lists.freedesktop.org
> 
> ---
> 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).

Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

If Mesa gets ready with this and Compute folks also agree I believe we
should already take this patch in.

> 
> 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_query.c  |  6 ++++
>  3 files changed, 63 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index e77a3a3db73d..7decc6510681 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,10 @@ 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),
> +};
> +
>  static long xe_drm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  {
>  	struct drm_file *file_priv = file->private_data;
> @@ -387,7 +392,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 = {
>  	/* Don't use MTRRs here; the Xserver or userspace app should
>  	 * deal with them for Intel hardware.
>  	 */
> @@ -415,6 +420,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,
> +
> +	.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)
> +{
> +	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 +478,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 +755,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)) {
> +		drm_err(&xe->drm, "Admin-only PF mode is enabled in non 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 c4d267002661..4695761585e6 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 4852fdcb4b95..a42b6606a55b 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);
> +
>  	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);
> +
>  	if (!copy_to_user(query_ptr, mem_regions, size))
>  		ret = 0;
>  	else
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v6 1/1] drm/xe/pf: Restrict device query responses in admin-only PF mode
  2026-03-16  6:41 ` [RFC v6 1/1] drm/xe/pf: Restrict device query responses in admin-only PF mode Satyanarayana K V P
  2026-03-23 22:03   ` Rodrigo Vivi
@ 2026-03-24 21:17   ` Dixit, Ashutosh
  2026-03-25  5:21     ` K V P, Satyanarayana
  2026-03-25  8:38     ` Michal Wajdeczko
  1 sibling, 2 replies; 8+ messages in thread
From: Dixit, Ashutosh @ 2026-03-24 21:17 UTC (permalink / raw)
  To: Satyanarayana K V P
  Cc: intel-xe, Michal Wajdeczko, Rodrigo Vivi, Piotr Piórkowski,
	Matthew Brost, Thomas Hellström, Michał Winiarski,
	Dunajski Bartosz, dri-devel

On Sun, 15 Mar 2026 23:41:02 -0700, Satyanarayana K V P wrote:
>
> ---
> 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).

Can someone explain the reason to move away from the approach in v5? Afais
v6 has issues of this sort:

* query_engines will return 0 engines but query_hwconfig will return > 0
  engines
* query_engines will return 0 engines but query_oa_units will list out the
  engines
* query_oa_units will return valid oa support but observation ioctl will
  fail

v5 seems to have avoided contradictions of this sort. Or this doesn't
matter? Thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v6 1/1] drm/xe/pf: Restrict device query responses in admin-only PF mode
  2026-03-24 21:17   ` Dixit, Ashutosh
@ 2026-03-25  5:21     ` K V P, Satyanarayana
  2026-03-25 13:11       ` Rodrigo Vivi
  2026-03-25  8:38     ` Michal Wajdeczko
  1 sibling, 1 reply; 8+ messages in thread
From: K V P, Satyanarayana @ 2026-03-25  5:21 UTC (permalink / raw)
  To: Dixit, Ashutosh
  Cc: intel-xe, Michal Wajdeczko, Rodrigo Vivi, Piotr Piórkowski,
	Matthew Brost, Thomas Hellström, Michał Winiarski,
	Dunajski Bartosz, dri-devel

[-- Attachment #1: Type: text/plain, Size: 2250 bytes --]


On 25-Mar-26 2:47 AM, Dixit, Ashutosh wrote:
> On Sun, 15 Mar 2026 23:41:02 -0700, Satyanarayana K V P wrote:
>> ---
>> 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).
> Can someone explain the reason to move away from the approach in v5? Afais
> v6 has issues of this sort:
The V6 is due to comment from Rodrigo from V5 "

Looking to this patch I got myself wondering if query.size == 0 is
long-term proof...   I mean, if we need to get some OA/RAS/telemetry
kind of API that end up needing some kind of query information....

Michals, thoughts on this?

I know, I know, this solution is indeed much better than my
proposal of no ioctl exposed. But I mean, since we are taking
this path and allowing some ioctl. Shouldn't we prepare at least
the query for that and then only limiting the number of engines
and memory regions to zero?"


and comment from you (Dixit, Ashutosh)as well.


"Also, there were some questions about supporting OA in the
admin-only-pf. Here are some high level points about this:

* OA/EUSTALL is not supported in VF's in current products, this might
   change in the future
* OA/EUSTALL in admin-only-pf can be used to profile global counts across
   VF's
* OA/EUSTALL themselves don't allow workload submission etc. They only
   allow reading profiling data from HW
* OA/EUSTALL in admin-only-pf will need the observation ioctl to be
   supported. Also, some other query ioctls such as hw configuration,
   topology, frequency will be needed to make sense of OA/EUSTALL data."

>
> * query_engines will return 0 engines but query_hwconfig will return > 0
>    engines
> * query_engines will return 0 engines but query_oa_units will list out the
>    engines
> * query_oa_units will return valid oa support but observation ioctl will
>    fail
>
> v5 seems to have avoided contradictions of this sort. Or this doesn't
> matter? Thanks.
Since we are not exposing any other IOCTLs, even if query_hwconfig  and

query_oa_units find number of engines > 0, the UMD can't submit any WL. So, should be 
fine right.

[-- Attachment #2: Type: text/html, Size: 4671 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v6 1/1] drm/xe/pf: Restrict device query responses in admin-only PF mode
  2026-03-24 21:17   ` Dixit, Ashutosh
  2026-03-25  5:21     ` K V P, Satyanarayana
@ 2026-03-25  8:38     ` Michal Wajdeczko
  2026-03-27  5:34       ` Dixit, Ashutosh
  1 sibling, 1 reply; 8+ messages in thread
From: Michal Wajdeczko @ 2026-03-25  8:38 UTC (permalink / raw)
  To: Dixit, Ashutosh, Satyanarayana K V P
  Cc: intel-xe, Rodrigo Vivi, Piotr Piórkowski, Matthew Brost,
	Thomas Hellström, Michał Winiarski, Dunajski Bartosz,
	dri-devel



On 3/24/2026 10:17 PM, Dixit, Ashutosh wrote:
> On Sun, 15 Mar 2026 23:41:02 -0700, Satyanarayana K V P wrote:
>>
>> ---
>> 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).
> 
> Can someone explain the reason to move away from the approach in v5? Afais
> v6 has issues of this sort:
> 
> * query_engines will return 0 engines but query_hwconfig will return > 0
>   engines

but those are separate queries on purpose, right?
and I guess that even today there could be a mismatch between these numbers:

* query_engines = engines available for use by the user software
* query_hwconfig.engines = report engines present on the hardware

> * query_engines will return 0 engines but query_oa_units will list out the
>   engines

and that IMO should be considered as a desired outcome, as I guess (again)
that this will allow us to do some OA reporting, even if PF alone is not
submitting any workloads and we want to monitor how VFs are doing

> * query_oa_units will return valid oa support but observation ioctl will
>   fail

my initial idea [1] was to expose observation ioctl as well, maybe we need
to add it back?

[1] https://patchwork.freedesktop.org/patch/706445/?series=160349&rev=2#comment_1299475

> 
> v5 seems to have avoided contradictions of this sort. Or this doesn't
> matter? Thanks.

but since I'm not using any of those ioctls on daily basis, I might be wrong


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v6 1/1] drm/xe/pf: Restrict device query responses in admin-only PF mode
  2026-03-25  5:21     ` K V P, Satyanarayana
@ 2026-03-25 13:11       ` Rodrigo Vivi
  0 siblings, 0 replies; 8+ messages in thread
From: Rodrigo Vivi @ 2026-03-25 13:11 UTC (permalink / raw)
  To: K V P, Satyanarayana
  Cc: Dixit, Ashutosh, intel-xe, Michal Wajdeczko,
	Piotr Piórkowski, Matthew Brost, Thomas Hellström,
	Michał Winiarski, Dunajski Bartosz, dri-devel

On Wed, Mar 25, 2026 at 10:51:18AM +0530, K V P, Satyanarayana wrote:
>    On 25-Mar-26 2:47 AM, Dixit, Ashutosh wrote:                                 
>                                                                                 
>    On Sun, 15 Mar 2026 23:41:02 -0700, Satyanarayana K V P wrote:               
>                                                                                 
>    >---                                                                         
>    >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).                                                         
>                                                                                 
>    Can someone explain the reason to move away from the approach in v5? Afais   
>    v6 has issues of this sort:                                                  
>                                                                                 
>    The V6 is due to comment from Rodrigo from V5 "                              
>                                                                                 
>  Looking to this patch I got myself wondering if query.size == 0 is             
>  long-term proof...   I mean, if we need to get some OA/RAS/telemetry           
>  kind of API that end up needing some kind of query information....             

Exactly my concern with the previous approach as well. It is not future proof.
If we later decide that any query is needed in the admin only mode we wouldn't
be able to reuse this generic query uapi and would be forced to create a new
entry.

>                                                                                 
>  Michals, thoughts on this?                                                     
>                                                                                 
>  I know, I know, this solution is indeed much better than my                    
>  proposal of no ioctl exposed. But I mean, since we are taking                  
>  this path and allowing some ioctl. Shouldn't we prepare at least               
>  the query for that and then only limiting the number of engines                
>  and memory regions to zero?"                                                   
>                                                                                 
>  and comment from you (Dixit, Ashutosh)as well.                                 
>                                                                                 
>  "Also, there were some questions about supporting OA in the                    
>  admin-only-pf. Here are some high level points about this:                     
>                                                                                 
>  * OA/EUSTALL is not supported in VF's in current products, this might          
>    change in the future                                                         
>  * OA/EUSTALL in admin-only-pf can be used to profile global counts across      
>    VF's                                                                         
>  * OA/EUSTALL themselves don't allow workload submission etc. They only         
>    allow reading profiling data from HW                                         
>  * OA/EUSTALL in admin-only-pf will need the observation ioctl to be            
>    supported. Also, some other query ioctls such as hw configuration,           
>    topology, frequency will be needed to make sense of OA/EUSTALL data."        
>                                                                                 
>                                                                                 
>    * query_engines will return 0 engines but query_hwconfig will return > 0     
>      engines                                                                    
>    * query_engines will return 0 engines but query_oa_units will list out the   
>      engines                                                                    
>    * query_oa_units will return valid oa support but observation ioctl will     
>      fail                                                                       
>                                                                                 
>    v5 seems to have avoided contradictions of this sort. Or this doesn't        
>    matter? Thanks.                                                              
>                                                                                 
>    Since we are not exposing any other IOCTLs, even if query_hwconfig  and      
>                                                                                 
>  query_oa_units  find number of engines > 0, the UMD can't submit any WL. So,   
>  should be fine right.                                                          

I agree. What we really want with the admin only mode right now is to prevent
job scheduling and memory allocation. So it should be fine.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v6 1/1] drm/xe/pf: Restrict device query responses in admin-only PF mode
  2026-03-25  8:38     ` Michal Wajdeczko
@ 2026-03-27  5:34       ` Dixit, Ashutosh
  2026-03-27 13:26         ` Rodrigo Vivi
  0 siblings, 1 reply; 8+ messages in thread
From: Dixit, Ashutosh @ 2026-03-27  5:34 UTC (permalink / raw)
  To: Michal Wajdeczko
  Cc: Satyanarayana K V P, intel-xe, Rodrigo Vivi,
	Piotr Piórkowski, Matthew Brost, Thomas Hellström,
	Michał Winiarski, Dunajski Bartosz, dri-devel,
	Robert Krzemien

On Wed, 25 Mar 2026 01:38:51 -0700, Michal Wajdeczko wrote:
>

Hi Michal,

> On 3/24/2026 10:17 PM, Dixit, Ashutosh wrote:
> > On Sun, 15 Mar 2026 23:41:02 -0700, Satyanarayana K V P wrote:
> >>
> >> ---
> >> 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).
> >
> > Can someone explain the reason to move away from the approach in v5? Afais
> > v6 has issues of this sort:
> >
> > * query_engines will return 0 engines but query_hwconfig will return > 0
> >   engines
>
> but those are separate queries on purpose, right?
> and I guess that even today there could be a mismatch between these numbers:
>
> * query_engines = engines available for use by the user software
> * query_hwconfig.engines = report engines present on the hardware

OK, agreed.

>
> > * query_engines will return 0 engines but query_oa_units will list out the
> >   engines
>
> and that IMO should be considered as a desired outcome, as I guess (again)
> that this will allow us to do some OA reporting, even if PF alone is not
> submitting any workloads and we want to monitor how VFs are doing
>
> > * query_oa_units will return valid oa support but observation ioctl will
> >   fail
>
> my initial idea [1] was to expose observation ioctl as well, maybe we need
> to add it back?
>
> [1]
> https://patchwork.freedesktop.org/patch/706445/?series=160349&rev=2#comment_1299475

OK, maybe I am thinking, we can expose the observation ioctl, though there
are both pros and cons to doing this.

Pros: get some OA reporting out of the box. Though the tools etc. will
      likely not work out of the box because of other missing
      queries/ioctl's.

Cons: Not sure if it is ok to "snoop" on VF information out of the
      box. Customers might insist this is not ok and insist on the
      observation ioctl be removed. Also, on platforms on which OA is
      supported in VF, there might be a conflict between OA in PF vs VF.

Also, even if we add the observation ioctl, only the base OA feature will
work. But there are other OA features which require other ioctl's (say
exec) which will not work in the admin-only-pf mode.

The other option is not add the OA ioctl. And insist that to get regular OA
reporting/tools to work, the device must be unbound and rebound in the
normal (non-admin-only) mode.

So we could go with either of these approaches. I am ok either way. Maybe
just add the observation ioctl for now and revisit after feedback from
customers/UMD's?

Thanks.
--
Ashutosh



>
> >
> > v5 seems to have avoided contradictions of this sort. Or this doesn't
> > matter? Thanks.
>
> but since I'm not using any of those ioctls on daily basis, I might be wrong
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v6 1/1] drm/xe/pf: Restrict device query responses in admin-only PF mode
  2026-03-27  5:34       ` Dixit, Ashutosh
@ 2026-03-27 13:26         ` Rodrigo Vivi
  0 siblings, 0 replies; 8+ messages in thread
From: Rodrigo Vivi @ 2026-03-27 13:26 UTC (permalink / raw)
  To: Dixit, Ashutosh
  Cc: Michal Wajdeczko, Satyanarayana K V P, intel-xe,
	Piotr Piórkowski, Matthew Brost, Thomas Hellström,
	Michał Winiarski, Dunajski Bartosz, dri-devel,
	Robert Krzemien

On Thu, Mar 26, 2026 at 10:34:20PM -0700, Dixit, Ashutosh wrote:
> On Wed, 25 Mar 2026 01:38:51 -0700, Michal Wajdeczko wrote:
> >
> 
> Hi Michal,
> 
> > On 3/24/2026 10:17 PM, Dixit, Ashutosh wrote:
> > > On Sun, 15 Mar 2026 23:41:02 -0700, Satyanarayana K V P wrote:
> > >>
> > >> ---
> > >> 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).
> > >
> > > Can someone explain the reason to move away from the approach in v5? Afais
> > > v6 has issues of this sort:
> > >
> > > * query_engines will return 0 engines but query_hwconfig will return > 0
> > >   engines
> >
> > but those are separate queries on purpose, right?
> > and I guess that even today there could be a mismatch between these numbers:
> >
> > * query_engines = engines available for use by the user software
> > * query_hwconfig.engines = report engines present on the hardware
> 
> OK, agreed.
> 
> >
> > > * query_engines will return 0 engines but query_oa_units will list out the
> > >   engines
> >
> > and that IMO should be considered as a desired outcome, as I guess (again)
> > that this will allow us to do some OA reporting, even if PF alone is not
> > submitting any workloads and we want to monitor how VFs are doing
> >
> > > * query_oa_units will return valid oa support but observation ioctl will
> > >   fail
> >
> > my initial idea [1] was to expose observation ioctl as well, maybe we need
> > to add it back?
> >
> > [1]
> > https://patchwork.freedesktop.org/patch/706445/?series=160349&rev=2#comment_1299475
> 
> OK, maybe I am thinking, we can expose the observation ioctl, though there
> are both pros and cons to doing this.
> 
> Pros: get some OA reporting out of the box. Though the tools etc. will
>       likely not work out of the box because of other missing
>       queries/ioctl's.
> 
> Cons: Not sure if it is ok to "snoop" on VF information out of the
>       box. Customers might insist this is not ok and insist on the
>       observation ioctl be removed. Also, on platforms on which OA is
>       supported in VF, there might be a conflict between OA in PF vs VF.
> 
> Also, even if we add the observation ioctl, only the base OA feature will
> work. But there are other OA features which require other ioctl's (say
> exec) which will not work in the admin-only-pf mode.
> 
> The other option is not add the OA ioctl. And insist that to get regular OA
> reporting/tools to work, the device must be unbound and rebound in the
> normal (non-admin-only) mode.
> 
> So we could go with either of these approaches. I am ok either way. Maybe
> just add the observation ioctl for now and revisit after feedback from
> customers/UMD's?

yes, that's probably the way to go since we still only have the oa in the PF.

In the future we might add a knob to steer where the oa is-or-not available.

> 
> Thanks.
> --
> Ashutosh
> 
> 
> 
> >
> > >
> > > v5 seems to have avoided contradictions of this sort. Or this doesn't
> > > matter? Thanks.
> >
> > but since I'm not using any of those ioctls on daily basis, I might be wrong
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-03-27 13:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260316064100.2542412-3-satyanarayana.k.v.p@intel.com>
2026-03-16  6:41 ` [RFC v6 1/1] drm/xe/pf: Restrict device query responses in admin-only PF mode Satyanarayana K V P
2026-03-23 22:03   ` Rodrigo Vivi
2026-03-24 21:17   ` Dixit, Ashutosh
2026-03-25  5:21     ` K V P, Satyanarayana
2026-03-25 13:11       ` Rodrigo Vivi
2026-03-25  8:38     ` Michal Wajdeczko
2026-03-27  5:34       ` Dixit, Ashutosh
2026-03-27 13:26         ` Rodrigo Vivi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox