Re: [PATCH i-g-t 04/10] lib/xe: Cache workaround information in xe_device

On Tue, Jul 21, 2026 at 03:59:57PM -0300, Gustavo Sousa wrote:
> One existing inconvenience with xe_wa() is that it needs to dump from
> debugfs every time it is called.  A more concerning issue is that the
> dump will fail if xe_wa() is called from a context without the
> required privileges.
> 
> We currently have only one user of xe_wa(), but shortly we will have
> another user that will need to check for a certain workaround in a
> non-root user context (in test cases that use igt_drop_root()).
> 
> Let's resolve those issues by ensuring that xe_device_get() caches the
> workaround information.  Tests that need to check for workarounds in
> underprivileged context will need to make sure to call xe_device_get()
> before dropping privileges.
> 
> Signed-off-by: Gustavo Sousa <[email protected]>
> ---
>  lib/xe/xe_query.c |  6 ++++++
>  lib/xe/xe_query.h |  9 +++++++--
>  lib/xe/xe_wa.c    | 37 ++++++++++++++++++++++++++++---------
>  lib/xe/xe_wa.h    |  5 +++++
>  4 files changed, 46 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
> index 68e60ddc75a2..ea095b207534 100644
> --- a/lib/xe/xe_query.c
> +++ b/lib/xe/xe_query.c
> @@ -28,6 +28,7 @@
>  
>  #include "xe_query.h"
>  #include "xe_ioctl.h"
> +#include "xe_wa.h"
>  
>  /**
>   * xe_query_device_may_fail:
> @@ -377,6 +378,9 @@ static void xe_device_free(struct xe_device *xe_dev)
>  	free(xe_dev->vram_size);
>  	free(xe_dev->eu_stall);
>  	free(xe_dev->pat_cache);
> +
> +	xe_wa_free_cache(xe_dev);
> +
>  	free(xe_dev);
>  }
>  
> @@ -441,6 +445,8 @@ 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);
>  
> +	xe_wa_build_cache(xe_dev);
> +
>  	/*
>  	 * Populate the PAT cache while we still have sufficient privileges
>  	 * to read debugfs.  Forked children that inherit this xe_device
> diff --git a/lib/xe/xe_query.h b/lib/xe/xe_query.h
> index 59330d80fd1b..f70476945dee 100644
> --- a/lib/xe/xe_query.h
> +++ b/lib/xe/xe_query.h
> @@ -80,6 +80,9 @@ struct xe_device {
>  	/** @pat_cache: cached PAT index configuration, NULL if not yet populated */
>  	struct intel_pat_cache *pat_cache;
>  
> +	/** @wa_cache: cached data for xe_wa() and related functions. */
> +	void *wa_cache;
> +
>  	/**
>  	 * @multi_lrc_mask: bitmask of engine classes supporting multi-LRC.
>  	 * UINT16_MAX if not available (older kernel).
> @@ -99,10 +102,12 @@ struct xe_device {
>  #define xe_for_each_engine_class(__class) \
>  	for (__class = 0; __class < DRM_XE_ENGINE_CLASS_COMPUTE + 1; \
>  	     ++__class)
> -#define xe_for_each_gt(__fd, __gt) \
> -	for (uint64_t igt_unique(__mask) = xe_device_get(__fd)->gt_mask; \
> +#define xe_for_each_gt_from_mask(__gt_mask, __gt) \

The gt_mask changes in this commit don't seem necessary/related to the
rest of the patch?  We still pass xe_dev->gt_mask to
get_wa_debugfs_dumps() so it doesn't seem like there's any change of
behavior related to this.


Matt

> +	for (uint64_t igt_unique(__mask) = __gt_mask; \
>  	     __gt = ffsll(igt_unique(__mask)) - 1, igt_unique(__mask) != 0; \
>  	     igt_unique(__mask) &= ~(1ull << __gt))
> +#define xe_for_each_gt(__fd, __gt) \
> +	xe_for_each_gt_from_mask(xe_device_get(__fd)->gt_mask, __gt)
>  #define xe_for_each_tile(__fd, __tile) \
>  	for (uint64_t igt_unique(__mask) = xe_device_get(__fd)->tile_mask; \
>  	     __tile = ffsll(igt_unique(__mask)) - 1, igt_unique(__mask) != 0; \
> diff --git a/lib/xe/xe_wa.c b/lib/xe/xe_wa.c
> index e4e1b0bb7972..0f4d2edc0795 100644
> --- a/lib/xe/xe_wa.c
> +++ b/lib/xe/xe_wa.c
> @@ -22,14 +22,14 @@ static void free_wa_debugfs_dumps(char **dumps)
>  	free(dumps);
>  }
>  
> -static char **get_wa_debugfs_dumps(int fd)
> +static char **get_wa_debugfs_dumps(int fd, uint64_t gt_mask)
>  {
>  	char **dumps;
>  	int gt;
>  	int debugfs_fd;
>  	int count = 1; /* Device workarounds */
>  
> -	xe_for_each_gt(fd, gt)
> +	xe_for_each_gt_from_mask(gt_mask, gt)
>  		count++;
>  
>  	dumps = calloc(count + 1, sizeof(*dumps));
> @@ -45,7 +45,7 @@ static char **get_wa_debugfs_dumps(int fd)
>  	if (!(dumps[count++] = igt_sysfs_get(debugfs_fd, "workarounds")))
>  		goto err;
>  
> -	xe_for_each_gt(fd, gt) {
> +	xe_for_each_gt_from_mask(gt_mask, gt) {
>  		char name[32];
>  
>  		snprintf(name, sizeof(name), "gt%d/workarounds", gt);
> @@ -112,17 +112,36 @@ static bool debugfs_dump_has_wa(char *dump, const char *wa)
>   */
>  int xe_wa(int fd, const char *wa)
>  {
> -	char **dumps = get_wa_debugfs_dumps(fd);
> -	int ret = 0;
> +	char **dumps = xe_device_get(fd)->wa_cache;
>  
>  	if (igt_warn_on(!dumps))
>  		return -1;
>  
>  	for (char **dump = dumps; *dump; dump++)
> -		if ((ret = debugfs_dump_has_wa(*dump, wa)))
> -			break;
> +		if (debugfs_dump_has_wa(*dump, wa))
> +			return 1;
>  
> -	free_wa_debugfs_dumps(dumps);
> +	return 0;
> +}
> +
> +/**
> + * xe_wa_build_cache: Build cached data for xe_wa().
> + * @xe_dev: Xe device where the cache will be stashed.
> + */
> +void xe_wa_build_cache(struct xe_device *xe_dev)
> +{
> +	xe_dev->wa_cache = get_wa_debugfs_dumps(xe_dev->fd, xe_dev->gt_mask);
> +}
> +
> +/**
> + * xe_wa_free_cache: Free cached data that was built with xe_wa_build_cache().
> + * @xe_dev: Xe device where the cached data is stashed.
> + */
> +void xe_wa_free_cache(struct xe_device *xe_dev)
> +{
> +	if (!xe_dev->wa_cache)
> +		return;
>  
> -	return ret;
> +	free_wa_debugfs_dumps(xe_dev->wa_cache);
> +	xe_dev->wa_cache = NULL;
>  }
> diff --git a/lib/xe/xe_wa.h b/lib/xe/xe_wa.h
> index 4ff897196545..f0a826553df8 100644
> --- a/lib/xe/xe_wa.h
> +++ b/lib/xe/xe_wa.h
> @@ -6,6 +6,11 @@
>  #ifndef XE_WA_H
>  #define XE_WA_H
>  
> +struct xe_device;
> +
>  int xe_wa(int fd, const char *wa);
>  
> +void xe_wa_build_cache(struct xe_device *xe_dev);
> +void xe_wa_free_cache(struct xe_device *xe_dev);
> +
>  #endif /* XE_WA_H */
> 
> -- 
> 2.55.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation