Re: [PATCH 1/3] drm/xe/pf: Skip fair VFs provisioning if already provisioned

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

Michal Wajdeczko <michal.wajdeczko@intel.com> wrote on pon [2024-lip-01 12:27:36 +0200]:
> Our debugfs allows to view and change VFs' provisioning configs.
> 
> If we attempt to experiment with VFs provisioning before enabling
> them, this early config will affect fair provisioning calculations,
> and will also be overwritten, which is undesirable behavior.
> 
> To improve this, check if the VFs configs are empty (unprovisioned)
> before starting the fair provisioning procedure.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c | 48 ++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h |  2 +
>  drivers/gpu/drm/xe/xe_pci_sriov.c          | 13 ++++++
>  3 files changed, 63 insertions(+)
> 
> 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 694671497f6e..810b579a0025 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> @@ -1922,6 +1922,54 @@ int xe_gt_sriov_pf_config_push(struct xe_gt *gt, unsigned int vfid, bool refresh
>  	return err;
>  }
>  
> +static int pf_validate_vf_config(struct xe_gt *gt, unsigned int vfid)
> +{
> +	struct xe_gt *primary_gt = gt_to_tile(gt)->primary_gt;
> +	struct xe_device *xe = gt_to_xe(gt);
> +	bool valid_ggtt, valid_ctxs, valid_dbs;
> +	bool valid_any, valid_all;
> +
> +	valid_ggtt = pf_get_vf_config_ggtt(primary_gt, vfid);
> +	valid_ctxs = pf_get_vf_config_ctxs(gt, vfid);
> +	valid_dbs = pf_get_vf_config_dbs(gt, vfid);
> +
> +	/* note that GuC doorbells are optional */
> +	valid_any = valid_ggtt || valid_ctxs || valid_dbs;
> +	valid_all = valid_ggtt && valid_ctxs;
> +
> +	if (IS_DGFX(xe)) {
> +		bool valid_lmem = pf_get_vf_config_ggtt(primary_gt, vfid);
> +
> +		valid_any = valid_any || valid_lmem;
> +		valid_all = valid_all && valid_lmem;
> +	}
> +
> +	return valid_all ? 1 : valid_any ? -ENOKEY : -ENODATA;
> +}
> +
> +/**
> + * xe_gt_sriov_pf_config_is_empty - Check VF's configuration.
> + * @gt: the &xe_gt
> + * @vfid: the VF identifier (can't be PF)
> + *
> + * This function can only be called on PF.
> + *
> + * Return: true if VF mandatory configuration (GGTT, LMEM, ...) is empty.
> + */
> +bool xe_gt_sriov_pf_config_is_empty(struct xe_gt *gt, unsigned int vfid)
> +{
> +	bool empty;
> +
> +	xe_gt_assert(gt, IS_SRIOV_PF(gt_to_xe(gt)));
> +	xe_gt_assert(gt, vfid);
> +
> +	mutex_lock(xe_gt_sriov_pf_master_mutex(gt));
> +	empty = pf_validate_vf_config(gt, vfid) == -ENODATA;
> +	mutex_unlock(xe_gt_sriov_pf_master_mutex(gt));
> +
> +	return empty;
> +}
> +
>  /**
>   * xe_gt_sriov_pf_config_print_ggtt - Print GGTT configurations.
>   * @gt: the &xe_gt
> 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 e8238c1ad06a..58c8f879d7ab 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h
> @@ -53,6 +53,8 @@ int xe_gt_sriov_pf_config_set_fair(struct xe_gt *gt, unsigned int vfid, unsigned
>  int xe_gt_sriov_pf_config_release(struct xe_gt *gt, unsigned int vfid, bool force);
>  int xe_gt_sriov_pf_config_push(struct xe_gt *gt, unsigned int vfid, bool refresh);
>  
> +bool xe_gt_sriov_pf_config_is_empty(struct xe_gt *gt, unsigned int vfid);
> +
>  int xe_gt_sriov_pf_config_print_ggtt(struct xe_gt *gt, struct drm_printer *p);
>  int xe_gt_sriov_pf_config_print_ctxs(struct xe_gt *gt, struct drm_printer *p);
>  int xe_gt_sriov_pf_config_print_dbs(struct xe_gt *gt, struct drm_printer *p);
> diff --git a/drivers/gpu/drm/xe/xe_pci_sriov.c b/drivers/gpu/drm/xe/xe_pci_sriov.c
> index 74c8fadc9365..aaceee748287 100644
> --- a/drivers/gpu/drm/xe/xe_pci_sriov.c
> +++ b/drivers/gpu/drm/xe/xe_pci_sriov.c
> @@ -13,6 +13,17 @@
>  #include "xe_sriov_pf_helpers.h"
>  #include "xe_sriov_printk.h"
>  
> +static int pf_needs_provisioning(struct xe_gt *gt, unsigned int num_vfs)
> +{
> +	unsigned int n;
> +
> +	for (n = 1; n <= num_vfs; n++)
> +		if (!xe_gt_sriov_pf_config_is_empty(gt, n))
> +			return false;
> +
> +	return true;
> +}
> +
>  static int pf_provision_vfs(struct xe_device *xe, unsigned int num_vfs)
>  {
>  	struct xe_gt *gt;
> @@ -20,6 +31,8 @@ static int pf_provision_vfs(struct xe_device *xe, unsigned int num_vfs)
>  	int result = 0, err;
>  
>  	for_each_gt(gt, xe, id) {
> +		if (!pf_needs_provisioning(gt, num_vfs))
> +			continue;
>  		err = xe_gt_sriov_pf_config_set_fair(gt, VFID(1), num_vfs);
>  		result = result ?: err;
>  	}

LGTM:
Reviewed-by: Piotr Piórkowski <piotr.piorkowski@intel.com>

> -- 
> 2.43.0
> 

--