* [PATCH RESEND 0/3] dmaengine: xilinx_dma: Fixes and optimizations for AXIDMA and MCDMA channel management
@ 2026-06-20 20:34 Suraj Gupta
2026-06-20 20:34 ` [PATCH RESEND 1/3] dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers Suraj Gupta
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Suraj Gupta @ 2026-06-20 20:34 UTC (permalink / raw)
To: vkoul, Frank.Li, michal.simek, linux-kernel
Cc: dmaengine, linux-arm-kernel, srinivas.neeli, dev
This patch series addresses issues and optimizations in the Xilinx
AXI DMA and MCDMA drivers:
1. Fix channel idle state management in the interrupt handlers.
2. Enable transfer chaining by removing unnecessary idle restrictions.
3. Optimize control register writes and channel start logic.
Changes in V2:
- Apply similar fixes and optimizations to MCDMA as well.
- Expand the 1/3 commit description with when the described issue occurs.
Suraj Gupta (3):
dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and
MCDMA interrupt handlers
dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA
by removing idle restriction
dmaengine: xilinx_dma: Optimize control register write and channel
start logic for AXIDMA and MCDMA in corresponding start_transfer()
drivers/dma/xilinx/xilinx_dma.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH RESEND 1/3] dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers
2026-06-20 20:34 [PATCH RESEND 0/3] dmaengine: xilinx_dma: Fixes and optimizations for AXIDMA and MCDMA channel management Suraj Gupta
@ 2026-06-20 20:34 ` Suraj Gupta
2026-06-20 20:47 ` sashiko-bot
2026-06-20 20:34 ` [PATCH RESEND 2/3] dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction Suraj Gupta
2026-06-20 20:34 ` [PATCH RESEND 3/3] dmaengine: xilinx_dma: Optimize control register write and channel start logic for AXIDMA and MCDMA in corresponding start_transfer() Suraj Gupta
2 siblings, 1 reply; 7+ messages in thread
From: Suraj Gupta @ 2026-06-20 20:34 UTC (permalink / raw)
To: vkoul, Frank.Li, michal.simek, linux-kernel
Cc: dmaengine, linux-arm-kernel, srinivas.neeli, dev
Fix a race condition in AXIDMA and MCDMA irq handlers where the channel
could be incorrectly marked as idle and attempt spurious transfers when
descriptors are still being processed.
The issue occurs when:
1. Multiple descriptors are queued and active.
2. An interrupt fires after completing some descriptors.
3. xilinx_dma_complete_descriptor() moves completed descriptors to
done_list.
4. Channel is marked idle and start_transfer() is called even though
active_list still contains unprocessed descriptors.
5. This leads to premature transfer attempts and potential descriptor
corruption or missed completions.
Only mark the channel as idle and start new transfers when the active list
is actually empty, ensuring proper channel state management and avoiding
spurious transfer attempts.
Fixes: c0bba3a99f07 ("dmaengine: vdma: Add Support for Xilinx AXI Direct Memory Access Engine")
Tested-by: Folker Schwesinger <dev@folker-schwesinger.de>
Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
Co-developed-by: Srinivas Neeli <srinivas.neeli@amd.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
drivers/dma/xilinx/xilinx_dma.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 404235c17353..ca396b709742 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -1893,8 +1893,10 @@ static irqreturn_t xilinx_mcdma_irq_handler(int irq, void *data)
if (status & XILINX_MCDMA_IRQ_IOC_MASK) {
spin_lock(&chan->lock);
xilinx_dma_complete_descriptor(chan);
- chan->idle = true;
- chan->start_transfer(chan);
+ if (list_empty(&chan->active_list)) {
+ chan->idle = true;
+ chan->start_transfer(chan);
+ }
spin_unlock(&chan->lock);
}
@@ -1950,8 +1952,10 @@ static irqreturn_t xilinx_dma_irq_handler(int irq, void *data)
XILINX_DMA_DMASR_DLY_CNT_IRQ)) {
spin_lock(&chan->lock);
xilinx_dma_complete_descriptor(chan);
- chan->idle = true;
- chan->start_transfer(chan);
+ if (list_empty(&chan->active_list)) {
+ chan->idle = true;
+ chan->start_transfer(chan);
+ }
spin_unlock(&chan->lock);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH RESEND 2/3] dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction
2026-06-20 20:34 [PATCH RESEND 0/3] dmaengine: xilinx_dma: Fixes and optimizations for AXIDMA and MCDMA channel management Suraj Gupta
2026-06-20 20:34 ` [PATCH RESEND 1/3] dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers Suraj Gupta
@ 2026-06-20 20:34 ` Suraj Gupta
2026-06-20 20:47 ` sashiko-bot
2026-06-20 20:34 ` [PATCH RESEND 3/3] dmaengine: xilinx_dma: Optimize control register write and channel start logic for AXIDMA and MCDMA in corresponding start_transfer() Suraj Gupta
2 siblings, 1 reply; 7+ messages in thread
From: Suraj Gupta @ 2026-06-20 20:34 UTC (permalink / raw)
To: vkoul, Frank.Li, michal.simek, linux-kernel
Cc: dmaengine, linux-arm-kernel, srinivas.neeli, dev
Remove the restrictive idle check in xilinx_dma_start_transfer() and
xilinx_mcdma_start_transfer() that prevented new transfers from being
queued when the channel was busy.
Additionally, only update the CURDESC register when the channel is
running in scatter-gather mode and active list is empty to avoid
interfering with transfers already in progress. When the active list
contains transfers, the hardware tail pointer extension mechanism
handles chaining automatically.
Tested-by: Folker Schwesinger <dev@folker-schwesinger.de>
Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
Co-developed-by: Srinivas Neeli <srinivas.neeli@amd.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
drivers/dma/xilinx/xilinx_dma.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index ca396b709742..35b553ee3205 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -1580,9 +1580,6 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
return;
}
- if (!chan->idle)
- return;
-
head_desc = list_first_entry(&chan->pending_list,
struct xilinx_dma_tx_descriptor, node);
tail_desc = list_last_entry(&chan->pending_list,
@@ -1599,7 +1596,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
}
- if (chan->has_sg)
+ if (chan->has_sg && list_empty(&chan->active_list))
xilinx_write(chan, XILINX_DMA_REG_CURDESC,
head_desc->async_tx.phys);
reg &= ~XILINX_DMA_CR_DELAY_MAX;
@@ -1660,9 +1657,6 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
if (chan->err)
return;
- if (!chan->idle)
- return;
-
if (list_empty(&chan->pending_list))
return;
@@ -1685,8 +1679,9 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
/* Program current descriptor */
- xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
- head_desc->async_tx.phys);
+ if (chan->has_sg && list_empty(&chan->active_list))
+ xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
+ head_desc->async_tx.phys);
/* Program channel enable register */
reg = dma_ctrl_read(chan, XILINX_MCDMA_CHEN_OFFSET);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH RESEND 3/3] dmaengine: xilinx_dma: Optimize control register write and channel start logic for AXIDMA and MCDMA in corresponding start_transfer()
2026-06-20 20:34 [PATCH RESEND 0/3] dmaengine: xilinx_dma: Fixes and optimizations for AXIDMA and MCDMA channel management Suraj Gupta
2026-06-20 20:34 ` [PATCH RESEND 1/3] dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers Suraj Gupta
2026-06-20 20:34 ` [PATCH RESEND 2/3] dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction Suraj Gupta
@ 2026-06-20 20:34 ` Suraj Gupta
2026-06-20 20:46 ` sashiko-bot
2 siblings, 1 reply; 7+ messages in thread
From: Suraj Gupta @ 2026-06-20 20:34 UTC (permalink / raw)
To: vkoul, Frank.Li, michal.simek, linux-kernel
Cc: dmaengine, linux-arm-kernel, srinivas.neeli, dev
Optimize AXI DMA control register programming by consolidating
coalesce count and delay configuration into a single register write.
Previously, the coalesce count was written separately from the delay
configuration, resulting in two register writes. Combine these into
one write operation to reduce bus overhead.
Additionally, avoid redundant channel starts in xilinx_dma_start_transfer()
and xilinx_mcdma_start_transfer() by only calling xilinx_dma_start() when
the channel is actually idle.
Tested-by: Folker Schwesinger <dev@folker-schwesinger.de>
Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
Co-developed-by: Srinivas Neeli <srinivas.neeli@amd.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
drivers/dma/xilinx/xilinx_dma.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 35b553ee3205..aa3dee0dc2fc 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -1593,7 +1593,6 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
reg &= ~XILINX_DMA_CR_COALESCE_MAX;
reg |= chan->desc_pendingcount <<
XILINX_DMA_CR_COALESCE_SHIFT;
- dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
}
if (chan->has_sg && list_empty(&chan->active_list))
@@ -1604,7 +1603,8 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
reg |= XILINX_DMA_DMAXR_ALL_IRQ_MASK;
dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
- xilinx_dma_start(chan);
+ if (chan->idle)
+ xilinx_dma_start(chan);
if (chan->err)
return;
@@ -1693,7 +1693,8 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
reg |= XILINX_MCDMA_CR_RUNSTOP_MASK;
dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
- xilinx_dma_start(chan);
+ if (chan->idle)
+ xilinx_dma_start(chan);
if (chan->err)
return;
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH RESEND 3/3] dmaengine: xilinx_dma: Optimize control register write and channel start logic for AXIDMA and MCDMA in corresponding start_transfer()
2026-06-20 20:34 ` [PATCH RESEND 3/3] dmaengine: xilinx_dma: Optimize control register write and channel start logic for AXIDMA and MCDMA in corresponding start_transfer() Suraj Gupta
@ 2026-06-20 20:46 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-06-20 20:46 UTC (permalink / raw)
To: Suraj Gupta; +Cc: vkoul, dmaengine, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent read-modify-write on shared global MCDMA registers due to lack of a controller-level lock.
--
commit ecde894a65e016199009c2bd5c7fcf89cdab08cf
Author: Suraj Gupta <suraj.gupta2@amd.com>
dmaengine: xilinx_dma: Optimize control register write and channel start logic for AXIDMA and MCDMA in corresponding start_transfer()
This commit optimizes AXI DMA control register programming by combining
coalesce count and delay configuration into a single register write to
reduce bus overhead. It also avoids redundant channel starts by only calling
xilinx_dma_start() when the channel is actually idle.
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 35b553ee3205c..aa3dee0dc2fcb 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
[ ... ]
> @@ -1693,7 +1693,8 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
> reg |= XILINX_MCDMA_CR_RUNSTOP_MASK;
> dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
>
> - xilinx_dma_start(chan);
> + if (chan->idle)
> + xilinx_dma_start(chan);
[Severity: High]
This is a pre-existing issue, but does the lack of a controller-level lock
cause concurrent read-modify-write operations on shared global MCDMA
registers to race?
In xilinx_mcdma_start_transfer(), just before this diff context, there
is an unprotected read-modify-write on the shared XILINX_MCDMA_CHEN_OFFSET
register:
reg = dma_ctrl_read(chan, XILINX_MCDMA_CHEN_OFFSET);
reg |= BIT(chan->tdest);
dma_ctrl_write(chan, XILINX_MCDMA_CHEN_OFFSET, reg);
Similarly, when xilinx_dma_start() is called here, it performs an
unprotected read-modify-write on the shared XILINX_DMA_REG_DMACR register:
dma_ctrl_set(chan, XILINX_DMA_REG_DMACR, XILINX_DMA_DMACR_RUNSTOP);
These operations appear to be protected only by the per-channel spinlock
(chan->lock). If two channels on the same controller execute their start
transfer logic concurrently on different CPUs, can the read-modify-write
operations race and lead to a channel's enable bit being silently
overwritten and lost?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620203417.4000360-1-suraj.gupta2@amd.com?part=3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RESEND 2/3] dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction
2026-06-20 20:34 ` [PATCH RESEND 2/3] dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction Suraj Gupta
@ 2026-06-20 20:47 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-06-20 20:47 UTC (permalink / raw)
To: Suraj Gupta; +Cc: dmaengine, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Removing the channel idle check unconditionally overwrites hardware registers in Direct (non-SG) mode, corrupting active transfers.
- [Critical] Appending new transfers to a running Scatter-Gather chain breaks the hardware descriptor chain, as new descriptors are never physically linked in memory.
- [High] Updating the interrupt coalesce threshold while the channel is active uses a partial descriptor count, disrupting interrupt generation.
--
commit 59ccde00ae0d672019658b79ee8d2055ebb8fa7a
Author: Suraj Gupta <suraj.gupta2@amd.com>
dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction
This commit removes the idle check in xilinx_dma_start_transfer() and
xilinx_mcdma_start_transfer() to allow queuing new transfers while the
channel is busy. It also updates the CURDESC register only when the active
list is empty in scatter-gather mode, relying on the hardware tail pointer
extension mechanism to handle chaining.
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index ca396b7097427..35b553ee3205c 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -1580,9 +1580,6 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
> return;
> }
>
> - if (!chan->idle)
> - return;
> -
[Severity: Critical]
Does removing this early return cause active transfers to be corrupted in
Direct (non-SG) mode?
Without the idle check, xilinx_dma_start_transfer() can proceed while a
transfer is running. In Direct DMA mode (has_sg is false), the code falls
through and executes:
xilinx_write(chan, XILINX_DMA_REG_SRCDSTADDR,
xilinx_prep_dma_addr_t(hw->buf_addr));
dma_ctrl_write(chan, XILINX_DMA_REG_BTT,
hw->control & chan->xdev->max_buffer_len);
Could this unconditionally overwrite the active hardware registers if a
transfer is already in progress?
Additionally, moving the newly queued descriptor into active_list appears to
cause xilinx_dma_complete_descriptor() to prematurely complete newly appended
descriptors, because the loop break condition is skipped for non-SG mode:
if (!(seg->hw.status & XILINX_DMA_BD_COMP_MASK) && chan->has_sg)
break;
[Severity: Critical]
Can appending new transfers to a running Scatter-Gather chain break the
hardware descriptor chain?
When the idle check is removed, we update the TAILDESC register to append
transfers on-the-fly. However, the Xilinx AXI DMA engine requires SG
descriptors to be physically linked in memory via their next_desc pointers.
In the driver, descriptor linking occurs in append_desc_queue():
if (list_empty(&chan->pending_list))
goto append;
...
tail_desc = list_last_entry(&chan->pending_list, ...);
Since start_transfer() moves all queued descriptors to chan->active_list and
leaves pending_list empty, a subsequent submission will find an empty
pending_list.
Will this cause it to skip linking its next_desc pointer to the tail of the
actively running descriptor chain in active_list?
When the DMA engine reaches the end of the original active chain, it might
follow a stale next_desc pointer instead of the physically unlinked new
descriptors.
> head_desc = list_first_entry(&chan->pending_list,
> struct xilinx_dma_tx_descriptor, node);
> tail_desc = list_last_entry(&chan->pending_list,
> @@ -1599,7 +1596,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
> dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
> }
>
> - if (chan->has_sg)
> + if (chan->has_sg && list_empty(&chan->active_list))
[Severity: High]
Does updating the interrupt coalesce threshold while the channel is active
disrupt interrupt generation?
The driver updates the hardware interrupt coalesce threshold using
chan->desc_pendingcount before writing to the DMACR register:
if (chan->desc_pendingcount <= XILINX_DMA_COALESCE_MAX) {
reg &= ~XILINX_DMA_CR_COALESCE_MAX;
reg |= chan->desc_pendingcount << XILINX_DMA_CR_COALESCE_SHIFT;
}
With the removal of the idle check, this function now executes while
descriptors are actively running. Since desc_pendingcount is reset to 0 at
the end of each start_transfer() call, it now only represents the number
of newly submitted descriptors in the latest pending_list batch.
Will overwriting the active coalesce threshold with this potentially much
smaller partial count disrupt the DMA engine's interrupt generation and cause
erratic coalescing behavior?
> xilinx_write(chan, XILINX_DMA_REG_CURDESC,
> head_desc->async_tx.phys);
> reg &= ~XILINX_DMA_CR_DELAY_MAX;
[ ... ]
> @@ -1660,9 +1657,6 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
> if (chan->err)
> return;
>
> - if (!chan->idle)
> - return;
> -
[Severity: Critical]
Does MCDMA suffer from the same unlinked descriptor chain vulnerability when
TDESC is updated while active?
Like xilinx_dma_start_transfer(), appending new transfers to a running
Scatter-Gather chain might break the hardware descriptor chain because the
new descriptors are not physically linked in memory to the active list.
> if (list_empty(&chan->pending_list))
> return;
>
> @@ -1685,8 +1679,9 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
> dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
>
> /* Program current descriptor */
> - xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
> - head_desc->async_tx.phys);
> + if (chan->has_sg && list_empty(&chan->active_list))
[Severity: High]
Does this also overwrite the MCDMA CR coalesce field with a partial count
while the engine may be active?
if (chan->desc_pendingcount <= XILINX_MCDMA_COALESCE_MAX) {
reg &= ~XILINX_MCDMA_COALESCE_MASK;
reg |= chan->desc_pendingcount << XILINX_MCDMA_COALESCE_SHIFT;
}
Similar to the AXIDMA case, using desc_pendingcount here while active might
disrupt interrupt generation.
> + xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
> + head_desc->async_tx.phys);
>
> /* Program channel enable register */
> reg = dma_ctrl_read(chan, XILINX_MCDMA_CHEN_OFFSET);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620203417.4000360-1-suraj.gupta2@amd.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RESEND 1/3] dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers
2026-06-20 20:34 ` [PATCH RESEND 1/3] dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers Suraj Gupta
@ 2026-06-20 20:47 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-06-20 20:47 UTC (permalink / raw)
To: Suraj Gupta; +Cc: vkoul, dmaengine, Frank.Li
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The patch's intended fix is completely ineffective for MCDMA and CDMA channels due to a pre-existing bug where descriptors are unconditionally completed without checking hardware status.
- [High] Cyclic DMA completion callbacks are permanently starved and will never be executed because the cyclic descriptor is moved out of `done_list` before the cleanup tasklet runs.
- [High] The MCDMA IRQ handler fails to process descriptor completions when a delay interrupt occurs, leading to indefinite stalls.
--
commit bb06990d374374bf16e72c807c81d0d1449ff514
Author: Suraj Gupta <suraj.gupta2@amd.com>
dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers
Fix a race condition in AXIDMA and MCDMA irq handlers where the channel
could be incorrectly marked as idle and attempt spurious transfers when
descriptors are still being processed.
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 404235c173538..ca396b7097427 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -1893,8 +1893,10 @@ static irqreturn_t xilinx_mcdma_irq_handler(int irq, void *data)
> if (status & XILINX_MCDMA_IRQ_IOC_MASK) {
> spin_lock(&chan->lock);
> xilinx_dma_complete_descriptor(chan);
> - chan->idle = true;
> - chan->start_transfer(chan);
> + if (list_empty(&chan->active_list)) {
> + chan->idle = true;
> + chan->start_transfer(chan);
> + }
[Severity: Critical]
This is a pre-existing issue, but does this list_empty() check actually
prevent the race condition for MCDMA channels?
Looking at drivers/dma/xilinx/xilinx_dma.c in xilinx_dma_complete_descriptor(),
hardware completion status is only validated for AXIDMA:
xilinx_dma_complete_descriptor() {
...
if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
struct xilinx_axidma_tx_segment *seg;
seg = list_last_entry(&desc->segments,
struct xilinx_axidma_tx_segment, node);
if (!(seg->hw.status & XILINX_DMA_BD_COMP_MASK) && chan->has_sg)
break;
}
...
}
For MCDMA and CDMA channels, descriptors are unconditionally unlinked and
moved to done_list. This means active_list is always emptied by
xilinx_dma_complete_descriptor(), causing list_empty(&chan->active_list)
to always evaluate to true.
Doesn't this make the patch's fix ineffective for MCDMA, while also risking
data corruption since clients might reuse memory before the hardware actually
completes the transfer?
[Severity: High]
This is a pre-existing issue, but does the MCDMA IRQ handler properly process
descriptor completions when a delay interrupt occurs?
Unlike the AXIDMA handler which checks both FRM_CNT_IRQ and DLY_CNT_IRQ,
if a delay interrupt fires for MCDMA in drivers/dma/xilinx/xilinx_dma.c,
the handler only logs a message:
xilinx_mcdma_irq_handler() {
...
if (status & XILINX_MCDMA_IRQ_DELAY_MASK) {
/*
* Device takes too long to do the transfer when user requires
* responsiveness.
*/
dev_dbg(chan->dev, "Inter-packet latency too long\n");
}
...
}
If a delay interrupt occurs without an accompanying IOC interrupt, which
happens when the transferred count hasn't reached the coalesce threshold but
the timer expires, descriptor completions are never processed.
Could this lead to indefinite stalls for the DMA client since the channel
becomes unresponsive?
> spin_unlock(&chan->lock);
> }
>
> @@ -1950,8 +1952,10 @@ static irqreturn_t xilinx_dma_irq_handler(int irq, void *data)
> XILINX_DMA_DMASR_DLY_CNT_IRQ)) {
> spin_lock(&chan->lock);
> xilinx_dma_complete_descriptor(chan);
> - chan->idle = true;
> - chan->start_transfer(chan);
> + if (list_empty(&chan->active_list)) {
> + chan->idle = true;
> + chan->start_transfer(chan);
> + }
[Severity: High]
This is a pre-existing issue, but will cyclic DMA completion callbacks ever
be executed since start_transfer() is called here?
When a cyclic transfer completes, xilinx_dma_complete_descriptor() moves
the cyclic descriptor from active_list to done_list. When start_transfer()
is invoked immediately after, it preemptively moves the descriptor back into
active_list because pending_list is empty in cyclic mode:
drivers/dma/xilinx/xilinx_dma.c:xilinx_dma_start_transfer() {
...
list_splice_tail_init(&chan->done_list, &chan->active_list);
...
}
After the IRQ handler returns, the cleanup tasklet executes to iterate over
done_list and trigger the callback:
drivers/dma/xilinx/xilinx_dma.c:xilinx_dma_chan_desc_cleanup() {
...
list_for_each_entry_safe(desc, next, &chan->done_list, node) {
struct dmaengine_result result;
if (desc->cyclic) {
xilinx_dma_chan_handle_cyclic(chan, desc, &flags);
...
}
Because done_list was already completely emptied by start_transfer(), won't
this loop be bypassed entirely, causing the cyclic notification mechanism
to fail and clients to stall?
> spin_unlock(&chan->lock);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620203417.4000360-1-suraj.gupta2@amd.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-06-20 20:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-20 20:34 [PATCH RESEND 0/3] dmaengine: xilinx_dma: Fixes and optimizations for AXIDMA and MCDMA channel management Suraj Gupta
2026-06-20 20:34 ` [PATCH RESEND 1/3] dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers Suraj Gupta
2026-06-20 20:47 ` sashiko-bot
2026-06-20 20:34 ` [PATCH RESEND 2/3] dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction Suraj Gupta
2026-06-20 20:47 ` sashiko-bot
2026-06-20 20:34 ` [PATCH RESEND 3/3] dmaengine: xilinx_dma: Optimize control register write and channel start logic for AXIDMA and MCDMA in corresponding start_transfer() Suraj Gupta
2026-06-20 20:46 ` sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.