Re: [PATCH v3 34/34] dmaengine: switchtec: use channel BH helpers

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

Pre-existing issues:
- [High] Stale hardware completion state is incorrectly processed after channel synchronization, leading to data corruption.
--

--- Patch [34]: [PATCH v3 34/34] dmaengine: switchtec: use channel BH helpers ---
commit d57e70e2d9fa1493ca8fd6bc467970ca44d80a8e
Author: Allen Pais <[email protected]>
Subject: dmaengine: switchtec: use channel BH helpers

Replace the per-channel descriptor completion tasklet with the shared
dmaengine BH helpers. Client callbacks remain in softirq context while
using the common scheduling and teardown mechanism.

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 3ef9286406159..fdc28a45e8586 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
[ ... ]
> @@ -584,6 +582,7 @@ static void switchtec_dma_synchronize(struct dma_chan *chan)
>  
>  	int rc;
>  
> +	dmaengine_kill_bh(chan);
>  	switchtec_dma_abort_desc(swdma_chan, 1);
>  
>  	rc = enable_channel(swdma_chan);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for stale
hardware completion state to be incorrectly processed after channel
synchronization?

Further down in switchtec_dma_synchronize(), the software state variables are
reset:

	swdma_chan->comp_ring_active = true;
	swdma_chan->phase_tag = 0;
	swdma_chan->tail = 0;
	swdma_chan->cq_tail = 0;
	swdma_chan->cid = 0;

However, the hardware completion queue memory (swdma_chan->hw_cq) is not
cleared. If previous hardware completions left non-zero phase tags in the
completion queue, would switchtec_dma_cleanup_completed() see
le16_to_cpu(phase_tag) != swdma_chan->phase_tag and process the stale entries
as new completions?

This might cause the client driver to read incomplete data or reuse buffers
that are still under active DMA.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=34