Re: [PATCH v5 22/25] drm/xe: Add ULLS migration job support to GuC submission

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

On Fri, Sep 04, 2026 at 02:49:34AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing MMIO readback to flush the posted write to RING_TAIL before signaling the ULLS semaphore in memory.
> --
> 
> commit 454390e5dedb020c4422d2a8b279ed19fa3eb789
> Author: Matthew Brost <matthew.brost@intel.com>
> 
> drm/xe: Add ULLS migration job support to GuC submission
> 
> This commit adds ULLS migration job support to the GuC submission backend.
> It updates the migration queue max jobs, directly sets the hardware engine
> tail via a MMIO write for ULLS jobs, and sets the ULLS semaphore for the
> current job releasing the last job.
> 
> Link: https://patch.msgid.link/20260228013501.106680-24-matthew.brost@intel.com
> 
> > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> > index dcb4b8a4f3b78..1578d80b01b82 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > @@ -1207,6 +1207,11 @@ static void submit_exec_queue(struct xe_exec_queue *q, struct xe_sched_job *job)
> >  	if (exec_queue_suspended(q))
> >  		return;
> >  
> > +	if (job->is_ulls && !job->is_ulls_first) {
> > +		xe_hw_engine_write_ring_tail(q->hwe, lrc->ring.tail);
> > +		xe_lrc_set_ulls_semaphore(lrc, xe_sched_job_lrc_seqno(job));
> 
> [Severity: High]
> Does this sequence miss an MMIO readback to flush the posted write?
> 
> A posted MMIO write is issued to the hardware engine's RING_TAIL via
> xe_hw_engine_write_ring_tail(), and is immediately followed by a memory write
> to signal the semaphore in xe_lrc_set_ulls_semaphore(). There is no
> intermediate MMIO readback to flush the posted write.
> 
> If the CS wakes up from the semaphore wait before the MMIO RING_TAIL write
> arrives, it can observe HEAD == TAIL, assume the ring is empty, and go idle.
> The context is then switched out. When the delayed MMIO write eventually
> arrives, it hits an idle engine (or worse, a different context), failing to
> execute the submitted ULLS job and hanging the migration queue.
> 
> Because xe_migrate_ulls_exit would then hang indefinitely waiting for the
> unexecuted job's fence, and the migration queue handles GPU page faults and
> SVM prefetches, this can hang workloads utilizing SVM.
> 
> Does this violate PCIe ordering requirements for flushing posted writes before
> dependent memory writes?
> 

Maybe. Again [1], it's something to keep an eye on, but I've never seen a
failure on BMG, and I've run `xe_exec_system_allocator` thousands of
times on this code over the past year.

I'd like to avoid adding a read unless we start seeing failures, as it
adds latency that we're trying very hard to avoid in this series.

If this does turn out to be an issue, we'll most likely move to a pure
memory-based model for ULLS with a single ring tail update. I have a
prototype of this, but it is significantly more invasive to the ring
operations and submission code than the changes in this series.

Matt

[1] https://patchwork.freedesktop.org/patch/750801/?series=149888&rev=6#comment_1383766

> > +	}
> > +
> >  	if (!exec_queue_enabled(q)) {
> >  		action[len++] = XE_GUC_ACTION_SCHED_CONTEXT_MODE_SET;
> >  		action[len++] = q->guc->id;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260904022207.3490018-1-matthew.brost@intel.com?part=22