dma-mapping: migrate to physical address-based API

From: Leon Romanovsky <[email protected]>

Convert the KMSAN DMA handling function from page-based to physical address-based interface.

The refactoring renames kmsan_handle_dma() parameters from accepting (struct page *page, size_t offset, size_t size) to (phys_addr_t phys, size_t size).

A PFN_VALID check is added to prevent KMSAN operations on non-page memory, preventing from non struct page backed address,

As part of this change, support for highmem addresses is implemented using kmap_local_page() to handle both lowmem and highmem regions properly. All callers throughout the codebase are updated to use the new phys_addr_t based interface.

Quoted 2 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Hi Leon,

Quoted 4 lines

Convert the KMSAN DMA handling function from page-based to physical address-based interface.

Quoted 3 lines

The refactoring renames kmsan_handle_dma() parameters from accepting (struct page *page, size_t offset, size_t size) to (phys_addr_t phys, size_t size).

Could you please elaborate a bit why this is needed? Are you fixing some particular issue?

Quoted 8 lines

A PFN_VALID check is added to prevent KMSAN operations on non-page memory, preventing from non struct page backed address,

As part of this change, support for highmem addresses is implemented using kmap_local_page() to handle both lowmem and highmem regions properly. All callers throughout the codebase are updated to use the new phys_addr_t based interface.

KMSAN only works on 64-bit systems, do we actually have highmem on any of these?

This series refactors the DMA mapping to use physical addresses as the primary interface instead of page+offset parameters. This change aligns the DMA API with the underlying hardware reality where DMA operations work with physical addresses, not page structures.

The series consists of 8 patches that progressively convert the DMA mapping infrastructure from page-based to physical address-based APIs:

The series maintains backward compatibility by keeping the old page-based API as wrapper functions around the new physical address-based implementations.

Quoted 3 lines · Alexander Potapenko

Quoted 2 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Quoted 7 lines

Hi Leon,

Quoted 4 lines

Convert the KMSAN DMA handling function from page-based to physical address-based interface.

Quoted 6 lines

Quoted 3 lines

The refactoring renames kmsan_handle_dma() parameters from accepting (struct page *page, size_t offset, size_t size) to (phys_addr_t phys, size_t size).

Could you please elaborate a bit why this is needed? Are you fixing some particular issue?

It is soft of the fix and improvement at the same time. Improvement:
It allows direct call to kmsan_handle_dma() without need to convert from phys_addr_t to struct page for newly introduced dma_map_phys() routine.

Fix:
It prevents us from executing kmsan for addresses that don't have struct page (for example PCI_P2PDMA_MAP_THRU_HOST_BRIDGE pages), which we are doing with original code.

