Re: [PATCH v3 1/2] drm/xe/vf: Late fixups of LRCs after VF migration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] drm/xe/vf: unchecked drmm_mutex_init() in
  xe_gt_sriov_vf_init_early()
- [Low] drm/xe/vf: inaccurate commit message on LRC fixups for
  registered queues
--

commit 23a112cec6cc9cc925edc77bd85cca15d49b52e4
Author: Tomasz Lis <tomasz.lis@intel.com>

drm/xe/vf: Late fixups of LRCs after VF migration

This commit moves LRC HWSP GGTT rebasing out of vf_post_migration_fixups()
to register_exec_queue() and guc_exec_queue_unpause_prepare(), protecting
the migration scratch buffer with a drm-managed mutex.

> Do not fix LRCs of existing contexts during VF post-migration
> recovery. Instead, perform fixups before every exec queue
> registration.

[Severity: Low]
Is this commit message description accurate for already-registered
contexts?

Contexts that are already registered across VF migration do not run
register_exec_queue() again; instead, their LRCs are still rebased during
post-migration recovery in guc_exec_queue_unpause_prepare() (via
vf_post_migration_recovery() -> vf_post_migration_rearm() ->
xe_guc_submit_unpause_prepare_vf()).  Only unregistered contexts defer LRC
fixups until register_exec_queue().

> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> index 2a5f2064ad3e8..4412ed6eb422f 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
[ ... ]
> @@ -1498,17 +1494,19 @@ static void vf_migration_fini(void *arg)
>   */
>  int xe_gt_sriov_vf_init_early(struct xe_gt *gt)
>  {
> +	struct xe_device *xe = gt_to_xe(gt);
>  	void *buf;
>  
> -	if (!xe_sriov_vf_migration_supported(gt_to_xe(gt)))
> +	if (!xe_sriov_vf_migration_supported(xe))
>  		return 0;
>  
> -	buf = drmm_kmalloc(&gt_to_xe(gt)->drm,
> -			   post_migration_scratch_size(gt_to_xe(gt)),
> +	buf = drmm_kmalloc(&xe->drm,
> +			   post_migration_scratch_size(xe),
>  			   GFP_KERNEL);
>  	if (!buf)
>  		return -ENOMEM;
>  
> +	drmm_mutex_init(&xe->drm, &gt->sriov.vf.migration.scratch_lock);
>  	gt->sriov.vf.migration.scratch = buf;

[Severity: Medium]
Should xe_gt_sriov_vf_init_early() check and return the error from
drmm_mutex_init()?

drmm_mutex_init() expands to drmm_add_action_or_reset(), which is annotated
__must_check and triggers a -Wunused-result warning (breaking builds with
-Werror).  If the drmres allocation fails, __drmm_mutex_release() destroys
scratch_lock while xe_gt_sriov_vf_init_early() returns 0, leaving
xe_exec_queue_lrcs_hwsp_rebase() to lock a destroyed mutex.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106215700.1218318-1-tomasz.lis@intel.com?part=1