[PATCH] tests/intel/xe_pmu: Tie preempt timeout to exec quantum

[PATCH] tests/intel/xe_pmu: Tie preempt timeout to exec quantum

[Jonathan Cavitt]

When updating the exec quantum in enable_and_provision_vfs, update the
preempt timeout value to be double the exec quantum value.  Also, reset
the preempt timeout value on test end in unprovision_and_disable_vfs.
The value for the preempt timeout here was chosen because it's what
we've used historically for cases like this.

Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
---
 tests/intel/xe_pmu.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
index 9525655ba2..e7c2d82da4 100644
--- a/tests/intel/xe_pmu.c
+++ b/tests/intel/xe_pmu.c
@@ -987,12 +987,18 @@ static unsigned int enable_and_provision_vfs(int fd)
 	num_vfs = igt_sriov_get_enabled_vfs(fd);
 	igt_require(num_vfs == 2);
 
-	/* Set 32ms for VF execution quantum and 64ms for PF execution quantum */
+	/*
+	 * Set 32ms for VF execution quantum and 64ms for PF execution quantum.
+	 * Set the preempt timeout for the VF and PF to be double their execution quantum.
+	 */
 	xe_for_each_gt(fd, gt) {
 		xe_sriov_set_sched_if_idle(fd, gt, 0);
-		for (int fn = 0; fn <= num_vfs; fn++)
+		for (int fn = 0; fn <= num_vfs; fn++) {
 			xe_sriov_set_exec_quantum_ms(fd, fn, gt, fn ? vf_exec_quantum :
 						     pf_exec_quantum);
+			xe_sriov_set_preempt_timeout_us(fd, fn, gt, 2000 *
+							(fn ? vf_exec_quantum : pf_exec_quantum));
+		}
 	}
 
 	/* probe VFs */
@@ -1011,8 +1017,10 @@ static void unprovision_and_disable_vfs(int fd)
 
 	xe_for_each_gt(fd, gt) {
 		xe_sriov_set_sched_if_idle(fd, gt, 0);
-		for (int fn = 0; fn <= num_vfs; fn++)
+		for (int fn = 0; fn <= num_vfs; fn++) {
 			xe_sriov_set_exec_quantum_ms(fd, fn, gt, 0);
+			xe_sriov_set_preempt_timeout_us(fd, fn, gt, 0);
+		}
 	}
 
 	xe_sriov_disable_vfs_restore_auto_provisioning(fd);
-- 
2.43.0

Re: [PATCH] tests/intel/xe_pmu: Tie preempt timeout to exec quantum

[Michal Wajdeczko]

On 11/6/2025 11:12 PM, Jonathan Cavitt wrote:
> When updating the exec quantum in enable_and_provision_vfs, update the
> preempt timeout value to be double the exec quantum value.  Also, reset
> the preempt timeout value on test end in unprovision_and_disable_vfs.
> The value for the preempt timeout here was chosen because it's what
> we've used historically for cases like this.

