Re: [PATCH v5 3/5] drm/xe/vf: Fix GuC FW check for VF migration support

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

On 10/21/2025 2:39 PM, Tomasz Lis wrote:
> The check whether GuC ABI version meets requirements shall be
> performed after said version is received from GuC.
> 
> Doing it in wrong order was triggering a warning:
> xe 0000:00:02.1: [drm] Assertion `gt->sriov.vf.guc_version.major` failed!
> 
> With this change, dislodge part of the VF migration support check
> and moved it to after GuC handshake.
> 
> v2: Use xe_sriov_vf_ccs_migration_bb_needed()
> 
> v3: Update commit message, move check funct to ccs module (Michal),
>  rename xe_sriov_vf_migration_disable(), remove its duplicate
> 
> v4: Limit scope of some functions to xe_sriov_vf_ccs file,
>  switched 'Fixes:' tag to a different commit (Michal)

btw, please move change log under ---

> 
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Tested-by: Matthew Brost <matthew.brost@intel.com> #v1
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/6349
> Fixes: ff1d2b5e3d28 ("drm/xe: Read VF GMD_ID with a specifically-allocated dummy GT")
> Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_sriov_vf.c     | 29 ++++++----------------
>  drivers/gpu/drm/xe/xe_sriov_vf.h     |  1 +
>  drivers/gpu/drm/xe/xe_sriov_vf_ccs.c | 37 ++++++++++++++++++++++++----
>  drivers/gpu/drm/xe/xe_sriov_vf_ccs.h |  2 --
>  4 files changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_sriov_vf.c b/drivers/gpu/drm/xe/xe_sriov_vf.c
> index 5ad39e315cb5..0a1331db2a9a 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_vf.c
> +++ b/drivers/gpu/drm/xe/xe_sriov_vf.c
> @@ -133,7 +133,7 @@ bool xe_sriov_vf_migration_supported(struct xe_device *xe)
>  	return !xe->sriov.vf.migration.disabled;
>  }
>  
> -static void vf_disable_migration(struct xe_device *xe, const char *fmt, ...)

as it is promoted to public function, now it needs a kernel-doc

> +void xe_sriov_vf_migration_disable(struct xe_device *xe, const char *fmt, ...)
>  {
>  	struct va_format vaf;
>  	va_list va_args;
> @@ -156,25 +156,15 @@ static void vf_migration_init_early(struct xe_device *xe)
>  	 * supported at production quality.
>  	 */
>  	if (!IS_ENABLED(CONFIG_DRM_XE_DEBUG))
> -		return vf_disable_migration(xe,
> -					    "experimental feature not available on production builds");
> +		return xe_sriov_vf_migration_disable(xe,
> +				"experimental feature not available on production builds");
>  
>  	if (GRAPHICS_VER(xe) < 20)
> -		return vf_disable_migration(xe, "requires gfx version >= 20, but only %u found",
> -					    GRAPHICS_VER(xe));
> -
> -	if (!IS_DGFX(xe)) {
> -		struct xe_uc_fw_version guc_version;
> -
> -		xe_gt_sriov_vf_guc_versions(xe_device_get_gt(xe, 0), NULL, &guc_version);
> -		if (MAKE_GUC_VER_STRUCT(guc_version) < MAKE_GUC_VER(1, 23, 0))
> -			return vf_disable_migration(xe,
> -						    "CCS migration requires GuC ABI >= 1.23 but only %u.%u found",
> -						    guc_version.major, guc_version.minor);
> -	}
> +		return xe_sriov_vf_migration_disable(xe,
> +				"requires gfx version >= 20, but only %u found",
> +				GRAPHICS_VER(xe));
>  
>  	xe->sriov.vf.migration.disabled = false;
> -	xe_sriov_dbg(xe, "migration support enabled\n");
>  }
>  
>  /**
> @@ -196,12 +186,7 @@ void xe_sriov_vf_init_early(struct xe_device *xe)
>   */
>  int xe_sriov_vf_init_late(struct xe_device *xe)
>  {
> -	int err = 0;
> -
> -	if (xe_sriov_vf_migration_supported(xe))
> -		err = xe_sriov_vf_ccs_init(xe);
> -
> -	return err;
> +	return xe_sriov_vf_ccs_init(xe);
>  }
>  
>  static int sa_info_vf_ccs(struct seq_file *m, void *data)
> diff --git a/drivers/gpu/drm/xe/xe_sriov_vf.h b/drivers/gpu/drm/xe/xe_sriov_vf.h
> index 4df95266b261..e967d4166a43 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_vf.h
> +++ b/drivers/gpu/drm/xe/xe_sriov_vf.h
> @@ -14,6 +14,7 @@ struct xe_device;
>  void xe_sriov_vf_init_early(struct xe_device *xe);
>  int xe_sriov_vf_init_late(struct xe_device *xe);
>  bool xe_sriov_vf_migration_supported(struct xe_device *xe);
> +void xe_sriov_vf_migration_disable(struct xe_device *xe, const char *fmt, ...);
>  void xe_sriov_vf_debugfs_register(struct xe_device *xe, struct dentry *root);
>  
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_sriov_vf_ccs.c b/drivers/gpu/drm/xe/xe_sriov_vf_ccs.c
> index a2d61b37ff21..11851c18513b 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_vf_ccs.c
> +++ b/drivers/gpu/drm/xe/xe_sriov_vf_ccs.c
> @@ -10,6 +10,8 @@
>  #include "xe_device.h"
>  #include "xe_exec_queue.h"
>  #include "xe_exec_queue_types.h"
> +#include "xe_gt_sriov_vf.h"
> +#include "xe_guc.h"
>  #include "xe_guc_submit.h"
>  #include "xe_lrc.h"
>  #include "xe_migrate.h"
> @@ -260,21 +262,44 @@ int xe_sriov_vf_ccs_register_context(struct xe_device *xe)
>  	return err;
>  }
>  
> -/**
> - * xe_sriov_vf_migration_ccs_bb_needed - Whether GuC requires CCS copy BBs for VF migration.
> +/*
> + * Whether GuC requires CCS copy BBs for VF migration.
>   * @xe: the &xe_device instance.
>   *
>   * Only selected platforms require VF KMD to maintain CCS copy BBs and linked LRCAs.
>   *
>   * Return: true if VF driver must participate in the CCS migration, false otherwise.
>   */
> -bool xe_sriov_vf_migration_ccs_bb_needed(struct xe_device *xe)
> +static bool vf_migration_ccs_bb_needed(struct xe_device *xe)
>  {
>  	xe_assert(xe, IS_SRIOV_VF(xe));
>  
>  	return !IS_DGFX(xe) && xe_device_has_flat_ccs(xe);
>  }
>  
> +/*
> + * Check for disable migration due to no CCS BBs support in GuC FW.
> + * @xe: the &xe_device instance.
> + *
> + * Performs late disable of VF migration feature in case GuC FW cannot support it.
> + */
> +static void vf_migration_ccs_bb_support_check(struct xe_device *xe)

