The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v2] dma-mapping: move dma_map_resource() sanity check into debug code
@ 2026-05-11  8:31 Jianpeng Chang
  2026-05-11 15:32 ` Robin Murphy
  2026-05-11 17:56 ` Leon Romanovsky
  0 siblings, 2 replies; 3+ messages in thread
From: Jianpeng Chang @ 2026-05-11  8:31 UTC (permalink / raw)
  To: m.szyprowski
  Cc: robin.murphy, leon, kbusch, jgg, iommu, linux-kernel, stable,
	Jianpeng Chang

dma_map_resource() uses pfn_valid() to ensure the range is not RAM.
However, pfn_valid() only checks for availability of the memory map for
a PFN but it does not ensure that the PFN is actually backed by RAM. On
ARM64 with SPARSEMEM (128MB section granularity), MMIO addresses that
share a section with RAM will falsely trigger the WARN_ON_ONCE and cause
dma_map_resource() to return DMA_MAPPING_ERROR.

This causes a WARNING on Raspberry Pi 4 during spi_bcm2835 probe because
the SPI FIFO register (0xfe204004) falls in the same sparsemem section
as the end of RAM (0xf8000000-0xfbffffff), both in section 31
(0xf8000000-0xffffffff).

Move the sanity check from dma_map_resource() into debug_dma_map_phys()
and replace the unreliable pfn_valid() with pfn_valid() &&
!PageReserved(), which correctly identifies actual usable RAM without
false positives for MMIO regions that happen to have struct pages.

Since dma_map_resource() is dma_map_phys(DMA_ATTR_MMIO), the check
applies equally to both APIs. Any non-reserved page represents kernel
memory to a sufficient degree that using DMA_ATTR_MMIO on it is almost
certainly wrong and risks breaking coherency on non-coherent platforms.
ZONE_DEVICE pages used for PCI P2P DMA (MEMORY_DEVICE_PCI_P2PDMA) have
PageReserved set, so they will not trigger a false positive.

The check is now a WARN_ONCE that no longer blocks the mapping, since
being unobtrusive is more important than being exhaustive for what is
merely a debug sanity check.

Fixes: f7326196a781 ("dma-mapping: export new dma_*map_phys() interface")
Signed-off-by: Jianpeng Chang <jianpeng.chang.cn@windriver.com>
---
v2:
  - move check to debug_dma_map_phys and replace pfn_valid() with
    pfn_valid() && !PageReserved() as Robin suggested.
  - update commit message to explain why PageReserved is safe for
    ZONE_DEVICE PCI_P2PDMA pages
v1: https://lore.kernel.org/all/20260507032120.4072283-1-jianpeng.chang.cn@windriver.com/
 kernel/dma/debug.c   | 9 +++++++++
 kernel/dma/mapping.c | 4 ----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
