Re: [PATCH v3 06/10] mailbox: imx: Start splitting the IRQ handler in primary and threaded handler

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Channel shutdown race condition leads to an unhandled IRQ storm on shared interrupt line.
--

commit 7ed2b361db99c9ca608465a64760164124f3d0d2
Author: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>

mailbox: imx: Start splitting the IRQ handler in primary and threaded handler

This commit splits the mailbox IRQ handling into primary and threaded
handlers. The primary handler masks the event, and the threaded handler
runs the mailbox core functions like mbox_chan_txdone() in a preemptible
context.

> diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> index 408cd083c64ee..87acc43cb99c4 100644
> --- a/drivers/mailbox/imx-mailbox.c
> +++ b/drivers/mailbox/imx-mailbox.c
> @@ -540,11 +540,31 @@ static void imx_mu_txdb_work(struct work_struct *t)
>  	mbox_chan_txdone(cp->chan, 0);
>  }
>  
> +static irqreturn_t imx_mu_isr_th(int irq, void *p)
> +{
> +	struct mbox_chan *chan = p;
> +	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> +	struct imx_mu_con_priv *cp = chan->con_priv;
> +
> +	switch (cp->type) {
> +	case IMX_MU_TYPE_TX:
> +		mbox_chan_txdone(chan, 0);

[Severity: High]
This is a pre-existing issue, but does moving this into a threaded handler
expose a race condition during channel shutdown?

When mbox_free_channel() is called, it invokes the driver's imx_mu_shutdown()
callback, which masks the TX interrupt and calls free_irq().

Because free_irq() waits for any executing threaded handlers to finish,
it will block if imx_mu_isr_th() is currently executing mbox_chan_txdone().

This in turn triggers the mailbox core to submit the next previously queued
message via msg_submit() -> chan->mbox->ops->send_data().

The send callback (such as imx_mu_generic_tx() or imx_mu_specific_tx()) then
unconditionally unmasks the TX interrupt by calling imx_mu_xcr_rmw(),
completely bypassing any shutdown state.

Once the threaded handler finishes, free_irq() completes and unregisters
the interrupt action, but the hardware interrupt remains unmasked.

Since IRQF_SHARED is used, could the unhandled interrupt fire continuously,
resulting in a storm of IRQ_NONE that eventually causes the kernel to disable
the shared IRQ line and break IPC globally?

> +		break;
> +
> +	default:
> +		dev_warn_ratelimited(priv->dev, "Unhandled channel type %d\n",
> +				     cp->type);
> +		return IRQ_NONE;
> +	}
> +	return IRQ_HANDLED;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-imx_mbox_rproc-v3-0-77948112defc@xxxxxxxxxxxxx?part=6