Re: [PATCH v2 i-g-t 1/1] tests/intel/xe_pmu: Provision VFs inside dynamic subtests

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

On 10/13/2026 8:30 PM, Gustavo Sousa wrote:
> Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com> writes:
>
>> VF setup helpers can call igt_assert(), which is not allowed in the outer
>> igt_subtest_with_dynamic() scope. Set up VFs in the first dynamic case,
>> reuse them across engine cases, and clean up after the group. This keeps
>> VF setup out of unrelated subtests.
>>
>> Assisted-by: Copilot:Claude-Opus-5
>> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
>> Cc: Gustavo Sousa <gustavo.sousa@intel.com>
>> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
>> Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
>> ---
>>   tests/intel/xe_pmu.c | 48 +++++++++++++++++++++++++++++++++-----------
>>   1 file changed, 36 insertions(+), 12 deletions(-)
>>
>> diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
>> index d41ebba1f..159a7f8a2 100644
>> --- a/tests/intel/xe_pmu.c
>> +++ b/tests/intel/xe_pmu.c
>> @@ -117,6 +117,8 @@ static char xe_device[NAME_MAX];
>>   static bool autoprobe;
>>   static int total_exec_quantum;
>>   static bool has_engine_active_ticks;
>> +static unsigned int num_functions;
> I feel like num_functions as a name for a global variable is a bit too
> generic for what its purpose is.
>
> Maybe sriov_num_fns (or the longer sriov_num_functions).
>
> And then, for consistency, sriov_vfs_setup_started?
>
> Anyway, the patch looks good to me. Thanks for this!
>
> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

Agreed, num_functions is too generic here. I’ll rename them to 
sriov_num_functions and sriov_vfs_setup_started.

Thanks for the review!

>
>> +static bool vfs_setup_started;
>>   
>>   #define test_each_engine(test, fd, hwe) \
>>   	igt_subtest_with_dynamic(test) \
>> @@ -999,6 +1001,7 @@ static unsigned int enable_and_provision_vfs(int fd)
>>   	num_vfs = total_vfs > 2 ? 2 : total_vfs;
>>   
>>   	/* Enable VF's */
>> +	vfs_setup_started = true;
>>   	igt_sriov_disable_driver_autoprobe(fd);
>>   	igt_sriov_enable_vfs(fd, num_vfs);
>>   	igt_require(igt_sriov_get_enabled_vfs(fd) == num_vfs);
>> @@ -1021,6 +1024,16 @@ static unsigned int enable_and_provision_vfs(int fd)
>>   	return num_vfs;
>>   }
>>   
>> +static void ensure_vfs_enabled(int fd)
>> +{
>> +	if (num_functions)
>> +		return;
>> +
>> +	igt_require_f(!vfs_setup_started,
>> +		      "VF setup failed in an earlier dynamic subtest; see its log\n");
>> +	num_functions = enable_and_provision_vfs(fd) + 1;
>> +}
>> +
>>   static void unprovision_and_disable_vfs(int fd)
>>   {
>>   	int ret;
>> @@ -1039,6 +1052,16 @@ static void unprovision_and_disable_vfs(int fd)
>>   	igt_sriov_clear_exit_handler();
>>   }
>>   
>> +static void cleanup_vfs_setup(int fd)
>> +{
>> +	if (!vfs_setup_started)
>> +		return;
>> +
>> +	unprovision_and_disable_vfs(fd);
>> +	vfs_setup_started = false;
>> +	num_functions = 0;
>> +}
>> +
>>   static void stash_gt_freq(int fd, uint32_t **stash_min, uint32_t **stash_max)
>>   {
>>   	int max_gt, gt;
>> @@ -1190,28 +1213,29 @@ int igt_main()
>>   	}
>>   
>>   	igt_subtest_group() {
>> -		unsigned int num_fns;
>> -
>> -		igt_fixture()
>> -			num_fns = enable_and_provision_vfs(fd) + 1;
>> -
>>   		igt_describe("Validate engine activity on all functions");
>> -		test_each_engine("all-fn-engine-activity-load", fd, eci)
>> -			engine_activity_all_fn(fd, eci, num_fns);
>> +		test_each_engine("all-fn-engine-activity-load", fd, eci) {
>> +			ensure_vfs_enabled(fd);
>> +			engine_activity_all_fn(fd, eci, num_functions);
>> +		}
>>   
>>   		igt_describe("Validate per-function engine activity");
>> -		test_each_engine("fn-engine-activity-load", fd, eci)
>> -			for (int fn = 0; fn < num_fns; fn++)
>> -				engine_activity_fn(fd, eci, fn, XE_SRIOV_SCHED_PRIORITY_LOW);
>> +		test_each_engine("fn-engine-activity-load", fd, eci) {
>> +			ensure_vfs_enabled(fd);
>> +			for (int fn = 0; fn < num_functions; fn++)
>> +				engine_activity_fn(fd, eci, fn,
>> +						   XE_SRIOV_SCHED_PRIORITY_LOW);
>> +		}
>>   
>>   		igt_describe("Validate per-function engine activity when sched-if-idle is set");
>>   		test_each_engine("fn-engine-activity-sched-if-idle", fd, eci) {
>> -			for (int fn = 0; fn < num_fns; fn++)
>> +			ensure_vfs_enabled(fd);
>> +			for (int fn = 0; fn < num_functions; fn++)
>>   				engine_activity_fn(fd, eci, fn, XE_SRIOV_SCHED_PRIORITY_NORMAL);
>>   		}
>>   
>>   		igt_fixture()
>> -			unprovision_and_disable_vfs(fd);
>> +			cleanup_vfs_setup(fd);
>>   	}
>>   
>>   	igt_subtest_group() {
>> -- 
>> 2.43.0