if you make this function bool

> +{
> +	if (!xe_sriov_vf_migration_supported(xe))
> +		return;
> +
> +	if (vf_migration_ccs_bb_needed(xe)) {
> +		struct xe_gt *gt = xe_device_get_gt(xe, 0);
> +		struct xe_uc_fw_version guc_version;
> +
> +		xe_gt_sriov_vf_guc_versions(gt, NULL, &guc_version);
> +		if (MAKE_GUC_VER_STRUCT(guc_version) < MAKE_GUC_VER(1, 23, 0))
> +			return xe_sriov_vf_migration_disable(xe,
> +				"CCS migration requires GuC ABI >= 1.23 but only %u.%u found",
> +				guc_version.major, guc_version.minor);
> +	}
> +}
> +
>  static void xe_sriov_vf_ccs_fini(void *arg)
>  {
>  	struct xe_sriov_vf_ccs_ctx *ctx = arg;
> @@ -307,9 +332,11 @@ int xe_sriov_vf_ccs_init(struct xe_device *xe)
>  	int err;
>  
>  	xe_assert(xe, IS_SRIOV_VF(xe));
> -	xe_assert(xe, xe_sriov_vf_migration_supported(xe));
>  
> -	if (!xe_sriov_vf_migration_ccs_bb_needed(xe))
> +	vf_migration_ccs_bb_support_check(xe);
> +
> +	if (!xe_sriov_vf_migration_supported(xe) ||
> +	    !vf_migration_ccs_bb_needed(xe))

then you can simplify the whole block here as:

	if (!xe_sriov_vf_migration_supported(xe))
		return 0;

	if (!vf_migration_ccs_bb_needed(xe))
		return 0;

	if (!vf_migration_ccs_bb_support_check(xe))
		return 0;

>  		return 0;
>  
>  	for_each_ccs_rw_ctx(ctx_id) {
> diff --git a/drivers/gpu/drm/xe/xe_sriov_vf_ccs.h b/drivers/gpu/drm/xe/xe_sriov_vf_ccs.h
> index 0e6b27016dac..f8ca6efce9ee 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_vf_ccs.h
> +++ b/drivers/gpu/drm/xe/xe_sriov_vf_ccs.h
> @@ -14,8 +14,6 @@ struct drm_printer;
>  struct xe_device;
>  struct xe_bo;
>  
> -bool xe_sriov_vf_migration_ccs_bb_needed(struct xe_device *xe);
> -
>  int xe_sriov_vf_ccs_init(struct xe_device *xe);
>  int xe_sriov_vf_ccs_attach_bo(struct xe_bo *bo);
>  int xe_sriov_vf_ccs_detach_bo(struct xe_bo *bo);