Re: [PATCH i-g-t] tests/intel/xe_configfs: Check ctx_restore_post_bb

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Sep 05, 2025 at 10:52:21PM -0700, Lucas De Marchi wrote:
> Add sanity checks for the values possibly written to
> ctx_restore_post_bb, according to pending patch series:
> https://lore.kernel.org/intel-xe/20250905-wa-bb-cmds-v3-0-3da2b7bdc73e@intel.com/
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  tests/intel/xe_configfs.c | 113 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 113 insertions(+)
> 
> diff --git a/tests/intel/xe_configfs.c b/tests/intel/xe_configfs.c
> index 086f99ba5..d778065f3 100644
> --- a/tests/intel/xe_configfs.c
> +++ b/tests/intel/xe_configfs.c
> @@ -122,6 +122,111 @@ static void test_engines_allowed(int configfs_device_fd)
>  	}
>  }
>  
> +/**
> + * SUBTEST: ctx-restore-post-bb-invalid
> + * Description: Validate ctx_restore_post_bb attribute for invalid values
> + */
> +static void test_ctx_restore_post_bb_invalid(int configfs_device_fd)
> +{
> +	static const struct value {
> +		const char *test;
> +		const char *in;
> +		const char *out;

you don't need the out here right?
or do you have plans to move the declaration out of the function?

> +	} values[] = {
> +		{ .test = "invalid-engine",
> +		  .in = "foobar cmd 11000001 4F100 DEADBEEF",
> +		},
> +		{ .test = "invalid-type",
> +		  .in = "rcs 11000001 4F100 DEADBEEF",
> +		},
> +		{ .test = "invalid-number",
> +		  .in = "rcs cmd 1100000g 4F100 DEADBEEF",
> +		},
> +		{ .test = "invalid-number",
> +		  .in = "rcs cmd 1100000g 4F100 DEADBEEF",
> +		},
> +		{ .test = "invalid-reg-addr-only",
> +		  .in = "rcs reg 4F100",
> +		},
> +	};
> +
> +	/*
> +	 * These only test if command parsing is correct,
> +	 * so just make sure there's no device bound
> +	 */
> +	igt_kmod_unbind("xe", bus_addr);
> +
> +	for (size_t i = 0; i < ARRAY_SIZE(values); i++) {
> +		const struct value *v = &values[i];
> +
> +		igt_info("%s\n", v->test);
> +		igt_debug("Test '%s' to ctx_restore_post_bb\n", v->in);
> +		igt_assert(!igt_sysfs_set(configfs_device_fd,
> +					  "ctx_restore_post_bb", v->in));

worth binding the driver and reading the mmio register with intel_reg
and comparing with set value?

> +	}
> +}
> +
> +/**
> + * SUBTEST: ctx-restore-post-bb
> + * Description: Validate ctx_restore_post_bb attribute
> + */
> +static void test_ctx_restore_post_bb(int configfs_device_fd)
> +{
> +	static const struct value {
> +		const char *test;
> +		const char *in;
> +		const char *out;
> +	} values[] = {
> +		{ .test = "cmd-single",
> +		  .in = "rcs cmd 11000001 4F100 DEADBEEF",
> +		  .out = "rcs: 11000001 0004f100 deadbeef\n",
> +		},
> +		{ .test = "cmd-single-multi-values",
> +		  .in = "rcs cmd 11000003 4F100 DEADBEEF 4F104 DEADBEEF",
> +		  .out = "rcs: 11000003 0004f100 deadbeef 0004f104 deadbeef\n",
> +		},
> +		{ .test = "cmd-multi",
> +		  .in = "rcs cmd 11000001 4F100 DEADBEEF\n"
> +			"rcs cmd 11000001 4F104 FFFFFFFF",
> +		  .out = "rcs: 11000001 0004f100 deadbeef 11000001 0004f104 ffffffff\n",
> +		},
> +		{ .test = "reg-single",
> +		  .in = "rcs reg 4F100 DEADBEEF",
> +		  .out = "rcs: 11000001 0004f100 deadbeef\n",
> +		},
> +		{ .test = "reg-multi",
> +		  .in = "rcs reg 4F100 DEADBEEF\n"
> +			"rcs reg 4F104 FFFFFFFF",
> +		  .out = "rcs: 11000001 0004f100 deadbeef 11000001 0004f104 ffffffff\n",
> +		},
> +	};
> +	char buf[4096] = { };
> +
> +	/*
> +	 * These only test if command parsing is correct,
> +	 * so just make sure there's no device bound
> +	 */
> +	igt_kmod_unbind("xe", bus_addr);
> +
> +	for (size_t i = 0; i < ARRAY_SIZE(values); i++) {
> +		const struct value *v = &values[i];
> +
> +		igt_info("%s\n", v->test);
> +		igt_debug("Test '%s' to ctx_restore_post_bb\n", v->in);
> +		igt_assert(igt_sysfs_set(configfs_device_fd,
> +					 "ctx_restore_post_bb", v->in));
> +
> +		if (v->out) {
> +			igt_assert(igt_sysfs_read(configfs_device_fd, "ctx_restore_post_bb", buf,
> +						  sizeof(buf) - 1));
> +			if (strcmp(v->out, buf)) {
> +				igt_debug("Expecting '%s' but found '%s'", v->out, buf);
> +				igt_fail(IGT_EXIT_FAILURE);
> +			}
> +		}
> +	}
> +}
> +
>  static void set_bus_addr(int fd)
>  {
>  	struct pci_device *pci_dev;
> @@ -173,6 +278,14 @@ igt_main
>  	igt_subtest("engines-allowed")
>  		test_engines_allowed(configfs_device_fd);
>  
> +	igt_describe("Validate ctx_restore_post_bb with invalid options");
> +	igt_subtest("ctx-restore-post-bb-invalid")
> +		test_ctx_restore_post_bb_invalid(configfs_device_fd);
> +
> +	igt_describe("Validate ctx_restore_post_bb");
> +	igt_subtest("ctx-restore-post-bb")
> +		test_ctx_restore_post_bb(configfs_device_fd);
> +
>  	igt_fixture {
>  		close(configfs_device_fd);
>  		close(configfs_fd);
> -- 
> 2.50.1
>