Re: [PATCH v2 i-g-t 2/6] lib/igt_sriov_device: add helper for resetting SR-IOV device

["Bernatowicz, Marcin" <marcin.bernatowicz@linux.intel.com>]

On 10/22/2024 12:10 PM, Michal Wajdeczko wrote:
> 
> 
> On 21.10.2024 22:07, Marcin Bernatowicz wrote:
>> Reset is initiated by writing 1 to device's sysfs reset attribute.
> 
> FLR is not SR-IOV only concept, FLR can be supported by any other PCI
> device, it's just mandatory for the VFs, so maybe these SR-IOV helpers
> should be built on top native helpers:
> 
> igt_trigger_reset(dir)
> {
> 	igt_sysfs_set(dir, "reset", "1")
> }
> 
> igt_reset_device(fd)
> {
> 	dir = igt_get_device_dir(fd)
> 	igt_trigger_reset(dir)
> }
> 
> igt_sriov_get_vf_device_dir(fd, n)
> {
> 	igt_assert(n);
> 	return ...("virtfn%u", n)
> }
> 
> igt_sriov_reset_vf(fd, n)
> {
> 	dir = igt_sriov_get_vf_device_dir(fd, n)
> 	igt_trigger_reset(dir)
> }
> 
Maybe I don't follow, but this patch contains the equivalent 
implementation for:

igt_sriov_get_vf_device_dir(fd, n)
igt_sriov_reset_vf(fd, n)

with the difference that we allow n == 0 (treat PF as VF0).

The common point would be int igt_device_trigger_reset(int dev_dir) (if 
placed in lib/igt_device), but I find it a bit confusing as the param is 
a device directory and not the DRM device fd.

igt_reset_device(fd) is not very usable for existing device_reset test 
as we unbind the driver before reset.

>>
>> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
>> Reviewed-by: Adam Miszczak <adam.miszczak@linux.intel.com>
>> Cc: Adam Miszczak <adam.miszczak@linux.intel.com>
>> Cc: Jakub Kolakowski <jakub1.kolakowski@intel.com>
>> Cc: Lukasz Laguna <lukasz.laguna@intel.com>
>> Cc: Michał Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Cc: Narasimha C V <narasimha.c.v@intel.com>
>> Cc: Piotr Piórkowski <piotr.piorkowski@intel.com>
>> Cc: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
>> Cc: Tomasz Lis <tomasz.lis@intel.com>
>> ---
>>   lib/igt_sriov_device.c | 51 ++++++++++++++++++++++++++++++++++++++++++
>>   lib/igt_sriov_device.h |  2 ++
>>   2 files changed, 53 insertions(+)
>>
>> diff --git a/lib/igt_sriov_device.c b/lib/igt_sriov_device.c
>> index d20c74823..2b83cd43c 100644
>> --- a/lib/igt_sriov_device.c
>> +++ b/lib/igt_sriov_device.c
>> @@ -413,3 +413,54 @@ int igt_sriov_device_sysfs_open(int pf, unsigned int vf_num)
>>   
>>   	return fd;
>>   }
>> +
>> +/**
>> + * igt_sriov_device_reset_exists:
>> + * @pf: PF device file descriptor
>> + * @vf_num: VF number (1-based to identify single VF) or 0 for PF
>> + *
>> + * Check if reset attribute exists for a given SR-IOV device.
>> + *
>> + * Returns:
>> + * True if reset attribute exists, false otherwise.
>> + */
>> +bool igt_sriov_device_reset_exists(int pf, unsigned int vf_num)
>> +{
>> +	int sysfs;
>> +	bool reset_exists;
>> +
>> +	sysfs = igt_sriov_device_sysfs_open(pf, vf_num);
>> +	if (sysfs < 0)
>> +		return false;
>> +
>> +	reset_exists = igt_sysfs_has_attr(sysfs, "reset");
>> +	close(sysfs);
>> +
>> +	return reset_exists;
>> +}
>> +
>> +/**
>> + * igt_sriov_device_reset:
>> + * @pf: PF device file descriptor
>> + * @vf_num: VF number (1-based to identify single VF) or 0 for PF
>> + *
>> + * Trigger FLR on a given VF.
>> + *
>> + * Returns:
>> + * True on success, false on failure.
>> + */
>> +bool igt_sriov_device_reset(int pf, unsigned int vf_num)
>> +{
>> +	int sysfs;
>> +	bool ret;
>> +
>> +	sysfs = igt_sriov_device_sysfs_open(pf, vf_num);
>> +	if (sysfs < 0)
>> +		return false;
>> +
>> +	igt_debug("Initiating FLR on VF%d\n", vf_num);
>> +	ret = igt_sysfs_set(sysfs, "reset", "1");
>> +	close(sysfs);
>> +
>> +	return ret;
>> +}
>> diff --git a/lib/igt_sriov_device.h b/lib/igt_sriov_device.h
>> index dc95a4c78..4b63ceb22 100644
>> --- a/lib/igt_sriov_device.h
>> +++ b/lib/igt_sriov_device.h
>> @@ -31,6 +31,8 @@ bool igt_sriov_is_vf_drm_driver_probed(int pf, unsigned int vf_num);
>>   void igt_sriov_bind_vf_drm_driver(int pf, unsigned int vf_num);
>>   void igt_sriov_unbind_vf_drm_driver(int pf, unsigned int vf_num);
>>   int igt_sriov_device_sysfs_open(int pf, unsigned int vf_num);
>> +bool igt_sriov_device_reset_exists(int pf, unsigned int vf_num);
>> +bool igt_sriov_device_reset(int pf, unsigned int vf_num);
>>   
>>   /**
>>    * for_each_sriov_vf - Helper for running code on each VF
>