Re: [PATCH v7 07/10] dmaengine: dw-edma: Add non_ll_start() callback

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The first burst and chunk are systematically ignored and dropped during DMA transfers, causing transfers to silently fail.
- [High] Data race and list corruption due to accessing channel state and virt-dma structures without holding the `chan->vc.lock`.
--

--- Patch [7]: [PATCH v7 07/10] dmaengine: dw-edma: Add non_ll_start() callback ---
commit 7dc5c95dae459bfa0e6a411b33405e84c6e8c394
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Add non_ll_start() callback

This commit adds a non_ll_start() callback and moves common non-linked-list
channel handling into the EDMA core so it can be shared by both EDMA and
HDMA. It also prepares for the upcoming reorganization of the burst and chunk
structures.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index bab4d49c92feb..e18d6e827c2c9 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
[ ... ]
> @@ -201,7 +202,16 @@ dw_edma_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
>  static inline
>  void dw_edma_core_start(struct dw_edma *dw, struct dw_edma_chunk *chunk, bool first)
>  {
> -	dw->core->start(chunk, first);
> +	if (chunk->chan->non_ll) {
> +		struct dw_edma_burst *child;
> +
> +		child = list_first_entry_or_null(&chunk->burst->list,
> +						 struct dw_edma_burst, list);

[Severity: High]
This isn't a bug introduced by this patch, but does this skip the first burst?

The driver implements linked lists by treating the first allocated element's
list member as the head of the list for subsequent elements, instead of using
a standalone struct list_head. When standard Linux list macros like
list_first_entry_or_null() are used on &chunk->burst->list, they
automatically skip the head node.

If only one burst is allocated in non-LL mode, would this return NULL and
silently skip starting the transfer?

> +		if (child)
> +			dw->core->non_ll_start(chunk->chan, child);
> +	} else {
> +		dw->core->start(chunk, first);
> +	}
>  }

[Severity: High]
This is a pre-existing issue, but can data races and list corruption occur
due to accessing channel state and virt-dma structures without holding the
chan->vc.lock?

For example, when calling dmaengine_resume() on a DMA channel, the driver's
dw_edma_device_resume() calls dw_edma_start_transfer(). This reads from the
virt-dma lists without holding chan->vc.lock:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_device_resume() {
    ...
    chan->status = EDMA_ST_BUSY;
    dw_edma_start_transfer(chan);
    ...
}

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_start_transfer() {
    ...
    vd = vchan_next_desc(&chan->vc);
    ...
}

Concurrently, dw_edma_done_interrupt() handles completed descriptors and
modifies the same lists under vc.lock, causing a direct data race.

Additionally, dw_edma_device_terminate_all() mutates chan->request and
chan->status locklessly.

Could this lead to list corruption or kernel panics when racing with
interrupt handlers?

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