Re: [PATCH i-g-t 3/3] tests/xe/xe_sriov_flr: Improve clear-ggtt subcheck initialization

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

On 11/20/2024 11:53 AM, Laguna, Lukasz wrote:
> On 11/19/2024 16:55, Marcin Bernatowicz wrote:
>> Mark the ggtt-clear subcheck as SKIP when prerequisites, such as scanning
>> GGTT provisioned offsets, are not met. Asserting in these cases falsely
>> implies that FLR is broken, while the test has not even started.
>>
>> Signed-off-by: Marcin Bernatowicz<marcin.bernatowicz@linux.intel.com>
>> Cc: Adam Miszczak<adam.miszczak@linux.intel.com>
>> Cc: C V Narasimha<narasimha.c.v@intel.com>
>> Cc: Jakub Kolakowski<jakub1.kolakowski@intel.com>
>> Cc: K V P Satyanarayana<satyanarayana.k.v.p@intel.com>
>> Cc: Lukasz Laguna<lukasz.laguna@intel.com>
>> Cc: Michał Wajdeczko<michal.wajdeczko@intel.com>
>> Cc: Michał Winiarski<michal.winiarski@intel.com>
>> Cc: Piotr Piórkowski<piotr.piorkowski@intel.com>
>> Cc: Tomasz Lis<tomasz.lis@intel.com>
>> ---
>>   tests/intel/xe_sriov_flr.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/intel/xe_sriov_flr.c b/tests/intel/xe_sriov_flr.c
>> index 1049cffec..502691765 100644
>> --- a/tests/intel/xe_sriov_flr.c
>> +++ b/tests/intel/xe_sriov_flr.c
>> @@ -393,7 +393,12 @@ static int populate_ggtt_pte_offsets(struct ggtt_data *gdata)
>>   		if (vf_id == 0)
>>   			continue;
>>   
>> -		igt_assert(vf_id >= 1 && vf_id <= num_vfs);
>> +		if (vf_id < 1 || vf_id > num_vfs) {
>> +			set_skip_reason(&gdata->base, "Unexpected VF%u at range entry %u [%#lx-%#lx], num_vfs=%u\n",
> 
> nit: Unexpected VF ID (%u)...
> 
>> +					vf_id, i, ranges[i].start, ranges[i].end, num_vfs);
>> +			free(ranges);
>> +			return -1;
>> +		}
>>   
>>   		if (gdata->pte_offsets[vf_id].end) {
>>   			set_skip_reason(&gdata->base, "Duplicate GGTT PTE offset range for VF%u\n",
>> @@ -410,7 +415,7 @@ static int populate_ggtt_pte_offsets(struct ggtt_data *gdata)
>>   
>>   	for (int vf_id = 1; vf_id <= num_vfs; ++vf_id)
>>   		if (!gdata->pte_offsets[vf_id].end) {
>> -			set_fail_reason(&gdata->base,
>> +			set_skip_reason(&gdata->base,
>>   					"Failed to find VF%u provisioned GGTT PTE offset range\n",
>>   					vf_id);
>>   			return -1;
>> @@ -440,7 +445,7 @@ static void ggtt_subcheck_init(struct subcheck_data *data)
>>   
>>   		populate_ggtt_pte_offsets(gdata);
>>   	} else {
>> -		set_fail_reason(data, "xe_mmio is NULL\n");
>> +		set_skip_reason(data, "xe_mmio is NULL\n");
>>   	}
>>   }
>>   
> 
> Looks good overall,but I see that we still fail in case of other 
> prerequisites not met (LMEM, GGTT write/read, no access to MMIO, etc). 
> Maybe we should skip in these cases too?

Makes sense, I'll extend the skips to the init/prepare phase for all checks.

Marcin

> 
> Lukasz
> 
>