Re: [PATCH i-g-t] lib/intel_pat: Cache PAT config for unprivileged processes

["Sharma, Nishit" <nishit.sharma@intel.com>]

On 3/1/2026 9:05 PM, himanshu.girotra@intel.com wrote:
> From: Himanshu Girotra <himanshu.girotra@intel.com>
>
> Commit 4e59b8e7779b ("lib/intel_pat: use kernel debugfs as
> authoritative PAT source for Xe") changed the Xe PAT lookup to
> read from debugfs, which requires root access. Some xe_oa subtests
> fork a child process that drops root privileges before performing
> GPU operations that internally need the PAT write-back index.
> After dropping root, the debugfs read fails.
>
> Fix this by caching the PAT configuration on first access. A new
> precache helper lets tests populate the cache while still running
> as root, so later lookups succeed without debugfs.
>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Xin Wang <x.wang@intel.com>
> Signed-off-by: Himanshu Girotra <himanshu.girotra@intel.com>
> ---
>   lib/intel_pat.c     | 39 +++++++++++++++++++++++++++++++++++++++
>   lib/intel_pat.h     |  2 ++
>   tests/intel/xe_oa.c | 10 ++++++++++
>   3 files changed, 51 insertions(+)
>
> diff --git a/lib/intel_pat.c b/lib/intel_pat.c
> index 8660a2515..16dfc0217 100644
> --- a/lib/intel_pat.c
> +++ b/lib/intel_pat.c
> @@ -7,6 +7,16 @@
>   #include "igt.h"
>   #include "intel_pat.h"
>   
> +/*
> + * Global PAT cache.  PAT configuration is a hardware/driver property that
> + * does not change over the lifetime of a device, so we cache the result of
> + * the first successful query.  This allows unprivileged processes (e.g.
> + * after igt_drop_root()) to use the cached values without needing debugfs
> + * access.
> + */
> +static struct intel_pat_cache pat_cache;
> +static bool pat_cached;
Instead declaring global structures declare own structure and use above 
as their member
> +
>   /**
>    * xe_get_pat_sw_config - Helper to read PAT (Page Attribute Table) software configuration
>    * from debugfs
> @@ -98,6 +108,11 @@ static void intel_get_pat_idx(int fd, struct intel_pat_cache *pat)
>   {
>   	uint16_t dev_id;
>   
> +	if (pat_cached) {
> +		*pat = pat_cache;
> +		return;
> +	}
> +
>   	/*
>   	 * For Xe driver, query the kernel's PAT software configuration
>   	 * via debugfs. The kernel is the authoritative source for PAT
> @@ -110,6 +125,8 @@ static void intel_get_pat_idx(int fd, struct intel_pat_cache *pat)
>   		igt_assert_f(parsed > 0,
>   			     "Failed to get PAT sw_config from debugfs (parsed=%d)\n",
>   			     parsed);
> +		pat_cache = *pat;
> +		pat_cached = true;
>   		return;
>   	}
>   
> @@ -134,6 +151,28 @@ static void intel_get_pat_idx(int fd, struct intel_pat_cache *pat)
>   	} else {
>   		igt_critical("Platform is missing PAT settings for uc/wt/wb\n");
>   	}
> +
> +	pat_cache = *pat;
> +	pat_cached = true;
> +}
> +
> +/**
> + * intel_pat_precache:
> + * @fd: DRM file descriptor
> + *
> + * Pre-populate the global PAT cache while the process still has sufficient
> + * privileges (e.g. root) to read debugfs.  After calling this function,
> + * subsequent intel_get_pat_idx_*() calls will return cached values even if
> + * the process has dropped privileges via igt_drop_root().
> + *
> + * Tests that fork a child and drop root before using bufops/intel_bb
> + * should call this in their fixture or before the fork.
> + */
> +void intel_pat_precache(int fd)
> +{
> +	struct intel_pat_cache pat = {};
> +
> +	intel_get_pat_idx(fd, &pat);
>   }
>   
>   uint8_t intel_get_max_pat_index(int fd)
> diff --git a/lib/intel_pat.h b/lib/intel_pat.h
> index e9ade2e2e..c204eeac7 100644
> --- a/lib/intel_pat.h
> +++ b/lib/intel_pat.h
> @@ -28,6 +28,8 @@ struct intel_pat_cache {
>   	uint32_t pat_ats;
>   };
>   
> +void intel_pat_precache(int fd);
> +
>   uint8_t intel_get_max_pat_index(int fd);
>   
>   uint8_t intel_get_pat_idx_uc(int fd);
> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> index 927f3f4f2..37449c87c 100644
> --- a/tests/intel/xe_oa.c
> +++ b/tests/intel/xe_oa.c
> @@ -27,6 +27,7 @@
>   #include "xe/xe_ioctl.h"
>   #include "xe/xe_query.h"
>   #include "xe/xe_oa.h"
> +#include "intel_pat.h"
>   
>   /**
>    * TEST: perf
> @@ -5094,6 +5095,15 @@ int igt_main_args("b:t", long_options, help_str, opt_handler, NULL)
>   		write_u64_file("/proc/sys/dev/xe/observation_paranoid", 1);
>   
>   		render_copy = igt_get_render_copyfunc(drm_fd);
> +
> +		/*
> +		 * Pre-cache PAT configuration while we still have root
> +		 * privileges.  Several subtests fork a child that drops
> +		 * root before using bufops/intel_bb, which internally
> +		 * needs the PAT WB index.  Without this, the child would
> +		 * fail to read debugfs after dropping privileges.
> +		 */
> +		intel_pat_precache(drm_fd);
>   	}
>   
>   	igt_subtest("non-system-wide-paranoid")

Rest LGTM

Acked-by: Nishit Sharma <nishit.sharma@intel.com>