Re: [PATCH V2 3/3] dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts

[sashiko-bot@kernel.org]

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

New issues:
- [High] The interrupt handler drops valid interrupts when CONFIG_PM is disabled.

Pre-existing issues:
- [High] Memory and runtime PM reference leaks on error paths in zynqmp_dma_alloc_chan_resources().
- [High] Missing synchronize_irq() before tasklet_kill() allows the tasklet to run after channel resources and PM references are freed.
--

commit 62db01b8d81ad56c8fb04b0d8f2adecdae634703
Author: Golla Nagendra <nagendra.golla@amd.com>

dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts

This commit adds a pm_runtime_get_if_active() check in the
zynqmp_dma_irq_handler() to safely handle spurious interrupts. It
returns IRQ_NONE immediately if the device is not runtime-active,
preventing potential synchronous external aborts.

> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index a9dfec3c0ca3d..ce9163138be74 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> @@ -730,6 +730,9 @@ static irqreturn_t zynqmp_dma_irq_handler(int irq, void *data)
>  	u32 isr, imr, status;
>  	irqreturn_t ret = IRQ_NONE;
>  
> +	if (pm_runtime_get_if_active(chan->dev) <= 0)
> +		return IRQ_NONE;

[Severity: High]
When CONFIG_PM is disabled, the pm_runtime_get_if_active() inline stub
automatically returns -EINVAL. Does this check unconditionally drop valid
interrupts in non-PM configurations?

Because -EINVAL is less than or equal to 0, the handler might always return
IRQ_NONE. This could potentially lead to an unacknowledged interrupt storm
or the kernel permanently disabling the IRQ line.

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but do the error paths in
zynqmp_dma_alloc_chan_resources() leak the runtime PM reference?

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_alloc_chan_resources() {
    ...
    chan->sw_desc_pool = kzalloc_objs(*desc, ZYNQMP_DMA_NUM_DESCS);
    if (!chan->sw_desc_pool)
        return -ENOMEM;
    ...
    if (!chan->desc_pool_v)
        return -ENOMEM;
    ...
}

If pm_runtime_resume_and_get() is called earlier, returning directly
on memory allocation failures without calling pm_runtime_put() will
leak the PM reference and previously allocated memory.


[Severity: High]
This is a pre-existing issue, but does zynqmp_dma_synchronize() miss
a required IRQ serialization step?

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_synchronize() {
        struct zynqmp_dma_chan *chan = to_chan(dchan);

        tasklet_kill(&chan->tasklet);
}

If zynqmp_dma_irq_handler() is executing concurrently and gets preempted
just before calling tasklet_schedule(), tasklet_kill() might see no
scheduled tasklet and return immediately. 

When the IRQ handler resumes and schedules the tasklet, could it execute
asynchronously after channel resources and PM references are freed,
potentially causing a synchronous external abort? 

Would calling synchronize_irq() first resolve this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618071056.2024286-1-nagendra.golla@amd.com?part=3