DMA Engine development
 help / color / mirror / Atom feed
* [PATCH 0/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal
@ 2026-06-30  6:48 Golla Nagendra
  2026-06-30  6:48 ` [PATCH 1/2] " Golla Nagendra
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Golla Nagendra @ 2026-06-30  6:48 UTC (permalink / raw)
  To: vkoul, Frank.Li, michal.simek, nagendra.golla, abin.joseph, kees,
	ptsm, sakari.ailus, radhey.shyam.pandey, u.kleine-koenig
  Cc: git, dmaengine, linux-arm-kernel, linux-kernel

This patch series addresses two issues in the zynqmp_dma_remove() function:
 
1. Fix the race condition between runtime PM and device removal where
   pm_runtime_disable() was called after the power state check, leaving
   a window for the runtime PM framework to change state unexpectedly.
 
2. Update the stale kernel doc comment that still references a return
   value after the function was converted to return void.



Golla Nagendra (2):
  dmaengine: zynqmp_dma: fix race between runtime PM and device removal
  dmaengine: zynqmp_dma: fix kernel doc for zynqmp_dma_remove()

 drivers/dma/xilinx/zynqmp_dma.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

-- 
2.49.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal
  2026-06-30  6:48 [PATCH 0/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal Golla Nagendra
@ 2026-06-30  6:48 ` Golla Nagendra
  2026-06-30  7:01   ` sashiko-bot
  2026-06-30  7:21   ` Pandey, Radhey Shyam
  2026-06-30  6:48 ` [PATCH 2/2] dmaengine: zynqmp_dma: fix kernel doc for zynqmp_dma_remove() Golla Nagendra
  2026-07-02 16:03 ` [PATCH 0/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal Vinod Koul
  2 siblings, 2 replies; 7+ messages in thread
From: Golla Nagendra @ 2026-06-30  6:48 UTC (permalink / raw)
  To: vkoul, Frank.Li, michal.simek, nagendra.golla, abin.joseph, kees,
	ptsm, sakari.ailus, radhey.shyam.pandey, u.kleine-koenig
  Cc: git, dmaengine, linux-arm-kernel, linux-kernel

In zynqmp_dma_remove(), runtime PM was disabled only after checking
state and doing a manual suspend. This can race with runtime PM in the
remove/unbind (rmmod) path.

Disable runtime PM first, then suspend only if the device is not already
suspended. To prevent any further runtime PM transitions.

Fixes: 72dd8b2914b5 ("dmaengine: zynqmp_dma: Add shutdown operation support")
Co-developed-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
 drivers/dma/xilinx/zynqmp_dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index 1402331f7ef5..26f097db593d 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -1188,9 +1188,9 @@ static void zynqmp_dma_remove(struct platform_device *pdev)
 	dma_async_device_unregister(&zdev->common);
 
 	zynqmp_dma_chan_remove(zdev->chan);
-	if (pm_runtime_active(zdev->dev))
-		zynqmp_dma_runtime_suspend(zdev->dev);
 	pm_runtime_disable(zdev->dev);
+	if (!pm_runtime_status_suspended(zdev->dev))
+		zynqmp_dma_runtime_suspend(zdev->dev);
 }
 
 static const struct of_device_id zynqmp_dma_of_match[] = {
-- 
2.49.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] dmaengine: zynqmp_dma: fix kernel doc for zynqmp_dma_remove()
  2026-06-30  6:48 [PATCH 0/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal Golla Nagendra
  2026-06-30  6:48 ` [PATCH 1/2] " Golla Nagendra
@ 2026-06-30  6:48 ` Golla Nagendra
  2026-06-30  7:28   ` Pandey, Radhey Shyam
  2026-07-02 16:03 ` [PATCH 0/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal Vinod Koul
  2 siblings, 1 reply; 7+ messages in thread
From: Golla Nagendra @ 2026-06-30  6:48 UTC (permalink / raw)
  To: vkoul, Frank.Li, michal.simek, nagendra.golla, abin.joseph, kees,
	ptsm, sakari.ailus, radhey.shyam.pandey, u.kleine-koenig
  Cc: git, dmaengine, linux-arm-kernel, linux-kernel

The zynqmp_dma_remove() function was converted from returning int to
void, but the kernel doc comment was not updated to reflect this change.
Remove the stale "Return: Always '0'" documentation that no longer
applies to the void function.

Fixes: b1c50ac25425 ("dmaengine: xilinx: zynqmp_dma: Convert to platform remove callback returning void")
Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
 drivers/dma/xilinx/zynqmp_dma.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index 26f097db593d..ba6604dd7153 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -1177,8 +1177,6 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
 /**
  * zynqmp_dma_remove - Driver remove function
  * @pdev: Pointer to the platform_device structure
- *
- * Return: Always '0'
  */
 static void zynqmp_dma_remove(struct platform_device *pdev)
 {
-- 
2.49.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal
  2026-06-30  6:48 ` [PATCH 1/2] " Golla Nagendra
@ 2026-06-30  7:01   ` sashiko-bot
  2026-06-30  7:21   ` Pandey, Radhey Shyam
  1 sibling, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-06-30  7:01 UTC (permalink / raw)
  To: Golla Nagendra; +Cc: vkoul, dmaengine, Frank.Li

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

Pre-existing issues:
- [High] The probe error path (`err_disable_pm`) fails to drop the PM runtime usage counter and leaves hardware clocks enabled if PM runtime is active.
- [High] Memory and PM runtime usage counter leak in `zynqmp_dma_alloc_chan_resources()` on error paths.
- [High] Device removal and shutdown paths disable clocks without first halting the DMA hardware, which can cause an AXI bus hang.
- [High] In `zynqmp_dma_prep_memcpy`, a 0-length transfer results in an invalid descriptor allocation count leading to free_list memory corruption.
- [High] Missing dma_wmb() before clearing the STOP bit when appending DMA descriptors.
--

commit 14cd36b7d6a47ddc5e88925238c330623d079cf9
Author: Golla Nagendra <nagendra.golla@amd.com>
Subject: dmaengine: zynqmp_dma: fix race between runtime PM and device removal

This commit changes zynqmp_dma_remove() to disable runtime PM before
attempting to manually suspend the device. This prevents a potential race
condition where runtime PM transitions could happen concurrently during the
driver's unbind path.

> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index f6a812e49ddca..ca2dee0012c87 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> @@ -1170,9 +1170,9 @@ static void zynqmp_dma_remove(struct platform_device *pdev)
>  	dma_async_device_unregister(&zdev->common);
>  
>  	zynqmp_dma_chan_remove(zdev->chan);
> -	if (pm_runtime_active(zdev->dev))
> -		zynqmp_dma_runtime_suspend(zdev->dev);
>  	pm_runtime_disable(zdev->dev);
> +	if (!pm_runtime_status_suspended(zdev->dev))
> +		zynqmp_dma_runtime_suspend(zdev->dev);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does zynqmp_dma_remove() need
to halt the DMA hardware before disabling clocks?

The function disables device clocks via zynqmp_dma_runtime_suspend() but never
writes to the hardware to halt active transfers or waits for idle.

Could stopping clocks while an AXI master is mid-transaction violate the bus
protocol and cause an interconnect hang?


[Severity: High]
This isn't a bug introduced by this patch, but does the probe error path fail
to drop the PM runtime usage counter?

If zynqmp_dma_chan_probe() or subsequent initialization steps fail, the
err_disable_pm path is taken:

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_probe() {
...
err_disable_pm:
	if (!pm_runtime_enabled(zdev->dev))
		zynqmp_dma_runtime_suspend(zdev->dev);
	pm_runtime_disable(zdev->dev);
	return ret;
}

When CONFIG_PM is enabled, pm_runtime_enabled() evaluates to true, so it
skips calling zynqmp_dma_runtime_suspend().

Doesn't this also fail to call pm_runtime_put_noidle() to balance the
earlier pm_runtime_resume_and_get(), permanently leaking the usage
counter and leaving hardware clocks running?


[Severity: High]
This isn't a bug introduced by this patch, but do the error paths in
zynqmp_dma_alloc_chan_resources() leak memory and PM usage counters?

If memory allocation fails:

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_alloc_chan_resources() {
...
	chan->sw_desc_pool = kzalloc_objs(*desc, ZYNQMP_DMA_NUM_DESCS);
	if (!chan->sw_desc_pool)
		return -ENOMEM;
...
}

This returns -ENOMEM directly without calling pm_runtime_put() to balance
the earlier pm_runtime_resume_and_get(), and without freeing previously
allocated structures.

Since the dmaengine core doesn't automatically invoke free_chan_resources()
when alloc_chan_resources() fails, does this permanently leak the usage
counter and descriptor memory?


[Severity: High]
This isn't a bug introduced by this patch, but can a 0-length transfer cause
memory corruption in zynqmp_dma_prep_memcpy()?

If a client driver requests a DMA memcpy with len == 0:

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_prep_memcpy() {
...
	desc_cnt = DIV_ROUND_UP(len, ZYNQMP_DMA_MAX_TRANS_LEN);
...
	chan->desc_free_cnt = chan->desc_free_cnt - desc_cnt;
...
	do {
		new = zynqmp_dma_get_descriptor(chan);
...
	} while (len);
...
}

DIV_ROUND_UP(0, ...) is 0, so desc_cnt is 0. The availability check passes
and desc_free_cnt is decremented by 0.

However, the subsequent do-while loop executes once, consuming one descriptor
from free_list.

When the descriptor is later freed, desc_free_cnt is incremented by 1,
inflating the available count.

Could subsequent transfers then bypass the free count check and extract a
descriptor from an empty list?


[Severity: High]
This isn't a bug introduced by this patch, but is a memory barrier missing
before clearing the STOP bit when appending descriptors?

In zynqmp_dma_tx_submit():

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_tx_submit() {
...
		desc->src_v->nxtdscraddr = new->src_p;
		desc->src_v->ctrl &= ~ZYNQMP_DMA_DESC_CTRL_STOP;
		desc->dst_v->nxtdscraddr = new->dst_p;
		desc->dst_v->ctrl &= ~ZYNQMP_DMA_DESC_CTRL_STOP;
...
}

The CPU updates the previous descriptor's nxtdscraddr and then clears its
ZYNQMP_DMA_DESC_CTRL_STOP bit.

Without a dma_wmb() between these stores to coherent memory, could the CPU
or memory controller reorder them?

If the DMA controller fetches the cleared ctrl word before the nxtdscraddr
update is globally visible, might it jump to a stale or invalid address?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630064844.705173-1-nagendra.golla@amd.com?part=1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal
  2026-06-30  6:48 ` [PATCH 1/2] " Golla Nagendra
  2026-06-30  7:01   ` sashiko-bot
@ 2026-06-30  7:21   ` Pandey, Radhey Shyam
  1 sibling, 0 replies; 7+ messages in thread
From: Pandey, Radhey Shyam @ 2026-06-30  7:21 UTC (permalink / raw)
  To: Golla Nagendra, vkoul, Frank.Li, michal.simek, abin.joseph, kees,
	ptsm, sakari.ailus, radhey.shyam.pandey, u.kleine-koenig
  Cc: git, dmaengine, linux-arm-kernel, linux-kernel

> In zynqmp_dma_remove(), runtime PM was disabled only after checking
> state and doing a manual suspend. This can race with runtime PM in the
> remove/unbind (rmmod) path.
> 
> Disable runtime PM first, then suspend only if the device is not already
> suspended. To prevent any further runtime PM transitions.
> 
> Fixes: 72dd8b2914b5 ("dmaengine: zynqmp_dma: Add shutdown operation support")
> Co-developed-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
> Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
> Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Thanks!
> ---
>   drivers/dma/xilinx/zynqmp_dma.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index 1402331f7ef5..26f097db593d 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> @@ -1188,9 +1188,9 @@ static void zynqmp_dma_remove(struct platform_device *pdev)
>   	dma_async_device_unregister(&zdev->common);
>   
>   	zynqmp_dma_chan_remove(zdev->chan);
> -	if (pm_runtime_active(zdev->dev))
> -		zynqmp_dma_runtime_suspend(zdev->dev);
>   	pm_runtime_disable(zdev->dev);
> +	if (!pm_runtime_status_suspended(zdev->dev))
> +		zynqmp_dma_runtime_suspend(zdev->dev);
>   }
>   
>   static const struct of_device_id zynqmp_dma_of_match[] = {


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] dmaengine: zynqmp_dma: fix kernel doc for zynqmp_dma_remove()
  2026-06-30  6:48 ` [PATCH 2/2] dmaengine: zynqmp_dma: fix kernel doc for zynqmp_dma_remove() Golla Nagendra
@ 2026-06-30  7:28   ` Pandey, Radhey Shyam
  0 siblings, 0 replies; 7+ messages in thread
From: Pandey, Radhey Shyam @ 2026-06-30  7:28 UTC (permalink / raw)
  To: Golla Nagendra, vkoul, Frank.Li, michal.simek, abin.joseph, kees,
	ptsm, sakari.ailus, radhey.shyam.pandey, u.kleine-koenig
  Cc: git, dmaengine, linux-arm-kernel, linux-kernel

On 6/30/2026 12:18 PM, Golla Nagendra wrote:
> The zynqmp_dma_remove() function was converted from returning int to
> void, but the kernel doc comment was not updated to reflect this change.
> Remove the stale "Return: Always '0'" documentation that no longer
> applies to the void function.
> 
> Fixes: b1c50ac25425 ("dmaengine: xilinx: zynqmp_dma: Convert to platform remove callback returning void")
> Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
> ---

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Thanks!

>   drivers/dma/xilinx/zynqmp_dma.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index 26f097db593d..ba6604dd7153 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> @@ -1177,8 +1177,6 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
>   /**
>    * zynqmp_dma_remove - Driver remove function
>    * @pdev: Pointer to the platform_device structure
> - *
> - * Return: Always '0'
>    */
>   static void zynqmp_dma_remove(struct platform_device *pdev)
>   {


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal
  2026-06-30  6:48 [PATCH 0/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal Golla Nagendra
  2026-06-30  6:48 ` [PATCH 1/2] " Golla Nagendra
  2026-06-30  6:48 ` [PATCH 2/2] dmaengine: zynqmp_dma: fix kernel doc for zynqmp_dma_remove() Golla Nagendra
@ 2026-07-02 16:03 ` Vinod Koul
  2 siblings, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2026-07-02 16:03 UTC (permalink / raw)
  To: Frank.Li, michal.simek, abin.joseph, kees, ptsm, sakari.ailus,
	radhey.shyam.pandey, u.kleine-koenig, Golla Nagendra
  Cc: git, dmaengine, linux-arm-kernel, linux-kernel


On Tue, 30 Jun 2026 12:18:42 +0530, Golla Nagendra wrote:
> This patch series addresses two issues in the zynqmp_dma_remove() function:
> 
> 1. Fix the race condition between runtime PM and device removal where
>    pm_runtime_disable() was called after the power state check, leaving
>    a window for the runtime PM framework to change state unexpectedly.
> 
> 2. Update the stale kernel doc comment that still references a return
>    value after the function was converted to return void.
> 
> [...]

Applied, thanks!

[1/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal
      commit: 516ba2d8b7aac4238f9fcbd58579c43c71b9b695
[2/2] dmaengine: zynqmp_dma: fix kernel doc for zynqmp_dma_remove()
      commit: f7e89cba18a1ca6462712ae459a88dd40537b693

Best regards,
-- 
~Vinod



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-02 16:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-30  6:48 [PATCH 0/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal Golla Nagendra
2026-06-30  6:48 ` [PATCH 1/2] " Golla Nagendra
2026-06-30  7:01   ` sashiko-bot
2026-06-30  7:21   ` Pandey, Radhey Shyam
2026-06-30  6:48 ` [PATCH 2/2] dmaengine: zynqmp_dma: fix kernel doc for zynqmp_dma_remove() Golla Nagendra
2026-06-30  7:28   ` Pandey, Radhey Shyam
2026-07-02 16:03 ` [PATCH 0/2] dmaengine: zynqmp_dma: fix race between runtime PM and device removal Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox