All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] dmaengine: Remove obsolete 32-bit DMA mask fallbacks
@ 2026-09-03 11:54 Ruizhe Zhou
  2026-09-03 11:54 ` [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback Ruizhe Zhou
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Ruizhe Zhou @ 2026-09-03 11:54 UTC (permalink / raw)
  To: Vinod Koul, Basavaraj Natikar, Logan Gunthorpe, Orson Zhai,
	Baolin Wang
  Cc: Frank Li, Chunyan Zhang, dmaengine, linux-kernel, Ruizhe Zhou

A few DMAengine drivers set a >32 bit DMA mask and retry with a 32-bit
mask if the first call fails. This treats the return value of
dma_set_mask_and_coherent() as an indication that the platform requires
a narrower DMA width.

That is not a correct interpretation of dma_set_mask_and_coherent().
The mask passed to the DMA API describes the highest address a device can
reach. It asks the DMA layer to keep mappings within that limit; it is not
a probe asking the platform which addressing mode the device should use.
A wider mask includes every address permitted by a 32-bit mask, including
addresses from a platform that only produces 32-bit DMA addresses.

Since commit 91ef26f914171 ("dma-direct: relax addressability checks in
dma_direct_supported"), the DMA API guarantees support for masks
of 32 bits or wider. A failed wider request is therefore not evidence that
the driver should advertise a smaller capability. Retrying with a 32-bit
mask cannot repair the underlying DMA setup failure. The DMA API HOWTO
explicitly identifies this fallback pattern as incorrect [1]. See [2] and
[3] for details.

This series removes the fallbacks from the AMD PTDMA, PLX and
Spreadtrum DMA drivers without changing the mask selected for each device.
The return-value checks are retained. DMA setup errors are still reported
and still abort device initialization.

Testing was compile-only. No hardware testing was performed.

[1] DMA API HOWTO, "DMA addressing capabilities"
https://docs.kernel.org/core-api/dma-api-howto.html#dma-addressing-capabilities

[2] commit 91ef26f914171 ("dma-direct: relax addressability checks in
dma_direct_supported")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=91ef26f914171cf753330f13724fd9142b5b1640

[3] commit f7ae20f2fc4e ("docs: dma: correct dma_set_mask() sample code")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f7ae20f2fc4e

Best regards,
Ruizhe Zhou

Ruizhe Zhou (3):
  dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback
  dmaengine: plx_dma: Remove obsolete 32-bit DMA mask fallback
  dmaengine: sprd: Remove obsolete 32-bit DMA mask fallback

 drivers/dma/amd/ptdma/ptdma-pci.c | 8 ++------
 drivers/dma/plx_dma.c             | 2 --
 drivers/dma/sprd-dma.c            | 7 ++-----
 3 files changed, 4 insertions(+), 13 deletions(-)


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
--
2.27.0

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

* [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback
  2026-09-03 11:54 [PATCH 0/3] dmaengine: Remove obsolete 32-bit DMA mask fallbacks Ruizhe Zhou
@ 2026-09-03 11:54 ` Ruizhe Zhou
  2026-09-03 19:26   ` Frank Li
  2026-09-03 11:54 ` [PATCH 2/3] dmaengine: plx_dma: " Ruizhe Zhou
  2026-09-03 11:54 ` [PATCH 3/3] dmaengine: sprd: " Ruizhe Zhou
  2 siblings, 1 reply; 11+ messages in thread
From: Ruizhe Zhou @ 2026-09-03 11:54 UTC (permalink / raw)
  To: Vinod Koul, Basavaraj Natikar, Logan Gunthorpe, Orson Zhai,
	Baolin Wang
  Cc: Frank Li, Chunyan Zhang, dmaengine, linux-kernel, Ruizhe Zhou

The DMA API guarantees support for masks of 32 bits or wider and
explicitly identifies retrying a 32-bit mask after a wider request as
incorrect:
https://docs.kernel.org/core-api/dma-api-howto.html#dma-addressing-capabilities

Remove the obsolete fallback while retaining the error check so that a
genuine DMA setup failure is still reported and aborts initialization.

Signed-off-by: Ruizhe Zhou <zhouruizhe@resnics.com>
---
 drivers/dma/amd/ptdma/ptdma-pci.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/amd/ptdma/ptdma-pci.c b/drivers/dma/amd/ptdma/ptdma-pci.c
index 22739ff0c3c5..d36bb9c67325 100644
--- a/drivers/dma/amd/ptdma/ptdma-pci.c
+++ b/drivers/dma/amd/ptdma/ptdma-pci.c
@@ -178,12 +178,8 @@ static int pt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(48));
 	if (ret) {
-		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
-		if (ret) {
-			dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n",
-				ret);
-			goto e_err;
-		}
+		dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n", ret);
+		goto e_err;
 	}
 
 	dev_set_drvdata(dev, pt);
-- 
2.27.0


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

* [PATCH 2/3] dmaengine: plx_dma: Remove obsolete 32-bit DMA mask fallback
  2026-09-03 11:54 [PATCH 0/3] dmaengine: Remove obsolete 32-bit DMA mask fallbacks Ruizhe Zhou
  2026-09-03 11:54 ` [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback Ruizhe Zhou
@ 2026-09-03 11:54 ` Ruizhe Zhou
  2026-09-03 14:29   ` sashiko-bot
  2026-09-03 22:00   ` Logan Gunthorpe
  2026-09-03 11:54 ` [PATCH 3/3] dmaengine: sprd: " Ruizhe Zhou
  2 siblings, 2 replies; 11+ messages in thread
From: Ruizhe Zhou @ 2026-09-03 11:54 UTC (permalink / raw)
  To: Vinod Koul, Basavaraj Natikar, Logan Gunthorpe, Orson Zhai,
	Baolin Wang
  Cc: Frank Li, Chunyan Zhang, dmaengine, linux-kernel, Ruizhe Zhou

The DMA API guarantees support for masks of 32 bits or wider and
explicitly identifies retrying a 32-bit mask after a wider request as
incorrect:
https://docs.kernel.org/core-api/dma-api-howto.html#dma-addressing-capabilities

Remove the obsolete fallback while retaining the error check so that a
genuine DMA setup failure is still propagated and aborts initialization.

Signed-off-by: Ruizhe Zhou <zhouruizhe@resnics.com>
---
 drivers/dma/plx_dma.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/dma/plx_dma.c b/drivers/dma/plx_dma.c
index 84941a918b01..ad9187448ed2 100644
--- a/drivers/dma/plx_dma.c
+++ b/drivers/dma/plx_dma.c
@@ -562,8 +562,6 @@ static int plx_dma_probe(struct pci_dev *pdev,
 		return rc;
 
 	rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(48));
-	if (rc)
-		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 	if (rc)
 		return rc;
 
-- 
2.27.0


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

* [PATCH 3/3] dmaengine: sprd: Remove obsolete 32-bit DMA mask fallback
  2026-09-03 11:54 [PATCH 0/3] dmaengine: Remove obsolete 32-bit DMA mask fallbacks Ruizhe Zhou
  2026-09-03 11:54 ` [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback Ruizhe Zhou
  2026-09-03 11:54 ` [PATCH 2/3] dmaengine: plx_dma: " Ruizhe Zhou
@ 2026-09-03 11:54 ` Ruizhe Zhou
  2026-09-03 14:26   ` sashiko-bot
  2 siblings, 1 reply; 11+ messages in thread
From: Ruizhe Zhou @ 2026-09-03 11:54 UTC (permalink / raw)
  To: Vinod Koul, Basavaraj Natikar, Logan Gunthorpe, Orson Zhai,
	Baolin Wang
  Cc: Frank Li, Chunyan Zhang, dmaengine, linux-kernel, Ruizhe Zhou

The DMA API guarantees support for masks of 32 bits or wider and
explicitly identifies retrying a 32-bit mask after a wider request as
incorrect:
https://docs.kernel.org/core-api/dma-api-howto.html#dma-addressing-capabilities

Remove the obsolete fallback while retaining the error check so that a
genuine DMA setup failure is still reported and aborts initialization.

Signed-off-by: Ruizhe Zhou <zhouruizhe@resnics.com>
---
 drivers/dma/sprd-dma.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 087fea3af2e4..b9f705dd80e5 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -1117,11 +1117,8 @@ static int sprd_dma_probe(struct platform_device *pdev)
 
 	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(36));
 	if (ret) {
-		ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
-		if (ret) {
-			dev_err(&pdev->dev, "unable to set coherent mask to 32\n");
-			return ret;
-		}
+		dev_err(&pdev->dev, "unable to set 36-bit DMA mask\n");
+		return ret;
 	}
 
 	/* Parse new and deprecated dma-channels properties */
