Re: [RFC 1/1] drm/xe/vf: Skip creating DRM device entries in PF admin‑only mode

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Jan 20, 2026 at 11:20:10AM +0000, Satyanarayana K V P wrote:
> When the PF is configured for admin‑only mode, it is restricted to
> management functions and should not expose a device node that would allow
> users to run workloads.
> 
> In this mode, no DRM device entry is created; however, sysfs and debugfs
> interfaces for the PF remain available at:
> 
> sysfs: /sys/devices/pci0000:00/<B:D:F>
> debugfs: /sys/kernel/debug/dri/<B:D:F>

+dri-devel since this can be useful to other drivers as well.

btw, on the subject it should be drm/xe/pf instead of vf right?!

> 
> 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>
> ---
>  drivers/gpu/drm/xe/xe_debugfs.c          | 16 ++++++++++++++++
>  drivers/gpu/drm/xe/xe_device.c           | 11 +++++++----
>  drivers/gpu/drm/xe/xe_device.h           |  6 ++++++
>  drivers/gpu/drm/xe/xe_oa.c               |  2 +-
>  drivers/gpu/drm/xe/xe_sriov_pf_helpers.h |  2 +-
>  5 files changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index 0907868b32d6..134d2e661c7c 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -458,6 +458,20 @@ static ssize_t disable_late_binding_set(struct file *f, const char __user *ubuf,
>  	return size;
>  }
>  
> +static void update_minor_dev(struct xe_device *xe, struct drm_minor **minor,
> +			     __maybe_unused struct dentry **root)
> +{
> +	struct drm_device *drm = &xe->drm;
> +
> +	if (!xe_sriov_pf_admin_only(xe))
> +		return;
> +
> +	(*minor)->dev = drm;
> +	(*minor)->kdev = drm->dev;
> +	(*minor)->debugfs_root = drm->debugfs_root;
> +	*root = (*minor)->debugfs_root;

I know that I had a few of them in my experiments, but I was wondering
if we could minimize this to just

minor->debugfs_root = dev->debugfs_root;

before the create files, instead this big hammer here....

and perhaps even minimize the use of the drm_debugfs in the cases
where we need in this admin only mode?

in other words, do we have another way to avoid a big hammer like
this?

> +}
> +
>  static const struct file_operations disable_late_binding_fops = {
>  	.owner = THIS_MODULE,
>  	.read = disable_late_binding_show,
> @@ -475,6 +489,8 @@ void xe_debugfs_register(struct xe_device *xe)
>  	u8 tile_id;
>  	u8 id;
>  
> +	update_minor_dev(xe, &minor, &root);

also, can/should we make this conditional on the admin mode only
instead of always?!

> +
>  	drm_debugfs_create_files(debugfs_list,
>  				 ARRAY_SIZE(debugfs_list),
>  				 root, minor);
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 00afc84a8683..fdd8668bd565 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -993,9 +993,11 @@ int xe_device_probe(struct xe_device *xe)
>  	if (err)
>  		return err;
>  
> -	err = drm_dev_register(&xe->drm, 0);
> -	if (err)
> -		return err;
> +	if (!xe_sriov_pf_admin_only(xe)) {
> +		err = drm_dev_register(&xe->drm, 0);
> +		if (err)
> +			return err;
> +	}

ack on this...

>  
>  	xe_display_register(xe);
>  
> @@ -1046,7 +1048,8 @@ void xe_device_remove(struct xe_device *xe)
>  
>  	xe_nvm_fini(xe);
>  
> -	drm_dev_unplug(&xe->drm);
> +	if (!xe_sriov_pf_admin_only(xe))
> +		drm_dev_unplug(&xe->drm);

ack on this too...

>  
>  	xe_bo_pci_dev_remove_all(xe);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> index 6604b89330d5..bb1e7bc8bf4f 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -11,6 +11,7 @@
>  #include "xe_device_types.h"
>  #include "xe_gt_types.h"
>  #include "xe_sriov.h"
> +#include "xe_sriov_pf_helpers.h"
>  
>  static inline struct xe_device *to_xe_device(const struct drm_device *dev)
>  {
> @@ -177,6 +178,11 @@ static inline bool xe_device_has_mert(struct xe_device *xe)
>  	return xe->info.has_mert;
>  }
>  
> +static inline struct device *xe_to_drm_kdev(struct xe_device *xe)
> +{
> +	return xe_sriov_pf_admin_only(xe) ? xe->drm.dev : xe->drm.primary->kdev;
> +}
> +
>  u32 xe_device_ccs_bytes(struct xe_device *xe, u64 size);
>  
>  void xe_device_snapshot_print(struct xe_device *xe, struct drm_printer *p);
> diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
> index abf87fe0b345..90fa30de85ce 100644
> --- a/drivers/gpu/drm/xe/xe_oa.c
> +++ b/drivers/gpu/drm/xe/xe_oa.c
> @@ -2512,7 +2512,7 @@ int xe_oa_register(struct xe_device *xe)
>  		return 0;
>  
>  	oa->metrics_kobj = kobject_create_and_add("metrics",
> -						  &xe->drm.primary->kdev->kobj);
> +						  &(xe_to_drm_kdev(xe)->kobj));

hmmm... what about skipping oa entirely if in admin mode?!
OA metrics is usage and should be working inside the VM, not in the admin mode.

>  	if (!oa->metrics_kobj)
>  		return -ENOMEM;
>  
> diff --git a/drivers/gpu/drm/xe/xe_sriov_pf_helpers.h b/drivers/gpu/drm/xe/xe_sriov_pf_helpers.h
> index 9054fdc34597..9a99fafbe77d 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_pf_helpers.h
> +++ b/drivers/gpu/drm/xe/xe_sriov_pf_helpers.h
> @@ -56,7 +56,7 @@ static inline unsigned int xe_sriov_pf_num_vfs(const struct xe_device *xe)
>   */
>  static inline bool xe_sriov_pf_admin_only(const struct xe_device *xe)
>  {
> -	return !xe->info.probe_display;
> +	return !xe->info.probe_display && xe->sriov.pf.driver_max_vfs > 0;

I'm confused on why the probe_display is in the equation...

>  }
>  
>  static inline struct mutex *xe_sriov_pf_master_mutex(struct xe_device *xe)
> -- 
> 2.43.0
> 

Thank you so much for taking care of this.
We definitely need it.

Thanks,
Rodrigo.