Re: [PATCH i-g-t v2 2/2] tests/intel/xe_configfs: Test engines_allowed

[Riana Tauro <riana.tauro@intel.com>]

Hi Lucas

On 6/2/2025 9:54 PM, Lucas De Marchi wrote:
> Add tests for the parsing of engines_allowed configfs. This doesn't try
> to bind the driver and ensure only those engines are available. That's
> because the engine uapi instance doesn't necessarily match what is used
> for configfs configuration. Maybe we will need an engine map in debugfs
> for covering that.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   tests/intel/xe_configfs.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 56 insertions(+)
> 
> diff --git a/tests/intel/xe_configfs.c b/tests/intel/xe_configfs.c
> index 074cd04d9..e4fffd0b9 100644
> --- a/tests/intel/xe_configfs.c
> +++ b/tests/intel/xe_configfs.c
> @@ -10,6 +10,7 @@
>   #include "igt_fs.h"
>   #include "igt_kmod.h"
>   #include "igt_sysfs.h"
> +#include "xe/xe_query.h"
>   
>   /**
>    * TEST: Check configfs userspace API
> @@ -54,6 +55,49 @@ static void test_survivability_mode(int configfs_device_fd)
>   	close(fd);
>   }
>   
> +/**
> + * SUBTEST: engines-allowed-invalid
> + * Description: Validate engines_allowed attribute for invalid values
> + */
> +static void test_engines_allowed_invalid(int configfs_device_fd)
> +{
> +	static const char *values[] = {
> +		"xcs0",
> +		"abcsdcs0",
> +		"rcs0,abcsdcs0",
> +		"rcs9",
> +		"rcs10",
> +		"rcs0asdf",
> +	};
> +
> +	for (size_t i = 0; i < ARRAY_SIZE(values); i++) {
> +		const char *v = values[i];
> +
> +		igt_debug("Writing '%s' to engines_allowed\n", v);
> +		igt_assert(!igt_sysfs_set(configfs_device_fd, "engines_allowed", v));
> +	}
> +}
> +
> +/**
> + * SUBTEST: engines-allowed
> + * Description: Validate engines_allowed attribute
> + */
> +static void test_engines_allowed(int configfs_device_fd)
> +{
> +	static const char *values[] = {
> +		"rcs0", "rcs*", "rcs0,bcs0", "bcs0,rcs0",
> +		"bcs0\nrcs0", "bcs0\nrcs0\n",
> +		"rcs000",
> +	};
> +
> +	for (size_t i = 0; i < ARRAY_SIZE(values); i++) {
> +		const char *v = values[i];
> +
> +		igt_debug("Writing '%s' to engines_allowed\n", v);
> +		igt_assert(igt_sysfs_set(configfs_device_fd, "engines_allowed", v));
> +	}
> +}
> +
>   static int create_device_configfs_group(int configfs_fd, int fd)
>   {
>   	int configfs_device_fd;
> @@ -88,6 +132,18 @@ igt_main
>   		test_survivability_mode(configfs_device_fd);
>   	}
>   
> +	igt_describe("Validate engines_allowed with invalid options");
> +	igt_subtest("engines-allowed-invalid") {
> +		igt_install_exit_handler(restore);

Exit handler can be common in the fixture if it's needed by all tests

Otherwise, Looks good to me
Reviewed-by: Riana Tauro <riana.tauro@intel.com>
> +		test_engines_allowed_invalid(configfs_device_fd);
> +	}
> +
> +	igt_describe("Validate engines_allowed");
> +	igt_subtest("engines-allowed") {
> +		igt_install_exit_handler(restore);
> +		test_engines_allowed(configfs_device_fd);
> +	}
> +
>   	igt_fixture {
>   		igt_fs_remove_dir(configfs_fd, bus_addr);
>   		close(configfs_device_fd);
>