Re: [PATCH v4 03/10] dmaengine: dw-edma: Move ll_region from struct dw_edma_chunk to struct dw_edma_chan

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] dw_edma_device_resume() accesses and modifies channel states and calls dw_edma_start_transfer() without holding the channel lock.
--

--- Patch [3]: [PATCH v4 03/10] dmaengine: dw-edma: Move ll_region from struct dw_edma_chunk to struct dw_edma_chan ---
commit 1e85ecaff6d1547be3e5049908d131f91651e11f
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Move ll_region from struct dw_edma_chunk to struct dw_edma_chan

ll_region is identical for all chunks belonging to the same DMA channel,
so there is no need to copy it into each chunk. Move ll_region to
struct dw_edma_chan to avoid redundant copies.

Tested-by: Koichiro Den <den@valinux.co.jp>
Signed-off-by: Frank Li <Frank.Li@nxp.com>

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

[ ... ]

While reviewing this file, I noticed a pre-existing issue in dw_edma_device_resume():

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_device_resume() {
    ...
	} else if (chan->request != EDMA_REQ_NONE) {
		err = -EPERM;
	} else {
		chan->status = EDMA_ST_BUSY;
		dw_edma_start_transfer(chan);
	}

	return err;
}

[Severity: High]
This isn't a bug introduced by this patch, but I noticed that
dw_edma_device_resume() modifies the channel state and calls
dw_edma_start_transfer() without holding the channel lock.

The vchan_next_desc() function, which is called by dw_edma_start_transfer(),
explicitly asserts lockdep_assert_held(&vc->lock). Modifying channel state
and manipulating virt-dma lists without holding chan->vc.lock can cause data
races and list corruption.

Any client driver triggering a channel resume will consistently hit the
lockdep splat and risk kernel panics from list corruption.

Could we acquire chan->vc.lock before updating chan->status and calling
dw_edma_start_transfer() here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-edma_ll-v4-0-cc128f0afb61@nxp.com?part=3