Re: [PATCH v7 08/10] dmaengine: dw-edma: Use common dw_edma_core_start() for both eDMA and HDMA

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Calling `dw_edma_start_transfer()` and modifying channel status from `dw_edma_device_resume()` without holding the virtual channel lock.
--

--- Patch [8]: [PATCH v7 08/10] dmaengine: dw-edma: Use common dw_edma_core_start() for both eDMA and HDMA ---
commit 44d14981017ae5dfd9faba4190eb71b1ca628fdb
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Use common dw_edma_core_start() for both eDMA and HDMA
    
This commit consolidates the start routines for eDMA and HDMA into a single
dw_edma_core_start() function. It removes the separate .start() callbacks
from both eDMA and HDMA implementations.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 2652ad8e7a8f6..f52d9fd18e573 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

> @@ -183,7 +211,7 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
>  	if (!child)
>  		return 0;
>  
> -	dw_edma_core_start(dw, child, !desc->xfer_sz);
> +	dw_edma_core_start(child, !desc->xfer_sz);
>  	desc->xfer_sz += child->xfer_sz;
>  	dw_edma_free_burst(child);
>  	list_del(&child->list);

[Severity: High]
This is a pre-existing issue, but is there a race condition when calling
dw_edma_start_transfer() from dw_edma_device_resume() without holding the
virtual channel lock?

When a client calls dmaengine_resume(), dw_edma_device_resume() updates
chan->status and calls dw_edma_start_transfer() without acquiring
chan->vc.lock:

static int dw_edma_device_resume(struct dma_chan *dchan)
{
    ...
	} else {
		chan->status = EDMA_ST_BUSY;
		dw_edma_start_transfer(chan);
	}
    ...
}

Once the hardware is started via dw_edma_core_start(), a completion interrupt
can fire immediately if the chunk completes quickly or encounters an error.
The interrupt handler dw_edma_done_interrupt() acquires chan->vc.lock, checks
that the channel is not paused, and calls dw_edma_start_transfer() again.

Because dw_edma_device_resume() does not hold the lock, both threads could
concurrently fetch the same chunk, start the hardware, and execute
dw_edma_free_burst() and list_del().

Can this lead to a double-free of the burst structure and list corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-edma_ll-v7-0-6fb7498c901e@nxp.com?part=8