dma_map_sg_attrs()
-> __dma_map_sg_attrs()
-> dma_direct_map_sg()
-> PCI_P2PDMA_MAP_THRU_HOST_BRIDGE and nents > 0

    -> kmsan_handle_dma_sg();
     -> kmsan_handle_dma(g_page(item) <---- this is "fake" page.

We are trying to build DMA API that doesn't require struct pages.

Quoted 1 line

KMSAN only works on 64-bit systems, do we actually have highmem on any of these?

I don't know, but the original code had this check:
344 if (PageHighMem(page))
345 return;

Thanks

Quoted 8 lines · Leon Romanovsky

This series refactors the DMA mapping to use physical addresses as the primary interface instead of page+offset parameters. This change aligns the DMA API with the underlying hardware reality where DMA operations work with physical addresses, not page structures.

The series consists of 8 patches that progressively convert the DMA mapping infrastructure from page-based to physical address-based APIs:

Quoted 3 lines

The series maintains backward compatibility by keeping the old page-based API as wrapper functions around the new physical address-based implementations.

Thanks for this rework! I assume that the next step is to add map_phys callback also to the dma_map_ops and teach various dma-mapping providers to use it to avoid more phys-to-page-to-phys conversions.

I only wonder if this newly introduced dma_map_phys()/dma_unmap_phys() API is also suitable for the recently discussed PCI P2P DMA? While adding a new API maybe we should take this into account?

My main concern is the lack of the source phys addr passed to the dma_unmap_phys() function and I'm aware that this might complicate a bit code conversion from old dma_map/unmap_page() API.

Best regards

Signature · 2 lines

Marek Szyprowski, PhD
Samsung R&D Institute Poland

Quoted 3 lines

Thanks for this rework! I assume that the next step is to add map_phys callback also to the dma_map_ops and teach various dma-mapping providers to use it to avoid more phys-to-page-to-phys conversions.

Probably Christoph will say yes, however I personally don't see any benefit in this. Maybe I wrong here, but all existing .map_page() implementation platforms don't support p2p anyway. They won't benefit from this such conversion.

Quoted 9 lines · Marek Szyprowski

Quoted 8 lines · Leon Romanovsky

This series refactors the DMA mapping to use physical addresses as the primary interface instead of page+offset parameters. This change aligns the DMA API with the underlying hardware reality where DMA operations work with physical addresses, not page structures.

The series consists of 8 patches that progressively convert the DMA mapping infrastructure from page-based to physical address-based APIs:

Quoted 7 lines

Quoted 3 lines

The series maintains backward compatibility by keeping the old page-based API as wrapper functions around the new physical address-based implementations.

Thanks for this rework! I assume that the next step is to add map_phys callback also to the dma_map_ops and teach various dma-mapping providers to use it to avoid more phys-to-page-to-phys conversions.

Probably Christoph will say yes, however I personally don't see any benefit in this. Maybe I wrong here, but all existing .map_page() implementation platforms don't support p2p anyway. They won't benefit from this such conversion.

Quoted 4 lines

I only wonder if this newly introduced dma_map_phys()/dma_unmap_phys() API is also suitable for the recently discussed PCI P2P DMA? While adding a new API maybe we should take this into account?

First, immediate user (not related to p2p) is blk layer:
https://lore.kernel.org/linux-nvme/[email protected]/T/#m7e715697d4b2e3997622a3400243477c75cab406

(quoted diff) +12 -0

+static bool blk_dma_map_direct(struct request *req, struct device *dma_dev,
+		struct blk_dma_iter *iter, struct phys_vec *vec)
+{
+	iter->addr = dma_map_page(dma_dev, phys_to_page(vec->paddr),
+			offset_in_page(vec->paddr), vec->len, rq_dma_dir(req));
+	if (dma_mapping_error(dma_dev, iter->addr)) {
+		iter->status = BLK_STS_RESOURCE;
+		return false;
+	}
+	iter->len = vec->len;
+	return true;
+}

Block layer started to store phys addresses instead of struct pages and this phys_to_page() conversion in data-path will be avoided.

Quoted 4 lines

My main concern is the lack of the source phys addr passed to the dma_unmap_phys() function and I'm aware that this might complicate a bit code conversion from old dma_map/unmap_page() API.

Quoted 7 lines

Best regards

Signature · 2 lines

Marek Szyprowski, PhD
Samsung R&D Institute Poland

Quoted 7 lines · Leon Romanovsky

Quoted 3 lines

Thanks for this rework! I assume that the next step is to add map_phys callback also to the dma_map_ops and teach various dma-mapping providers to use it to avoid more phys-to-page-to-phys conversions.

Probably Christoph will say yes, however I personally don't see any benefit in this. Maybe I wrong here, but all existing .map_page() implementation platforms don't support p2p anyway. They won't benefit from this such conversion.

I think that conversion should eventually happen, and rather sooner than later.

Quoted 8 lines · Christoph Hellwig

Quoted 7 lines · Leon Romanovsky

Quoted 3 lines

Thanks for this rework! I assume that the next step is to add map_phys callback also to the dma_map_ops and teach various dma-mapping providers to use it to avoid more phys-to-page-to-phys conversions.

Probably Christoph will say yes, however I personally don't see any benefit in this. Maybe I wrong here, but all existing .map_page() implementation platforms don't support p2p anyway. They won't benefit from this such conversion.

Quoted 2 lines

I think that conversion should eventually happen, and rather sooner than later.

Agreed.

Applied patches 1-7 to my dma-mapping-next branch. Let me know if one needs a stable branch with it.

Leon, it would be great if You could also prepare an incremental patch adding map_phys callback to the dma_maps_ops, so the individual arch-specific dma-mapping providers can be then converted (or simplified in many cases) too.

Best regards

Signature · 2 lines

Marek Szyprowski, PhD
Samsung R&D Institute Poland

Quoted 9 lines · Marek Szyprowski

Quoted 8 lines · Christoph Hellwig

Quoted 7 lines · Leon Romanovsky

Quoted 3 lines

Thanks for this rework! I assume that the next step is to add map_phys callback also to the dma_map_ops and teach various dma-mapping providers to use it to avoid more phys-to-page-to-phys conversions.

Probably Christoph will say yes, however I personally don't see any benefit in this. Maybe I wrong here, but all existing .map_page() implementation platforms don't support p2p anyway. They won't benefit from this such conversion.

Quoted 6 lines

Quoted 2 lines

I think that conversion should eventually happen, and rather sooner than later.

Agreed.

Applied patches 1-7 to my dma-mapping-next branch. Let me know if one needs a stable branch with it.

Thanks a lot, I don't think that stable branch is needed. Realistically speaking, my VFIO DMA work won't be merged this cycle, We are in -rc5, it is complete rewrite from RFC version and touches pci-p2p code (to remove dependency on struct page) in addition to VFIO, so it will take time.

Regarding, last patch (hmm), it will be great if you can take it. We didn't touch anything in hmm.c this cycle and have no plans to send PR. It can safely go through your tree.

Quoted 4 lines

Leon, it would be great if You could also prepare an incremental patch adding map_phys callback to the dma_maps_ops, so the individual arch-specific dma-mapping providers can be then converted (or simplified in many cases) too.

Sure, will do.

Quoted 22 lines · Leon Romanovsky

Quoted 9 lines · Marek Szyprowski

Quoted 8 lines · Christoph Hellwig

Quoted 7 lines · Leon Romanovsky

Quoted 3 lines

Thanks for this rework! I assume that the next step is to add map_phys callback also to the dma_map_ops and teach various dma-mapping providers to use it to avoid more phys-to-page-to-phys conversions.

Probably Christoph will say yes, however I personally don't see any benefit in this. Maybe I wrong here, but all existing .map_page() implementation platforms don't support p2p anyway. They won't benefit from this such conversion.

Quoted 6 lines

Quoted 2 lines

I think that conversion should eventually happen, and rather sooner than later.

Agreed.

Applied patches 1-7 to my dma-mapping-next branch. Let me know if one needs a stable branch with it.

Thanks a lot, I don't think that stable branch is needed. Realistically speaking, my VFIO DMA work won't be merged this cycle, We are in -rc5, it is complete rewrite from RFC version and touches pci-p2p code (to remove dependency on struct page) in addition to VFIO, so it will take time.

Quoted 3 lines

Regarding, last patch (hmm), it will be great if you can take it. We didn't touch anything in hmm.c this cycle and have no plans to send PR. It can safely go through your tree.

Okay, then I would like to get an explicit ack from Jérôme for this.

Quoted 23 lines · Marek Szyprowski

Quoted 22 lines · Leon Romanovsky

Quoted 9 lines · Marek Szyprowski

Quoted 8 lines · Christoph Hellwig

Quoted 7 lines · Leon Romanovsky

Quoted 3 lines

Thanks for this rework! I assume that the next step is to add map_phys callback also to the dma_map_ops and teach various dma-mapping providers to use it to avoid more phys-to-page-to-phys conversions.

Probably Christoph will say yes, however I personally don't see any benefit in this. Maybe I wrong here, but all existing .map_page() implementation platforms don't support p2p anyway. They won't benefit from this such conversion.

Quoted 6 lines

Quoted 2 lines

I think that conversion should eventually happen, and rather sooner than later.

Agreed.

Applied patches 1-7 to my dma-mapping-next branch. Let me know if one needs a stable branch with it.

Thanks a lot, I don't think that stable branch is needed. Realistically speaking, my VFIO DMA work won't be merged this cycle, We are in -rc5, it is complete rewrite from RFC version and touches pci-p2p code (to remove dependency on struct page) in addition to VFIO, so it will take time.

Quoted 4 lines

Quoted 3 lines

Regarding, last patch (hmm), it will be great if you can take it. We didn't touch anything in hmm.c this cycle and have no plans to send PR. It can safely go through your tree.

Okay, then I would like to get an explicit ack from Jérôme for this.

Jerome is not active in HMM world for a long time already.
HMM tree is managed by us (RDMA) https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm
➜ kernel git:(m/dmabuf-vfio) git log --merges mm/hmm.c
...
Pull HMM updates from Jason Gunthorpe:
...

https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=58ba80c4740212c29a1cf9b48f588e60a7612209
+hmm git git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git#hmm

We just never bothered to reflect current situation in MAINTAINERS file.

Quoted 47 lines · Leon Romanovsky

Quoted 28 lines · Marek Szyprowski

Quoted 27 lines · Leon Romanovsky

Quoted 12 lines · Marek Szyprowski

Quoted 11 lines · Christoph Hellwig

Quoted 10 lines · Leon Romanovsky

Quoted 5 lines

Thanks for this rework! I assume that the next step is to add map_phys
callback also to the dma_map_ops and teach various dma-mapping providers
to use it to avoid more phys-to-page-to-phys conversions.

Probably Christoph will say yes, however I personally don't see any benefit in this. Maybe I wrong here, but all existing .map_page() implementation platforms don't support p2p anyway. They won't benefit
from this such conversion.

Quoted 7 lines

Quoted 3 lines

I think that conversion should eventually happen, and rather sooner than
later.

Agreed.

Applied patches 1-7 to my dma-mapping-next branch. Let me know if one needs a stable branch with it.

Thanks a lot, I don't think that stable branch is needed. Realistically
speaking, my VFIO DMA work won't be merged this cycle, We are in -rc5, it is complete rewrite from RFC version and touches pci-p2p code (to remove dependency on struct page) in addition to VFIO, so it will take time.

Quoted 5 lines

Quoted 4 lines

Regarding, last patch (hmm), it will be great if you can take it. We didn't touch anything in hmm.c this cycle and have no plans to send PR.
It can safely go through your tree.

Okay, then I would like to get an explicit ack from Jérôme for this.

Jerome is not active in HMM world for a long time already. HMM tree is managed by us (RDMA)
https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm
➜  kernel git:(m/dmabuf-vfio) git log --merges mm/hmm.c
...
Pull HMM updates from Jason Gunthorpe:
...

https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=58ba80c4740212c29a1cf9b48f588e60a7612209

+hmm        git
git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git#hmm

Quoted 1 line

We just never bothered to reflect current situation in MAINTAINERS file.

Maybe this is the time to update it :)

