Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Removing local_irq_disable() re-enables interrupts before and during 
ELSP hardware writes, risking incomplete submissions and GPU hangs.
--

commit ff9840660b858aab1bd255c1588d895283bda28f
Author: Sebastian Andrzej Siewior <[email protected]>
    
drm/i915/gt: Use spin_lock_irq() instead of local_irq_disable() + spin_lock()

This commit replaces local_irq_disable() and spin_lock() with spin_lock_irq()
in the execlists submission path to improve PREEMPT_RT compatibility.
However, releasing the lock earlier in the function leaves the hardware ELSP
MMIO writes exposed to interrupts and preemption.

> diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c 
> b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> index 1359fc9cb88ef..e11db81dca9c5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c

[ ... ]

> @@ -1588,7 +1588,7 @@ static void execlists_dequeue(struct intel_engine_cs 
> *engine)
>        */
>       sched_engine->queue_priority_hint = queue_prio(sched_engine);
>       i915_sched_engine_reset_on_empty(sched_engine);
> -     spin_unlock(&sched_engine->lock);
> +     spin_unlock_irq(&sched_engine->lock);

[Severity: High]
Does this change leave the hardware ELSP writes exposed to interrupts and
preemption?

By removing local_irq_disable() around the entire execlists_dequeue() call
and replacing it with spin_lock_irq() internally, interrupts are now
re-enabled at this spin_unlock_irq() call.

This means the subsequent hardware writes in execlists_submit_ports() now
occur with interrupts enabled:

execlists_dequeue() {
    ...
    spin_unlock_irq(&sched_engine->lock);
    
    /* ... */
    if (submit && ...) {
        ...
        execlists_submit_ports(engine);
    }
}

Since execlists_submit_ports() performs 4-DWord MMIO writes to the ELSP
that must remain strictly atomic, could being interrupted or preempted
during these writes result in incomplete context descriptors being fetched,
potentially causing GPU hangs?

[ ... ]

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to