-- 
2.27.0


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

* Re: [PATCH 3/3] dmaengine: sprd: Remove obsolete 32-bit DMA mask fallback
  2026-09-03 11:54 ` [PATCH 3/3] dmaengine: sprd: " Ruizhe Zhou
@ 2026-09-03 14:26   ` sashiko-bot
  0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-03 14:26 UTC (permalink / raw)
  To: Ruizhe Zhou; +Cc: dmaengine, Frank.Li, vkoul

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

Pre-existing issues:
- [High] Premature IRQ request leading to potential NULL pointer dereference and uninitialized spinlock access.
- [Medium] PM runtime usage counter and enabled state leak on the `pm_runtime_get_sync()` error path.
--

commit 323a1a8d4f9b18f09fa8dd3e9872da5d0d05321b
Author: Ruizhe Zhou <zhouruizhe@resnics.com>

dmaengine: sprd: Remove obsolete 32-bit DMA mask fallback

This commit removes the obsolete 32-bit DMA mask fallback from
sprd_dma_probe(), relying solely on the 36-bit DMA mask request, while
retaining the error check to report genuine DMA setup failures.

> diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
> index 087fea3af2e41..b9f705dd80e52 100644
> --- a/drivers/dma/sprd-dma.c
> +++ b/drivers/dma/sprd-dma.c
> @@ -1117,11 +1117,8 @@ static int sprd_dma_probe(struct platform_device *pdev)
>  
>  	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(36));
>  	if (ret) {
> -		ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> -		if (ret) {
> -			dev_err(&pdev->dev, "unable to set coherent mask to 32\n");
> -			return ret;
> -		}
> +		dev_err(&pdev->dev, "unable to set 36-bit DMA mask\n");
> +		return ret;
>  	}
>  
>  	/* Parse new and deprecated dma-channels properties */

[Severity: High]
This isn't a bug introduced by this patch, but looking further down in
sprd_dma_probe(), could the premature IRQ request lead to a NULL pointer
dereference or uninitialized spinlock access if an interrupt fires immediately?

If an interrupt is pending at the controller level, the handler dma_irq_handle()
will execute as soon as it is requested:

	ret = devm_request_irq(&pdev->dev, sdev->irq, dma_irq_handle,
			       0, "sprd_dma", (void *)sdev);

The interrupt handler will then dereference sdev->glb_base, which is mapped
later:

	sdev->glb_base = devm_platform_ioremap_resource(pdev, 0);

And it will attempt to acquire the spinlock, which is initialized even later:

	vchan_init(&dma_chn->vc, &sdev->dma_dev);

[Severity: Medium]
This is a pre-existing issue, but does the pm_runtime_get_sync() error path
leak the PM runtime usage counter and leave PM runtime enabled?

	ret = pm_runtime_get_sync(&pdev->dev);
	if (ret < 0)
		goto err_rpm;

If pm_runtime_get_sync() fails and returns a negative error code, it jumps
to the err_rpm label:

err_rpm:
	sprd_dma_disable(sdev);
	return ret;

Since pm_runtime_get_sync() increments the usage counter even on failure,
shouldn't this path call pm_runtime_put_noidle() to drop the usage counter,
and balance the earlier pm_runtime_enable() call with pm_runtime_disable()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903115441.912500-1-zhouruizhe@resnics.com?part=3

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

* Re: [PATCH 2/3] dmaengine: plx_dma: Remove obsolete 32-bit DMA mask fallback
  2026-09-03 11:54 ` [PATCH 2/3] dmaengine: plx_dma: " Ruizhe Zhou
@ 2026-09-03 14:29   ` sashiko-bot
  2026-09-03 22:00   ` Logan Gunthorpe
  1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-03 14:29 UTC (permalink / raw)
  To: Ruizhe Zhou; +Cc: dmaengine, Frank.Li, vkoul

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

Pre-existing issues:
- [High] Potential NULL pointer dereference in plx_dma_isr() because the interrupt handler is registered before plxdev->bar is initialized.
--

commit ebe0c3fecaf3a5fe091d793b49cde50217868ab8
Author: Ruizhe Zhou <zhouruizhe@resnics.com>

dmaengine: plx_dma: Remove obsolete 32-bit DMA mask fallback

The DMA API guarantees support for masks of 32 bits or wider. This commit
removes the obsolete 32-bit DMA mask fallback while retaining the error
check so that a genuine DMA setup failure is still propagated.

> diff --git a/drivers/dma/plx_dma.c b/drivers/dma/plx_dma.c
> index 84941a918b01f..ad9187448ed20 100644
> --- a/drivers/dma/plx_dma.c
> +++ b/drivers/dma/plx_dma.c
> @@ -562,8 +562,6 @@ static int plx_dma_probe(struct pci_dev *pdev,
>  		return rc;
>  
>  	rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(48));
> -	if (rc)
> -		rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>  	if (rc)
>  		return rc;
>  

[Severity: High]
This is a pre-existing issue, but is there a potential NULL pointer
dereference in plx_dma_isr() due to the order of initialization in
plx_dma_create()?

When plx_dma_probe() calls plx_dma_create(), the interrupt handler is
registered before plxdev->bar is initialized:

drivers/dma/plx_dma.c:plx_dma_create() {
    ...
    rc = request_irq(pci_irq_vector(pdev, 0), plx_dma_isr, 0,
                     KBUILD_MODNAME, plxdev);
    if (rc)
        goto free_plx;

    ...
    plxdev->bar = pcim_iomap_table(pdev)[0];
    ...
}

If an interrupt triggers immediately after request_irq() (e.g., from a
spurious interrupt or a pending interrupt left by a previous kernel/kexec),
the handler will execute:

drivers/dma/plx_dma.c:plx_dma_isr() {
    ...
    status = readw(plxdev->bar + PLX_REG_INTR_STATUS);
    ...
}

Since plxdev is zero-allocated and plxdev->bar is initialized only after
the IRQ is registered, could this cause a kernel panic due to a NULL
pointer dereference? Should the driver state be fully initialized before
unmasking the interrupt?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903115441.912500-1-zhouruizhe@resnics.com?part=2

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

* Re: [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback
  2026-09-03 11:54 ` [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback Ruizhe Zhou
@ 2026-09-03 19:26   ` Frank Li
  2026-09-04 11:15     ` 周睿哲
  0 siblings, 1 reply; 11+ messages in thread
From: Frank Li @ 2026-09-03 19:26 UTC (permalink / raw)
  To: Ruizhe Zhou
  Cc: Vinod Koul, Basavaraj Natikar, Logan Gunthorpe, Orson Zhai,
	Baolin Wang, Frank Li, Chunyan Zhang, dmaengine, linux-kernel

On Thu, Sep 03, 2026 at 07:54:39PM +0800, Ruizhe Zhou wrote:
> [You don't often get email from zhouruizhe@resnics.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> The DMA API guarantees support for masks of 32 bits or wider and
> explicitly identifies retrying a 32-bit mask after a wider request as
> incorrect:
> https://docs.kernel.org/core-api/dma-api-howto.html#dma-addressing-capabilities
>
> Remove the obsolete fallback while retaining the error check so that a
> genuine DMA setup failure is still reported and aborts initialization.
>
> Signed-off-by: Ruizhe Zhou <zhouruizhe@resnics.com>
> ---
>  drivers/dma/amd/ptdma/ptdma-pci.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma/amd/ptdma/ptdma-pci.c b/drivers/dma/amd/ptdma/ptdma-pci.c
> index 22739ff0c3c5..d36bb9c67325 100644
> --- a/drivers/dma/amd/ptdma/ptdma-pci.c
> +++ b/drivers/dma/amd/ptdma/ptdma-pci.c
> @@ -178,12 +178,8 @@ static int pt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>
>         ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(48));
>         if (ret) {
> -               ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> -               if (ret) {
> -                       dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n",
> -                               ret);
> -                       goto e_err;
> -               }
> +               dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n", ret);
> +               goto e_err;

also needn't check return value, it always return success if mask >= 32.

Frank
>         }
>
>         dev_set_drvdata(dev, pt);
> --
> 2.27.0
>

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

