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

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

[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.

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.

Fixes: f7326196a781 ("dma-mapping: export new dma_*map_phys() interface")
Signed-off-by: Jianpeng Chang <jianpeng.chang.cn@windriver.com>
---
Hi,

I found the WARNING in dma_map_resource() on Raspberry Pi 4 when using the
downstream kernel from https://github.com/raspberrypi/linux, which calls
dma_map_resource() in bcm2835-dma.c (mainline uses the physical address
directly instead).

Although mainline bcm2835-dma does not call dma_map_resource(), the bogus
pfn_valid() check can still affect any other driver that does, so it
should be removed again.

Thanks,
Jianpeng

 kernel/dma/mapping.c | 4 ----
 1 file changed, 4 deletions(-)

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

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

[Robin Murphy]

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,
Robin.

> Fixes: f7326196a781 ("dma-mapping: export new dma_*map_phys() interface")
> Signed-off-by: Jianpeng Chang <jianpeng.chang.cn@windriver.com>
> ---
> Hi,
> 
> I found the WARNING in dma_map_resource() on Raspberry Pi 4 when using the
> downstream kernel from https://github.com/raspberrypi/linux, which calls
> dma_map_resource() in bcm2835-dma.c (mainline uses the physical address
> directly instead).
> 
> Although mainline bcm2835-dma does not call dma_map_resource(), the bogus
> pfn_valid() check can still affect any other driver that does, so it
> should be removed again.
> 
> Thanks,
> Jianpeng
> 
>   kernel/dma/mapping.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> 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);

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

[Jianpeng Chang]

在 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.

Thanks,
Jianpeng>
> Thanks,
> Robin.
> 
>> Fixes: f7326196a781 ("dma-mapping: export new dma_*map_phys() interface")
>> Signed-off-by: Jianpeng Chang <jianpeng.chang.cn@windriver.com>
>> ---
>> Hi,
>>
>> I found the WARNING in dma_map_resource() on Raspberry Pi 4 when using 
>> the
>> downstream kernel from https://github.com/raspberrypi/linux, which calls
>> dma_map_resource() in bcm2835-dma.c (mainline uses the physical address
>> directly instead).
>>
>> Although mainline bcm2835-dma does not call dma_map_resource(), the bogus
>> pfn_valid() check can still affect any other driver that does, so it
>> should be removed again.
>>
>> Thanks,
>> Jianpeng
>>
>>   kernel/dma/mapping.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> 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);
> 

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

[Robin Murphy]

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.

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

[Jason Gunthorpe]

On Fri, May 08, 2026 at 06:01:01PM +0800, Jianpeng Chang wrote:
> > 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.

I'm not sure that is right. IIRC pfn_valid() is true for ZONE_DEVICE
P2P pages that are used with map_phys but never with map_resource.

PageReserved isn't enough to fix it.

Jason

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

[Robin Murphy]

On 2026-05-08 12:31 pm, Jason Gunthorpe wrote:
> On Fri, May 08, 2026 at 06:01:01PM +0800, Jianpeng Chang wrote:
>>> 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.
> 
> I'm not sure that is right. IIRC pfn_valid() is true for ZONE_DEVICE
> P2P pages that are used with map_phys but never with map_resource.
> 
> PageReserved isn't enough to fix it.

It fixes the false-positive on non-reserved pages, which is the 
important thing. Yes, we'll get false-negatives on reserved ZONE_DEVICE 
pages and similar, but that's still an improvement over getting 
false-negatives on _everything_ by not checking at all. Realistically, 
dma-debug can never be exhaustive and 100% accurate, but there's still 
value in catching as much obvious misuse as is straightforward to do.

In the long term, perhaps we could add some kind of "DMAable 
memory/P2Pable MMIO/neither" attribute to our wishlist of physical 
address range properties along with CoCO shared/private, such that 
dma_map_phys() could then do the right thing all by itself and we 
wouldn't even need DMA_ATTR_MMIO any more...

Thanks,
Robin.

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

[Jason Gunthorpe]

