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

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

[Rui Wang]

The driver implements device_pause/device_resume, but device_tx_status
keeps reporting DMA_IN_PROGRESS for a paused channel, so clients cannot
tell a paused channel apart from a running one. Report DMA_PAUSED when
the channel is paused and the cookie is still in flight, including for
callers that pass a NULL dma_tx_state, and clear the stale is_paused
flag in dma_chan_terminate_all(), which disables the channel and thus
implicitly cancels the paused state.

Also, the residue of an in-flight transfer is currently derived from
the number of completed LLI blocks, so it only advances in block-size
steps and stays stale for the duration of a large block. Read the
current hardware pointer (CH_SAR for MEM_TO_DEV and MEM_TO_MEM, CH_DAR
for DEV_TO_MEM) and walk the descriptor's LLIs to compute how many
bytes have actually been transferred. The 64-bit pointer is sampled
with a tearing-safe double read, as the transfer may be running
concurrently. The pointer is only consulted for the descriptor most
recently programmed into the hardware, tracked in the previously
unused chan->desc field; any other descriptor has not been started yet
and keeps reporting its full length. When the transfer has just
completed but the descriptor has not been reaped yet, the pointer sits
at the end of the last block and the residue naturally reads as 0.
Cyclic descriptors keep the block-granular accounting.

Tested on an FPGA platform.

Signed-off-by: Rui Wang <wr574332525@163.com>
---
Changes in v5 (thanks to Frank Li's review):
- Rewrite the tearing-safe register read as a do/while loop.
- Drop the "never report full completion while the channel is enabled"
  clamp: chan->desc matching already keeps never-started descriptors
  away from the stale pointer, and the residual stale-read window is
  bounded by the first LLI fetch after channel enable.
- Use min() instead of min_t().
- Link to v4: https://lore.kernel.org/dmaengine/20260923060045.5571-1-wr574332525@163.com/
Changes in v4:
- Track which descriptor the hardware pointer registers belong to via
  the (previously unused) chan->desc field instead of testing the head
  of desc_issued: a queued-but-never-started descriptor must not be
  matched against the stale pointer left by a previous transfer, which
  could otherwise falsely report full completion when its buffer is
  reused.
- Clear chan->desc when its descriptor is reaped or the channel is
  terminated.
- Link to v3: https://lore.kernel.org/dmaengine/20260923044104.3234-1-wr574332525@163.com/
Changes in v3:
- Report DMA_PAUSED also to callers passing a NULL dma_tx_state.
- Never report full completion while the channel enable bit is still
  set, so a stale pointer (e.g. a new transfer reusing the buffer of a
  just-finished one) is not mistaken for completion.
- Link to v2: https://lore.kernel.org/dmaengine/20260923034653.1413-1-wr574332525@163.com/
Changes in v2:
- Sample the 64-bit SAR/DAR with a tearing-safe double read instead of
  lo_hi_readq(), avoiding a torn pointer when the low half wraps.
- Read the hardware pointer for the in-flight descriptor even after the
  hardware has just completed it (channel enable self-cleared, IRQ not
  yet handled), so the residue reads 0 instead of jumping back to the
  full length.
- Link to v1: https://lore.kernel.org/dmaengine/20260923030201.859-1-wr574332525@163.com/
---
 .../dma/dw-axi-dmac/dw-axi-dmac-platform.c    | 102 ++++++++++++++++--
 1 file changed, 94 insertions(+), 8 deletions(-)

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 eebed2474..f0cb3c795 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
+ * platforms, so sample the high half before and after the low half
+ * and retry if it moved (the low half wrapped in between).
+ */
+static u64 axi_chan_readq(struct axi_dma_chan *chan, u32 reg)
+{
+	u32 hi, lo, hi2;
+
+	do {
+		hi = readl(chan->chan_regs + reg + 4);
+		lo = readl(chan->chan_regs + reg);
+		hi2 = readl(chan->chan_regs + reg + 4);
+	} while (unlikely(hi != hi2));
+
+	return (u64)hi << 32 | lo;
+}
+
+/*
+ * Return the number of bytes already transferred by the descriptor
+ * currently on the hardware (running, paused or just completed), based
+ * on the current read or write position: CH_SAR for MEM_TO_DEV and
+ * MEM_TO_MEM, CH_DAR for DEV_TO_MEM. Must be called with vc.lock held.
+ */
+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);
+
+		xferred += hw_desc->len;
+	}
+
+	/* Position doesn't match any block, be conservative */
+	return 0;
+}
+
 static enum dma_status
 dma_chan_tx_status(struct dma_chan *dchan, dma_cookie_t cookie,
 		   struct dma_tx_state *txstate)
 {
 	struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan);
 	struct virt_dma_desc *vdesc;
