[PATCH i-g-t v7 3/3] tests/intel/xe_fault_injection: Do not assert for probe_guc_fail_* functions.

[K V P, Satyanarayana]

Hi
> -----Original Message-----
> From: Bernatowicz, Marcin <marcin.bernatowicz at linux.intel.com>
> Sent: Tuesday, April 22, 2025 3:17 PM
> To: K V P, Satyanarayana <satyanarayana.k.v.p at intel.com>; igt-
> dev at lists.freedesktop.org
> Cc: Wajdeczko, Michal <Michal.Wajdeczko at intel.com>; Dugast, Francois
> <francois.dugast at intel.com>
> Subject: Re: [PATCH i-g-t v7 3/3] tests/intel/xe_fault_injection: Do not assert
> for probe_guc_fail_* functions.
> 
> 
> 
> On 4/21/2025 9:45 AM, Satyanarayana K V P wrote:
> > In the current implementation, test asserts if the captured error is not same
> as
> > injected error. It is possible that the error received is translated to other
> > error which can be returned to application and in some scenarios driver
> retries
> > in case of failure and so, no error might be captured.
> >
> > Considering above cases, not asserting the test in inject_fault_probe(), but
> > returing error to the caller. Caller can check and assert if needed. Test does
> 
> returning
> 
> > not assert for probe_guc_fail_* functions as driver some times retries in case
> > of failure. The main idea of injecting errors for these guc functions is to
> > check the robustness of the driver.
> 
> Looks description not related to the patch.
> 
> >
> > Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p at intel.com>
> > ---
> > Cc: Michał Wajdeczko <michal.wajdeczko at intel.com>
> > Cc: Francois Dugast <francois.dugast at intel.com>
> > Cc: Marcin Bernatowicz <marcin.bernatowicz at linux.intel.com>
> > ---
> >   tests/intel/xe_fault_injection.c | 12 +++++++-----
> >   1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/tests/intel/xe_fault_injection.c b/tests/intel/xe_fault_injection.c
> > index d95b51f2d..ae8f79aee 100644
> > --- a/tests/intel/xe_fault_injection.c
> > +++ b/tests/intel/xe_fault_injection.c
> > @@ -224,7 +224,7 @@ static void set_retval(const char function_name[],
> long long retval)
> >    * @xe_wa_init:				xe_wa_init
> >    * @xe_wopcm_init:			xe_wopcm_init
> >    */
> > -static void
> > +static int
> >   inject_fault_probe(int fd, char pci_slot[], const char function_name[])
> >   {
> >   	igt_info("Injecting error \"%s\" (%d) in function \"%s\"\n",
> > @@ -234,8 +234,8 @@ inject_fault_probe(int fd, char pci_slot[], const char
> function_name[])
> >   	injection_list_add(function_name);
> >   	set_retval(function_name, INJECT_ERRNO);
> >   	xe_sysfs_driver_do(fd, pci_slot, XE_SYSFS_DRIVER_TRY_BIND);
> > -	igt_assert_eq(-errno, INJECT_ERRNO);
> >   	injection_list_remove(function_name);
> > +	return -errno;
> 
> It's better to store the errno after the operation for which the result
> is to be returned.
> 
Fixed and sent new version.
-Satya.
> >   }
> >
> >   /**
> > @@ -470,7 +470,7 @@ const char *help_str =
> >
> >   igt_main_args("I:", NULL, help_str, opt_handler, NULL)
> >   {
> > -	int fd, sysfs;
> > +	int fd, sysfs, ret;
> >   	struct drm_xe_engine_class_instance *hwe;
> >   	struct fault_injection_params fault_params;
> >   	static uint32_t devid;
> > @@ -576,8 +576,10 @@ igt_main_args("I:", NULL, help_str, opt_handler,
> NULL)
> >   	}
> >
> >   	for (const struct section *s = probe_fail_functions; s->name; s++)
> > -		igt_subtest_f("inject-fault-probe-function-%s", s->name)
> > -			inject_fault_probe(fd, pci_slot, s->name);
> > +		igt_subtest_f("inject-fault-probe-function-%s", s->name) {
> > +			ret = inject_fault_probe(fd, pci_slot,	s->name);
> > +			igt_assert_eq(ret, INJECT_ERRNO);
> > +		}
> >
> >   	for (const struct section *s = guc_fail_functions; s->name; s++)
> >   		igt_subtest_f("probe-fail-guc-%s", s->name) {