[PATCH 0/3] dmaengine: dw-axi-dmac: Fix LLP search, error reporting, and error dump

[PATCH 0/3] dmaengine: dw-axi-dmac: Fix LLP search, error reporting, and error dump

[Roland Dreier via B4 Relay]

One patch that build's on Jia Wang's fix for error-path LLI dump: the
same bug is present in axi_chan_block_xfer_complete(), although it's
not clear that it could ever be hit in practice, because the llp being
searched for should always be found.

Then two fixes for failed transfer handling: one to report status in
tx_result so that clients can know when a transfer hit an error, and
another to make log messages more informative than "Bad descriptor
submitted".

Signed-off-by: Roland Dreier <rolanddreier@rivian.com>
---
Roland Dreier (3):
      dmaengine: dw-axi-dmac: Bound the cyclic LLP search to the descriptor
      dmaengine: dw-axi-dmac: Report failed transfers in the descriptor result
      dmaengine: dw-axi-dmac: Decode errors in the transfer failure log

 drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 56 +++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 6 deletions(-)
---
base-commit: 67f0943b394d920b6c142aad8c6af94340342ae7
change-id: 20261006-dw-axi-fixes-4f066a7d9720

Best regards,
--  
Roland Dreier <rolanddreier@rivian.com>

[PATCH 1/3] dmaengine: dw-axi-dmac: Bound the cyclic LLP search to the descriptor

[Roland Dreier via B4 Relay]

From: Roland Dreier <rolanddreier@rivian.com>

For a cyclic transfer, axi_chan_block_xfer_complete() looks for the
LLI whose address matches CH_LLP by walking the descriptor's hw_desc
array, but it bounds the walk with chan->descs_allocated. That counts
the LLIs of every descriptor allocated on the channel, not just this
one. If CH_LLP matches none of the descriptor's LLIs (should never
happen, but...) while another descriptor is allocated, the loop reads
past the end of the array.

Bound the walk with the descriptor's nr_hw_descs, as in the
already-queued patch "dmaengine: dw-axi-dmac: Fix LLI dump
out-of-bounds access".

