Re: [PATCH v2] dma-mapping: move dma_map_resource() sanity check into debug code

[Robin Murphy]

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@xxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxx/
>   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@xxxxxxx>

> +
>   	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);