* Re: [PATCH 2/3] dmaengine: plx_dma: Remove obsolete 32-bit DMA mask fallback
  2026-09-03 11:54 ` [PATCH 2/3] dmaengine: plx_dma: " Ruizhe Zhou
  2026-09-03 14:29   ` sashiko-bot
@ 2026-09-03 22:00   ` Logan Gunthorpe
  1 sibling, 0 replies; 11+ messages in thread
From: Logan Gunthorpe @ 2026-09-03 22:00 UTC (permalink / raw)
  To: Ruizhe Zhou, Vinod Koul, Basavaraj Natikar, Orson Zhai,
	Baolin Wang
  Cc: Frank Li, Chunyan Zhang, dmaengine, linux-kernel



On 2026-09-03 5:54 a.m., Ruizhe Zhou wrote:
> The DMA API guarantees support for masks of 32 bits or wider and
> explicitly identifies retrying a 32-bit mask after a wider request as
> incorrect:
> https://docs.kernel.org/core-api/dma-api-howto.html#dma-addressing-capabilities
> 
> Remove the obsolete fallback while retaining the error check so that a
> genuine DMA setup failure is still propagated and aborts initialization.
> 
> Signed-off-by: Ruizhe Zhou <zhouruizhe@resnics.com>

Makes sense to me, thanks.

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

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

* Re:Re: [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback
  2026-09-03 19:26   ` Frank Li
@ 2026-09-04 11:15     ` 周睿哲
  2026-09-11 22:37       ` Frank Li
  0 siblings, 1 reply; 11+ messages in thread
From: 周睿哲 @ 2026-09-04 11:15 UTC (permalink / raw)
  To: Frank Li
  Cc: Vinod Koul, Basavaraj Natikar, Logan Gunthorpe, Orson Zhai,
	Baolin Wang, Frank Li, Chunyan Zhang, dmaengine, linux-kernel

Hi Frank,


From: Frank Li <Frank.li@oss.nxp.com>
Date: 2026-09-04 03:26:36
To:  Ruizhe Zhou <zhouruizhe@resnics.com>
Cc:  Vinod Koul <vkoul@kernel.org>,Basavaraj Natikar <Basavaraj.Natikar@amd.com>,Logan Gunthorpe <logang@deltatee.com>,Orson Zhai <orsonzhai@gmail.com>,Baolin Wang <baolin.wang@linux.alibaba.com>,Frank Li <Frank.Li@kernel.org>,Chunyan Zhang <zhang.lyra@gmail.com>,dmaengine@vger.kernel.org,linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback>On Thu, Sep 03, 2026 at 07:54:39PM +0800, Ruizhe Zhou wrote:
>> [You don't often get email from zhouruizhe@resnics.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> The DMA API guarantees support for masks of 32 bits or wider and
>> explicitly identifies retrying a 32-bit mask after a wider request as
>> incorrect:
>> https://docs.kernel.org/core-api/dma-api-howto.html#dma-addressing-capabilities
>>
>> Remove the obsolete fallback while retaining the error check so that a
>> genuine DMA setup failure is still reported and aborts initialization.
>>
>> Signed-off-by: Ruizhe Zhou <zhouruizhe@resnics.com>
>> ---
>>  drivers/dma/amd/ptdma/ptdma-pci.c | 8 ++------
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/dma/amd/ptdma/ptdma-pci.c b/drivers/dma/amd/ptdma/ptdma-pci.c
>> index 22739ff0c3c5..d36bb9c67325 100644
>> --- a/drivers/dma/amd/ptdma/ptdma-pci.c
>> +++ b/drivers/dma/amd/ptdma/ptdma-pci.c
>> @@ -178,12 +178,8 @@ static int pt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>
>>         ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(48));
>>         if (ret) {
>> -               ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>> -               if (ret) {
>> -                       dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n",
>> -                               ret);
>> -                       goto e_err;
>> -               }
>> +               dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n", ret);
>> +               goto e_err;
>
>also needn't check return value, it always return success if mask >= 32.

I did some more checking after your reply to make sure I understand
whether dma_set_mask_and_coherent() can actually fail for a mask wider
than 32 bits, and whether keeping the return-value check is necessary.

I also read your August discussion with Michal Pecio on the same
question:

https://lore.kernel.org/all/aoMty9Dvt2bnWm74@SMW015318/

In that discussion you pointed out that dev->dma_mask should already be
initialized by the bus before driver probe, and asked whether there are
any dma_supported() implementations which can actually reject such a
mask.

I agree that !dev->dma_mask does not look like an interesting failure
case for a normally probed device. However, after going through the
current dma_supported() paths and the in-tree .dma_supported callbacks,
I found several other cases which appear to make the stronger "cannot
fail for >32 bits" statement incorrect.

The current path in kernel/dma/mapping.c is:

static int dma_supported(struct device *dev, u64 mask)
{
        const struct dma_map_ops *ops = get_dma_ops(dev);

        if (use_dma_iommu(dev)) {
                if (WARN_ON(ops))
                        return false;
                return true;
        }

        if (ops) {
                if (!ops->dma_supported)
                        return true;
                return ops->dma_supported(dev, mask);
        }

        return dma_direct_supported(dev, mask);
}