Assisted-by: LLM
Signed-off-by: Roland Dreier <rolanddreier@rivian.com>
---
 drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 3 +--
 1 file changed, 1 insertion(+), 2 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 eebed2474210..813d17a278e7 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -1095,7 +1095,6 @@ static noinline void axi_chan_handle_err(struct axi_dma_chan *chan, u32 status)
 
 static void axi_chan_block_xfer_complete(struct axi_dma_chan *chan)
 {
-	int count = atomic_read(&chan->descs_allocated);
 	struct axi_dma_hw_desc *hw_desc;
 	struct axi_dma_desc *desc;
 	struct virt_dma_desc *vd;
@@ -1122,7 +1121,7 @@ static void axi_chan_block_xfer_complete(struct axi_dma_chan *chan)
 		desc = vd_to_axi_desc(vd);
 		if (desc) {
 			llp = lo_hi_readq(chan->chan_regs + CH_LLP);
-			for (i = 0; i < count; i++) {
+			for (i = 0; i < desc->nr_hw_descs; i++) {
 				hw_desc = &desc->hw_desc[i];
 				if (hw_desc->llp == llp) {
 					axi_chan_irq_clear(chan, hw_desc->lli->status_lo);

-- 
2.54.0

[PATCH 2/3] dmaengine: dw-axi-dmac: Report failed transfers in the descriptor result

[Roland Dreier via B4 Relay]

From: Roland Dreier <rolanddreier@rivian.com>

axi_chan_handle_err() completes a failed descriptor without updating
vd->tx_result. So clients using tx_result always get DMA_TRANS_NOERROR,
even for incomplete / failed transfers (bus error or other reason).

Set the result so a client can detect failures:
 - DMA_TRANS_READ_FAILED for source decode or slave errors
 - DMA_TRANS_WRITE_FAILED for destination decode or slave errors
 - DMA_TRANS_ABORTED for everything else

Assisted-by: LLM
Signed-off-by: Roland Dreier <rolanddreier@rivian.com>
---
 drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 7 +++++++
 1 file changed, 7 insertions(+)

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 813d17a278e7..88362f181207 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -1084,6 +1084,13 @@ static noinline void axi_chan_handle_err(struct axi_dma_chan *chan, u32 status)
 		axi_chan_name(chan), vd->tx.cookie, status);
 	axi_chan_list_dump_lli(chan, vd_to_axi_desc(vd));
 
+	if (status & (DWAXIDMAC_IRQ_SRC_DEC_ERR | DWAXIDMAC_IRQ_SRC_SLV_ERR))
+		vd->tx_result.result = DMA_TRANS_READ_FAILED;
+	else if (status & (DWAXIDMAC_IRQ_DST_DEC_ERR | DWAXIDMAC_IRQ_DST_SLV_ERR))
+		vd->tx_result.result = DMA_TRANS_WRITE_FAILED;
+	else
+		vd->tx_result.result = DMA_TRANS_ABORTED;
+
 	vchan_cookie_complete(vd);
 
 	/* Try to restart the controller */

-- 
2.54.0

[PATCH 3/3] dmaengine: dw-axi-dmac: Decode errors in the transfer failure log

[Roland Dreier via B4 Relay]

From: Roland Dreier <rolanddreier@rivian.com>

axi_chan_handle_err() reports every error with "Bad descriptor
submitted" and dumps the transfer's LLIs. Common errors are source and
destination decode or slave errors, from an undecoded address or a
target returning an error response. These errors do not necessarily
indicate a malformed descriptor, so the message is misleading. The
dump also has one line per LLI without any indication where the
transfer failed.

Report the first matching error name while retaining the raw interrupt
status so additional error bits remain visible. Include SAR, DAR and
LLP to provide hardware progress and linked-list context without
dumping every LLI. These registers are diagnostic snapshots, not
necessarily the exact addresses of the failing transactions.

Dump the LLIs only when a descriptor or programming error is present.
Leave the transfer result and recovery path unchanged.

Assisted-by: LLM
Signed-off-by: Roland Dreier <rolanddreier@rivian.com>
---
 drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 46 +++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 4 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 88362f181207..e5a259e79a38 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -1059,6 +1059,36 @@ static void axi_chan_list_dump_lli(struct axi_dma_chan *chan,
 		axi_chan_dump_lli(chan, &desc_head->hw_desc[i]);
 }
 
+#define AXI_DMA_DATA_ERR	(DWAXIDMAC_IRQ_SRC_DEC_ERR | \
+			 DWAXIDMAC_IRQ_SRC_SLV_ERR | \
+			 DWAXIDMAC_IRQ_DST_DEC_ERR | \
+			 DWAXIDMAC_IRQ_DST_SLV_ERR)
+
+static const char *axi_chan_err_str(u32 status)
+{
+	if (status & DWAXIDMAC_IRQ_SRC_DEC_ERR)
+		return "source decode error";
+	if (status & DWAXIDMAC_IRQ_SRC_SLV_ERR)
+		return "source slave error";
+	if (status & DWAXIDMAC_IRQ_DST_DEC_ERR)
+		return "destination decode error";
+	if (status & DWAXIDMAC_IRQ_DST_SLV_ERR)
+		return "destination slave error";
+	if (status & DWAXIDMAC_IRQ_LLI_RD_DEC_ERR)
+		return "LLI read decode error";
+	if (status & DWAXIDMAC_IRQ_LLI_RD_SLV_ERR)
+		return "LLI read slave error";
+	if (status & DWAXIDMAC_IRQ_LLI_WR_DEC_ERR)
+		return "LLI write-back decode error";
+	if (status & DWAXIDMAC_IRQ_LLI_WR_SLV_ERR)
+		return "LLI write-back slave error";
+	if (status & DWAXIDMAC_IRQ_INVALID_ERR)
+		return "invalid LLI or shadow register";
+	if (status & DWAXIDMAC_IRQ_MULTIBLKTYPE_ERR)
+		return "invalid multi-block transfer type";
+	return "slave interface error";
+}
+
 static noinline void axi_chan_handle_err(struct axi_dma_chan *chan, u32 status)
 {
 	struct virt_dma_desc *vd;
@@ -1078,11 +1108,19 @@ 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);
 
-	/* WARN about bad descriptor */
+	/*
+	 * SAR and DAR advance as each burst is issued, so with bursts still
+	 * outstanding they can be a little past the one that failed.
+	 */
 	dev_err(chan2dev(chan),
-		"Bad descriptor submitted for %s, cookie: %d, irq: 0x%08x\n",
-		axi_chan_name(chan), vd->tx.cookie, status);
-	axi_chan_list_dump_lli(chan, vd_to_axi_desc(vd));
+		"%s for %s, cookie: %d, irq: 0x%08x, SAR: 0x%llx DAR: 0x%llx LLP: 0x%llx\n",
+		axi_chan_err_str(status), axi_chan_name(chan), vd->tx.cookie,
+		status, lo_hi_readq(chan->chan_regs + CH_SAR),
+		lo_hi_readq(chan->chan_regs + CH_DAR),
+		lo_hi_readq(chan->chan_regs + CH_LLP));
+	/* Only dump LLIs on non-data errors */
+	if (status & (DWAXIDMAC_IRQ_ALL_ERR & ~AXI_DMA_DATA_ERR))
+		axi_chan_list_dump_lli(chan, vd_to_axi_desc(vd));
 
 	if (status & (DWAXIDMAC_IRQ_SRC_DEC_ERR | DWAXIDMAC_IRQ_SRC_SLV_ERR))
 		vd->tx_result.result = DMA_TRANS_READ_FAILED;

-- 
2.54.0

Re: [PATCH 3/3] dmaengine: dw-axi-dmac: Decode errors in the transfer failure log

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dmaengine: dw-axi-dmac: torn reads of hardware registers during error logging
--

commit 6d61e30d4aa304e36fef59b004f04b0033e152dd
Author: Roland Dreier <rolanddreier@rivian.com>
dmaengine: dw-axi-dmac: Decode errors in the transfer failure log

This commit decodes errors in the transfer failure log for the dw-axi-dmac
driver. It reports the first matching error name and includes the SAR, DAR,
and LLP registers for diagnostic context, avoiding dumping every LLI on
standard data errors.

> 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 88362f1812071..e5a259e79a386 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c

[ ... ]

> @@ -1078,11 +1108,19 @@ 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);
>  
> -	/* WARN about bad descriptor */
> +	/*
> +	 * SAR and DAR advance as each burst is issued, so with bursts still
> +	 * outstanding they can be a little past the one that failed.
> +	 */
>  	dev_err(chan2dev(chan),
> -		"Bad descriptor submitted for %s, cookie: %d, irq: 0x%08x\n",
> -		axi_chan_name(chan), vd->tx.cookie, status);
> -	axi_chan_list_dump_lli(chan, vd_to_axi_desc(vd));
> +		"%s for %s, cookie: %d, irq: 0x%08x, SAR: 0x%llx DAR: 0x%llx LLP: 0x%llx\n",
> +		axi_chan_err_str(status), axi_chan_name(chan), vd->tx.cookie,
> +		status, lo_hi_readq(chan->chan_regs + CH_SAR),
> +		lo_hi_readq(chan->chan_regs + CH_DAR),
> +		lo_hi_readq(chan->chan_regs + CH_LLP));

