[PATCH i-g-t v2 2/2] tests/intel/xe_fault_injection: Inject errors during xe_guc_ct_send_recv

[K V P, Satyanarayana]

> -----Original Message-----
> From: Dugast, Francois <francois.dugast at intel.com>
> Sent: Monday, January 20, 2025 7:11 PM
> To: K V P, Satyanarayana <satyanarayana.k.v.p at intel.com>
> Cc: igt-dev at lists.freedesktop.org; Brost, Matthew
> <matthew.brost at intel.com>; Wajdeczko, Michal
> <Michal.Wajdeczko at intel.com>
> Subject: Re: [PATCH i-g-t v2 2/2] tests/intel/xe_fault_injection: Inject errors
> during xe_guc_ct_send_recv
> 
> On Mon, Jan 20, 2025 at 02:04:10PM +0530, Satyanarayana K V P wrote:
> > Use the kernel fault injection infrastructure to test error handling
> > of xe at enabling of VFs stage when executing xe_guc_ct_send_recv()
> > so that more code paths are tested, such as error handling and unwinding.
> >
> > Error can be injected using:
> > igt at xe_fault_injection@enable-vfs-fail-xe_guc_ct_send_recv
> >
> > v2: Updated guc_fail_* to enable_vfs_*
> > Added igt_skip_on(!igt_sriov_is_pf(fd)) to skip test when run without
> > enabling sriov.
> >
> > Cc: Matthew Brost <matthew.brost at intel.com>
> > Cc: Michał Wajdeczko <michal.wajdeczko at intel.com>
> > Cc: Francois Dugast <francois.dugast at intel.com>
> > Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p at intel.com>
> > ---
> >  tests/intel/xe_fault_injection.c | 63
> ++++++++++++++++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> >
> > diff --git a/tests/intel/xe_fault_injection.c b/tests/intel/xe_fault_injection.c
> > index 3a0e2aa29..56616ee17 100644
> > --- a/tests/intel/xe_fault_injection.c
> > +++ b/tests/intel/xe_fault_injection.c
> > @@ -19,12 +19,14 @@
> >  #include "igt_sysfs.h"
> >  #include "lib/igt_syncobj.h"
> >  #include "lib/intel_pat.h"
> > +#include "lib/igt_sriov_device.h"
> >  #include "xe/xe_ioctl.h"
> >  #include "xe/xe_query.h"
> >
> >  #define INJECT_ERRNO	-ENOMEM
> >  #define BO_ADDR		0x1a0000
> >  #define BO_SIZE		(1024*1024)
> > +#define NUM_VFS		1
> >
> >  enum injection_list_action {
> >  	INJECTION_LIST_ADD,
> > @@ -281,6 +283,55 @@ vm_bind_fail(int fd, const char function_name[])
> >  	igt_assert_eq(simple_vm_bind(fd, vm), 0);
> >  }
> >
> > +static int sriov_enable_vfs(int fd, int num_vfs)
> > +{
> > +	int sysfs;
> > +	bool ret;
> > +
> > +	sysfs = igt_sysfs_open(fd);
> > +	igt_assert_fd(sysfs);
> > +
> > +	ret = __igt_sysfs_set_u32(sysfs, "device/sriov_numvfs", num_vfs);
> > +	close(sysfs);
> > +
> > +	return ret;
> > +}
> > +
> > +/**
> > + * SUBTEST: guc-fail-%s
> 
> Hi,
> 
> The test name must be updated in the documentation as well:
> 
>     ERROR: Missing documentation for igt at xe_fault_injection@enable-vfs-fail-
> xe_guc_ct_send_recv
>     ERROR: Unneeded documentation for igt at xe_fault_injection@guc-fail-
> xe_guc_ct_send_recv
> 
Updated and pushed V3 patch.
> > + * Description: inject an error in function %arg[1] used when xe interacts
> with guc to make it fail
> > + * Functionality: fault
> > + *
> > + * arg[1]:
> > + * @xe_guc_ct_send_recv:               xe_guc_ct_send_recv
> > + */
> > +
> > +static void
> > +enable_vfs_fail(int fd, int num_vfs, const char function_name[])
> > +{
> > +	bool autoprobe_en = 0;
> > +
> > +	ignore_faults_in_dmesg(function_name);
> > +	injection_list_do(INJECTION_LIST_ADD, function_name);
> > +	set_retval(function_name, INJECT_ERRNO);
> > +
> > +	autoprobe_en = igt_sriov_is_driver_autoprobe_enabled(fd);
> > +
> > +	if (autoprobe_en)
> > +		igt_sriov_disable_driver_autoprobe(fd);
> > +
> > +	/* igt_sriov_enable_vfs can't be used here as it is causing abort on any
> error.
> > +	 * Since error in this test is expected, we have written our own static
> function here.
> > +	 */
> > +	sriov_enable_vfs(fd, num_vfs);
> > +
> > +	igt_assert_eq(-errno, INJECT_ERRNO);
> > +	injection_list_do(INJECTION_LIST_REMOVE, function_name);
> > +
> > +	if (autoprobe_en)
> > +		igt_sriov_enable_driver_autoprobe(fd);
> > +}
> > +
> >  igt_main
> >  {
> >  	int fd;
> > @@ -319,6 +370,10 @@ igt_main
> >  		{ "xe_vma_ops_alloc" },
> >  		{ }
> >  	};
> > +	const struct section enable_vfs_fail_functions[] = {
> > +		{ "xe_guc_ct_send_recv" },
> > +		{ }
> > +	};
> >
> >  	igt_fixture {
> >  		igt_require(fail_function_injection_enabled());
> > @@ -335,6 +390,14 @@ igt_main
> >  		igt_subtest_f("vm-bind-fail-%s", s->name)
> >  			vm_bind_fail(fd, s->name);
> >
> > +	for (const struct section *s = enable_vfs_fail_functions; s->name; s++)
> > +		igt_subtest_f("enable-vfs-fail-%s", s->name) {
> > +			/* Skip the test if not running with SRIOV
> > +			 */
> 
> Is there an accidental newline between the extra white space after "SRIOV"
> and "*/" on a new line?
> 
Was just trying to follow syntax guidelines. Intentionally added */ in the new line.
> > +			igt_skip_on(!igt_sriov_is_pf(fd));
> > +			enable_vfs_fail(fd, NUM_VFS, s->name);
> > +		}
> > +
> >  	igt_fixture {
> >  		xe_sysfs_driver_do(fd, pci_slot, XE_SYSFS_DRIVER_UNBIND);
> >  	}
> > --
> > 2.35.3
> >