Re: Re: [PATCH 1/3] dmaengine: ptdma: Remove obsolete 32-bit DMA mask fallback
From: Frank Li
Date: Fri Sep 11 2026 - 22:58:53 EST
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@xxxxxxxxxxx>
> > Date: 2026-09-04 03:26:36
> > To: Ruizhe Zhou <zhouruizhe@xxxxxxxxxxx>
> > Cc: Vinod Koul <vkoul@xxxxxxxxxx>,Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>,Logan Gunthorpe <logang@xxxxxxxxxxxx>,Orson Zhai <orsonzhai@xxxxxxxxx>,Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>,Frank Li <Frank.Li@xxxxxxxxxx>,Chunyan Zhang <zhang.lyra@xxxxxxxxx>,dmaengine@xxxxxxxxxxxxxxx,linux-kernel@xxxxxxxxxxxxxxx
> > 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@xxxxxxxxxxx. 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@xxxxxxxxxxx>
> > >> ---
> > >> 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@xxxxxxxxxx/
>
> 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
> > >>
> > >