index 1a725edbbbf6..180aa2c930b5 100644
--- a/kernel/dma/debug.c
+++ b/kernel/dma/debug.c
@@ -1239,6 +1239,15 @@ void debug_dma_map_phys(struct device *dev, phys_addr_t phys, size_t size,
 	if (dma_mapping_error(dev, dma_addr))
 		return;
 
+	if (attrs & DMA_ATTR_MMIO) {
+		unsigned long pfn = PHYS_PFN(phys);
+
+		WARN_ONCE(pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn)),
+			  "dma_map_resource called for RAM address %pa\n",
+			  &phys);
+		return;
+	}
+
 	entry = dma_entry_alloc();
 	if (!entry)
 		return;
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 23ed8eb9233e..e6b07f160d20 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -365,10 +365,6 @@ EXPORT_SYMBOL(dma_unmap_sg_attrs);
 dma_addr_t dma_map_resource(struct device *dev, phys_addr_t phys_addr,
 		size_t size, enum dma_data_direction dir, unsigned long attrs)
 {
-	if (IS_ENABLED(CONFIG_DMA_API_DEBUG) &&
-	    WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
-		return DMA_MAPPING_ERROR;
-
 	return dma_map_phys(dev, phys_addr, size, dir, attrs | DMA_ATTR_MMIO);
 }
 EXPORT_SYMBOL(dma_map_resource);
-- 
2.54.0


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

* Re: [PATCH v2] dma-mapping: move dma_map_resource() sanity check into debug code
  2026-05-11  8:31 [PATCH v2] dma-mapping: move dma_map_resource() sanity check into debug code Jianpeng Chang
@ 2026-05-11 15:32 ` Robin Murphy
  2026-05-11 17:56 ` Leon Romanovsky
  1 sibling, 0 replies; 3+ messages in thread
From: Robin Murphy @ 2026-05-11 15:32 UTC (permalink / raw)
  To: Jianpeng Chang, m.szyprowski
  Cc: leon, kbusch, jgg, iommu, linux-kernel, stable

On 2026-05-11 9:31 am, Jianpeng Chang wrote:
> dma_map_resource() uses pfn_valid() to ensure the range is not RAM.
> However, pfn_valid() only checks for availability of the memory map for
> a PFN but it does not ensure that the PFN is actually backed by RAM. On
> ARM64 with SPARSEMEM (128MB section granularity), MMIO addresses that
> share a section with RAM will falsely trigger the WARN_ON_ONCE and cause
> dma_map_resource() to return DMA_MAPPING_ERROR.
> 
> This causes a WARNING on Raspberry Pi 4 during spi_bcm2835 probe because
> the SPI FIFO register (0xfe204004) falls in the same sparsemem section
> as the end of RAM (0xf8000000-0xfbffffff), both in section 31
> (0xf8000000-0xffffffff).
> 
> Move the sanity check from dma_map_resource() into debug_dma_map_phys()
> and replace the unreliable pfn_valid() with pfn_valid() &&
> !PageReserved(), which correctly identifies actual usable RAM without
> false positives for MMIO regions that happen to have struct pages.
> 
> Since dma_map_resource() is dma_map_phys(DMA_ATTR_MMIO), the check
> applies equally to both APIs. Any non-reserved page represents kernel
> memory to a sufficient degree that using DMA_ATTR_MMIO on it is almost
> certainly wrong and risks breaking coherency on non-coherent platforms.
> ZONE_DEVICE pages used for PCI P2P DMA (MEMORY_DEVICE_PCI_P2PDMA) have
> PageReserved set, so they will not trigger a false positive.
> 
> The check is now a WARN_ONCE that no longer blocks the mapping, since
> being unobtrusive is more important than being exhaustive for what is
> merely a debug sanity check.
> 
> Fixes: f7326196a781 ("dma-mapping: export new dma_*map_phys() interface")
> Signed-off-by: Jianpeng Chang <jianpeng.chang.cn@windriver.com>
> ---
> v2:
>    - move check to debug_dma_map_phys and replace pfn_valid() with
>      pfn_valid() && !PageReserved() as Robin suggested.
>    - update commit message to explain why PageReserved is safe for
>      ZONE_DEVICE PCI_P2PDMA pages
> v1: https://lore.kernel.org/all/20260507032120.4072283-1-jianpeng.chang.cn@windriver.com/
>   kernel/dma/debug.c   | 9 +++++++++
>   kernel/dma/mapping.c | 4 ----
>   2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
> index 1a725edbbbf6..180aa2c930b5 100644
> --- a/kernel/dma/debug.c
> +++ b/kernel/dma/debug.c
> @@ -1239,6 +1239,15 @@ void debug_dma_map_phys(struct device *dev, phys_addr_t phys, size_t size,
>   	if (dma_mapping_error(dev, dma_addr))
>   		return;
>   
> +	if (attrs & DMA_ATTR_MMIO) {
> +		unsigned long pfn = PHYS_PFN(phys);
> +
> +		WARN_ONCE(pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn)),
> +			  "dma_map_resource called for RAM address %pa\n",
> +			  &phys);
> +		return;
> +	}

This wants to pair with the existing !MMIO checks slightly further down 
- since we're no longer aborting the map request itself, if we bail out 
early without actually creating the debug entry, we'll end up firing 
another spurious warning if the caller (correctly) unmaps the address. 
Then we can also finish the job of turning it into a proper dma-debug 
check by using err_printk() instead of the open-coded WARN(), so that 
it's subject to filtering etc. as well.

