Re: [PATCH i-g-t v2] tests/intel/xe_debugfs: Add subtest for reg_sr programming failures

[Matt Atwood <matthew.s.atwood@intel.com>]

On Tue, Mar 24, 2026 at 02:11:16PM -0700, Matt Roper wrote:
> The kernel provides a 'register-save-restore-check' debugfs entry that
> allows developers to easily check and see whether any of the driver's
> 'register save/restore' (reg_sr) programming is no longer in effect.
> Wrap a simple IGT test around this debugfs entry so that CI can help
> flag any unexpected changes via a dedicated test.
> 
> Note that we're intentionally avoiding i915's approach of having the
> driver do immediate readback and verification of workaround/tuning
> programming.  That wound up being very problematic since any programming
> failure (even benign/expected failures) would show up as a problem on
> driver probe, and CI would treat that as a fatal error and refuse to run
> any other tests.
> 
> At the moment this test will already report gt0 failures on some Xe2
> platforms (specifically for workaround registers 0xb104, 0xb108, and
> 0xb158) --- this reflects a legitimate kernel bug that's been root
> caused to incorrect bspec documentation about MCR register steering
> (fortunately the bug only affects the register readback used for
> verification; the actual programming did indeed reach the hardware as
> expected in this case).  The fix for that failure will be implemented in
> the kernel once the necessary hardware documentation is available, at
> which point this test should start passing on those platforms.
> 
> At the moment there's an "exception" list containing one register
> (GUC_INTR_CHICKEN_GUC_REG) which is expected to show up in the debugfs
> entry.  This is a case where once the KMD completes its initial
> programming, ownership of the register transfers to an external agent
> (the GuC firmware) and further changes to its value are legitimate and
> not indicative of any hardware or software problem.  Other exceptions
> may show up in the future, either due to cases where ownership of a
> register transfers, or cases where reg_sr programming targets "write
> only" registers that are expected to not read back properly.
> 
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
> Changes in v2:
> - Moved subtest to xe_debugfs. (Kamil)
> - Add missing parameter to igt_info().
> - Link to v1: https://lore.kernel.org/r/20260318-reg_sr_check-v1-1-845d09d27bd1@intel.com
> ---
>  tests/intel/xe_debugfs.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/intel/xe_debugfs.c b/tests/intel/xe_debugfs.c
> index 7fc4a3cbec4b925a40a37a18a5ebd256ba927e0f..587e3e785f6782317f4555035ca59dee060d82ef 100644
> --- a/tests/intel/xe_debugfs.c
> +++ b/tests/intel/xe_debugfs.c
> @@ -28,6 +28,8 @@ struct {
>   * Feature: core
>   * Test category: uapi
>   *
> + * SUBTEST: check-gt-reg-sr
> + * Description: Check the reg_sr list associated with GTs for missing reg values
>   */
>  
>  IGT_TEST_DESCRIPTION("Validate Xe debugfs devnodes and their contents");
> @@ -365,6 +367,50 @@ static void test_info_read(struct xe_device *xe_dev)
>  
>  }
>  
> +/*
> + * A small number of reg_sr programming mismatches are expected and not
> + * indicative of hardware/software problems.
> + */
> +static const unsigned long reg_sr_exceptions[] = {
> +	/* GUC_INTR_CHICKEN_GUC_REG: GuC takes ownership after initial programming */
> +	0xC50C,
> +};
> +
> +static void check_gt_reg_sr(int fd, int gt)
> +{
> +	char buf[1024];
> +	int debugfs_fd;
> +	FILE *file;
> +	int problems = 0;
> +
> +	debugfs_fd = igt_debugfs_gt_open(fd, gt, "register-save-restore-check",
> +					 O_RDONLY);
> +	igt_require(debugfs_fd);
> +	file = fdopen(debugfs_fd, "r");
> +	while (fgets(buf, sizeof(buf), file) != NULL) {
> +		unsigned long offset = strtoul(buf, NULL, 16);
> +		bool ok = false;
> +
> +		for (int ex = 0; ex < ARRAY_SIZE(reg_sr_exceptions); ex++) {
> +			if (offset == reg_sr_exceptions[ex]) {
> +				igt_info("Mismatch on %#lx is not a problem\n", offset);
> +				ok = true;
> +				break;
> +			}
> +		}
> +
> +		if (!ok) {
> +			igt_warn("Mismatch on %#lx, Driver reports: %s", offset, buf);
> +			problems++;
> +		}
> +	}
> +
> +	fclose(file);
> +	close(debugfs_fd);
> +
> +	igt_assert_eq(problems, 0);
> +}
> +
>  const char *help_str =
>  	"  --warn-not-hit|--w\tWarn about devfs nodes that have no tests";
>  
> @@ -390,7 +436,7 @@ int igt_main_args("", long_options, help_str, opt_handler, NULL)
>  {
>  	struct xe_device *xe_dev;
>  	unsigned int t;
> -	int fd = -1;
> +	int fd = -1, gt;
>  
>  	igt_fixture() {
>  		fd = drm_open_driver_master(DRIVER_XE);
> @@ -412,6 +458,12 @@ int igt_main_args("", long_options, help_str, opt_handler, NULL)
>  	igt_describe("Check info debugfs devnode contents.");
>  	igt_subtest("info-read")
>  		test_info_read(xe_dev);
> +
> +	igt_subtest_with_dynamic("check-gt-reg-sr")
> +		xe_for_each_gt(fd, gt)
> +			igt_dynamic_f("gt%d", gt)
> +				check_gt_reg_sr(fd, gt);
> +
>  	igt_fixture() {
>  		drm_close_driver(fd);
>  	}
> 
> ---
> base-commit: 4c8773922f643932cc017ba94d164d2b9d3dd546
> change-id: 20260312-reg_sr_check-95efc4248b54
> 
> Best regards,
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation
>