Re: [PATCH 2/5] mailbox: imx: Move the RX part of the mailbox into the threaded handler

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-05-29 17:26:35 [+0000], sashiko-bot@kernel.org wrote:
> commit 7ab3fb54b60e0decf23b64d1d78df0e2b4409f2a
> Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> mailbox: imx: Move the RX part of the mailbox into the threaded handler
> 
> This commit moves the RX mailbox callback handling into the threaded
> interrupt handler. It updates the driver to mask the RX interrupt in the
> primary handler and unmask it in the threaded handler upon success.
> 
> > diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> > --- a/drivers/mailbox/imx-mailbox.c
> > +++ b/drivers/mailbox/imx-mailbox.c
> 
> [ ... ]
> 
> > @@ -524,6 +522,12 @@ static irqreturn_t imx_mu_isr_th(int irq, void *p)
> >  		mbox_chan_txdone(chan, 0);
> >  		return IRQ_HANDLED;
> >  
> > +	case IMX_MU_TYPE_RX:
> > +		cp->pending = 0;
> > +		if (!priv->dcfg->rx(priv, cp))
> > +			imx_mu_xcr_rmw(priv, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx), 0);
> 
> [Severity: High]
> This is a pre-existing issue, but does bypassing the unmasking step on
> error permanently disable the channel?

Yes, I think this is the intention. The RX callback does not unmask the
event on error.

> If imx_mu_specific_rx() encounters an error, such as a read timeout or
> receiving a message that exceeds the maximum size, it returns a negative
> error code. When this happens, the condition evaluates to false and the
> interrupt remains masked.
> 
> Since there does not seem to be a mechanism to automatically reset the
> channel after these errors, could a buggy remote processor trigger this
> to cause a permanent denial of service for the RX channel?
> 
> [Severity: High]
> Can this unmasking step also race with channel shutdown?
> 
> If mbox_free_channel() is called concurrently, imx_mu_shutdown() masks the
> interrupt and invokes free_irq(). The free_irq() function synchronizes
> with the threaded handler and will block until imx_mu_isr_th() finishes
> processing.
> 
> Because imx_mu_isr_th() proceeds to unmask the interrupt here after
> processing, the hardware interrupt will be left enabled after the kernel
> IRQ handler is fully unregistered. If a new message arrives afterward,
> won't this cause an interrupt storm and prompt the genirq core to disable
> the shared IRQ line entirely?

Yes, we had this.

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

Sebastian