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

["Piotr Piórkowski" <piotr.piorkowski@intel.com>]

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.

> +}
> +
>  /**
>   * 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>

> -- 
> 2.47.1
> 

--