Re: [PATCH v2 09/19] dmaengine: dw-edma: Reclaim issued descriptors from IRQ-paired LL progress

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Processing EDMA_REQ_STOP unconditionally terminates descriptors and marks the channel IDLE on any interrupt, leaving the hardware running against freed memory.
- [High] Hardware abort interrupts fail to reset the continuous ring buffer state, permanently deadlocking the DMA channel for subsequent descriptors.
--

--- Patch [9]: [PATCH v2 09/19] dmaengine: dw-edma: Reclaim issued descriptors from IRQ-paired LL progress ---
commit b319de2e00cdfc3228e59ce503f4609c77c06d4b
Author: Koichiro Den <den@valinux.co.jp>
Date:   Thu Jul 23 08:41:40 2026 +0000

    dmaengine: dw-edma: Reclaim issued descriptors from IRQ-paired LL progress
    
    Dynamic append can put entries from several descriptors in the ring.
    Track the consumed boundary in ll_done, reuse consumed entries, and
    complete descriptors in issued order. Keep one data entry free so a
    physical index is unambiguous within the active producer window.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index e88bfb417ad4a..44a0ff906f2f7 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -726,14 +890,15 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
>  			break;
>  		}
>  
> -		chan->status = dw_edma_start_transfer(chan) ? EDMA_ST_BUSY : EDMA_ST_IDLE;
> +		active = false;
> +		if (vd)
> +			active = dw_edma_start_transfer(chan);
> +		if (!chan->non_ll)
> +			active = dw_edma_ll_pending(chan);
> +		chan->status = active ? EDMA_ST_BUSY : EDMA_ST_IDLE;
>  		break;
>  
>  	case EDMA_REQ_STOP:
> -		vd = vchan_next_desc(&chan->vc);
> -		if (!vd)
> -			break;
> -
>  		dw_edma_terminate_all_descs(chan);
>  		chan->request = EDMA_REQ_NONE;
>  		chan->status = EDMA_ST_IDLE;
>  		break;

[Severity: Critical]
If a client calls dmaengine_terminate_all(), it sets chan->request to
EDMA_REQ_STOP without immediately writing a hardware stop command.

If the DMA engine then fires a progress interrupt, such as a periodic
watermark or eDMA block completion, before it has actually stopped, does
this unconditionally terminate all descriptors and mark the channel IDLE?

If the client sees the termination complete and frees the DMA buffers, but
the hardware is still executing the remainder of the continuous circular
ring buffer against the freed memory, could this lead to a use-after-free
and IOMMU faults?

[ ... ]
> @@ -789,12 +954,34 @@ static void dw_edma_queue_irq_work(struct dw_edma_chan *chan,
>  	queue_work(chan->dw->wq, &chan->irq_work);
>  }
>  
[ ... ]
> -static void dw_edma_abort_interrupt_deferred(struct dw_edma_chan *chan)
> +static void dw_edma_abort_interrupt_deferred(struct dw_edma_chan *chan,
> +					     bool stopped)
>  {
>  	dw_edma_queue_irq_work(chan, DW_EDMA_IRQ_ABORT);
>  }

[Severity: High]
Does dw_edma_abort_interrupt() completely reset the continuous ring buffer
state when handling a hardware abort?

If the hardware encounters a bus error and fires an ABORT interrupt, it
appears dw_edma_abort_interrupt() pops a single descriptor and sets the
channel to IDLE, but fails to clear chan->ll_valid or flush the remaining
published descriptors.

When the next transfer is submitted, would dw_edma_start_transfer() skip
resetting and re-enabling the channel hardware (via
dw_edma_core_ch_enable) because ll_valid is still true?

Could this leave the hardware permanently halted while software blindly
appends to the stale ring?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=9