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

[Manszewski, Christoph]

Hi Dominik,

On 27.02.2025 17:39, Grzegorzek, Dominik wrote:
> 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 at 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.

Ok

>> +{
>> +	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.

Makes sense, I will fix that.

Thanks,
Christoph

> 
> 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
>