Re: [PATCH v6 1/2] drm/tyr: add TyrIrq threaded IRQ wrapper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The IRQ wrapper requests the threaded IRQ before clearing the device's pending interrupt status, creating an initialization race condition that can drop valid interrupts or cause IRQ storms.
--

commit e2f89c4d0d12a403d87f79e5dbf89539040c84df
Author: Laura Nao <laura.nao@collabora.com>

drm/tyr: add TyrIrq threaded IRQ wrapper

Introduce a generic TyrIrq threaded IRQ wrapper along with the
TyrIrqTrait trait describing the operations required to handle a
Tyr interrupt source.

> diff --git a/drivers/gpu/drm/tyr/irq.rs b/drivers/gpu/drm/tyr/irq.rs
> new file mode 100644
> index 0000000000000..e6a739532daa2
> --- /dev/null
> +++ b/drivers/gpu/drm/tyr/irq.rs

[ ... ]

> +impl<T: TyrIrqTrait> TyrIrq<T> {
> +    /// Requests a threaded IRQ registration for irq, using name to look up the interrupt.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must not mem::forget() the resulting registration or otherwise prevent its
> +    /// [Drop] implementation from running.
> +    pub(crate) unsafe fn request<'a>(
> +        pdev: &'a platform::Device<Bound>,
> +        name: &'static CStr,
> +        irq: T,
> +    ) -> Result<impl PinInit<ThreadedRegistration<'a, Self>, Error> + 'a>
> +    where
> +        T: 'a,
> +    {
> +        let handler = try_pin_init!(Self {
> +            irq,
> +            _pin: PhantomPinned,
> +        });
> +
> +        // SAFETY: The caller guarantees the resulting registration will not be leaked.
> +        let registration =
> +            unsafe { pdev.request_threaded_irq_by_name(Flags::SHARED, name, name, handler) };
> +
> +        // Clear the status and unmask once the handler has been
> +        // successfully registered.
> +        Ok(registration.pin_chain(|reg| {
> +            let irq = &reg.handler().irq;
> +            irq.clear_status(irq.mask());

[Severity: High]
Does this code request the threaded IRQ before clearing the device's pending
interrupt status?

Calling pdev.request_threaded_irq_by_name() immediately registers the
handler and enables the IRQ at the interrupt controller. However, the
hardware status is only cleared later when the .pin_chain closure executes.

If the device line is shared and level-triggered, could a pending stale
interrupt cause the hardirq handler to fire immediately? If masked, it
returns None, leading the kernel to see an unhandled level interrupt, which
could trigger an IRQ storm and permanently disable the line.

Furthermore, if a valid hardware interrupt arrives after registration but
before the .pin_chain closure runs, does irq.clear_status(irq.mask())
unconditionally wipe it out, permanently losing the event?

Should the device be quiesced (clearing status and masking) before
registering the IRQ?

> +            irq.reenable_mask();
> +            Ok(())
> +        }))
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-tyr-irq-v2-v6-0-15c90baed949@collabora.com?part=1