Re: [PATCH v6 5/9] irqchip/imx-irqsteer: Mask all interrupts in probe()

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Oct 08, 2026 at 09:18:11AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] irqchip/imx-irqsteer: missing wmb() allows interrupt storm race
> --
>
> commit e530007174d6dce33c6ec542e006f51e4a17dc73
> Author: Zhipeng Wang <zhipeng.wang_1@nxp.com>
>
> irqchip/imx-irqsteer: Mask all interrupts in probe()
>
> This commit masks all input interrupts in probe() before setting up
> chained handlers. This prevents an interrupt storm if a child interrupt
> was left unmasked during driver unload, as the hardware would otherwise
> assert it immediately upon re-mapping in the new domain.
>
> > diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> > index 5acc04504e520..fa233de9bc2b2 100644
> > --- a/drivers/irqchip/irq-imx-irqsteer.c
> > +++ b/drivers/irqchip/irq-imx-irqsteer.c
>
> [ ... ]
>
> > @@ -239,6 +239,14 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
> >  	if (irqsteer_has_chanctrl(data->devtype_data))
> >  		writel_relaxed(BIT(data->channel), data->regs + CHANCTRL);
> >
> > +	/*
> > +	 * Mask all interrupts before wiring up the chained handlers. CHANMASK
> > +	 * has inverted polarity (a set bit enables the interrupt), so writing
> > +	 * zero masks the source.
> > +	 */
> > +	for (i = 0; i < data->reg_num; i++)
> > +		writel_relaxed(0, data->regs + CHANMASK(i, data->reg_num));
> > +
>
> [Severity: Medium]
> Is an explicit memory barrier needed here to ensure the masks are applied
> before the parent interrupt is unmasked?
>
> Since writel_relaxed() does not provide a barrier, these writes to the IRQ
> Steer hardware could be reordered. The parent GIC interrupt may be unmasked
> before the IRQ Steer actually processes the interrupt masks, leaving a window
> for the interrupt storm this patch is designed to prevent.
>
> The parent GIC driver uses writel_relaxed() to unmask (for example,
> gic_unmask_irq() in GICv2 or gic_poke_irq() in GICv3). Furthermore,
> intervening locks like desc->lock in the IRQ core or irq_domain_mutex
> provide only ACQUIRE and RELEASE semantics. On ARM64, an intervening RELEASE
> followed by an ACQUIRE to different lock addresses can be reordered by the
> CPU.
>
> Could this allow the subsequent GIC relaxed write to cross the prior IRQ
> Steer relaxed write, meaning a full wmb() is strictly required to order
> writes to different I/O peripherals?

On our platform, all IO read/write is strict orderred. So no this problem

Frank
>
> >  	ret = devm_pm_runtime_set_active_enabled(&pdev->dev);
> >  	if (ret)
> >  		goto out;
>
> [ ... ]
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261008090231.796511-1-Zhipeng.wang_1@oss.nxp.com?part=5