I was just a bit confused that no-one commented the HMM patch, but if You maintain it, then this is okay.

From: Leon Romanovsky <[email protected]>

Convert HMM DMA operations from the legacy page-based API to the new physical address-based dma_map_phys() and dma_unmap_phys() functions. This demonstrates the preferred approach for new code that should use physical addresses directly rather than page+offset parameters.

The change replaces dma_map_page() and dma_unmap_page() calls with dma_map_phys() and dma_unmap_phys() respectively, using the physical address that was already available in the code. This eliminates the redundant page-to-physical address conversion and aligns with the DMA subsystem's move toward physical address-centric interfaces.

This serves as an example of how new code should be written to leverage the more efficient physical address API, which provides cleaner interfaces for drivers that already have access to physical addresses.

Hi Leon,

Quoted 13 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

Convert HMM DMA operations from the legacy page-based API to the new physical address-based dma_map_phys() and dma_unmap_phys() functions. This demonstrates the preferred approach for new code that should use physical addresses directly rather than page+offset parameters.

The change replaces dma_map_page() and dma_unmap_page() calls with dma_map_phys() and dma_unmap_phys() respectively, using the physical address that was already available in the code. This eliminates the redundant page-to-physical address conversion and aligns with the DMA subsystem's move toward physical address-centric interfaces.

