* [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 = ®ular_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 = ®ular_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-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-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 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