Re: [PATCH v3 i-g-t 3/4] tests/xe/xe_sriov_flr: Use SKIP for unmet subcheck prerequisites

["Laguna, Lukasz" <lukasz.laguna@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4156 bytes --]

On 11/21/2024 15:18, Marcin Bernatowicz wrote:
> Handle missing prerequisites by marking subchecks as SKIP instead of FAIL.
> This avoids misleading results suggesting FLR issues when tests haven't
> started or conditions aren't met.
>
> 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 | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/tests/intel/xe_sriov_flr.c b/tests/intel/xe_sriov_flr.c
> index 1049cffec..550d58bb9 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",
> +					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");
>   	}
>   }
>   
> @@ -460,7 +465,7 @@ static void ggtt_subcheck_prepare_vf(int vf_id, struct subcheck_data *data)
>   	for_each_pte_offset(pte_offset, &gdata->pte_offsets[vf_id]) {
>   		if (!set_pte_gpa(&gdata->ggtt, gdata->mmio, data->gt, pte_offset,
>   				 (uint8_t)vf_id, &pte)) {
> -			set_fail_reason(data,
> +			set_skip_reason(data,
>   					"Prepare VF%u failed, unexpected gpa: Read PTE: %#lx at offset: %#x\n",
>   					vf_id, pte, pte_offset);
>   			return;
> @@ -696,7 +701,7 @@ static void lmem_subcheck_prepare_vf(int vf_id, struct subcheck_data *data)
>   
>   	if (!lmem_mmap_write_munmap(data->pf_fd, vf_id,
>   				    ldata->vf_lmem_size[vf_id], vf_id)) {
> -		set_fail_reason(data, "LMEM write failed on VF%u\n", vf_id);
> +		set_skip_reason(data, "LMEM write failed on VF%u\n", vf_id);
>   	}
>   }
>   
> @@ -758,12 +763,12 @@ static void regs_subcheck_prepare_vf(int vf_id, struct subcheck_data *data)
>   		struct pci_device *pci_dev = __igt_device_get_pci_device(data->pf_fd, vf_id);
>   
>   		if (!pci_dev) {
> -			set_fail_reason(data, "No PCI device found for VF%u\n", vf_id);
> +			set_skip_reason(data, "No PCI device found for VF%u\n", vf_id);
>   			return;
>   		}
>   
>   		if (intel_register_access_init(&rdata->mmio[vf_id], pci_dev, false)) {
> -			set_fail_reason(data, "Failed to get access to VF%u MMIO\n", vf_id);
> +			set_skip_reason(data, "Failed to get access to VF%u MMIO\n", vf_id);
>   			return;
>   		}
>   	}
> @@ -773,7 +778,7 @@ static void regs_subcheck_prepare_vf(int vf_id, struct subcheck_data *data)
>   
>   		intel_register_write(&rdata->mmio[vf_id], reg, vf_id);
>   		if (intel_register_read(&rdata->mmio[vf_id], reg) != vf_id) {
> -			set_fail_reason(data, "Registers write/read check failed on VF%u\n", vf_id);
> +			set_skip_reason(data, "Registers write/read check failed on VF%u\n", vf_id);
>   			return;
>   		}
>   	}


LGTM,
Reviewed-by: Lukasz Laguna <lukasz.laguna@intel.com>

[-- Attachment #2: Type: text/html, Size: 5551 bytes --]