On Fri, May 08, 2026 at 01:16:25PM +0100, Robin Murphy wrote:
> On 2026-05-08 12:31 pm, Jason Gunthorpe wrote:
> > On Fri, May 08, 2026 at 06:01:01PM +0800, Jianpeng Chang wrote:
> > > > 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.
> > 
> > I'm not sure that is right. IIRC pfn_valid() is true for ZONE_DEVICE
> > P2P pages that are used with map_phys but never with map_resource.
> > 
> > PageReserved isn't enough to fix it.
> 
> It fixes the false-positive on non-reserved pages, which is the important
> thing. Yes, we'll get false-negatives on reserved ZONE_DEVICE pages and
> similar, but that's still an improvement over getting false-negatives on
> _everything_ by not checking at all. Realistically, dma-debug can never be
> exhaustive and 100% accurate, but there's still value in catching as much
> obvious misuse as is straightforward to do.

I'm saying I think the new expression still has a false positive for
the common case of map_phys with ZONE_DEVICE P2P, and I don't want to
see debugging logging for normal as-designed scenarios in map_phys.

So we either need to narrow the expression further somehow, or leave
it in map_resource which has fewer users and doesn't accept
ZONE_DEVICE anyhow.

Jason

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

[Robin Murphy]

On 2026-05-08 4:18 pm, Jason Gunthorpe wrote:
> On Fri, May 08, 2026 at 01:16:25PM +0100, Robin Murphy wrote:
>> On 2026-05-08 12:31 pm, Jason Gunthorpe wrote:
>>> On Fri, May 08, 2026 at 06:01:01PM +0800, Jianpeng Chang wrote:
>>>>> 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.
>>>
>>> I'm not sure that is right. IIRC pfn_valid() is true for ZONE_DEVICE
>>> P2P pages that are used with map_phys but never with map_resource.
>>>
>>> PageReserved isn't enough to fix it.
>>
>> It fixes the false-positive on non-reserved pages, which is the important
>> thing. Yes, we'll get false-negatives on reserved ZONE_DEVICE pages and
>> similar, but that's still an improvement over getting false-negatives on
>> _everything_ by not checking at all. Realistically, dma-debug can never be
>> exhaustive and 100% accurate, but there's still value in catching as much
>> obvious misuse as is straightforward to do.
> 
> I'm saying I think the new expression still has a false positive for
> the common case of map_phys with ZONE_DEVICE P2P, and I don't want to
> see debugging logging for normal as-designed scenarios in map_phys.
> 
> So we either need to narrow the expression further somehow, or leave
> it in map_resource which has fewer users and doesn't accept
> ZONE_DEVICE anyhow.

But surely anything with a ZONE_DEVICE page is "memory" to the degree 
that mapping it with DMA_ATTR_MMIO would be wrong, no? However, IIRC 
ZONE_DEVICE pages _are_ reserved, so still wouldn't warn whether we'd 
like it or not. I'm confused as to what you're objecting to...

Robin.

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

[Jason Gunthorpe]

On Fri, May 08, 2026 at 05:04:31PM +0100, Robin Murphy wrote:
> On 2026-05-08 4:18 pm, Jason Gunthorpe wrote:
> > On Fri, May 08, 2026 at 01:16:25PM +0100, Robin Murphy wrote:
> > > On 2026-05-08 12:31 pm, Jason Gunthorpe wrote:
> > > > On Fri, May 08, 2026 at 06:01:01PM +0800, Jianpeng Chang wrote:
> > > > > > 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.
> > > > 
> > > > I'm not sure that is right. IIRC pfn_valid() is true for ZONE_DEVICE
> > > > P2P pages that are used with map_phys but never with map_resource.
> > > > 
> > > > PageReserved isn't enough to fix it.
> > > 
> > > It fixes the false-positive on non-reserved pages, which is the important
> > > thing. Yes, we'll get false-negatives on reserved ZONE_DEVICE pages and
> > > similar, but that's still an improvement over getting false-negatives on
> > > _everything_ by not checking at all. Realistically, dma-debug can never be
> > > exhaustive and 100% accurate, but there's still value in catching as much
> > > obvious misuse as is straightforward to do.
> > 
> > I'm saying I think the new expression still has a false positive for
> > the common case of map_phys with ZONE_DEVICE P2P, and I don't want to
> > see debugging logging for normal as-designed scenarios in map_phys.
> > 
> > So we either need to narrow the expression further somehow, or leave
> > it in map_resource which has fewer users and doesn't accept
> > ZONE_DEVICE anyhow.
> 
> But surely anything with a ZONE_DEVICE page is "memory" to the degree that
> mapping it with DMA_ATTR_MMIO would be wrong, no?