[Severity: Medium]
Can reading the SAR, DAR, and LLP registers using lo_hi_readq result in torn
reads here?

The comment notes that bursts might still be outstanding and the hardware
registers can actively advance concurrently. 

Because lo_hi_readq performs two independent non-atomic 32-bit reads, if the
hardware advances the register across a 32-bit boundary between the low and
high reads, the resulting 64-bit value will be torn.

Could this mislead debugging efforts by logging bogus, corrupted addresses in
the diagnostic output?

> +	/* Only dump LLIs on non-data errors */
> +	if (status & (DWAXIDMAC_IRQ_ALL_ERR & ~AXI_DMA_DATA_ERR))
> +		axi_chan_list_dump_lli(chan, vd_to_axi_desc(vd));
>  
>  	if (status & (DWAXIDMAC_IRQ_SRC_DEC_ERR | DWAXIDMAC_IRQ_SRC_SLV_ERR))
>  		vd->tx_result.result = DMA_TRANS_READ_FAILED;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006-dw-axi-fixes-v1-0-3b319e456d93@rivian.com?part=3

Re: [PATCH 2/3] dmaengine: dw-axi-dmac: Report failed transfers in the descriptor result

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dmaengine: dw-axi-dmac: missing residue update on error leaves it as 0
--

commit 0cfa0e9416a64e0fa0850f1489894a836df68692
Author: Roland Dreier <rolanddreier@rivian.com>

dmaengine: dw-axi-dmac: Report failed transfers in the descriptor result

This commit updates axi_chan_handle_err() to populate tx_result.result with an
error state. It previously completed failed descriptors without updating
vd->tx_result, which returned DMA_TRANS_NOERROR to clients even after failures.

> 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 813d17a278e72..88362f1812071 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -1084,6 +1084,13 @@ static noinline void axi_chan_handle_err(struct axi_dma_chan *chan, u32 status)
>  		axi_chan_name(chan), vd->tx.cookie, status);
>  	axi_chan_list_dump_lli(chan, vd_to_axi_desc(vd));
>  
> +	if (status & (DWAXIDMAC_IRQ_SRC_DEC_ERR | DWAXIDMAC_IRQ_SRC_SLV_ERR))
> +		vd->tx_result.result = DMA_TRANS_READ_FAILED;
> +	else if (status & (DWAXIDMAC_IRQ_DST_DEC_ERR | DWAXIDMAC_IRQ_DST_SLV_ERR))
> +		vd->tx_result.result = DMA_TRANS_WRITE_FAILED;
> +	else
> +		vd->tx_result.result = DMA_TRANS_ABORTED;
> +

[Severity: Medium]
Because vd->tx_result.residue is not updated here, it defaults to 0, which
indicates a full transfer completion despite the assigned error state.

Clients inspecting the residue to determine how much data was successfully
transferred before the error will incorrectly conclude that all data was
transferred.

Should this code also update the vd->tx_result.residue field?

>  	vchan_cookie_complete(vd);
>  
>  	/* Try to restart the controller */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006-dw-axi-fixes-v1-0-3b319e456d93@rivian.com?part=2