Re: [PATCH] dma-mapping: remove bogus test for pfn_valid from dma_map_resource

[Robin Murphy <robin.murphy@arm.com>]

On 2026-05-08 11:01 am, Jianpeng Chang wrote:
> 
> 
> 在 2026/5/7 下午9:18, Robin Murphy 写道:
>> CAUTION: This email comes from a non Wind River email account!
>> Do not click links or open attachments unless you recognize the sender 
>> and know the content is safe.
>>
>> On 07/05/2026 4:21 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.
>>>
>>> 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).
>>>
>>> The pfn_valid() check was originally removed by commit a9c38c5d267c
>>> ("dma-mapping: remove bogus test for pfn_valid from dma_map_resource")
>>> but was accidentally re-introduced by commit f7326196a781
>>> ("dma-mapping: export new dma_*map_phys() interface") during the
>>> refactoring of dma_map_resource() into a wrapper around dma_map_phys().
>>>
>>> Drop the pfn_valid() test from dma_map_resource() again.
>>
>> As I said last time, I think pfn_valid() && !PageReserved(pfn_to_page())
>> would be enough for what we want here, although now it's strictly under
>> CONFIG_DMA_API_DEBUG, perhaps the overhead of memblock_is_map_memory()
>> might be less of an issue. Either way though, now that it's all
>> channelled through the single dma_map_phys() path, it would probably
>> make sense to consolidate any MMIO sanity-checking into
>> dma_debug_map_phys() anyway :/
> Thanks for the suggestion. Move the check into debug_dma_map_phys() is 
> indeed better, and I will replace pfn_valid() with pfn_valid() && ! 
> PageReserved() as you suggested.

Oh, and just to clarify on the points DavidH raised last time, indeed 
PageReserved isn't 100% accurate for this, and there may well be 
reserved pages which shouldn't be DMA-mapped either, but that's a 
reasonable false-negative (especially compared to having no check at 
all!) - the main thing is any *non*-reserved page represents "kernel 
memory" to enough of a degree that using DMA_ATTR_MMIO is almost 
certainly wrong and liable to break coherency.

Cheers,
Robin.