Re: [PATCH v2 14/34] drm/xe/vf: Remove memory allocations from VF post migration recovery

["Lis, Tomasz" <tomasz.lis@intel.com>]

On 9/24/2025 3:15 AM, Matthew Brost wrote:
> VF post migration recovery is the path of dma-fence signaling / reclaim,
> avoid memory allocations in this path.
>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_gt_sriov_vf.c       | 29 ++++++++++++++---------
>   drivers/gpu/drm/xe/xe_gt_sriov_vf.h       |  2 +-
>   drivers/gpu/drm/xe/xe_gt_sriov_vf_types.h |  2 ++
>   drivers/gpu/drm/xe/xe_sriov.c             |  8 +++++--
>   drivers/gpu/drm/xe/xe_sriov_vf.c          | 14 ++++++++---
>   drivers/gpu/drm/xe/xe_sriov_vf.h          |  2 +-
>   6 files changed, 39 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> index cfb71b749e52..8304c26c076e 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> @@ -1214,17 +1214,13 @@ static size_t post_migration_scratch_size(struct xe_device *xe)
>   
>   static int vf_post_migration_fixups(struct xe_gt *gt)
>   {
> +	void *buf = gt->sriov.vf.migration.lrc_wa_bb;

can we just call it 'scratch', or 'recovery_scratch'? The current use is 
limited to lrc, but besides temp buffer for WAs it also stores MEMIRQ regs.

This name makes it look like something more important than it is, and 
does not describe all uses.

Besides that name, LGTM.

-Tomasz

>   	s64 shift;
> -	void *buf;
>   	int err;
>   
> -	buf = kmalloc(post_migration_scratch_size(gt_to_xe(gt)), GFP_ATOMIC);
> -	if (!buf)
> -		return -ENOMEM;
> -
>   	err = xe_gt_sriov_vf_query_config(gt);
>   	if (err)
> -		goto out;
> +		return err;
>   
>   	shift = xe_gt_sriov_vf_ggtt_shift(gt);
>   	if (shift) {
> @@ -1232,12 +1228,10 @@ static int vf_post_migration_fixups(struct xe_gt *gt)
>   		xe_gt_sriov_vf_default_lrcs_hwsp_rebase(gt);
>   		err = xe_guc_contexts_hwsp_rebase(&gt->uc.guc, buf);
>   		if (err)
> -			goto out;
> +			return err;
>   	}
>   
> -out:
> -	kfree(buf);
> -	return err;
> +	return 0;
>   }
>   
>   static void vf_post_migration_kickstart(struct xe_gt *gt)
> @@ -1314,15 +1308,28 @@ static void migration_worker_func(struct work_struct *w)
>   /**
>    * xe_gt_sriov_vf_migration_init_early() - VF post migration init early
>    * @gt: the &xe_gt
> + *
> + * Return 0 on success, errno on failure
>    */
> -void xe_gt_sriov_vf_migration_init_early(struct xe_gt *gt)
> +int xe_gt_sriov_vf_migration_init_early(struct xe_gt *gt)
>   {
> +	void *buf;
> +
> +	buf = drmm_kmalloc(&gt_to_xe(gt)->drm,
> +			   post_migration_scratch_size(gt_to_xe(gt)),
> +			   GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	gt->sriov.vf.migration.lrc_wa_bb = buf;
>   	init_rwsem(&gt->sriov.vf.self_config.lock);
>   	spin_lock_init(&gt->sriov.vf.migration.lock);
>   	INIT_WORK(&gt->sriov.vf.migration.worker, migration_worker_func);
>   
>   	if (!xe_sriov_vf_migration_supported(gt_to_xe(gt)))
>   		xe_gt_sriov_info(gt, "migration not supported by this module version\n");
> +
> +	return 0;
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.h b/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
> index 2ac6775b52f0..195dbebe941e 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
> @@ -23,7 +23,7 @@ int xe_gt_sriov_vf_connect(struct xe_gt *gt);
>   int xe_gt_sriov_vf_query_runtime(struct xe_gt *gt);
>   void xe_gt_sriov_vf_migrated_event_handler(struct xe_gt *gt);
>   
> -void xe_gt_sriov_vf_migration_init_early(struct xe_gt *gt);
> +int xe_gt_sriov_vf_migration_init_early(struct xe_gt *gt);
>   bool xe_gt_sriov_vf_recovery_inprogress(struct xe_gt *gt);
>   
>   u32 xe_gt_sriov_vf_gmdid(struct xe_gt *gt);
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf_types.h b/drivers/gpu/drm/xe/xe_gt_sriov_vf_types.h
> index 53680a2f188a..496b657119de 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf_types.h
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf_types.h
> @@ -58,6 +58,8 @@ struct xe_gt_sriov_vf_migration {
>   	struct work_struct worker;
>   	/** @lock: Protects recovery_queued */
>   	spinlock_t lock;
> +	/** @lrc_wa_bb: Scratch memory for LRC WA BB in recovery */
> +	void *lrc_wa_bb;
>   	/** @recovery_queued: VF post migration recovery in queued */
>   	bool recovery_queued;
>   	/** @recovery_inprogress: VF post migration recovery in progress */
> diff --git a/drivers/gpu/drm/xe/xe_sriov.c b/drivers/gpu/drm/xe/xe_sriov.c
> index 7d2d6de2aabf..358a35c80d5a 100644
> --- a/drivers/gpu/drm/xe/xe_sriov.c
> +++ b/drivers/gpu/drm/xe/xe_sriov.c
> @@ -116,8 +116,12 @@ int xe_sriov_init(struct xe_device *xe)
>   			return err;
>   	}
>   
> -	if (IS_SRIOV_VF(xe))
> -		xe_sriov_vf_init_early(xe);
> +	if (IS_SRIOV_VF(xe)) {
> +		int err = xe_sriov_vf_init_early(xe);
> +
> +		if (err)
> +			return err;
> +	}
>   
>   	xe_assert(xe, !xe->sriov.wq);
>   	xe->sriov.wq = alloc_workqueue("xe-sriov-wq", 0, 0);
> diff --git a/drivers/gpu/drm/xe/xe_sriov_vf.c b/drivers/gpu/drm/xe/xe_sriov_vf.c
> index 7d91553c4acc..e622a7c562c4 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_vf.c
> +++ b/drivers/gpu/drm/xe/xe_sriov_vf.c
> @@ -180,16 +180,24 @@ static void vf_migration_init_early(struct xe_device *xe)
>   /**
>    * xe_sriov_vf_init_early - Initialize SR-IOV VF specific data.
>    * @xe: the &xe_device to initialize
> + *
> + * Return: 0 on success or a negative error code on failure.
>    */
> -void xe_sriov_vf_init_early(struct xe_device *xe)
> +int xe_sriov_vf_init_early(struct xe_device *xe)
>   {
>   	struct xe_gt *gt;
>   	unsigned int id;
> +	int err;
>   
> -	for_each_gt(gt, xe, id)
> -		xe_gt_sriov_vf_migration_init_early(gt);
> +	for_each_gt(gt, xe, id) {
> +		err = xe_gt_sriov_vf_migration_init_early(gt);
> +		if (err)
> +			return err;
> +	}
>   
>   	vf_migration_init_early(xe);
> +
> +	return 0;
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/xe/xe_sriov_vf.h b/drivers/gpu/drm/xe/xe_sriov_vf.h
> index 4df95266b261..13969c6910ce 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_vf.h
> +++ b/drivers/gpu/drm/xe/xe_sriov_vf.h
> @@ -11,7 +11,7 @@
>   struct dentry;
>   struct xe_device;
>   
> -void xe_sriov_vf_init_early(struct xe_device *xe);
> +int 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_debugfs_register(struct xe_device *xe, struct dentry *root);