and dma_set_mask() does:

if (!dev->dma_mask || !dma_supported(dev, mask))
        return -EIO;

For the generic direct-DMA path, the situation is clear:

int dma_direct_supported(struct device *dev, u64 mask)
{
        ...

        if (mask >= DMA_BIT_MASK(32))
                return 1;

        ...
}

So dma_direct_supported() itself cannot reject a >=32-bit mask.

However, this does not appear to be true for every path through
dma_supported().

My audit of the relevant current callbacks looks roughly like this:

+----------------------------+---------------------+-----------------------+
| Backend / callback            | >32 can fail?           | 64 can fail?                |
+----------------------------+---------------------+-----------------------+
| dma_direct_supported()        | No                          | No                          |
| default dma-iommu              | Yes, conflicting       | Yes, conflicting       |
|                                                | dma_ops state       | dma_ops state        |
| ibmebus_dma_supported()    | Yes, !=64 fails         | No                          |
| xen_grant_dma_supported()  | Yes, !=64 fails         | No                          |
| xen_swiotlb_dma_supported | Yes, threshold         | Not due to width   |
| ppc dma_iommu_supported()  | Yes                       | Yes, if no table        |
| parisc sba_dma_supported()  | Yes                         | Yes, if no IOC           |
| parisc ccio_supported()         | No for >=32          | No for valid dev       |
| alpha_pci_supported()           | Yes, threshold /     | Can fail if no             |
|                                                | mapping dependent   | usable DMA path|
| dma_dummy_supported()      | Yes, always           | Yes, always                |
+----------------------------+---------------------+-----------------------+

There seem to be two separate issues here.

First, "any mask wider than 32 bits cannot fail" has direct
counterexamples.

For example, arch/powerpc/platforms/pseries/ibmebus.c contains:

static int ibmebus_dma_supported(struct device *dev, u64 mask)
{
        return mask == DMA_BIT_MASK(64);
}

and installs it in ibmebus_dma_ops.

Therefore a call such as:

dma_set_mask(dev, DMA_BIT_MASK(40))

will fail even though the mask is wider than 32 bits.

drivers/xen/grant-dma-ops.c has the same rule:

static int xen_grant_dma_supported(struct device *dev, u64 mask)
{
        return mask == DMA_BIT_MASK(64);
}

and xen_grant_dma_ops installs this as .dma_supported.

So, for that backend as well, DMA_BIT_MASK(40), for example, is
rejected.

These two cases seem to directly contradict the more general statement
that dma_set_mask_and_coherent() cannot fail for a mask wider than
32 bits.

There are also cases where even DMA_BIT_MASK(64) can fail.

One example is the PowerPC legacy IOMMU backend in
arch/powerpc/kernel/dma-iommu.c:

int dma_iommu_dma_supported(struct device *dev, u64 mask)
{
        struct iommu_table *tbl;

        ...

        tbl = get_iommu_table_base(dev);

        if (!tbl) {
                dev_err(dev,
                        "Warning: IOMMU dma not supported: "
                        "mask 0x%08llx, table unavailable\n",
                        mask);
                return 0;
        }

        if (tbl->it_offset >
            (mask >> tbl->it_page_shift)) {
                ...
                return 0;
        }

        return 1;
}

If get_iommu_table_base() returns NULL, the callback rejects the mask
regardless of whether it is 32, 40, or 64 bits.

I understand that an unavailable IOMMU table may represent an
unexpected or unusable DMA configuration rather than an ordinary
address-width limitation, but that seems exactly like a reason for the
driver to retain the error check and abort probe instead of proceeding
as if DMA setup succeeded.

There is a similar state-dependent failure in
drivers/parisc/sba_iommu.c:

ioc = GET_IOC(dev);
if (!ioc)
        return 0;

/*
 * The max IO Virt address will *always* < 30 bits.
 */
return mask >= ...;

For a valid IOC the required address range is below 32 bits, so a
64-bit mask works. But if GET_IOC(dev) fails, DMA_BIT_MASK(64) is still
rejected.

The default dma-iommu path also appears to contain an intentional
failure condition:

if (use_dma_iommu(dev)) {
        if (WARN_ON(ops))
                return false;
        return true;
}

The intended state is:

use_dma_iommu(dev) == true
get_dma_ops(dev)   == NULL

because the default IOMMU DMA implementation does not rely on a
dma_map_ops instance.

However, if use_dma_iommu(dev) is true and get_dma_ops(dev) is
non-NULL, dma_supported() deliberately returns false, independent of
mask width.

This looks particularly significant because this was explicitly
discussed when the default dma-iommu implementation was changed from
dma_ops indirect calls to direct calls.

Christoph suggested moving the consistency check out of the fast path
and doing it in dma_set_mask(), "And fail the call while we're at it."
Leon replied that he would add it to dma_supported():

https://lore.kernel.org/all/20240718070406.GK5630@unreal/

The same discussion also says that the default-IOMMU state implies
!ops.

So the resulting WARN_ON(ops) path appears to be an intentional reason
for dma_set_mask() to return an error when the DMA backend state is
inconsistent, including when mask == DMA_BIT_MASK(64).

Another concrete case is dma_dummy_ops.

kernel/dma/dummy.c has:

static int dma_dummy_supported(struct device *hwdev, u64 mask)
{
        return 0;
}

so dma_set_mask() fails for every mask, including DMA_BIT_MASK(64).

I initially wondered whether this was only a theoretical sentinel that
could never be seen by a driver probe, but there is a real setup path
for it. acpi_dma_configure_id() does:

if (attr == DEV_DMA_NOT_SUPPORTED) {
        set_dma_ops(dev, &dma_dummy_ops);
        return 0;
}

Since this returns 0 from DMA configuration after installing
dma_dummy_ops, the device can continue through the driver-core setup
with DMA deliberately marked unsupported. If such a DMA-using driver
then calls dma_set_mask_and_coherent(), the return-value check is what
prevents it from continuing with an unusable DMA backend.

I agree this is not representative of the normal case for a healthy
PCI device, but it does seem to demonstrate that a 64-bit mask is not an
unconditional success guarantee of the DMA API itself.

The formal DMA API documentation also seems to reflect this. In
Documentation/core-api/dma-api.rst, under "DMA addressing
limitations", it says:

All the below functions which set a DMA mask may fail if the
requested mask cannot be used with the device, or if the device is
not capable of doing DMA.

and dma_set_mask_and_coherent() is documented as returning zero on
success and a negative error on failure.

So I think there are two different statements here:

1. For a normally configured DMA-capable device, a 64-bit DMA mask
   should be supportable.

2. dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)) cannot return
   an error, so checking its return value is unnecessary.

The first looks like the intended normal-case invariant, but I do not
see how the second follows given the current dma_supported() paths
above.

In particular, the PowerPC IOMMU table failure, the
use_dma_iommu(dev) && ops consistency check, and dma_dummy_ops are all
paths where the mask width itself is not the problem but
dma_set_mask_and_coherent(..., DMA_BIT_MASK(64)) can nevertheless
report failure.

That is why I currently think code like:

ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
if (ret)
        return ret;

still has value: the check is not probing whether 64-bit DMA
addressing is supported versus some narrower address width. It is
checking whether the DMA setup as a whole succeeded before the driver
starts using DMA.