With that fixed,

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> +
>   	entry = dma_entry_alloc();
>   	if (!entry)
>   		return;
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index 23ed8eb9233e..e6b07f160d20 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -365,10 +365,6 @@ EXPORT_SYMBOL(dma_unmap_sg_attrs);
>   dma_addr_t dma_map_resource(struct device *dev, phys_addr_t phys_addr,
>   		size_t size, enum dma_data_direction dir, unsigned long attrs)
>   {
> -	if (IS_ENABLED(CONFIG_DMA_API_DEBUG) &&
> -	    WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
> -		return DMA_MAPPING_ERROR;
> -
>   	return dma_map_phys(dev, phys_addr, size, dir, attrs | DMA_ATTR_MMIO);
>   }
>   EXPORT_SYMBOL(dma_map_resource);


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

* Re: [PATCH v2] dma-mapping: move dma_map_resource() sanity check into debug code
  2026-05-11  8:31 [PATCH v2] dma-mapping: move dma_map_resource() sanity check into debug code Jianpeng Chang
  2026-05-11 15:32 ` Robin Murphy
@ 2026-05-11 17:56 ` Leon Romanovsky
  1 sibling, 0 replies; 3+ messages in thread
From: Leon Romanovsky @ 2026-05-11 17:56 UTC (permalink / raw)
  To: Jianpeng Chang
  Cc: m.szyprowski, robin.murphy, kbusch, jgg, iommu, linux-kernel,
	stable

On Mon, May 11, 2026 at 04:31:33PM +0800, Jianpeng Chang wrote:
> dma_map_resource() uses pfn_valid() to ensure the range is not RAM.
> However, pfn_valid() only checks for availability of the memory map for
> a PFN but it does not ensure that the PFN is actually backed by RAM. On
> ARM64 with SPARSEMEM (128MB section granularity), MMIO addresses that
> share a section with RAM will falsely trigger the WARN_ON_ONCE and cause
> dma_map_resource() to return DMA_MAPPING_ERROR.
> 
> This causes a WARNING on Raspberry Pi 4 during spi_bcm2835 probe because
> the SPI FIFO register (0xfe204004) falls in the same sparsemem section
> as the end of RAM (0xf8000000-0xfbffffff), both in section 31
> (0xf8000000-0xffffffff).
> 
> Move the sanity check from dma_map_resource() into debug_dma_map_phys()
> and replace the unreliable pfn_valid() with pfn_valid() &&
> !PageReserved(), which correctly identifies actual usable RAM without
> false positives for MMIO regions that happen to have struct pages.
> 
> Since dma_map_resource() is dma_map_phys(DMA_ATTR_MMIO), the check
> applies equally to both APIs. Any non-reserved page represents kernel
> memory to a sufficient degree that using DMA_ATTR_MMIO on it is almost
> certainly wrong and risks breaking coherency on non-coherent platforms.
> ZONE_DEVICE pages used for PCI P2P DMA (MEMORY_DEVICE_PCI_P2PDMA) have
> PageReserved set, so they will not trigger a false positive.
> 
> The check is now a WARN_ONCE that no longer blocks the mapping, since
> being unobtrusive is more important than being exhaustive for what is
> merely a debug sanity check.
> 
> Fixes: f7326196a781 ("dma-mapping: export new dma_*map_phys() interface")
> Signed-off-by: Jianpeng Chang <jianpeng.chang.cn@windriver.com>
> ---
> v2:
>   - move check to debug_dma_map_phys and replace pfn_valid() with
>     pfn_valid() && !PageReserved() as Robin suggested.
>   - update commit message to explain why PageReserved is safe for
>     ZONE_DEVICE PCI_P2PDMA pages
> v1: https://lore.kernel.org/all/20260507032120.4072283-1-jianpeng.chang.cn@windriver.com/
>  kernel/dma/debug.c   | 9 +++++++++
>  kernel/dma/mapping.c | 4 ----
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
> index 1a725edbbbf6..180aa2c930b5 100644
> --- a/kernel/dma/debug.c
> +++ b/kernel/dma/debug.c
> @@ -1239,6 +1239,15 @@ void debug_dma_map_phys(struct device *dev, phys_addr_t phys, size_t size,
>  	if (dma_mapping_error(dev, dma_addr))
>  		return;
>  
> +	if (attrs & DMA_ATTR_MMIO) {
> +		unsigned long pfn = PHYS_PFN(phys);
> +
> +		WARN_ONCE(pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn)),
> +			  "dma_map_resource called for RAM address %pa\n",
> +			  &phys);
> +		return;

I’m not comfortable with this return statement. It effectively disables  
DMA debugging for any caller that uses dma_map_phys(..., DMA_ATTR_MMIO),
like dma-buf, block layer, HMM, e.t.c

Thanks

> +	}
> +
>  	entry = dma_entry_alloc();
>  	if (!entry)
>  		return;
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index 23ed8eb9233e..e6b07f160d20 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -365,10 +365,6 @@ EXPORT_SYMBOL(dma_unmap_sg_attrs);
>  dma_addr_t dma_map_resource(struct device *dev, phys_addr_t phys_addr,
>  		size_t size, enum dma_data_direction dir, unsigned long attrs)
>  {
> -	if (IS_ENABLED(CONFIG_DMA_API_DEBUG) &&
> -	    WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
> -		return DMA_MAPPING_ERROR;
> -
>  	return dma_map_phys(dev, phys_addr, size, dir, attrs | DMA_ATTR_MMIO);
>  }
>  EXPORT_SYMBOL(dma_map_resource);
> -- 
> 2.54.0
> 

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

end of thread, other threads:[~2026-05-11 17:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-11  8:31 [PATCH v2] dma-mapping: move dma_map_resource() sanity check into debug code Jianpeng Chang
2026-05-11 15:32 ` Robin Murphy
2026-05-11 17:56 ` Leon Romanovsky

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