Re: [PATCH v9 3/5] tests/xe_pat: add pat-sanity subtest for debugfs vs getters

Matt Roper
2025-12-31T17:11:41Z

On Wed, Dec 31, 2025 at 05:55:23AM +0000, Xin Wang wrote:
> Add a new pat-sanity subtest that reads the Xe PAT software
> configuration from debugfs and validates it against the
> existing PAT index helper getters.
>
> CC: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> CC: Matt Roper <matthew.d.roper@intel.com>
> CC: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Xin Wang <x.wang@intel.com>
> ---
> tests/intel/xe_pat.c | 99 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 99 insertions(+)
>
> diff --git a/tests/intel/xe_pat.c b/tests/intel/xe_pat.c
> index 2fd3635fd..f003f0046 100644
> --- a/tests/intel/xe_pat.c
> +++ b/tests/intel/xe_pat.c
> @@ -18,6 +18,7 @@
> #include "intel_blt.h"
> #include "intel_mocs.h"
> #include "intel_pat.h"
> +#include "linux_scaffold.h"
>
> #include "xe/xe_ioctl.h"
> #include "xe/xe_query.h"
> @@ -76,6 +77,101 @@ static void userptr_coh_none(int fd)
> munmap(data, size);
> xe_vm_destroy(fd, vm);
> }
> +#define REG_FIELD_GET(__mask, __val) \
> + ((uint32_t)FIELD_GET(__mask, __val))
> +
> +#define XE2_NO_PROMOTE REG_BIT(10)
> +#define XE2_COMP_EN REG_BIT(9)
> +#define XE2_L3_CLOS GENMASK(7, 6)
> +#define XE2_L3_POLICY GENMASK(5, 4)
> +#define XE2_L4_POLICY GENMASK(3, 2)
> +#define XE2_COH_MODE GENMASK(1, 0)
> +
> +#define L3_CLOS1 1
> +#define L3_CLOS2 2
> +#define L3_CLOS3 3
> +
> +#define L3_CACHE_POLICY_WB 0
> +#define L3_CACHE_POLICY_XD 1
> +#define L3_CACHE_POLICY_UC 3
> +
> +#define L4_CACHE_POLICY_WB 0
> +#define L4_CACHE_POLICY_WT 1
> +#define L4_CACHE_POLICY_UC 3
> +
> +#define COH_MODE_NONE 0
> +#define COH_MODE_1WAY 2
> +#define COH_MODE_2WAY 3
> +
> +static int xe_fetch_pat_sw_config_or_skip(int fd, struct intel_pat_cache *pat_sw_config)
> +{
> + int32_t parsed = xe_get_pat_sw_config(fd, pat_sw_config);
> +
> + igt_require_f(parsed > 0, "Couldn't get Xe PAT software configuration\n");

Would there ever be a non-error reason for us to fail to read these
entries? All platforms supported by the Xe driver should have PAT
tables, so I think it would be an error if we can't find this debugfs
entry. So maybe this should be an igt_assert (to cause failure) rather
than igt_require (which skips)?

> +
> + return parsed;
> +}
> +
> +static void dump_xe2_pat_sw_entries(const struct intel_pat_cache *pat_sw_config,
> + int32_t parsed)
> +{
> + for (int i = 0; i < parsed; i++) {
> + uint32_t pat = pat_sw_config->entries[i].pat;
> +
> + igt_info("PAT[%2d] = [ %u, %u, %u, %u, %u, %u] (%#8x)%s\n", i,
> + !!(pat & XE2_NO_PROMOTE),
> + !!(pat & XE2_COMP_EN),
> + REG_FIELD_GET(XE2_L3_CLOS, pat),
> + REG_FIELD_GET(XE2_L3_POLICY, pat),
> + REG_FIELD_GET(XE2_L4_POLICY, pat),
> + REG_FIELD_GET(XE2_COH_MODE, pat),
> + pat,
> + pat_sw_config->entries[i].rsvd ? " *" : "");
> + }
> +}

I don't think there's much value in re-dumping this output while running
the test. A lot of our IGT tests already spam more information than we
really want, and the information here is super easy for the user to just
'cat' directly from debugfs if they want to check it by hand. In the
usual case where the test passes and everything is fine, there's no
need to dump a bunch of debugging information on the console.

> +
> +/**
> + * SUBTEST: pat-sanity
> + * Test category: functionality test
> + * Description: Test debugfs PAT config vs getters
> + */
> +static void pat_sanity(int fd)
> +{
> + uint16_t dev_id = intel_get_drm_devid(fd);
> + struct intel_pat_cache pat_sw_config = {};
> + int32_t parsed;
> + bool has_uc_comp = false, has_wt = false;
> +
> + parsed = xe_fetch_pat_sw_config_or_skip(fd, &pat_sw_config);
> +
> + if (intel_graphics_ver(dev_id) >= IP_VER(20, 0)) {
> + dump_xe2_pat_sw_entries(&pat_sw_config, parsed);
> + for (int i = 0; i < parsed; i++) {
> + uint32_t pat = pat_sw_config.entries[i].pat;
> + if (pat_sw_config.entries[i].rsvd)
> + continue;
> + if (!!(pat & XE2_COMP_EN) &&
> + REG_FIELD_GET(XE2_L3_POLICY, pat) == L3_CACHE_POLICY_UC &&
> + REG_FIELD_GET(XE2_L4_POLICY, pat) == L4_CACHE_POLICY_UC) {
> + has_uc_comp = true;
> + }
> + if (REG_FIELD_GET(XE2_L3_POLICY, pat) == L3_CACHE_POLICY_XD &&
> + REG_FIELD_GET(XE2_L4_POLICY, pat) == L4_CACHE_POLICY_WT) {
> + has_wt = true;
> + }
> + }
> + if (has_uc_comp)
> + igt_assert(pat_sw_config.uc_comp == intel_get_pat_idx_uc_comp(fd));

It's better to use igt_assert_eq() so that if there's a mismatch, the
error message will include the expected and actual values it
encountered during the comparison. Same for the other instances below.

Matt

> + }
> + else {
> + has_wt = true;
> + }
> + igt_assert(pat_sw_config.max_index == intel_get_max_pat_index(fd));
> + igt_assert(pat_sw_config.uc == intel_get_pat_idx_uc(fd));
> + igt_assert(pat_sw_config.wb == intel_get_pat_idx_wb(fd));
> + if (has_wt)
> + igt_assert(pat_sw_config.wt == intel_get_pat_idx_wt(fd));
> +}
>
> /**
> * SUBTEST: pat-index-all
> @@ -1230,6 +1326,9 @@ int igt_main_args("V", NULL, help_str, opt_handler, NULL)
> xe_device_get(fd);
> }
>
> + igt_subtest("pat-sanity")
> + pat_sanity(fd);
> +
> igt_subtest("pat-index-all")
> pat_index_all(fd);
>
> --
> 2.43.0
>

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