If the ZONE_DEVICE subtype is MEMORY_DEVICE_PCI_P2PDMA it is mapped as
MMIO and must be used with DMA_ATTR_MMIO.

> However, IIRC ZONE_DEVICE pages _are_ reserved, so still wouldn't
> warn whether we'd like it or not. 

I didn't think that was the case for PCI_P2PDMA, but yes it does look
like the reserved flag remains set.

> I'm confused as to what you're objecting to...

I don't want to see a warning, if it turns out it doesn't then it's
fine, but it certainly isn't obvious that it was going to be OK for
phys and I explained what we were worried about when we had left this
behind in map resource. 

So this should all be summarized in the commit message moving the
check

Jason

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

[Robin Murphy]

On 2026-05-08 6:36 pm, Jason Gunthorpe wrote:
> On Fri, May 08, 2026 at 05:04:31PM +0100, Robin Murphy wrote:
>> On 2026-05-08 4:18 pm, Jason Gunthorpe wrote:
>>> On Fri, May 08, 2026 at 01:16:25PM +0100, Robin Murphy wrote:
>>>> On 2026-05-08 12:31 pm, Jason Gunthorpe wrote:
>>>>> On Fri, May 08, 2026 at 06:01:01PM +0800, Jianpeng Chang wrote:
>>>>>>> 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.
>>>>>
>>>>> I'm not sure that is right. IIRC pfn_valid() is true for ZONE_DEVICE
>>>>> P2P pages that are used with map_phys but never with map_resource.
>>>>>
>>>>> PageReserved isn't enough to fix it.
>>>>
>>>> It fixes the false-positive on non-reserved pages, which is the important
>>>> thing. Yes, we'll get false-negatives on reserved ZONE_DEVICE pages and
>>>> similar, but that's still an improvement over getting false-negatives on
>>>> _everything_ by not checking at all. Realistically, dma-debug can never be
>>>> exhaustive and 100% accurate, but there's still value in catching as much
>>>> obvious misuse as is straightforward to do.
>>>
>>> I'm saying I think the new expression still has a false positive for
>>> the common case of map_phys with ZONE_DEVICE P2P, and I don't want to
>>> see debugging logging for normal as-designed scenarios in map_phys.
>>>
>>> So we either need to narrow the expression further somehow, or leave
>>> it in map_resource which has fewer users and doesn't accept
>>> ZONE_DEVICE anyhow.
>>
>> But surely anything with a ZONE_DEVICE page is "memory" to the degree that
>> mapping it with DMA_ATTR_MMIO would be wrong, no?
> 
> If the ZONE_DEVICE subtype is MEMORY_DEVICE_PCI_P2PDMA it is mapped as
> MMIO and must be used with DMA_ATTR_MMIO.
> 
>> However, IIRC ZONE_DEVICE pages _are_ reserved, so still wouldn't
>> warn whether we'd like it or not.
> 
> I didn't think that was the case for PCI_P2PDMA, but yes it does look
> like the reserved flag remains set.
> 
>> I'm confused as to what you're objecting to...
> 
> I don't want to see a warning, if it turns out it doesn't then it's
> fine, but it certainly isn't obvious that it was going to be OK for
> phys and I explained what we were worried about when we had left this
> behind in map resource.
> 
> So this should all be summarized in the commit message moving the
> check

Indeed. The general rule here is that DMA_ATTR_MMIO must not be used for 
anything which could have a cacheable CPU mapping because that could 
break coherency, while conversely any !DMA_ATTR_MMIO mappings must be 
valid for phys_to_virt()/kmap_atomic() so that a DMA API backend *can* 
perform cache maintenance by VA internally. Anything that is invalid for 
dma_map_resource() is inherently invalid for dma_map_phys(DMA_ATTR_MMIO) 
for the same reasons, because dma_map_resource() *is* 
dma_map_phys(DMA_ATTR_MMIO), and it doesn't make any sense to check the 
wrapper differently from the thing it wraps when they are both equally 
public APIs. The point of checking is not to enforce some 
arbitrarily-decided API policy, it is to flag up "you must not do this 
because it risks going badly wrong on non-coherent platforms" if a 
driver does try to do something obviously inappropriate.

Robin.