Re: [PATCH v5] dmaengine: dw-axi-dmac: report paused state and residue in tx_status

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Sep 24, 2026 at 03:18:27AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] dmaengine: dw-axi-dmac: stale hardware pointer race on buffer reuse
> - [Medium] dmaengine: dw-axi-dmac: incorrect residue for overlapping SG regions
> --
>
> commit 51268a3a8963bcf17968d0065986bd47e9b6a2a8
> Author: Rui Wang <wr574332525@163.com>
> dmaengine: dw-axi-dmac: report paused state and residue in tx_status
>
> The driver implements device_pause/device_resume, but device_tx_status
> keeps reporting DMA_IN_PROGRESS for a paused channel. This commit updates
> the reporting to handle paused state and calculates actual byte residue.
>
> > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> > index eebed2474210d..f0cb3c7956629 100644
> > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>
> [ ... ]
>
> > @@ -352,33 +352,110 @@ static void vchan_desc_put(struct virt_dma_desc *vdesc)
> >  	axi_desc_put(vd_to_axi_desc(vdesc));
> >  }
> >
> > +/*
> > + * Read a 64-bit channel address register while the transfer may be
> > + * running. The two 32-bit halves cannot be read atomically on all
>
> [ ... ]
>
> > +static u32 axi_chan_get_xferred(struct axi_dma_chan *chan,
> > +				struct axi_dma_desc *desc)
> > +{
> > +	struct axi_dma_hw_desc *hw_desc;
> > +	bool dst = chan->direction == DMA_DEV_TO_MEM;
> > +	u64 pos, start;
> > +	u32 xferred = 0;
> > +	int i;
> > +
> > +	pos = axi_chan_readq(chan, dst ? CH_DAR : CH_SAR);
> > +
> > +	for (i = 0; i < desc->nr_hw_descs; i++) {
> > +		hw_desc = &desc->hw_desc[i];
> > +		start = le64_to_cpu(dst ? hw_desc->lli->dar : hw_desc->lli->sar);
> > +
> > +		/* Current position is inside this block: partial progress */
> > +		if (pos >= start && pos <= start + hw_desc->len)
> > +			return xferred + (u32)(pos - start);
>
> [Severity: Medium]
> Will this logic correctly handle scatter-gather lists that contain
> overlapping or repeating physical memory addresses?
>
> If a DMA client submits an SG list using a common memory region for padding
> across multiple elements, pos could match an earlier block while the DMA
> engine is actually processing a subsequent block reusing the same physical
> address.
>
> Could this cause the residue to jump backwards and report an artificially
> high remaining byte count to clients that rely on a monotonically
> decreasing residue?
>
> > +
> > +		xferred += hw_desc->len;
> > +	}
> > +
> > +	/* Position doesn't match any block, be conservative */
> > +	return 0;
> > +}
>
> [ ... ]
>
> > @@ -468,6 +545,9 @@ static void axi_chan_block_xfer_start(struct axi_dma_chan *chan,
> >  	}
> >  	axi_chan_config_write(chan, &config);
> >
> > +	/* The hardware pointer registers now belong to this descriptor */
> > +	chan->desc = first;
> > +
> >  	write_chan_llp(chan, first->hw_desc[0].llp | lms);
> >
> >  	irq_mask = DWAXIDMAC_IRQ_DMA_TRF | DWAXIDMAC_IRQ_ALL_ERR;
> >  	axi_chan_irq_sig_set(chan, irq_mask);
> >
> >  	/* Generate 'suspend' status but don't generate interrupt */
> >  	irq_mask |= DWAXIDMAC_IRQ_SUSPENDED;
> >  	axi_chan_irq_set(chan, irq_mask);
> >
> >  	axi_chan_enable(chan);
> >  }
>
> [Severity: High]
> Is there a race condition between setting the descriptor as active and the
> hardware actually updating its pointer registers?
>
> If a client reuses a previously completed buffer for a new transfer,
> axi_chan_block_xfer_start() assigns chan->desc = first and enables the
> channel.
>
> If dma_chan_tx_status() is called to check the status before the DMA engine
> fetches the first LLI, it might evaluate a stale hardware pointer.
>
> Since the buffer is identical to the previous run, could the stale pointer
> match the end of the new transfer's memory range, causing
> axi_chan_get_xferred() to erroneously return the full block length and
> report a 0 residue for a newly started transfer?

Look like possible. Caller may reuse prevous DMA buffer. But I don't know
how to resolve it.

Frank

>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260924025739.844-1-wr574332525@163.com?part=1