Quoted 3 lines

This serves as an example of how new code should be written to leverage the more efficient physical address API, which provides cleaner interfaces for drivers that already have access to physical addresses.

I'm struggling a little to see how this is cleaner or more efficient than the old code.

From what I can tell, dma_map_page_attrs() takes a 'struct page *' and converts it to a physical address using page_to_phys() whilst your new dma_map_phys() interface takes a physical address and converts it to a 'struct page *' using phys_to_page(). In both cases, hmm_dma_map_pfn() still needs the page for other reasons. If anything, existing users of dma_map_page_attrs() now end up with a redundant page-to-phys-to-page conversion which hopefully the compiler folds away.

I'm assuming there's future work which builds on top of the new API and removes the reliance on 'struct page' entirely, is that right? If so, it would've been nicer to be clearer about that as, on its own, I'm not really sure this patch series achieves an awful lot and the efficiency argument looks quite weak to me.

Cheers,

Will

From: Leon Romanovsky <[email protected]>

All platforms which implement map_page interface don't support physical addresses without real struct page. Add condition to check it.

Signed-off-by Leon Romanovsky <[email protected]>

---
  kernel/dma/mapping.c | 15 ++++++++++++++-
  1 file changed, 14 insertions(+), 1 deletion(-)

kernel/dma/mapping.c +14 -1

