Re: [PATCH] tests/intel/xe_exec_system_allocator: Skip reserved PAT madvise indices

[Gustavo Sousa <gustavo.sousa@intel.com>]

<nishit.sharma@intel.com> writes:

> From: Nishit Sharma <nishit.sharma@intel.com>
>
> With Wa_14026539277 active, KMD marks the 2-way-coherent PAT entries as
> reserved in the debugfs pat_sw_config dump and they must not be used.
> Skip the PAT-index madvise subtests when the selected PAT index is
> reserved instead of decoding the coherency bits of the PAT entry.
>
> Signed-off-by: Nishit Sharma <nishit.sharma@intel.com>
> ---
>  tests/intel/xe_exec_system_allocator.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/tests/intel/xe_exec_system_allocator.c b/tests/intel/xe_exec_system_allocator.c
> index 45ec62813..2a0b89f7a 100644
> --- a/tests/intel/xe_exec_system_allocator.c
> +++ b/tests/intel/xe_exec_system_allocator.c
> @@ -26,6 +26,7 @@
>  #include "xe/xe_gt.h"
>  #include "xe/xe_ioctl.h"
>  #include "xe/xe_query.h"
> +#include "xe/xe_wa.h"
>  #include <string.h>
>  
>  #define USER_FENCE_VALUE	0xdeadbeefdeadbeefull
> @@ -1216,6 +1217,16 @@ xe_vm_madvixe_pat_attr(int fd, uint32_t vm, uint64_t addr, uint64_t range,
>  		      DRM_XE_MEM_RANGE_ATTR_PAT, pat_index, 0, 0);
>  }
>  
> +static bool pat_idx_is_reserved(const struct intel_pat_cache *pat_sw_config,
> +				int32_t num_pat_entries,
> +				uint8_t pat_idx)
> +{
> +	if (pat_idx >= num_pat_entries)
> +		return false;
> +
> +	return pat_sw_config->entries[pat_idx].rsvd;
> +}
> +
>  static void
>  xe_vm_madvise_atomic_attr(int fd, uint32_t vm, uint64_t addr, uint64_t range,
>  			  int mem_attr)
> @@ -2647,6 +2658,8 @@ int igt_main()
>  
>  	int fd, svm_compute_loops = getenv_int("IGT_SVM_COMPUTE_LOOPS", 1);
>  	uint16_t dev_id;
> +	struct intel_pat_cache pat_sw_config = {};
> +	int32_t num_pat_entries = 0;
>  
>  	igt_fixture() {
>  		struct xe_device *xe;
> @@ -2658,6 +2671,11 @@ int igt_main()
>  		va_bits = xe->va_bits;
>  		open_sync_file();
>  		dev_id = intel_get_drm_devid(fd);
> +		if (xe_wa(fd, "14026539277")) {
> +			num_pat_entries = xe_get_pat_sw_config(fd, &pat_sw_config, 0);
> +			igt_assert_f(num_pat_entries > 0,
> +				     "Couldn't get Xe PAT software configuration\n");
> +		}

I was thinking: maybe we could always read the PAT sw config,
irrespective to the workaround being enabled or not.  This would
future-proof the code for new workarounds (or other types of
restrictions) that could get added later.

>  	}
>  
>  	for (const struct section *s = sections; s->name; s++) {
> @@ -2887,6 +2905,10 @@ int igt_main()
>  			if (IS_PONTEVECCHIO(dev_id) &&
>  			    strstr(s->name, "madvise-pat-idx-uc-comp-"))
>  				igt_skip("Skipping compression-related PAT index on PVC\n");
> +
> +			igt_skip_on_f(pat_idx_is_reserved(&pat_sw_config, num_pat_entries,
> +							  s->fn(fd)),
> +				      "PAT index reserved by Wa_14026539277\n");

While Wa_14026539277 marks some PAT indices as reserved, this is not a
one-to-one relationship, meaning that a PAT index could theoretically be
reserved for a different reason.  As such, I think we should avoid
mentioning Wa_14026539277 here.

--
Gustavo Sousa

>  			xe_for_each_engine(fd, hwe)
>  				test_exec(fd, hwe, 1, 1, SZ_4M, 0, 0, NULL, NULL, s->flags, s->fn);
>  		}
> -- 
> 2.43.0