while PT = 2x EQ was a common setting for VFs expected to run regular WLs,
I'm not sure that this would be a correct value for this particular test,
which verifies reporting of the engine activity - maybe PT shall be very
small or equal PT (depending on actual WL's type submitted by the test)

> 
> Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> ---
>  tests/intel/xe_pmu.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
> index 9525655ba2..e7c2d82da4 100644
> --- a/tests/intel/xe_pmu.c
> +++ b/tests/intel/xe_pmu.c
> @@ -987,12 +987,18 @@ static unsigned int enable_and_provision_vfs(int fd)
>  	num_vfs = igt_sriov_get_enabled_vfs(fd);
>  	igt_require(num_vfs == 2);
>  
> -	/* Set 32ms for VF execution quantum and 64ms for PF execution quantum */
> +	/*
> +	 * Set 32ms for VF execution quantum and 64ms for PF execution quantum.
> +	 * Set the preempt timeout for the VF and PF to be double their execution quantum.
> +	 */
>  	xe_for_each_gt(fd, gt) {
>  		xe_sriov_set_sched_if_idle(fd, gt, 0);
> -		for (int fn = 0; fn <= num_vfs; fn++)
> +		for (int fn = 0; fn <= num_vfs; fn++) {
>  			xe_sriov_set_exec_quantum_ms(fd, fn, gt, fn ? vf_exec_quantum :
>  						     pf_exec_quantum);
> +			xe_sriov_set_preempt_timeout_us(fd, fn, gt, 2000 *
> +							(fn ? vf_exec_quantum : pf_exec_quantum));

maybe better to have some temp vars/const with these pf/vf preempt_timeout values
like it is done for vf/pf exec_quantum
so we will have them defined together

> +		}
>  	}
>  
>  	/* probe VFs */
> @@ -1011,8 +1017,10 @@ static void unprovision_and_disable_vfs(int fd)
>  
>  	xe_for_each_gt(fd, gt) {
>  		xe_sriov_set_sched_if_idle(fd, gt, 0);
> -		for (int fn = 0; fn <= num_vfs; fn++)
> +		for (int fn = 0; fn <= num_vfs; fn++) {
>  			xe_sriov_set_exec_quantum_ms(fd, fn, gt, 0);
> +			xe_sriov_set_preempt_timeout_us(fd, fn, gt, 0);
> +		}
>  	}
>  
>  	xe_sriov_disable_vfs_restore_auto_provisioning(fd);

RE: [PATCH] tests/intel/xe_pmu: Tie preempt timeout to exec quantum

[Cavitt, Jonathan]

-----Original Message-----
From: Wajdeczko, Michal <Michal.Wajdeczko@intel.com> 
Sent: Thursday, November 6, 2025 2:35 PM
To: Cavitt, Jonathan <jonathan.cavitt@intel.com>; igt-dev@lists.freedesktop.org
Cc: Gupta, saurabhg <saurabhg.gupta@intel.com>; Zuo, Alex <alex.zuo@intel.com>
Subject: Re: [PATCH] tests/intel/xe_pmu: Tie preempt timeout to exec quantum
> 
> On 11/6/2025 11:12 PM, Jonathan Cavitt wrote:
> > When updating the exec quantum in enable_and_provision_vfs, update the
> > preempt timeout value to be double the exec quantum value.  Also, reset
> > the preempt timeout value on test end in unprovision_and_disable_vfs.
> > The value for the preempt timeout here was chosen because it's what
> > we've used historically for cases like this.
> 
> while PT = 2x EQ was a common setting for VFs expected to run regular WLs,
> I'm not sure that this would be a correct value for this particular test,
> which verifies reporting of the engine activity - maybe PT shall be very
> small or equal PT (depending on actual WL's type submitted by the test)
> 
> > 
> > Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > ---
> >  tests/intel/xe_pmu.c | 14 +++++++++++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
> > index 9525655ba2..e7c2d82da4 100644
> > --- a/tests/intel/xe_pmu.c
> > +++ b/tests/intel/xe_pmu.c
> > @@ -987,12 +987,18 @@ static unsigned int enable_and_provision_vfs(int fd)
> >  	num_vfs = igt_sriov_get_enabled_vfs(fd);
> >  	igt_require(num_vfs == 2);
> >  
> > -	/* Set 32ms for VF execution quantum and 64ms for PF execution quantum */
> > +	/*
> > +	 * Set 32ms for VF execution quantum and 64ms for PF execution quantum.
> > +	 * Set the preempt timeout for the VF and PF to be double their execution quantum.
> > +	 */
> >  	xe_for_each_gt(fd, gt) {
> >  		xe_sriov_set_sched_if_idle(fd, gt, 0);
> > -		for (int fn = 0; fn <= num_vfs; fn++)
> > +		for (int fn = 0; fn <= num_vfs; fn++) {
> >  			xe_sriov_set_exec_quantum_ms(fd, fn, gt, fn ? vf_exec_quantum :
> >  						     pf_exec_quantum);
> > +			xe_sriov_set_preempt_timeout_us(fd, fn, gt, 2000 *
> > +							(fn ? vf_exec_quantum : pf_exec_quantum));
> 
> maybe better to have some temp vars/const with these pf/vf preempt_timeout values
> like it is done for vf/pf exec_quantum
> so we will have them defined together

Given that the preempt timeout value is going to be a fixed multiple of the exec quantum
(exact multiplier undecided, probably just 1 given your prior comment), it probably
doesn't make much sense to define the same values in two different locations.

What I can do, however, is define a temporary variable to store the target exec quantum
for the given fn number and just operate on that.  Something like:

"""
xe_for_each_gt(fd, gt) {
                xe_sriov_set_sched_if_idle(fd, gt, 0);
                for (int fn = 0; fn <= num_vfs; fn++) {
                        target_dur_ms = VF_EXEC_QUANTUM; // VF_EXEC_QUANTUM = 32
                        target_dur_ms *= fn ? 1 : PF_MULTIPLIER; // PF_MULTIPLIER = 2
                        xe_sriov_set_exec_quantum_ms(fd, fn, gt, target_dur_ms);
                        xe_sriov_set_preempt_timeout_us(fd, fn, gt,
                                                        PF_TO_EQ_RATIO * target_dur_ms); // PF_TO_EQ_RATIO = 1000
                }
        }
"""

Would that be amenable?

-Jonathan Cavitt

> 
> > +		}
> >  	}
> >  
> >  	/* probe VFs */
> > @@ -1011,8 +1017,10 @@ static void unprovision_and_disable_vfs(int fd)
> >  
> >  	xe_for_each_gt(fd, gt) {
> >  		xe_sriov_set_sched_if_idle(fd, gt, 0);
> > -		for (int fn = 0; fn <= num_vfs; fn++)
> > +		for (int fn = 0; fn <= num_vfs; fn++) {
> >  			xe_sriov_set_exec_quantum_ms(fd, fn, gt, 0);
> > +			xe_sriov_set_preempt_timeout_us(fd, fn, gt, 0);
> > +		}
> >  	}
> >  
> >  	xe_sriov_disable_vfs_restore_auto_provisioning(fd);
> 
> 

RE: [PATCH] tests/intel/xe_pmu: Tie preempt timeout to exec quantum

[Cavitt, Jonathan]

-----Original Message-----
From: Cavitt, Jonathan <jonathan.cavitt@intel.com> 
Sent: Thursday, November 6, 2025 2:49 PM
To: Wajdeczko, Michal <Michal.Wajdeczko@intel.com>; igt-dev@lists.freedesktop.org
Cc: Gupta, saurabhg <saurabhg.gupta@intel.com>; Zuo, Alex <alex.zuo@intel.com>; Cavitt, Jonathan <jonathan.cavitt@intel.com>
Subject: RE: [PATCH] tests/intel/xe_pmu: Tie preempt timeout to exec quantum
> 
> -----Original Message-----
> From: Wajdeczko, Michal <Michal.Wajdeczko@intel.com> 
> Sent: Thursday, November 6, 2025 2:35 PM
> To: Cavitt, Jonathan <jonathan.cavitt@intel.com>; igt-dev@lists.freedesktop.org
> Cc: Gupta, saurabhg <saurabhg.gupta@intel.com>; Zuo, Alex <alex.zuo@intel.com>
> Subject: Re: [PATCH] tests/intel/xe_pmu: Tie preempt timeout to exec quantum
> > 
> > On 11/6/2025 11:12 PM, Jonathan Cavitt wrote:
> > > When updating the exec quantum in enable_and_provision_vfs, update the
> > > preempt timeout value to be double the exec quantum value.  Also, reset
> > > the preempt timeout value on test end in unprovision_and_disable_vfs.
> > > The value for the preempt timeout here was chosen because it's what
> > > we've used historically for cases like this.
> > 
> > while PT = 2x EQ was a common setting for VFs expected to run regular WLs,
> > I'm not sure that this would be a correct value for this particular test,
> > which verifies reporting of the engine activity - maybe PT shall be very
> > small or equal PT (depending on actual WL's type submitted by the test)
> > 
> > > 
> > > Suggested-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > > ---
> > >  tests/intel/xe_pmu.c | 14 +++++++++++---
> > >  1 file changed, 11 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
> > > index 9525655ba2..e7c2d82da4 100644
> > > --- a/tests/intel/xe_pmu.c
> > > +++ b/tests/intel/xe_pmu.c
> > > @@ -987,12 +987,18 @@ static unsigned int enable_and_provision_vfs(int fd)
> > >  	num_vfs = igt_sriov_get_enabled_vfs(fd);
> > >  	igt_require(num_vfs == 2);
> > >  
> > > -	/* Set 32ms for VF execution quantum and 64ms for PF execution quantum */
> > > +	/*
> > > +	 * Set 32ms for VF execution quantum and 64ms for PF execution quantum.
> > > +	 * Set the preempt timeout for the VF and PF to be double their execution quantum.
> > > +	 */
> > >  	xe_for_each_gt(fd, gt) {
> > >  		xe_sriov_set_sched_if_idle(fd, gt, 0);
> > > -		for (int fn = 0; fn <= num_vfs; fn++)
> > > +		for (int fn = 0; fn <= num_vfs; fn++) {
> > >  			xe_sriov_set_exec_quantum_ms(fd, fn, gt, fn ? vf_exec_quantum :
> > >  						     pf_exec_quantum);
> > > +			xe_sriov_set_preempt_timeout_us(fd, fn, gt, 2000 *
> > > +							(fn ? vf_exec_quantum : pf_exec_quantum));
> > 
> > maybe better to have some temp vars/const with these pf/vf preempt_timeout values
> > like it is done for vf/pf exec_quantum
> > so we will have them defined together
> 
> Given that the preempt timeout value is going to be a fixed multiple of the exec quantum
> (exact multiplier undecided, probably just 1 given your prior comment), it probably
> doesn't make much sense to define the same values in two different locations.
> 
> What I can do, however, is define a temporary variable to store the target exec quantum
> for the given fn number and just operate on that.  Something like:
> 
> """
> xe_for_each_gt(fd, gt) {
>                 xe_sriov_set_sched_if_idle(fd, gt, 0);
>                 for (int fn = 0; fn <= num_vfs; fn++) {
>                         target_dur_ms = VF_EXEC_QUANTUM; // VF_EXEC_QUANTUM = 32
>                         target_dur_ms *= fn ? 1 : PF_MULTIPLIER; // PF_MULTIPLIER = 2
>                         xe_sriov_set_exec_quantum_ms(fd, fn, gt, target_dur_ms);
>                         xe_sriov_set_preempt_timeout_us(fd, fn, gt,
>                                                         PF_TO_EQ_RATIO * target_dur_ms); // PF_TO_EQ_RATIO = 1000
>                 }
>         }
> """
> 
> Would that be amenable?

Okay, I'm just going to apply this change and if it's not agreeable, you can let me know
in v2 where it's applied.
-Jonathan Cavitt

> 
> -Jonathan Cavitt
> 
> > 
> > > +		}
> > >  	}
> > >  
> > >  	/* probe VFs */
> > > @@ -1011,8 +1017,10 @@ static void unprovision_and_disable_vfs(int fd)
> > >  
> > >  	xe_for_each_gt(fd, gt) {
> > >  		xe_sriov_set_sched_if_idle(fd, gt, 0);
> > > -		for (int fn = 0; fn <= num_vfs; fn++)
> > > +		for (int fn = 0; fn <= num_vfs; fn++) {
> > >  			xe_sriov_set_exec_quantum_ms(fd, fn, gt, 0);
> > > +			xe_sriov_set_preempt_timeout_us(fd, fn, gt, 0);
> > > +		}
> > >  	}
> > >  
> > >  	xe_sriov_disable_vfs_restore_auto_provisioning(fd);
> > 
> > 
>