Re: [PATCH i-g-t v2] tests/intel/xe_pmu: Do not enable VFs in unrelated subtests

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

On 10/6/2026 4:11 PM, Gustavo Sousa wrote:
> The test xe_pmu has a subtest group that enables SRIOV VFs as part of
> its fixture.  Even though the fixture is part of a subtest group, the
> enabling/disabling will happen regardless of whether those subests are
> selected or not (via --run-subtest).
>
> As most subtests in xe_pmu don't need that, avoid doing that as a
> fixture and rather incorporate the enabling/disabling as part of a new
> macro called test_each_engine_with_vfs().
>
> v2:
>    - Add "intel" to commit subject prefix. (Kamil)
>    - Fix argument name of test_each_engine_with_vfs():
>      s/num_vfs/num_fns/. (Kamil)
>    - Enable/disable VFs only once per macro call by moving the call to
>      xe_for_each_engine() inside the for-loop. (Marcin)
>
> Cc: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>   tests/intel/xe_pmu.c | 21 ++++++++++++---------
>   1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
> index d41ebba1faef..eda56c078653 100644
> --- a/tests/intel/xe_pmu.c
> +++ b/tests/intel/xe_pmu.c
> @@ -124,6 +124,15 @@ static bool has_engine_active_ticks;
>   			igt_dynamic_f("engine-%s%d", xe_engine_class_string(hwe->engine_class), \
>   				      hwe->engine_instance)
>   
> +#define test_each_engine_with_vfs(test, fd, hwe, num_fns) \
> +	igt_subtest_with_dynamic(test) \
> +		for (num_fns = enable_and_provision_vfs(fd) + 1; \
> +		     num_fns; \
> +		     unprovision_and_disable_vfs(fd), num_fns = 0) \
> +			xe_for_each_engine(fd, hwe) \
> +				igt_dynamic_f("engine-%s%d", xe_engine_class_string(hwe->engine_class), \
> +					      hwe->engine_instance)
> +

igt_assert (from enable_and_provision_vfs) before entering igt_dynamic 
can hit an internal assertion and abort.

Could we move VF setup into the dynamic case and reuse the enabled VFs 
across engines, like xe_sriov_scheduling does?

Unfortunately it complicates the code, adds state and cleanup logic, but 
I don’t see a simpler way to keep setup assertions safe.

>   static bool has_event(const char *device, const char *event)
>   {
>   	char buf[512];
> @@ -1192,26 +1201,20 @@ 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)
> +		test_each_engine_with_vfs("all-fn-engine-activity-load", fd, eci, num_fns)
>   			engine_activity_all_fn(fd, eci, num_fns);
>   
>   		igt_describe("Validate per-function engine activity");
> -		test_each_engine("fn-engine-activity-load", fd, eci)
> +		test_each_engine_with_vfs("fn-engine-activity-load", fd, eci, num_fns)
>   			for (int fn = 0; fn < num_fns; 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) {
> +		test_each_engine_with_vfs("fn-engine-activity-sched-if-idle", fd, eci, num_fns) {
>   			for (int fn = 0; fn < num_fns; fn++)
>   				engine_activity_fn(fd, eci, fn, XE_SRIOV_SCHED_PRIORITY_NORMAL);
>   		}
> -
> -		igt_fixture()
> -			unprovision_and_disable_vfs(fd);
>   	}
>   
>   	igt_subtest_group() {
>
> ---
> base-commit: faad2cd0e3e8eb2e18111c2358823a7bdfb56d94
> change-id: 20261005-xe_pmu-do-not-enable-vfs-in-unrelated-subtests-e24c1cd95d71
>
> Best regards,
> --
> Gustavo Sousa <gustavo.sousa@intel.com>