Re: [PATCH v2 3/4] drm/xe/vf: Start post-migration fixups with provisinoning query

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

On 24.09.2024 22:25, Tomasz Lis wrote:
> During post-migration recovery, only MMIO communication to GuC is
> allowed. The VF KMD needs to use that channel to ask for the new
> provisioning, which includes a new GGTT range assigned to the VF.

you likely need first to remove below assert from the
xe_guc_mmio_send_recv()

	xe_assert(xe, !xe_guc_ct_enabled(&guc->ct));

> 
> v2: query config only instead of handshake; no need to get pm ref as
>  it's now kept through whole recovery (mwajdeczko)
> 
> Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_sriov_vf.c | 39 ++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_sriov_vf.c b/drivers/gpu/drm/xe/xe_sriov_vf.c
> index d0c5a0b7e170..fe5eefa736c8 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_vf.c
> +++ b/drivers/gpu/drm/xe/xe_sriov_vf.c
> @@ -24,6 +24,34 @@ void xe_sriov_vf_init_early(struct xe_device *xe)
>  	INIT_WORK(&xe->sriov.vf.migration.worker, migration_worker_func);
>  }
>  
> +/**
> + * vf_post_migration_requery_guc - Re-initialize GuC communication.
> + * @xe: the &xe_device struct instance
> + *
> + * After migration, we need to reestablish communication with GuC and
> + * re-query all VF configuration to make sure they match previous
> + * provisioning. Note that most of VF provisioning shall be the same,
> + * except GGTT range, since GGTT is not virtualized per-VF.
> + *
> + * Returns: 0 if the operation completed successfully, or a negative error
> + * code otherwise.
> + */
> +static int vf_post_migration_requery_guc(struct xe_device *xe)
> +{
> +	struct xe_gt *gt;
> +	unsigned int id;
> +	int err, ret;

	int err, ret = 0;

> +
> +	err = 0;
> +	for_each_gt(gt, xe, id) {
> +		ret = xe_gt_sriov_vf_query_config(gt);

		err = xe_gt_sriov_vf_query_config(gt);

> +		if (!err)
> +			err = ret;

		ret = ret ?: err;

> +	}
> +
> +	return err;

	return ret;

> +}
> +
>  /*
>   * vf_post_migration_notify_resfix_done - Notify all GuCs about resource fixups apply finished.
>   * @xe: the &xe_device struct instance
> @@ -44,12 +72,23 @@ static void vf_post_migration_notify_resfix_done(struct xe_device *xe)
>  
>  static void vf_post_migration_recovery(struct xe_device *xe)
>  {
> +	int err;
> +
>  	drm_dbg(&xe->drm, "migration recovery in progress\n");
>  	xe_pm_runtime_get(xe);
> +	err = vf_post_migration_requery_guc(xe);
> +	if (unlikely(err))
> +		goto fail;

shouldn't all this be below "add the recovery steps" line ?

> +
>  	/* FIXME: add the recovery steps */
>  	vf_post_migration_notify_resfix_done(xe);
>  	xe_pm_runtime_put(xe);
>  	drm_notice(&xe->drm, "migration recovery ended\n");
> +	return;
> +fail:
> +	xe_pm_runtime_put(xe);
> +	drm_err(&xe->drm, "migration recovery failed (%pe)\n", ERR_PTR(err));
> +	xe_device_declare_wedged(xe);
>  }
>  
>  static void migration_worker_func(struct work_struct *w)