Re: [PATCH v2 03/17] drm/xe/pf: Add _locked variants of the VF EQ config functions

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 10/29/2025 9:47 AM, Piotr Piórkowski wrote:
> Michal Wajdeczko <michal.wajdeczko@intel.com> wrote on wto [2025-paź-28 18:58:17 +0100]:
>> In upcoming patches we will want to configure VF's execution
>> quantum (EQ) on all GTs under single lock to avoid potential
>> races in parallel GT configuration attempts.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c | 58 +++++++++++++++++-----
>>  drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h |  4 ++
>>  2 files changed, 49 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
>> index c0c0215c0703..717f81e76b8c 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
>> @@ -1732,29 +1732,65 @@ static int pf_get_exec_quantum(struct xe_gt *gt, unsigned int vfid)
>>  }
>>  
>>  /**
>> - * xe_gt_sriov_pf_config_set_exec_quantum - Configure execution quantum for the VF.
>> + * xe_gt_sriov_pf_config_set_exec_quantum_locked() - Configure execution quantum of the VF.
>>   * @gt: the &xe_gt
>>   * @vfid: the VF identifier
>>   * @exec_quantum: requested execution quantum in milliseconds (0 is infinity)
>>   *
>> - * This function can only be called on PF.
>> + * This function can only be called on PF with the master mutex hold.
>> + * It will log the provisioned value or an error in case of the failure.
>>   *
>>   * Return: 0 on success or a negative error code on failure.
>>   */
>> -int xe_gt_sriov_pf_config_set_exec_quantum(struct xe_gt *gt, unsigned int vfid,
>> -					   u32 exec_quantum)
>> +int xe_gt_sriov_pf_config_set_exec_quantum_locked(struct xe_gt *gt, unsigned int vfid,
>> +						  u32 exec_quantum)
>>  {
>>  	int err;
>>  
>> -	mutex_lock(xe_gt_sriov_pf_master_mutex(gt));
>> +	lockdep_assert_held(xe_gt_sriov_pf_master_mutex(gt));
>> +
>>  	err = pf_provision_exec_quantum(gt, vfid, exec_quantum);
>> -	mutex_unlock(xe_gt_sriov_pf_master_mutex(gt));
>>  
>>  	return pf_config_set_u32_done(gt, vfid, exec_quantum,
>> -				      xe_gt_sriov_pf_config_get_exec_quantum(gt, vfid),
>> +				      pf_get_exec_quantum(gt, vfid),
>>  				      "execution quantum", exec_quantum_unit, err);
>>  }
>>  
>> +/**
>> + * xe_gt_sriov_pf_config_set_exec_quantum - Configure execution quantum for the VF.
>> + * @gt: the &xe_gt
>> + * @vfid: the VF identifier
>> + * @exec_quantum: requested execution quantum in milliseconds (0 is infinity)
>> + *
>> + * This function can only be called on PF.
>> + * It will log the provisioned value or na error in case of the failure.
> 
> typo: na -> an
>> + *
>> + * Return: 0 on success or a negative error code on failure.
>> + */
>> +int xe_gt_sriov_pf_config_set_exec_quantum(struct xe_gt *gt, unsigned int vfid,
>> +					   u32 exec_quantum)
>> +{
>> +	guard(mutex)(xe_gt_sriov_pf_master_mutex(gt));
>> +
>> +	return xe_gt_sriov_pf_config_set_exec_quantum_locked(gt, vfid, exec_quantum);
>> +}
>> +
>> +/**
>> + * xe_gt_sriov_pf_config_get_exec_quantum_locked() - Get VF's execution quantum.
>> + * @gt: the &xe_gt
>> + * @vfid: the VF identifier
>> + *
>> + * This function can only be called on PF with the master mutex hold.
>> + *
>> + * Return: VF's (or PF's) execution quantum in milliseconds.
>> + */
>> +u32 xe_gt_sriov_pf_config_get_exec_quantum_locked(struct xe_gt *gt, unsigned int vfid)
>> +{
>> +	lockdep_assert_held(xe_gt_sriov_pf_master_mutex(gt));
>> +
>> +	return pf_get_exec_quantum(gt, vfid);
> 
> NIT: Perhaps it would have been better to be consistent and call it the locked version here.

almost all static functions in this file are expecting lock to be
already taken, also enforced with lockdep_assert_held, and we use
_locked suffix only public functions (as those usually are _not_
expecting the lock)

> 
>> +}
>> +
>>  /**
>>   * xe_gt_sriov_pf_config_get_exec_quantum - Get VF's execution quantum.
>>   * @gt: the &xe_gt
>> @@ -1766,13 +1802,9 @@ int xe_gt_sriov_pf_config_set_exec_quantum(struct xe_gt *gt, unsigned int vfid,
>>   */
>>  u32 xe_gt_sriov_pf_config_get_exec_quantum(struct xe_gt *gt, unsigned int vfid)
>>  {
>> -	u32 exec_quantum;
>> +	guard(mutex)(xe_gt_sriov_pf_master_mutex(gt));
>>  
>> -	mutex_lock(xe_gt_sriov_pf_master_mutex(gt));
>> -	exec_quantum = pf_get_exec_quantum(gt, vfid);
>> -	mutex_unlock(xe_gt_sriov_pf_master_mutex(gt));
>> -
>> -	return exec_quantum;
>> +	return pf_get_exec_quantum(gt, vfid);
>>  }
>>  
>>  static const char *preempt_timeout_unit(u32 preempt_timeout)
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h
>> index 513e6512a575..b4beb5a97031 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h
>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h
>> @@ -40,6 +40,10 @@ int xe_gt_sriov_pf_config_bulk_set_lmem(struct xe_gt *gt, unsigned int vfid, uns
>>  u32 xe_gt_sriov_pf_config_get_exec_quantum(struct xe_gt *gt, unsigned int vfid);
>>  int xe_gt_sriov_pf_config_set_exec_quantum(struct xe_gt *gt, unsigned int vfid, u32 exec_quantum);
>>  
>> +u32 xe_gt_sriov_pf_config_get_exec_quantum_locked(struct xe_gt *gt, unsigned int vfid);
>> +int xe_gt_sriov_pf_config_set_exec_quantum_locked(struct xe_gt *gt, unsigned int vfid,
>> +						  u32 exec_quantum);
>> +
>>  u32 xe_gt_sriov_pf_config_get_preempt_timeout(struct xe_gt *gt, unsigned int vfid);
>>  int xe_gt_sriov_pf_config_set_preempt_timeout(struct xe_gt *gt, unsigned int vfid,
>>  					      u32 preempt_timeout);
> 
> but anyway it looks good:
> Reviewed-by: Piotr Piórkowski <piotr.piorkowski@intel.com>

thanks!

> 
>> -- 
>> 2.47.1
>>
>