Re: [PATCH v12 06/10] drm/xe/migrate: Introduce xe_migrate_reinit()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe/migrate: missing ULLS teardown and race in xe_migrate_reinit()
--

commit e6b24d47b598778e5ce6fbebf60700ba786119b2
Author: Raag Jadav <raag.jadav@intel.com>

drm/xe/migrate: Introduce xe_migrate_reinit()

This commit introduces xe_migrate_reinit() and wires it into xe_gt_reinit()
to re-initialize the migration VM page tables and migration exec queue after
a PCIe FLR.

> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 19c37eb315893..71dfb1b3362a3 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
> @@ -778,6 +778,18 @@ static void xe_migrate_ulls_exit(struct work_struct *work)
>  	drm_dev_exit(idx);
>  }
>  
> +/**
> + * xe_migrate_reinit() - Re-initialize a migrate context
> + * @m: The migration context
> + *
> + * Returns: 0 on success, negative error code otherwise.
> + */
> +int xe_migrate_reinit(struct xe_migrate *m)
> +{
> +	xe_migrate_prepare_vm(m->tile, m, m->q->vm, NULL);
> +	return xe_exec_queue_reinit(m->q);
> +}

[Severity: High]
Does xe_migrate_reinit() (or the preceding FLR preparation path) also need to
stop m->ulls.exit_work and reset m->ulls.enabled along with its runtime PM
reference?

On USM-capable dGFX platforms, xe_migrate_ulls_enter() takes a runtime PM
reference via xe_pm_runtime_get_noresume(), sets m->ulls.enabled = true,
parks the migration engine on a PPHWSP semaphore in VRAM, and schedules
m->ulls.exit_work (xe_migrate_ulls_exit()) on system_percpu_wq.

During teardown, xe_migrate_fini() cleans this up before destroying the
queue:

drivers/gpu/drm/xe/xe_migrate.c:xe_migrate_fini() {
    ...
	disable_delayed_work_sync(&m->ulls.exit_work);
	scoped_guard(mutex, &m->job_mutex) {
		if (xe_migrate_ulls_enabled(m)) {
			xe_pm_runtime_put(xe);
			xe_migrate_ulls_toggle_enable(m, false);
		}
	}
    ...
}

Across PCIe FLR, however, neither xe_gt_flr_prepare() nor xe_migrate_reinit()
cancels m->ulls.exit_work or resets m->ulls.enabled:

1. If m->ulls.exit_work has not fired yet when xe_migrate_reinit() and GT
   restart finish, m->ulls.enabled remains true and the runtime PM reference
   stays held, even though PCIe FLR and xe_lrc_ctx_init() wiped VRAM, zeroed
   the LRC PPHWSP semaphores, and reset the engine out of ULLS mode.
   Subsequent calls to xe_migrate_ulls_enter() will see m->ulls.enabled set
   and skip submitting a ULLS_ENTER job, while xe_migrate_job_push() will tag
   post-FLR jobs as ULLS_ACTIVE, suppressing H2G context submission and ring
   tail updates in submit_exec_queue() and stalling migrations.

2. If xe_migrate_ulls_exit() runs after uc_flr_sanitize() has stopped the
   scheduler (or concurrently with xe_migrate_reinit(), which does not take
   m->job_mutex), xe_migrate_ulls_exit() sees xe_exec_queue_is_idle(m->q, 1)
   and calls __xe_migrate_job_push(m, job, ULLS_EXIT), which arms the job
   (advancing lrc->fence_ctx.next_seqno) and pushes it to m->q->entity. When
   xe_migrate_reinit() -> xe_exec_queue_reinit() -> xe_lrc_ctx_init() runs:

   drivers/gpu/drm/xe/xe_lrc.c:xe_lrc_ctx_init() {
       ...
       map = __xe_lrc_seqno_map(lrc);
       xe_map_write32(lrc_to_xe(lrc), &map, lrc->fence_ctx.next_seqno - 1);
       ...
   }

   it writes lrc->fence_ctx.next_seqno - 1 into the LRC seqno map,
   prematurely signaling the unsubmitted ULLS_EXIT fence seqno while the
   stale job remains queued in m->q->entity.

3. If xe_migrate_ulls_exit() runs during xe_uc_flr_prepare() in the window
   after xe_guc_submit_pause_abort() (which restarts the scheduler with m->q
   marked killed and timeout set to 0) and before uc_flr_sanitize() stops the
   scheduler, the ULLS_EXIT job is popped without emitting to the ring
   (leaving job->ptrs[0].head == 0) and queues TDR on gt->ordered_wq, which
   can either restart scheduler submission before xe_sched_reinit() asserts
   drm_sched_is_stopped() or leave the un-emitted job on pending_list so
   guc_exec_queue_start() rewinds lrc->ring.tail to 0 over the instructions
   written by xe_lrc_ctx_init().

Should m->ulls.exit_work be canceled and m->ulls.enabled (along with its
runtime PM reference) be reset before re-initializing m->q?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103060649.1015468-1-raag.jadav@intel.com?part=6