Re: [PATCH v2 2/2] drm/xe: gate observation streams with perf_allow_cpu()

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Fri, 22 May 2026 18:33:26 -0700, John Hubbard wrote:
>
> xe OA and EU-stall paths open-code a partial copy of the system-wide
> perf CPU-event permission check:
>
>     if (xe_observation_paranoid && !perfmon_capable())
>             return -EACCES;
>
> This open-coded check skips two things perf_allow_cpu() handles: the
> graduated kernel.perf_event_paranoid policy that an administrator
> may have tuned, and the security_perf_event_open() LSM hook.
>
> Introduce xe_observation_paranoid_check() to wrap perf_allow_cpu(),
> and convert the open-coded sites in xe_oa.c and xe_eu_stall.c. The
> dev.xe.observation_paranoid sysctl still acts as an escape hatch
> when cleared.
>
> xe observation now consults kernel.perf_event_paranoid and the LSM
> perf hook on every open. Sites that have already configured an LSM
> perf policy or tuned the paranoid sysctl will see those settings
> extend to xe.
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>

LGTM:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_eu_stall.c    |  5 +++--
>  drivers/gpu/drm/xe/xe_oa.c          | 25 +++++++++++++---------
>  drivers/gpu/drm/xe/xe_observation.c | 32 ++++++++++++++++++++++++-----
>  drivers/gpu/drm/xe/xe_observation.h |  3 +--
>  4 files changed, 46 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_eu_stall.c b/drivers/gpu/drm/xe/xe_eu_stall.c
> index dddcdd0bb7a3..ede8e3c98b2b 100644
> --- a/drivers/gpu/drm/xe/xe_eu_stall.c
> +++ b/drivers/gpu/drm/xe/xe_eu_stall.c
> @@ -963,9 +963,10 @@ int xe_eu_stall_stream_open(struct drm_device *dev, u64 data, struct drm_file *f
>		return -ENODEV;
>	}
>
> -	if (xe_observation_paranoid && !perfmon_capable()) {
> +	ret = xe_observation_paranoid_check();
> +	if (ret) {
>		drm_dbg(&xe->drm,  "Insufficient privileges for EU stall monitoring\n");
> -		return -EACCES;
> +		return ret;
>	}
>
>	/* Initialize and set default values */
> diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
> index d908f4e03906..f3dcff66b336 100644
> --- a/drivers/gpu/drm/xe/xe_oa.c
> +++ b/drivers/gpu/drm/xe/xe_oa.c
> @@ -1676,9 +1676,10 @@ static int xe_oa_mmap(struct file *file, struct vm_area_struct *vma)
>	unsigned long start = vma->vm_start;
>	int i, ret;
>
> -	if (xe_observation_paranoid && !perfmon_capable()) {
> +	ret = xe_observation_paranoid_check();
> +	if (ret) {
>		drm_dbg(&stream->oa->xe->drm, "Insufficient privilege to map OA buffer\n");
> -		return -EACCES;
> +		return ret;
>	}
>
>	/* Can mmap the entire OA buffer or nothing (no partial OA buffer mmaps) */
> @@ -2054,10 +2055,12 @@ int xe_oa_stream_open_ioctl(struct drm_device *dev, u64 data, struct drm_file *f
>		privileged_op = true;
>	}
>
> -	if (privileged_op && xe_observation_paranoid && !perfmon_capable()) {
> -		drm_dbg(&oa->xe->drm, "Insufficient privileges to open xe OA stream\n");
> -		ret = -EACCES;
> -		goto err_exec_q;
> +	if (privileged_op) {
> +		ret = xe_observation_paranoid_check();
> +		if (ret) {
> +			drm_dbg(&oa->xe->drm, "Insufficient privileges to open xe OA stream\n");
> +			goto err_exec_q;
> +		}
>	}
>
>	if (!param.exec_q && !param.sample) {
> @@ -2336,9 +2339,10 @@ int xe_oa_add_config_ioctl(struct drm_device *dev, u64 data, struct drm_file *fi
>		return -ENODEV;
>	}
>
> -	if (xe_observation_paranoid && !perfmon_capable()) {
> +	err = xe_observation_paranoid_check();
> +	if (err) {
>		drm_dbg(&oa->xe->drm, "Insufficient privileges to add xe OA config\n");
> -		return -EACCES;
> +		return err;
>	}
>
>	err = copy_from_user(&param, u64_to_user_ptr(data), sizeof(param));
> @@ -2438,9 +2442,10 @@ int xe_oa_remove_config_ioctl(struct drm_device *dev, u64 data, struct drm_file
>		return -ENODEV;
>	}
>
> -	if (xe_observation_paranoid && !perfmon_capable()) {
> +	ret = xe_observation_paranoid_check();
> +	if (ret) {
>		drm_dbg(&oa->xe->drm, "Insufficient privileges to remove xe OA config\n");
> -		return -EACCES;
> +		return ret;
>	}
>
>	ret = get_user(arg, ptr);
> diff --git a/drivers/gpu/drm/xe/xe_observation.c b/drivers/gpu/drm/xe/xe_observation.c
> index e3f9b546207e..39e05b9131a7 100644
> --- a/drivers/gpu/drm/xe/xe_observation.c
> +++ b/drivers/gpu/drm/xe/xe_observation.c
> @@ -4,6 +4,7 @@
>   */
>
>  #include <linux/errno.h>
> +#include <linux/perf_event.h>
>  #include <linux/sysctl.h>
>
>  #include <uapi/drm/xe_drm.h>
> @@ -12,9 +13,28 @@
>  #include "xe_oa.h"
>  #include "xe_observation.h"
>
> -u32 xe_observation_paranoid = true;
> +static u32 xe_observation_paranoid = true;
>  static struct ctl_table_header *sysctl_header;
>
> +/**
> + * xe_observation_paranoid_check - Gate access to xe observation streams.
> + *
> + * When the xe-specific observation_paranoid sysctl is enabled (the
> + * default), defer to perf_allow_cpu() so that access is governed by the
> + * same policy as system-wide perf CPU events: kernel.perf_event_paranoid
> + * plus the security_perf_event_open() LSM hook. When the sysctl has been
> + * cleared by a privileged user, observation is open to all callers.
> + *
> + * Return: 0 if access is permitted, a negative errno otherwise.
> + */
> +int xe_observation_paranoid_check(void)
> +{
> +	if (!xe_observation_paranoid)
> +		return 0;
> +
> +	return perf_allow_cpu();
> +}
> +
>  static int xe_oa_ioctl(struct drm_device *dev, struct drm_xe_observation_param *arg,
>		       struct drm_file *file)
>  {
> @@ -83,11 +103,13 @@ static const struct ctl_table observation_ctl_table[] = {
>  };
>
>  /**
> - * xe_observation_sysctl_register - Register xe_observation_paranoid sysctl
> + * xe_observation_sysctl_register - Register the observation_paranoid sysctl
>   *
> - * Normally only superuser/root can access observation stream
> - * data. However, superuser can set xe_observation_paranoid sysctl to 0 to
> - * allow non-privileged users to also access observation data.
> + * When dev.xe.observation_paranoid is set (the default), access to
> + * observation streams follows the system-wide perf_allow_cpu() policy:
> + * kernel.perf_event_paranoid plus the security_perf_event_open() LSM
> + * hook. A privileged user can clear the sysctl to bypass that gate and
> + * allow unprivileged access to observation data.
>   *
>   * Return: always returns 0
>   */
> diff --git a/drivers/gpu/drm/xe/xe_observation.h b/drivers/gpu/drm/xe/xe_observation.h
> index 17816998e966..73a03e03c96a 100644
> --- a/drivers/gpu/drm/xe/xe_observation.h
> +++ b/drivers/gpu/drm/xe/xe_observation.h
> @@ -11,8 +11,7 @@
>  struct drm_device;
>  struct drm_file;
>
> -extern u32 xe_observation_paranoid;
> -
> +int xe_observation_paranoid_check(void);
>  int xe_observation_ioctl(struct drm_device *dev, void *data, struct drm_file *file);
>  int xe_observation_sysctl_register(void);
>  void xe_observation_sysctl_unregister(void);
> --
> 2.54.0
>