Re: [PATCH 0/3] drm/xe: Fix ULLS chained job loss and GT reset replay

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Sep 30, 2026 at 03:10:32PM +0530, Varun Gupta wrote:
> A chained ULLS migration job can be lost: its predecessor's postamble
> publishes the ring tail over the job's slot before parking on the
> semaphore, so the CS is free to fetch that slot while it still holds
> padding. When the job is later written and the semaphore signalled,
> the CS runs what it already fetched, drains to the tail and idles. A
> lost ULLS_EXIT goes unnoticed, a lost ULLS_ACTIVE hangs the kernel
> migration queue, and because the GT reset replay of a chained ULLS job
> does not work either, the second timeout wedges the device.
> 
> Patch 1 is the fix: park first, publish the tail after the wait, so the
> next slot stays beyond RING_TAIL until the job is in place. This puts
> the non-posted tail write on the wake-up path, which the original
> ordering was chosen to avoid. I could not find a way to keep the tail
> ahead of the wait without the CS being able to fetch the slot early.
> 

I saw patch 1 as a possible gap but at least on BMG I believe semaphore
acted as prefetch flush but if this isn't true on CRI, fine with change
going in but let's align on the issue (I don't have CRI myself to test
anything). I'd expect if this an issue on CRI a reproducer would simply
be run 'xe_exec_system_allocator' and we'd get an immediate hang. 

> Patches 2 and 3 make the GT reset replay of a chained ULLS job work,
> so a future ULLS hang degrades to a single recoverable reset rather
> than a wedge. Patch 3 covers a state patch 1 eliminates and is defence
> in depth.
> 

Patch 2 certainly looks corrcet, so does 3.

> A couple of related items I have left alone and would appreciate a
> view on:
> 
>  - The SR-IOV VF pause/unpause replay only routes the last_replay job
>    through the tail write, so a chained last job publishes nothing
>    there either. Adding "|| job->last_replay" to the patch 2 condition
>    looks right but I have no VF setup to test it for now.
> 

The VF code likely has gaps but pagefault + VF migrating shouldn't be
enabled (this combonation can deadlock), thus ULLS should be
unreachable. I do think we could workout the VF gaps if needed but agree
we'd have prove this on VF's with testing to have any level of
condidence.

>  - At replay, pending chained jobs still have their semaphore slot
>    signalled from before the reset, so the first re-emitted postamble
>    passes its wait immediately. The resubmit loop writes every job
>    before GuC processes the enable, so this has not been observed;
>    clearing the slots in guc_exec_queue_start() would close it.

I'd actually lean towards GT resets eliding the emit_job() like VF
replays do. This step isn't required as the ring instructions should
already be present in the ring and that memory should be persistent
across GT reset.

Thanks for looking in here - I figured I had some gaps which we'd catch
after merging my original series.

Matt

> 
> Varun Gupta (3):
>   drm/xe: Park on the ULLS semaphore before publishing the next job's
>     tail
>   drm/xe/guc: Publish the ring tail when replaying a chained ULLS job
>   drm/xe/guc: Rewind the LRC ring head when replaying a ULLS job
> 
>  drivers/gpu/drm/xe/xe_guc_submit.c | 19 +++++++++++++++++--
>  drivers/gpu/drm/xe/xe_migrate.c    | 24 +++++++++++++-----------
>  drivers/gpu/drm/xe/xe_ring_ops.c   | 15 +++++++++++----
>  3 files changed, 41 insertions(+), 17 deletions(-)
> 
> -- 
> 2.43.0
>