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

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

On Thu, 05 Mar 2026 10:54:31 -0800, <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 in struct xe_device at
> xe_device_get() time. Since xe_device_get() is called while still
> root, the cache is available to forked children even after
> igt_drop_root(). Tests that open a new fd after forking must call
> xe_device_get() explicitly before dropping root to populate the
> cache for that fd.
>
> v2: Cache PAT in struct xe_device instead of a global static;
>     drop the intel_pat_precache() helper — xe_device_get() already
>     populates the cache, so call it directly in xe_oa before
>     igt_drop_root() (Ashutosh Dixit)
>
> v3: Use pat_cache pointer, NULL indicates unpopulated cache;
>     remove redundant xe_oa.c change (Ashutosh Dixit)
>
>     Add FIXME for multi-GPU (Zbigniew Kempczyński)
>
> v4: nit fixes and use forward declaration in xe_query.h (Ashutosh Dixit)

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

>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Xin Wang <x.wang@intel.com>
> Cc: Nishit Sharma <nishit.sharma@intel.com>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
>
> Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Himanshu Girotra <himanshu.girotra@intel.com>
> ---
>  lib/intel_pat.c   | 17 +++++++++--------
>  lib/xe/xe_query.c | 20 ++++++++++++++++++++
>  lib/xe/xe_query.h |  5 +++++
>  3 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/lib/intel_pat.c b/lib/intel_pat.c
> index 8660a2515..9feeeb39d 100644
> --- a/lib/intel_pat.c
> +++ b/lib/intel_pat.c
> @@ -6,6 +6,7 @@
>  #include <fcntl.h>
>  #include "igt.h"
>  #include "intel_pat.h"
> +#include "xe/xe_query.h"
>
>  /**
>   * xe_get_pat_sw_config - Helper to read PAT (Page Attribute Table) software configuration
> @@ -99,17 +100,17 @@ static void intel_get_pat_idx(int fd, struct intel_pat_cache *pat)
>	uint16_t dev_id;
>
>	/*
> -	 * For Xe driver, query the kernel's PAT software configuration
> -	 * via debugfs. The kernel is the authoritative source for PAT
> -	 * indices, accounting for platform-specific workarounds
> -	 * (e.g. Wa_16023588340) at runtime.
> +	 * For Xe, use the PAT cache stored in struct xe_device.
> +	 * xe_device_get() populates the cache while still root; forked
> +	 * children that inherit the xe_device can use it post-drop_root().
>	 */
>	if (is_xe_device(fd)) {
> -		int32_t parsed = xe_get_pat_sw_config(fd, pat);
> +		struct xe_device *xe_dev = xe_device_get(fd);
>
> -		igt_assert_f(parsed > 0,
> -			     "Failed to get PAT sw_config from debugfs (parsed=%d)\n",
> -			     parsed);
> +		igt_assert_f(xe_dev->pat_cache,
> +			     "PAT sw_config not available -- "
> +			     "debugfs not accessible (missing root or not mounted?)\n");
> +		*pat = *xe_dev->pat_cache;
>		return;
>	}
>
> diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
> index 981d76948..3afca353e 100644
> --- a/lib/xe/xe_query.c
> +++ b/lib/xe/xe_query.c
> @@ -21,6 +21,7 @@
>  #include "drmtest.h"
>  #include "ioctl_wrappers.h"
>  #include "igt_map.h"
> +#include "intel_pat.h"
>
>  #include "xe_query.h"
>  #include "xe_ioctl.h"
> @@ -235,6 +236,7 @@ static void xe_device_free(struct xe_device *xe_dev)
>	free(xe_dev->mem_regions);
>	free(xe_dev->vram_size);
>	free(xe_dev->eu_stall);
> +	free(xe_dev->pat_cache);
>	free(xe_dev);
>  }
>
> @@ -299,6 +301,24 @@ struct xe_device *xe_device_get(int fd)
>	xe_dev->default_alignment = __mem_default_alignment(xe_dev->mem_regions);
>	xe_dev->has_vram = __mem_has_vram(xe_dev->mem_regions);
>
> +	/*
> +	 * Populate the PAT cache while we still have sufficient privileges
> +	 * to read debugfs.  Forked children that inherit this xe_device
> +	 * (via fork()) will be able to use the cached values even after
> +	 * dropping root with igt_drop_root(). pat_cache is left NULL if
> +	 * debugfs is not accessible.
> +	 *
> +	 * FIXME: the cache is keyed by fd; for multi-GPU support this
> +	 * should be extended to cache PAT entries by platform version/
> +	 * revision instead.
> +	 */
> +	xe_dev->pat_cache = calloc(1, sizeof(*xe_dev->pat_cache));
> +	igt_assert(xe_dev->pat_cache);
> +	if (xe_get_pat_sw_config(xe_dev->fd, xe_dev->pat_cache) <= 0) {
> +		free(xe_dev->pat_cache);
> +		xe_dev->pat_cache = NULL;
> +	}
> +
>	/* We may get here from multiple threads, use first cached xe_dev */
>	pthread_mutex_lock(&cache.cache_mutex);
>	prev = find_in_cache_unlocked(fd);
> diff --git a/lib/xe/xe_query.h b/lib/xe/xe_query.h
> index d7a9f95f9..05e2ad84f 100644
> --- a/lib/xe/xe_query.h
> +++ b/lib/xe/xe_query.h
> @@ -20,6 +20,8 @@
>  #define XE_DEFAULT_ALIGNMENT           SZ_4K
>  #define XE_DEFAULT_ALIGNMENT_64K       SZ_64K
>
> +struct intel_pat_cache;
> +
>  struct xe_device {
>	/** @fd: xe fd */
>	int fd;
> @@ -74,6 +76,9 @@ struct xe_device {
>
>	/** @dev_id: Device id of xe device */
>	uint16_t dev_id;
> +
> +	/** @pat_cache: cached PAT index configuration, NULL if not yet populated */
> +	struct intel_pat_cache *pat_cache;
>  };
>
>  #define xe_for_each_engine(__fd, __hwe) \
> --
> 2.50.1
>