+	struct axi_dma_desc *desc;
 	enum dma_status status;
 	u32 completed_length;
 	unsigned long flags;
-	u32 completed_blocks;
 	size_t bytes = 0;
 	u32 length;
-	u32 len;
 
 	status = dma_cookie_status(dchan, cookie, txstate);
-	if (status == DMA_COMPLETE || !txstate)
+	if (status == DMA_COMPLETE)
 		return status;
 
 	spin_lock_irqsave(&chan->vc.lock, flags);
 
+	if (chan->is_paused && status == DMA_IN_PROGRESS)
+		status = DMA_PAUSED;
+
+	if (!txstate) {
+		spin_unlock_irqrestore(&chan->vc.lock, flags);
+		return status;
+	}
+
 	vdesc = vchan_find_desc(&chan->vc, cookie);
 	if (vdesc) {
-		length = vd_to_axi_desc(vdesc)->length;
-		completed_blocks = vd_to_axi_desc(vdesc)->completed_blocks;
-		len = vd_to_axi_desc(vdesc)->hw_desc[0].len;
-		completed_length = completed_blocks * len;
-		bytes = length - completed_length;
+		desc = vd_to_axi_desc(vdesc);
+		length = desc->length;
+
+		if (chan->cyclic) {
+			completed_length = desc->completed_blocks *
+					   desc->hw_desc[0].len;
+		} else if (desc == chan->desc) {
+			/*
+			 * chan->desc is the descriptor last programmed into
+			 * the hardware, so the pointer registers belong to
+			 * it; never match a queued-but-never-started
+			 * descriptor against the stale pointer left by a
+			 * previous transfer. If the transfer has just
+			 * finished but the interrupt has not reaped the
+			 * descriptor yet, the pointer sits at the end and
+			 * the residue reads 0.
+			 */
+			completed_length = axi_chan_get_xferred(chan, desc);
+		} else {
+			/* Still queued, nothing transferred yet */
+			completed_length = 0;
+		}
+
+		bytes = length - min(completed_length, length);
 	}
 
 	spin_unlock_irqrestore(&chan->vc.lock, flags);
@@ -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;
@@ -1077,6 +1157,8 @@ static noinline void axi_chan_handle_err(struct axi_dma_chan *chan, u32 status)
 	}
 	/* Remove the completed descriptor from issued list */
 	list_del(&vd->node);
+	if (chan->desc == vd_to_axi_desc(vd))
+		chan->desc = NULL;
 
 	/* WARN about bad descriptor */
 	dev_err(chan2dev(chan),
@@ -1140,6 +1222,8 @@ static void axi_chan_block_xfer_complete(struct axi_dma_chan *chan)
 	} else {
 		/* Remove the completed descriptor from issued list before completing */
 		list_del(&vd->node);
+		if (chan->desc == vd_to_axi_desc(vd))
+			chan->desc = NULL;
 		vchan_cookie_complete(vd);
 	}
 
@@ -1205,7 +1289,9 @@ static int dma_chan_terminate_all(struct dma_chan *dchan)
 
 	vchan_get_all_descriptors(&chan->vc, &head);
 
+	chan->desc = NULL;
 	chan->cyclic = false;
+	chan->is_paused = false;
 	spin_unlock_irqrestore(&chan->vc.lock, flags);
 
 	vchan_dma_desc_free_list(&chan->vc, &head);
-- 
2.43.0

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

[sashiko-bot]

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?

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

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

[Frank Li]

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

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

[Rui Wang]

Hi Frank,

On Thu, Sep 24, 2026 at 01:38:34PM -0500, Frank Li wrote:
> Look like possible. Caller may reuse prevous DMA buffer. But I don't know
> how to resolve it.

How about pre-loading the pointer registers when programming the
channel? In v6, axi_chan_block_xfer_start() writes the first block's
SAR/DAR into CH_SAR/CH_DAR before setting the channel enable bit. The
hardware reloads them with the same values once it fetches the first
LLI, and in the window in between a reader observes the start of the
first block, i.e. a full residue, which is the truth for a transfer
that has not begun. A stale end-of-transfer pointer left by a previous
run can no longer be observed, so the buffer-reuse race is closed at
the source and tx_status stays free of special cases.

Best regards,
Rui Wang