Again, I do not work within the DMA subsystem, and therefore I won't exactly
claim myself to be an expert on this. The above are some findings after some
digging. So correct me if I am wrong. 

Am I missing an invariant which makes these failure paths unreachable
for a driver calling dma_set_mask_and_coherent()? If not, then I believe the return
value check is needed, and the DMA HOWTO document might need to mention that.

Thanks,
Ruizhe




>
>Frank
>>         }
>>
>>         dev_set_drvdata(dev, pt);
>> --
>> 2.27.0
>>
>

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

* Re: Re: [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback
  2026-09-04 11:15     ` 周睿哲
@ 2026-09-11 22:37       ` Frank Li
  2026-09-12  2:58         ` Frank Li
  0 siblings, 1 reply; 11+ messages in thread
From: Frank Li @ 2026-09-11 22:37 UTC (permalink / raw)
  To: 周睿哲, Christophe JAILLET, Christoph Hellwig
  Cc: Vinod Koul, Basavaraj Natikar, Logan Gunthorpe, Orson Zhai,
	Baolin Wang, Frank Li, Chunyan Zhang, dmaengine, linux-kernel

On Fri, Sep 04, 2026 at 07:15:36PM +0800, 周睿哲 wrote:
> Hi Frank,
>
>
> From: Frank Li <Frank.li@oss.nxp.com>
> Date: 2026-09-04 03:26:36
> To:  Ruizhe Zhou <zhouruizhe@resnics.com>
> Cc:  Vinod Koul <vkoul@kernel.org>,Basavaraj Natikar <Basavaraj.Natikar@amd.com>,Logan Gunthorpe <logang@deltatee.com>,Orson Zhai <orsonzhai@gmail.com>,Baolin Wang <baolin.wang@linux.alibaba.com>,Frank Li <Frank.Li@kernel.org>,Chunyan Zhang <zhang.lyra@gmail.com>,dmaengine@vger.kernel.org,linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback>On Thu, Sep 03, 2026 at 07:54:39PM +0800, Ruizhe Zhou wrote:
> >> [You don't often get email from zhouruizhe@resnics.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >>
> >> The DMA API guarantees support for masks of 32 bits or wider and
> >> explicitly identifies retrying a 32-bit mask after a wider request as
> >> incorrect:
> >> https://docs.kernel.org/core-api/dma-api-howto.html#dma-addressing-capabilities
> >>
> >> Remove the obsolete fallback while retaining the error check so that a
> >> genuine DMA setup failure is still reported and aborts initialization.
> >>
> >> Signed-off-by: Ruizhe Zhou <zhouruizhe@resnics.com>
> >> ---
> >>  drivers/dma/amd/ptdma/ptdma-pci.c | 8 ++------
> >>  1 file changed, 2 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/dma/amd/ptdma/ptdma-pci.c b/drivers/dma/amd/ptdma/ptdma-pci.c
> >> index 22739ff0c3c5..d36bb9c67325 100644
> >> --- a/drivers/dma/amd/ptdma/ptdma-pci.c
> >> +++ b/drivers/dma/amd/ptdma/ptdma-pci.c
> >> @@ -178,12 +178,8 @@ static int pt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> >>
> >>         ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(48));
> >>         if (ret) {
> >> -               ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> >> -               if (ret) {
> >> -                       dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n",
> >> -                               ret);
> >> -                       goto e_err;
> >> -               }
> >> +               dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n", ret);
> >> +               goto e_err;
> >
> >also needn't check return value, it always return success if mask >= 32.

Add Christophe JAILLET and Christoph Hellwig

https://lore.kernel.org/all/6a4df3e0a0849f179f9747f47b9c8cae53b29b59.1641752692.git.christophe.jaillet@wanadoo.fr/

Frank

>
> I did some more checking after your reply to make sure I understand
> whether dma_set_mask_and_coherent() can actually fail for a mask wider
> than 32 bits, and whether keeping the return-value check is necessary.
>
> I also read your August discussion with Michal Pecio on the same
> question:
>
> https://lore.kernel.org/all/aoMty9Dvt2bnWm74@SMW015318/
>
> In that discussion you pointed out that dev->dma_mask should already be
> initialized by the bus before driver probe, and asked whether there are
> any dma_supported() implementations which can actually reject such a
> mask.
>
> I agree that !dev->dma_mask does not look like an interesting failure
> case for a normally probed device. However, after going through the
> current dma_supported() paths and the in-tree .dma_supported callbacks,
> I found several other cases which appear to make the stronger "cannot
> fail for >32 bits" statement incorrect.
>
> The current path in kernel/dma/mapping.c is:
>
> static int dma_supported(struct device *dev, u64 mask)
> {
>         const struct dma_map_ops *ops = get_dma_ops(dev);
>
>         if (use_dma_iommu(dev)) {
>                 if (WARN_ON(ops))
>                         return false;
>                 return true;
>         }
>
>         if (ops) {
>                 if (!ops->dma_supported)
>                         return true;
>                 return ops->dma_supported(dev, mask);
>         }
>
>         return dma_direct_supported(dev, mask);
> }
>
> and dma_set_mask() does:
>
> if (!dev->dma_mask || !dma_supported(dev, mask))
>         return -EIO;
>
> For the generic direct-DMA path, the situation is clear:
>
> int dma_direct_supported(struct device *dev, u64 mask)
> {
>         ...
>
>         if (mask >= DMA_BIT_MASK(32))
>                 return 1;
>
>         ...
> }
>
> So dma_direct_supported() itself cannot reject a >=32-bit mask.
>
> However, this does not appear to be true for every path through
> dma_supported().
>
> My audit of the relevant current callbacks looks roughly like this:
>
> +----------------------------+---------------------+-----------------------+
> | Backend / callback            | >32 can fail?           | 64 can fail?                |
> +----------------------------+---------------------+-----------------------+
> | dma_direct_supported()        | No                          | No                          |
> | default dma-iommu              | Yes, conflicting       | Yes, conflicting       |
> |                                                | dma_ops state       | dma_ops state        |
> | ibmebus_dma_supported()    | Yes, !=64 fails         | No                          |
> | xen_grant_dma_supported()  | Yes, !=64 fails         | No                          |
> | xen_swiotlb_dma_supported | Yes, threshold         | Not due to width   |
> | ppc dma_iommu_supported()  | Yes                       | Yes, if no table        |
> | parisc sba_dma_supported()  | Yes                         | Yes, if no IOC           |
> | parisc ccio_supported()         | No for >=32          | No for valid dev       |
> | alpha_pci_supported()           | Yes, threshold /     | Can fail if no             |
> |                                                | mapping dependent   | usable DMA path|
> | dma_dummy_supported()      | Yes, always           | Yes, always                |
> +----------------------------+---------------------+-----------------------+
>
> There seem to be two separate issues here.
>
> First, "any mask wider than 32 bits cannot fail" has direct
> counterexamples.
>
> For example, arch/powerpc/platforms/pseries/ibmebus.c contains:
>
> static int ibmebus_dma_supported(struct device *dev, u64 mask)
> {
>         return mask == DMA_BIT_MASK(64);
> }
>
> and installs it in ibmebus_dma_ops.
>
> Therefore a call such as:
>
> dma_set_mask(dev, DMA_BIT_MASK(40))
>
> will fail even though the mask is wider than 32 bits.
>
> drivers/xen/grant-dma-ops.c has the same rule:
>
> static int xen_grant_dma_supported(struct device *dev, u64 mask)
> {
>         return mask == DMA_BIT_MASK(64);
> }
>
> and xen_grant_dma_ops installs this as .dma_supported.
>
> So, for that backend as well, DMA_BIT_MASK(40), for example, is
> rejected.
>
> These two cases seem to directly contradict the more general statement
> that dma_set_mask_and_coherent() cannot fail for a mask wider than
> 32 bits.
>
> There are also cases where even DMA_BIT_MASK(64) can fail.
>
> One example is the PowerPC legacy IOMMU backend in
> arch/powerpc/kernel/dma-iommu.c:
>
> int dma_iommu_dma_supported(struct device *dev, u64 mask)
> {
>         struct iommu_table *tbl;
>
>         ...
>
>         tbl = get_iommu_table_base(dev);
>
>         if (!tbl) {
>                 dev_err(dev,
>                         "Warning: IOMMU dma not supported: "
>                         "mask 0x%08llx, table unavailable\n",
>                         mask);
>                 return 0;
>         }
>
>         if (tbl->it_offset >
>             (mask >> tbl->it_page_shift)) {
>                 ...
>                 return 0;
>         }
>
>         return 1;
> }
>
> If get_iommu_table_base() returns NULL, the callback rejects the mask
> regardless of whether it is 32, 40, or 64 bits.
>
> I understand that an unavailable IOMMU table may represent an
> unexpected or unusable DMA configuration rather than an ordinary
> address-width limitation, but that seems exactly like a reason for the
> driver to retain the error check and abort probe instead of proceeding
> as if DMA setup succeeded.
>
> There is a similar state-dependent failure in
> drivers/parisc/sba_iommu.c:
>
> ioc = GET_IOC(dev);
> if (!ioc)
>         return 0;
>
> /*
>  * The max IO Virt address will *always* < 30 bits.
>  */
> return mask >= ...;
>
> For a valid IOC the required address range is below 32 bits, so a
> 64-bit mask works. But if GET_IOC(dev) fails, DMA_BIT_MASK(64) is still
> rejected.
>
> The default dma-iommu path also appears to contain an intentional
> failure condition:
>
> if (use_dma_iommu(dev)) {
>         if (WARN_ON(ops))
>                 return false;
>         return true;
> }
>
> The intended state is:
>
> use_dma_iommu(dev) == true
> get_dma_ops(dev)   == NULL
>
> because the default IOMMU DMA implementation does not rely on a
> dma_map_ops instance.
>
> However, if use_dma_iommu(dev) is true and get_dma_ops(dev) is
> non-NULL, dma_supported() deliberately returns false, independent of
> mask width.
>
> This looks particularly significant because this was explicitly
> discussed when the default dma-iommu implementation was changed from
> dma_ops indirect calls to direct calls.
>
> Christoph suggested moving the consistency check out of the fast path
> and doing it in dma_set_mask(), "And fail the call while we're at it."
> Leon replied that he would add it to dma_supported():
>
> https://lore.kernel.org/all/20240718070406.GK5630@unreal/
>
> The same discussion also says that the default-IOMMU state implies
> !ops.
>
> So the resulting WARN_ON(ops) path appears to be an intentional reason
> for dma_set_mask() to return an error when the DMA backend state is
> inconsistent, including when mask == DMA_BIT_MASK(64).
>
> Another concrete case is dma_dummy_ops.
>
> kernel/dma/dummy.c has:
>
> static int dma_dummy_supported(struct device *hwdev, u64 mask)
> {
>         return 0;
> }
>
> so dma_set_mask() fails for every mask, including DMA_BIT_MASK(64).
>
> I initially wondered whether this was only a theoretical sentinel that
> could never be seen by a driver probe, but there is a real setup path
> for it. acpi_dma_configure_id() does:
>
> if (attr == DEV_DMA_NOT_SUPPORTED) {
>         set_dma_ops(dev, &dma_dummy_ops);
>         return 0;
> }
>
> Since this returns 0 from DMA configuration after installing
> dma_dummy_ops, the device can continue through the driver-core setup
> with DMA deliberately marked unsupported. If such a DMA-using driver
> then calls dma_set_mask_and_coherent(), the return-value check is what
> prevents it from continuing with an unusable DMA backend.
>
> I agree this is not representative of the normal case for a healthy
> PCI device, but it does seem to demonstrate that a 64-bit mask is not an
> unconditional success guarantee of the DMA API itself.
>
> The formal DMA API documentation also seems to reflect this. In
> Documentation/core-api/dma-api.rst, under "DMA addressing
> limitations", it says:
>
> All the below functions which set a DMA mask may fail if the
> requested mask cannot be used with the device, or if the device is
> not capable of doing DMA.
>
> and dma_set_mask_and_coherent() is documented as returning zero on
> success and a negative error on failure.
>
> So I think there are two different statements here:
>
> 1. For a normally configured DMA-capable device, a 64-bit DMA mask
>    should be supportable.
>
> 2. dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)) cannot return
>    an error, so checking its return value is unnecessary.
>
> The first looks like the intended normal-case invariant, but I do not
> see how the second follows given the current dma_supported() paths
> above.
>
> In particular, the PowerPC IOMMU table failure, the
> use_dma_iommu(dev) && ops consistency check, and dma_dummy_ops are all
> paths where the mask width itself is not the problem but
> dma_set_mask_and_coherent(..., DMA_BIT_MASK(64)) can nevertheless
> report failure.
>
> That is why I currently think code like:
>
> ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> if (ret)
>         return ret;
>
> still has value: the check is not probing whether 64-bit DMA
> addressing is supported versus some narrower address width. It is
> checking whether the DMA setup as a whole succeeded before the driver
> starts using DMA.
>
> Again, I do not work within the DMA subsystem, and therefore I won't exactly
> claim myself to be an expert on this. The above are some findings after some
> digging. So correct me if I am wrong.
>
> Am I missing an invariant which makes these failure paths unreachable
> for a driver calling dma_set_mask_and_coherent()? If not, then I believe the return
> value check is needed, and the DMA HOWTO document might need to mention that.
>
> Thanks,
> Ruizhe
>
>
>
>
> >
> >Frank
> >>         }
> >>
> >>         dev_set_drvdata(dev, pt);
> >> --
> >> 2.27.0
> >>
> >

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

* Re: Re: [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback
  2026-09-11 22:37       ` Frank Li
@ 2026-09-12  2:58         ` Frank Li
  0 siblings, 0 replies; 11+ messages in thread
From: Frank Li @ 2026-09-12  2:58 UTC (permalink / raw)
  To: 周睿哲, Christophe JAILLET, Christoph Hellwig
  Cc: Vinod Koul, Basavaraj Natikar, Logan Gunthorpe, Orson Zhai,
	Baolin Wang, Frank Li, Chunyan Zhang, dmaengine, linux-kernel

On Fri, Sep 11, 2026 at 06:37:40PM -0400, Frank Li wrote:
> On Fri, Sep 04, 2026 at 07:15:36PM +0800, 周睿哲 wrote:
> > Hi Frank,
> >
> >
> > From: Frank Li <Frank.li@oss.nxp.com>
> > Date: 2026-09-04 03:26:36
> > To:  Ruizhe Zhou <zhouruizhe@resnics.com>
> > Cc:  Vinod Koul <vkoul@kernel.org>,Basavaraj Natikar <Basavaraj.Natikar@amd.com>,Logan Gunthorpe <logang@deltatee.com>,Orson Zhai <orsonzhai@gmail.com>,Baolin Wang <baolin.wang@linux.alibaba.com>,Frank Li <Frank.Li@kernel.org>,Chunyan Zhang <zhang.lyra@gmail.com>,dmaengine@vger.kernel.org,linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback>On Thu, Sep 03, 2026 at 07:54:39PM +0800, Ruizhe Zhou wrote:
> > >> [You don't often get email from zhouruizhe@resnics.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> > >>
> > >> The DMA API guarantees support for masks of 32 bits or wider and
> > >> explicitly identifies retrying a 32-bit mask after a wider request as
> > >> incorrect:
> > >> https://docs.kernel.org/core-api/dma-api-howto.html#dma-addressing-capabilities
> > >>
> > >> Remove the obsolete fallback while retaining the error check so that a
> > >> genuine DMA setup failure is still reported and aborts initialization.
> > >>
> > >> Signed-off-by: Ruizhe Zhou <zhouruizhe@resnics.com>
> > >> ---
> > >>  drivers/dma/amd/ptdma/ptdma-pci.c | 8 ++------
> > >>  1 file changed, 2 insertions(+), 6 deletions(-)
> > >>
> > >> diff --git a/drivers/dma/amd/ptdma/ptdma-pci.c b/drivers/dma/amd/ptdma/ptdma-pci.c
> > >> index 22739ff0c3c5..d36bb9c67325 100644
> > >> --- a/drivers/dma/amd/ptdma/ptdma-pci.c
> > >> +++ b/drivers/dma/amd/ptdma/ptdma-pci.c
> > >> @@ -178,12 +178,8 @@ static int pt_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > >>
> > >>         ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(48));
> > >>         if (ret) {
> > >> -               ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > >> -               if (ret) {
> > >> -                       dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n",
> > >> -                               ret);
> > >> -                       goto e_err;
> > >> -               }
> > >> +               dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n", ret);
> > >> +               goto e_err;
> > >
> > >also needn't check return value, it always return success if mask >= 32.
>
> Add Christophe JAILLET and Christoph Hellwig
>
> https://lore.kernel.org/all/6a4df3e0a0849f179f9747f47b9c8cae53b29b59.1641752692.git.christophe.jaillet@wanadoo.fr/
>
> Frank
>
> >
> > I did some more checking after your reply to make sure I understand
> > whether dma_set_mask_and_coherent() can actually fail for a mask wider
> > than 32 bits, and whether keeping the return-value check is necessary.
> >
> > I also read your August discussion with Michal Pecio on the same
> > question:
> >
> > https://lore.kernel.org/all/aoMty9Dvt2bnWm74@SMW015318/
> >
> > In that discussion you pointed out that dev->dma_mask should already be
> > initialized by the bus before driver probe, and asked whether there are
> > any dma_supported() implementations which can actually reject such a
> > mask.
> >
> > I agree that !dev->dma_mask does not look like an interesting failure
> > case for a normally probed device. However, after going through the
> > current dma_supported() paths and the in-tree .dma_supported callbacks,
> > I found several other cases which appear to make the stronger "cannot
> > fail for >32 bits" statement incorrect.
> >
> > The current path in kernel/dma/mapping.c is:
> >
> > static int dma_supported(struct device *dev, u64 mask)
> > {
> >         const struct dma_map_ops *ops = get_dma_ops(dev);
> >
> >         if (use_dma_iommu(dev)) {

I recall my memory. if use iommu, all mask should be supported because iova
is difference address space. iova always allocate matched device required
mask's io address space.

> >                 if (WARN_ON(ops))

This branch should be safety dead branch.

> >                         return false;
> >                 return true;
> >         }
> >
> >         if (ops) {
> >                 if (!ops->dma_supported)
> >                         return true;
> >                 return ops->dma_supported(dev, mask);
> >         }
> >
> >         return dma_direct_supported(dev, mask);
> > }
> >
> > and dma_set_mask() does:
> >
> > if (!dev->dma_mask || !dma_supported(dev, mask))
> >         return -EIO;
> >
> > For the generic direct-DMA path, the situation is clear:
> >
> > int dma_direct_supported(struct device *dev, u64 mask)
> > {
> >         ...
> >
> >         if (mask >= DMA_BIT_MASK(32))
> >                 return 1;
> >
> >         ...
> > }
> >
> > So dma_direct_supported() itself cannot reject a >=32-bit mask.
> >
> > However, this does not appear to be true for every path through
> > dma_supported().
> >
> > My audit of the relevant current callbacks looks roughly like this:
> >
> > +----------------------------+---------------------+-----------------------+
> > | Backend / callback            | >32 can fail?           | 64 can fail?                |
> > +----------------------------+---------------------+-----------------------+
> > | dma_direct_supported()        | No                          | No                          |
> > | default dma-iommu              | Yes, conflicting       | Yes, conflicting       |
> > |                                                | dma_ops state       | dma_ops state        |
> > | ibmebus_dma_supported()    | Yes, !=64 fails         | No                          |
> > | xen_grant_dma_supported()  | Yes, !=64 fails         | No                          |

This should be problem because many devices have dma_mask is 32bit.
if != 64 failure, many devices will not work.

> > | xen_swiotlb_dma_supported | Yes, threshold         | Not due to width   |
> > | ppc dma_iommu_supported()  | Yes                       | Yes, if no table        |
> > | parisc sba_dma_supported()  | Yes                         | Yes, if no IOC           |
> > | parisc ccio_supported()         | No for >=32          | No for valid dev       |
> > | alpha_pci_supported()           | Yes, threshold /     | Can fail if no             |
> > |                                                | mapping dependent   | usable DMA path|
> > | dma_dummy_supported()      | Yes, always           | Yes, always                |
> > +----------------------------+---------------------+-----------------------+
> >
> > There seem to be two separate issues here.
> >
> > First, "any mask wider than 32 bits cannot fail" has direct
> > counterexamples.
> >
> > For example, arch/powerpc/platforms/pseries/ibmebus.c contains:
> >
> > static int ibmebus_dma_supported(struct device *dev, u64 mask)
> > {
> >         return mask == DMA_BIT_MASK(64);
> > }
> >
> > and installs it in ibmebus_dma_ops.
> >
> > Therefore a call such as:
> >
> > dma_set_mask(dev, DMA_BIT_MASK(40))
> >
> > will fail even though the mask is wider than 32 bits.
> >
> > drivers/xen/grant-dma-ops.c has the same rule:
> >
> > static int xen_grant_dma_supported(struct device *dev, u64 mask)
> > {
> >         return mask == DMA_BIT_MASK(64);
> > }
> >
> > and xen_grant_dma_ops installs this as .dma_supported.
> >
> > So, for that backend as well, DMA_BIT_MASK(40), for example, is
> > rejected.
> >
> > These two cases seem to directly contradict the more general statement
> > that dma_set_mask_and_coherent() cannot fail for a mask wider than
> > 32 bits.
> >
> > There are also cases where even DMA_BIT_MASK(64) can fail.
> >
> > One example is the PowerPC legacy IOMMU backend in
> > arch/powerpc/kernel/dma-iommu.c:
> >
> > int dma_iommu_dma_supported(struct device *dev, u64 mask)
> > {
> >         struct iommu_table *tbl;
> >
> >         ...
> >
> >         tbl = get_iommu_table_base(dev);
> >
> >         if (!tbl) {
> >                 dev_err(dev,
> >                         "Warning: IOMMU dma not supported: "
> >                         "mask 0x%08llx, table unavailable\n",
> >                         mask);
> >                 return 0;
> >         }
> >
> >         if (tbl->it_offset >
> >             (mask >> tbl->it_page_shift)) {
> >                 ...
> >                 return 0;
> >         }
> >
> >         return 1;
> > }
> >
> > If get_iommu_table_base() returns NULL, the callback rejects the mask
> > regardless of whether it is 32, 40, or 64 bits.
> >
> > I understand that an unavailable IOMMU table may represent an
> > unexpected or unusable DMA configuration rather than an ordinary
> > address-width limitation, but that seems exactly like a reason for the
> > driver to retain the error check and abort probe instead of proceeding
> > as if DMA setup succeeded.
> >
> > There is a similar state-dependent failure in
> > drivers/parisc/sba_iommu.c:
> >
> > ioc = GET_IOC(dev);
> > if (!ioc)
> >         return 0;
> >
> > /*
> >  * The max IO Virt address will *always* < 30 bits.
> >  */
> > return mask >= ...;
> >
> > For a valid IOC the required address range is below 32 bits, so a
> > 64-bit mask works. But if GET_IOC(dev) fails, DMA_BIT_MASK(64) is still
> > rejected.
> >
> > The default dma-iommu path also appears to contain an intentional
> > failure condition:
> >
> > if (use_dma_iommu(dev)) {
> >         if (WARN_ON(ops))
> >                 return false;
> >         return true;
> > }
> >
> > The intended state is:
> >
> > use_dma_iommu(dev) == true
> > get_dma_ops(dev)   == NULL
> >
> > because the default IOMMU DMA implementation does not rely on a
> > dma_map_ops instance.
> >
> > However, if use_dma_iommu(dev) is true and get_dma_ops(dev) is
> > non-NULL, dma_supported() deliberately returns false, independent of
> > mask width.
> >
> > This looks particularly significant because this was explicitly
> > discussed when the default dma-iommu implementation was changed from
> > dma_ops indirect calls to direct calls.
> >
> > Christoph suggested moving the consistency check out of the fast path
> > and doing it in dma_set_mask(), "And fail the call while we're at it."
> > Leon replied that he would add it to dma_supported():
> >
> > https://lore.kernel.org/all/20240718070406.GK5630@unreal/
> >
> > The same discussion also says that the default-IOMMU state implies
> > !ops.
> >
> > So the resulting WARN_ON(ops) path appears to be an intentional reason
> > for dma_set_mask() to return an error when the DMA backend state is
> > inconsistent, including when mask == DMA_BIT_MASK(64).
> >
> > Another concrete case is dma_dummy_ops.
> >
> > kernel/dma/dummy.c has:
> >
> > static int dma_dummy_supported(struct device *hwdev, u64 mask)
> > {
> >         return 0;
> > }
> >
> > so dma_set_mask() fails for every mask, including DMA_BIT_MASK(64).
> >
> > I initially wondered whether this was only a theoretical sentinel that
> > could never be seen by a driver probe, but there is a real setup path
> > for it. acpi_dma_configure_id() does:
> >
> > if (attr == DEV_DMA_NOT_SUPPORTED) {
> >         set_dma_ops(dev, &dma_dummy_ops);
> >         return 0;
> > }
> >
> > Since this returns 0 from DMA configuration after installing
> > dma_dummy_ops, the device can continue through the driver-core setup
> > with DMA deliberately marked unsupported. If such a DMA-using driver
> > then calls dma_set_mask_and_coherent(), the return-value check is what
> > prevents it from continuing with an unusable DMA backend.
> >
> > I agree this is not representative of the normal case for a healthy
> > PCI device, but it does seem to demonstrate that a 64-bit mask is not an
> > unconditional success guarantee of the DMA API itself.
> >
> > The formal DMA API documentation also seems to reflect this. In
> > Documentation/core-api/dma-api.rst, under "DMA addressing
> > limitations", it says:
> >
> > All the below functions which set a DMA mask may fail if the
> > requested mask cannot be used with the device, or if the device is
> > not capable of doing DMA.
> >
> > and dma_set_mask_and_coherent() is documented as returning zero on
> > success and a negative error on failure.

dma_set_mask_and_coherent() actually indicate specific device's DMA address
width.

The real DMA map address need consider whole bus, for example, device
support 64bit, and parent bus may just support 32bits. DT's dma-range do
related mappings.

History reason, < 32bit, such as 16bit/24bit, some system have not low DMA
memory ragion, can't allocate memory for this DMA zone. so return failure.

I have not touch this area for the long time and need do more research to
anwser all of your questions.

Frank

> >
> > So I think there are two different statements here:
> >
> > 1. For a normally configured DMA-capable device, a 64-bit DMA mask
> >    should be supportable.
> >
> > 2. dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)) cannot return
> >    an error, so checking its return value is unnecessary.
> >
> > The first looks like the intended normal-case invariant, but I do not
> > see how the second follows given the current dma_supported() paths
> > above.
> >
> > In particular, the PowerPC IOMMU table failure, the
> > use_dma_iommu(dev) && ops consistency check, and dma_dummy_ops are all
> > paths where the mask width itself is not the problem but
> > dma_set_mask_and_coherent(..., DMA_BIT_MASK(64)) can nevertheless
> > report failure.
> >
> > That is why I currently think code like:
> >
> > ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> > if (ret)
> >         return ret;
> >
> > still has value: the check is not probing whether 64-bit DMA
> > addressing is supported versus some narrower address width. It is
> > checking whether the DMA setup as a whole succeeded before the driver
> > starts using DMA.
> >
> > Again, I do not work within the DMA subsystem, and therefore I won't exactly
> > claim myself to be an expert on this. The above are some findings after some
> > digging. So correct me if I am wrong.
> >
> > Am I missing an invariant which makes these failure paths unreachable
> > for a driver calling dma_set_mask_and_coherent()? If not, then I believe the return
> > value check is needed, and the DMA HOWTO document might need to mention that.
> >
> > Thanks,
> > Ruizhe
> >
> >
> >
> >
> > >
> > >Frank
> > >>         }
> > >>
> > >>         dev_set_drvdata(dev, pt);
> > >> --
> > >> 2.27.0
> > >>
> > >

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

end of thread, other threads:[~2026-09-12  2:58 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03 11:54 [PATCH 0/3] dmaengine: Remove obsolete 32-bit DMA mask fallbacks Ruizhe Zhou
2026-09-03 11:54 ` [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback Ruizhe Zhou
2026-09-03 19:26   ` Frank Li
2026-09-04 11:15     ` 周睿哲
2026-09-11 22:37       ` Frank Li
2026-09-12  2:58         ` Frank Li
2026-09-03 11:54 ` [PATCH 2/3] dmaengine: plx_dma: " Ruizhe Zhou
2026-09-03 14:29   ` sashiko-bot
2026-09-03 22:00   ` Logan Gunthorpe
2026-09-03 11:54 ` [PATCH 3/3] dmaengine: sprd: " Ruizhe Zhou
2026-09-03 14:26   ` 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.