Re: [PATCH v8 15/27] drm/i915/gt: Use spin_lock_irq() instead of local_irq_disable() + spin_lock()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing local_irq_disable() exposes the ELSP (ExecList Submit Port) MMIO writes to hardirq preemption, risking hardware hangs due to interleaved MMIO accesses.
--

commit 417d04153bfb15dc2291029341c85c1b4a51d8bb
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
drm/i915/gt: Use spin_lock_irq() instead of local_irq_disable() + spin_lock()

This patch modifies the execlists dequeue paths by replacing the explicit
local_irq_disable() wrapper with spin_lock_irq() semantics on the schedule
engine lock.

> 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);
>  
>  	/*
>  	 * We can skip poking the HW if we ended up with exactly the same set
[ ... ]
> -static void execlists_dequeue_irq(struct intel_engine_cs *engine)
> -{
> -	local_irq_disable(); /* Suspend interrupts across request submission */
> -	execlists_dequeue(engine);
> -	local_irq_enable(); /* flush irq_work (e.g. breadcrumb enabling) */
> -}

[Severity: High]
Does removing the local_irq_disable() wrapper expose the ELSP MMIO writes
to hardirq preemption?

With the move to spin_unlock_irq() earlier in the function, hardware 
interrupts are re-enabled before execlists_submit_ports() is called at the
end of execlists_dequeue().

Inside the submission path, write_desc() performs multiple 32-bit writel() 
operations to the ELSP register:

	writel(upper_32_bits(desc), execlists->submit_reg);
	writel(lower_32_bits(desc), execlists->submit_reg);

If an interrupt occurs between these consecutive MMIO writes and performs
other GPU MMIO accesses, could this disrupt the hardware submission state
machine and lead to GPU hangs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615201846.307297-1-dev@lankhorst.se?part=15