* [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
* 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: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
* [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
* 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 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
* [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
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.