Re: [PATCH i-g-t v2 2/2] tests/intel/xe_eudebug: Add subtests for eudebug/SR-IOV exclusion

["Grzegorzek, Dominik" <dominik.grzegorzek@intel.com>]

On Thu, 2025-02-20 at 15:59 +0100, Christoph Manszewski wrote:
> Eudebug is not supported in PF mode with VFs enabled and in VF mode.
> Provide subtests to ensure that:
>   1. enabling eudebug is not permitted in PF mode with VFs enabled
>   2. eudebug sysfs toggle is not available in VF mode
>   3. enabling VFs is not permitted when eudebug is enabled
> 
> Signed-off-by: Christoph Manszewski <christoph.manszewski@intel.com>
> ---
>  tests/intel/xe_eudebug.c       |   2 +
>  tests/intel/xe_eudebug_sriov.c | 153 +++++++++++++++++++++++++++++++++
>  tests/meson.build              |   1 +
>  3 files changed, 156 insertions(+)
>  create mode 100644 tests/intel/xe_eudebug_sriov.c
> 
> diff --git a/tests/intel/xe_eudebug.c b/tests/intel/xe_eudebug.c
> index 76870805c..3eedd543d 100644
> --- a/tests/intel/xe_eudebug.c
> +++ b/tests/intel/xe_eudebug.c
> @@ -20,7 +20,9 @@
>  #include <sys/prctl.h>
>  
>  #include "igt.h"
> +#include "igt_sysfs.h"
>  #include "intel_pat.h"
> +#include "lib/igt_sriov_device.h"
>  #include "lib/igt_syncobj.h"
>  #include "xe/xe_eudebug.h"
>  #include "xe/xe_ioctl.h"
> diff --git a/tests/intel/xe_eudebug_sriov.c b/tests/intel/xe_eudebug_sriov.c
> new file mode 100644
> index 000000000..e82dff98a
> --- /dev/null
> +++ b/tests/intel/xe_eudebug_sriov.c
> @@ -0,0 +1,153 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +/**
> + * TEST: Test EU Debugger and SR-IOV interaction
> + * Category: Core
> + * Mega feature: EUdebug/SR-IOV
> + * Sub-category: EUdebug tests
> + * Functionality: EU Debugger framework
> + * Test category: functionality test
> + */
> +
> +#include "igt.h"
> +#include "igt_sysfs.h"
> +#include "lib/igt_sriov_device.h"
> +#include "xe/xe_eudebug.h"
> +
> +static bool has_enable_eudebug_attr(int fd, unsigned int vf_num)
I know it is just a nitpicking, but may it be has_vf_enable_eudebug_attr? Easier to read.
> +{
> +	char path[PATH_MAX];
> +	int sysfs;
> +	bool ret;
> +
> +	igt_assert(vf_num > 0);
> +
> +	sysfs = igt_sysfs_open(fd);
> +	igt_assert_fd(sysfs);
> +	/* vf_num is 1-based, but virtfn is 0-based */
> +	snprintf(path, sizeof(path), "device/virtfn%u/enable_eudebug", vf_num - 1);
> +	ret = igt_sysfs_has_attr(sysfs, path);
> +	close(sysfs);
> +
> +	return ret;
> +}
> +
> +/**
> + * SUBTEST: deny-eudebug
> + * Description:
> + *	Check that eudebug toggle is not available for VFs, and that enabling
> + *	eudebug with VFs enabled is not permitted.
> + */
> +static void test_deny_eudebug(int fd)
> +{
> +	unsigned int num_vfs = igt_sriov_get_total_vfs(fd);
> +	bool eudebug_enable = true;
> +	bool err = false;
> +	int ret = 0;
> +
> +	igt_debug("Testing %u VFs\n", num_vfs);
> +
> +	xe_eudebug_enable(fd, false);
> +	igt_sriov_enable_driver_autoprobe(fd);
> +	igt_sriov_enable_vfs(fd, num_vfs);
> +	igt_assert_eq(num_vfs, igt_sriov_get_enabled_vfs(fd));
> +
> +	for (int vf_num = 1; vf_num <= num_vfs; ++vf_num) {
> +		if (!igt_sriov_is_vf_drm_driver_probed(fd, vf_num)) {
> +			igt_debug("VF%u probe failed\n", vf_num);
> +			err = true;
> +		} else if (has_enable_eudebug_attr(fd, vf_num)) {
> +			igt_debug("VF%u has enable_eudebug attribute\n", vf_num);
> +			err = true;
> +		}
> +	}
> +
> +	igt_assert(!err);
> +
> +	ret = __xe_eudebug_enable_getset(fd, NULL, &eudebug_enable);
> +	igt_assert_eq(ret, -1);
> +	igt_assert_eq(errno, EPERM);
Its quite late to read errno, isn't it? 
> +}
> +
> +/**
> + * SUBTEST: deny-sriov
> + * Description:
> + *	Check that VFs cannot be enabled when eudebug is enabled.
> + */
> +static void test_deny_sriov(int fd)
> +{
> +	unsigned int num_vfs = igt_sriov_get_total_vfs(fd);
> +	bool ret = false;
> +	int sysfs = 0;
> +
> +	igt_debug("Testing %u VFs\n", num_vfs);
> +
> +	igt_sriov_disable_vfs(fd);
> +	igt_assert_eq(0, igt_sriov_get_enabled_vfs(fd));
> +	xe_eudebug_enable(fd, true);
> +
> +	sysfs = igt_sysfs_open(fd);
> +	igt_assert_fd(sysfs);
> +
> +	ret = __igt_sysfs_set_u32(sysfs, "device/sriov_numvfs", num_vfs);
> +	close(sysfs);
> +
> +	igt_assert_eq(ret, false);
> +	igt_assert_eq(errno, EPERM);
Like above. Check errno right away after set. 
I would use even igt_sysfs_printf which returns -errno directly.

Regards,
Dominik
> +}
> +
> +static void restore_initial_driver_state(int fd, bool eudebug_enabled, bool vf_autoprobe)
> +{
> +	bool abort = false;
> +
> +	igt_sriov_disable_vfs(fd);
> +	if (igt_sriov_get_enabled_vfs(fd) > 0) {
> +		igt_debug("Failed to disable VF(s)\n");
> +		abort = true;
> +	}
> +
> +	vf_autoprobe ? igt_sriov_enable_driver_autoprobe(fd) :
> +		       igt_sriov_disable_driver_autoprobe(fd);
> +	if (vf_autoprobe != igt_sriov_is_driver_autoprobe_enabled(fd)) {
> +		igt_debug("Failed to restore sriov_drivers_autoprobe value\n");
> +		abort = true;
> +	}
> +
> +	if (__xe_eudebug_enable_getset(fd, NULL, &eudebug_enabled) < 0) {
> +		igt_debug("Failed to restore eudebug state\n");
> +		abort = true;
> +	}
> +
> +	/* abort to avoid execution of next tests with invalid driver state */
> +	igt_abort_on_f(abort, "Failed to restore initial driver state\n");
> +}
> +
> +igt_main
> +{
> +	bool eudebug_enabled;
> +	bool vf_autoprobe;
> +	int fd;
> +
> +	igt_fixture {
> +		fd = drm_open_driver(DRIVER_XE);
> +		igt_require(igt_sriov_is_pf(fd));
> +		igt_require(igt_sriov_vfs_supported(fd));
> +		igt_require(igt_sriov_get_enabled_vfs(fd) == 0);
> +		igt_require(__xe_eudebug_enable_getset(fd, &eudebug_enabled, NULL) == 0);
> +		vf_autoprobe = igt_sriov_is_driver_autoprobe_enabled(fd);
> +	}
> +
> +	igt_subtest("deny-eudebug")
> +		test_deny_eudebug(fd);
> +
> +	igt_subtest("deny-sriov")
> +		test_deny_sriov(fd);
> +
> +	igt_fixture {
> +		restore_initial_driver_state(fd, eudebug_enabled, vf_autoprobe);
> +		close(fd);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index a0f984b34..30d067c24 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -332,6 +332,7 @@ intel_xe_eudebug_progs = [
>  	'xe_eudebug',
>  	'xe_exec_sip_eudebug',
>  	'xe_eudebug_online',
> +	'xe_eudebug_sriov',
>  ]
>  
>  if build_xe_eudebug