index 709405d46b2b..74efb6909103 100644
@@ -158,6 +158,7 @@ dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page,
  {
  	const struct dma_map_ops *ops = get_dma_ops(dev);
  	phys_addr_t phys = page_to_phys(page) + offset;
+	bool is_pfn_valid = true;
  	dma_addr_t addr;
 
  	BUG_ON(!valid_dma_direction(dir));

@@ -170,8 +171,20 @@ dma_addr_t dma_map_page_attrs(struct device *dev, struct page *page,
  		addr = dma_direct_map_phys(dev, phys, size, dir, attrs);
  	else if (use_dma_iommu(dev))
  		addr = iommu_dma_map_phys(dev, phys, size, dir, attrs);
-	else
+	else {
+		if (IS_ENABLED(CONFIG_DMA_API_DEBUG))
+			is_pfn_valid = pfn_valid(PHYS_PFN(phys));
+
+		if (unlikely(!is_pfn_valid))
+			return DMA_MAPPING_ERROR;
+
+		/*
+		 * All platforms which implement .map_page() don't support
+		 * non-struct page backed addresses.
+		 */
  		addr = ops->map_page(dev, page, offset, size, dir, attrs);
+	}
+
  	kmsan_handle_dma(phys, size, dir);
  	trace_dma_map_phys(dev, phys, addr, size, dir, attrs);
  	debug_dma_map_phys(dev, phys, size, dir, addr, attrs);

This series refactors the DMA mapping to use physical addresses as the primary interface instead of page+offset parameters. This change aligns the DMA API with the underlying hardware reality where DMA operations work with physical addresses, not page structures.

The series consists of 8 patches that progressively convert the DMA mapping infrastructure from page-based to physical address-based APIs:

The series maintains backward compatibility by keeping the old page-based API as wrapper functions around the new physical address-based implementations.

Thanks

Leon Romanovsky (8):
dma-debug: refactor to use physical addresses for page mapping
dma-mapping: rename trace_dma_*map_page to trace_dma_*map_phys iommu/dma: rename iommu_dma_*map_page to iommu_dma_*map_phys
dma-mapping: convert dma_direct_*map_page to be phys_addr_t based
kmsan: convert kmsan_handle_dma to use physical addresses
dma-mapping: fail early if physical address is mapped through platform
callback
dma-mapping: export new dma_*map_phys() interface
mm/hmm: migrate to physical address-based DMA mapping API

  Documentation/core-api/dma-api.rst |  4 +-
  arch/powerpc/kernel/dma-iommu.c    |  4 +-
  drivers/iommu/dma-iommu.c          | 14 +++----
  drivers/virtio/virtio_ring.c       |  4 +-
  include/linux/dma-map-ops.h        |  8 ++--
  include/linux/dma-mapping.h        | 13 ++++++
  include/linux/iommu-dma.h          |  7 ++--
  include/linux/kmsan.h              | 12 +++---
  include/trace/events/dma.h         |  4 +-
  kernel/dma/debug.c                 | 28 ++++++++-----
  kernel/dma/debug.h                 | 16 ++++---
  kernel/dma/direct.c                |  6 +--
  kernel/dma/direct.h                | 13 +++---
  kernel/dma/mapping.c               | 67 +++++++++++++++++++++---------
  kernel/dma/ops_helpers.c           |  6 +--
  mm/hmm.c                           |  8 ++--
  mm/kmsan/hooks.c                   | 36 ++++++++++++----
  tools/virtio/linux/kmsan.h         |  2 +-
  18 files changed, 159 insertions(+), 93 deletions(-)

Quoted 4 lines · Leon Romanovsky

From: Leon Romanovsky <[email protected]>

All platforms which implement map_page interface don't support physical addresses without real struct page. Add condition to check it.

As-is, the condition also needs to cover iommu-dma, because that also still doesn't support non-page-backed addresses. You can't just do a simple s/page/phys/ rename and hope it's OK because you happen to get away with it for coherent, 64-bit, trusted devices.

But given what we do already know of from decades of experience, obvious
question: For the tiny minority of users who know full well when they're dealing with a non-page-backed physical address, what's wrong with using dma_map_resource?

Does it make sense to try to consolidate our p2p infrastructure so dma_map_resource() could return bus addresses where appropriate?

Are we trying to remove struct page from the kernel altogether?

Quoted 8 lines · Christoph Hellwig

Quoted 7 lines · Leon Romanovsky

Quoted 3 lines

Thanks for this rework! I assume that the next step is to add map_phys callback also to the dma_map_ops and teach various dma-mapping providers to use it to avoid more phys-to-page-to-phys conversions.

Probably Christoph will say yes, however I personally don't see any benefit in this. Maybe I wrong here, but all existing .map_page() implementation platforms don't support p2p anyway. They won't benefit from this such conversion.

Quoted 2 lines

I think that conversion should eventually happen, and rather sooner than later.

Agreed.

Applied patches 1-7 to my dma-mapping-next branch. Let me know if one needs a stable branch with it.

Leon, it would be great if You could also prepare an incremental patch adding map_phys callback to the dma_maps_ops, so the individual arch-specific dma-mapping providers can be then converted (or simplified in many cases) too.

Quoted 9 lines · Marek Szyprowski

Quoted 8 lines · Christoph Hellwig

Quoted 7 lines · Leon Romanovsky

Quoted 3 lines

Thanks for this rework! I assume that the next step is to add map_phys callback also to the dma_map_ops and teach various dma-mapping providers to use it to avoid more phys-to-page-to-phys conversions.

Probably Christoph will say yes, however I personally don't see any benefit in this. Maybe I wrong here, but all existing .map_page() implementation platforms don't support p2p anyway. They won't benefit from this such conversion.

Quoted 7 lines

Quoted 2 lines

I think that conversion should eventually happen, and rather sooner than later.

Agreed.

Applied patches 1-7 to my dma-mapping-next branch. Let me know if one needs a stable branch with it.

As the maintainer of iommu-dma, please drop the iommu-dma patch because it is broken. It does not in any way remove the struct page dependency from iommu-dma, it merely hides it so things can crash more easily in circumstances that clearly nobody's bothered to test.

Quoted 4 lines

Leon, it would be great if You could also prepare an incremental patch adding map_phys callback to the dma_maps_ops, so the individual arch-specific dma-mapping providers can be then converted (or simplified in many cases) too.

Marek, I'm surprised that even you aren't seeing why that would at best be pointless churn. The fundamental design of dma_map_page() operating on struct page is that it sits in between alloc_pages() at the caller and kmap_atomic() deep down in the DMA API implementation (which also subsumes any dependencies on having a kernel virtual address at the implementation end). The natural working unit for whatever replaces dma_map_page() will be whatever the replacement for alloc_pages() returns, and the replacement for kmap_atomic() operates on. Until that exists (and I simply cannot believe it would be an unadorned physical address) there cannot be any *meaningful* progress made towards removing the struct page dependency from the DMA API. If there is also a goal to kill off highmem before then, then logically we should just wait for that to land, then revert back to dma_map_single() being the first-class interface, and dma_map_page() can turn into a trivial page_to_virt() wrapper for the long tail of caller conversions.

Simply obfuscating the struct page dependency today by dressing it up as a phys_addr_t with implicit baggage is not not in any way helpful. It only makes the code harder to understand and more bug-prone. Despite the disingenuous claims, it is quite blatantly the opposite of "efficient" for callers to do extra work to throw away useful information with page_to_phys(), and the implementation then have to re-derive that information with pfn_valid()/phys_to_page().

And by "bug-prone" I also include greater distractions like this misguided idea that the same API could somehow work for non-memory addresses too, so then everyone can move on bikeshedding VFIO while overlooking the fundamental flaws in the whole premise. I mean, besides all the issues I've already pointed out in that regard, not least the glaring fact that it's literally just a worse version of *an API we already have*, as DMA API maintainer do you *really* approve of a design that depends on callers abusing DMA_ATTR_SKIP_CPU_SYNC, yet will still readily blow up if they did then call a dma_sync op?

Thanks,
Robin.

Quoted 5 lines

The natural working unit for whatever replaces dma_map_page() will be whatever the replacement for alloc_pages() returns, and the replacement for kmap_atomic() operates on. Until that exists (and I simply cannot believe it would be an unadorned physical address) there cannot be any *meaningful*

Quoted 5 lines

progress made towards removing the struct page dependency from the DMA API. If there is also a goal to kill off highmem before then, then logically we should just wait for that to land, then revert back to dma_map_single() being the first-class interface, and dma_map_page() can turn into a trivial page_to_virt() wrapper for the long tail of caller conversions.

The most reasonable way to prevent DMA_ATTR_SKIP_CPU_SYNC leakage is to introduce new DMA attribute (let's call it DMA_ATTR_MMIO for now) and pass it to both dma_map_phys() and dma_iova_link(). This flag will indicate that p2p type is PCI_P2PDMA_MAP_THRU_HOST_BRIDGE and call to right callbacks which will set IOMMU_MMIO flag and skip CPU sync,

Quoted 11 lines · Marek Szyprowski

Quoted 10 lines · Christoph Hellwig

Quoted 9 lines · Leon Romanovsky

Quoted 5 lines

Thanks for this rework! I assume that the next step is to add map_phys
callback also to the dma_map_ops and teach various dma-mapping providers
to use it to avoid more phys-to-page-to-phys conversions.

Probably Christoph will say yes, however I personally don't see any benefit in this. Maybe I wrong here, but all existing .map_page() implementation platforms don't support p2p anyway. They won't benefit from this such conversion.

Quoted 8 lines

Quoted 3 lines

I think that conversion should eventually happen, and rather sooner than
later.

Agreed.

Applied patches 1-7 to my dma-mapping-next branch. Let me know if one needs a stable branch with it.

As the maintainer of iommu-dma, please drop the iommu-dma patch because it is broken. It does not in any way remove the struct page dependency from iommu-dma, it merely hides it so things can crash more easily in circumstances that clearly nobody's bothered to test.

Quoted 4 lines

Leon, it would be great if You could also prepare an incremental patch adding map_phys callback to the dma_maps_ops, so the individual arch-specific dma-mapping providers can be then converted (or simplified in many cases) too.

Marek, I'm surprised that even you aren't seeing why that would at best be pointless churn. The fundamental design of dma_map_page() operating on struct page is that it sits in between alloc_pages() at the caller and kmap_atomic() deep down in the DMA API implementation (which also subsumes any dependencies on having a kernel virtual address at the implementation end). The natural working unit for whatever replaces dma_map_page() will be whatever the replacement for alloc_pages() returns, and the replacement for kmap_atomic() operates on. Until that exists (and I simply cannot believe it would be an unadorned physical address) there cannot be any *meaningful* progress made towards removing the struct page dependency from the DMA API. If there is also a goal to kill off highmem before then, then logically we should just wait for that to land, then revert back to dma_map_single() being the first-class interface, and dma_map_page() can turn into a trivial page_to_virt() wrapper for the long tail of caller conversions.

Simply obfuscating the struct page dependency today by dressing it up as a phys_addr_t with implicit baggage is not not in any way helpful. It only makes the code harder to understand and more bug-prone. Despite the disingenuous claims, it is quite blatantly the opposite of "efficient" for callers to do extra work to throw away useful information with page_to_phys(), and the implementation then have to re-derive that information with pfn_valid()/phys_to_page().

And by "bug-prone" I also include greater distractions like this misguided idea that the same API could somehow work for non-memory addresses too, so then everyone can move on bikeshedding VFIO while overlooking the fundamental flaws in the whole premise. I mean, besides all the issues I've already pointed out in that regard, not least the glaring fact that it's literally just a worse version of *an API we already have*, as DMA API maintainer do you *really* approve of a design that depends on callers abusing DMA_ATTR_SKIP_CPU_SYNC, yet will still readily blow up if they did then call a dma_sync op?

On Wed, Jul 30, 2025 at 04:40:26PM +0300, Leon Romanovsky wrote:

Quoted 5 lines

The most reasonable way to prevent DMA_ATTR_SKIP_CPU_SYNC leakage is to introduce new DMA attribute (let's call it DMA_ATTR_MMIO for now) and pass it to both dma_map_phys() and dma_iova_link(). This flag will indicate that p2p type is PCI_P2PDMA_MAP_THRU_HOST_BRIDGE and call to right callbacks which will set IOMMU_MMIO flag and skip CPU sync,

So the idea is if the memory is non-cachable, no-KVA you'd call dma_iova_link(phys_addr, DMA_ATTR_MMIO) and dma_map_phys(phys_addr, DMA_ATTR_MMIO) ?

And then internally the dma_ops and dma_iommu would use the existing map_page/map_resource variations based on the flag, thus ensuring that MMIO is never kmap'd or cache flushed?

dma_map_resource is really then just
dma_map_phys(phys_addr, DMA_ATTR_MMIO)?

I like this, I think it well addresses the concerns.

Jason

Marek, I'm surprised that even you aren't seeing why that would at best be pointless churn. The fundamental design of dma_map_page() operating on struct page is that it sits in between alloc_pages() at the caller and kmap_atomic() deep down in the DMA API implementation (which also subsumes any dependencies on having a kernel virtual address at the implementation end). The natural working unit for whatever replaces dma_map_page() will be

whatever the replacement for alloc_pages() returns, and the replacement for kmap_atomic() operates on. Until that exists (and I simply cannot believe it would be an unadorned physical address) there cannot be any *meaningful* progress made towards removing the struct page dependency from the DMA API. If there is also a goal to kill off highmem before then, then logically we should just wait for that to land, then revert back to dma_map_single() being the first-class interface, and dma_map_page() can turn into a trivial page_to_virt() wrapper for the long tail of caller conversions.

The replacement for kmap_atomic() is already here -- it's kmap_(atomic|local)_pfn(). If a simple wrapper like kmap_local_phys() would make this more palatable, that would be fine by me. Might save a bit of messing around with calculating offsets in each caller.

Matthew, do you think it makes sense to introduce types to make this clearer? We have two kinds of values that a phys_addr_t can store - something compatible with kmap_XX_phys(), and something that isn't.

This was recently a long discussion in ARM KVM as well which had a similar confusion that a phys_addr_t was actually two very different things inside its logic.

We clearly have these two different ideas floating around in code, page tables, etc.

Quoted 3 lines · Jason Gunthorpe

Matthew, do you think it makes sense to introduce types to make this clearer? We have two kinds of values that a phys_addr_t can store - something compatible with kmap_XX_phys(), and something that isn't.

I was with you up until this point. And then you said "What if we have a raccoon that isn't a raccoon" and my brain derailed.

- CPU untranslated. This is the "physical" address. Physical address 0 is what the CPU sees when it drives zeroes on the memory bus.

Quoted 2 lines

We clearly have these two different ideas floating around in code, page tables, etc.

No. No, we don't. I've never heard of this asininity before.