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

["Golla, Nagendra" <Nagendra.Golla@amd.com>]

On 6/18/2026 12:56 PM, sashiko-bot@kernel.org wrote:
> 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?

Hi,

Thanks for catching, agreed and will fix in v3 along with other RCs from 
the maintainers.

Thanks,
Nagendra>