| Message ID | 20220420124341.14982-1-quic_c_gdjako@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [5.15,1/2] dma-mapping: remove bogus test for pfn_valid from dma_map_resource | expand |
On 21.04.22 9:43, Greg KH wrote: > On Wed, Apr 20, 2022 at 05:43:40AM -0700, Georgi Djakov wrote: >> From: Mike Rapoport <rppt@linux.ibm.com> >> >> [ Upstream commit a9c38c5d267cb94871dfa2de5539c92025c855d7 ] >> >> 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. >> >> As dma_map_resource() is the only method in DMA mapping APIs that has this >> check, simply drop the pfn_valid() test from dma_map_resource(). >> >> Link: https://lore.kernel.org/all/20210824173741.GC623@arm.com/ >> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com> >> Reviewed-by: Christoph Hellwig <hch@lst.de> >> Acked-by: David Hildenbrand <david@redhat.com> >> Link: https://lore.kernel.org/r/20210930013039.11260-2-rppt@kernel.org >> Signed-off-by: Will Deacon <will@kernel.org> >> Fixes: 859a85ddf90e ("mm: remove pfn_valid_within() and CONFIG_HOLES_IN_ZONE") >> Link: https://lore.kernel.org/r/Yl0IZWT2nsiYtqBT@linux.ibm.com >> Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> >> --- >> kernel/dma/mapping.c | 4 ---- >> 1 file changed, 4 deletions(-) > > I took this, but I do not understand why patch 2/2 in this series is > needed, as Sasha points out. Cleanups are nice, but is it necessary > here? It's needed as it removes the "select HAVE_ARCH_PFN_VALID" from the arm64/Kconfig. This will make us use the generic pfn_valid() function in mmzone.h, instead of the arch-specific one, that we are dropping. Thanks, Georgi
On Thu, Apr 21, 2022 at 10:51:39AM +0300, Georgi Djakov wrote: > > On 21.04.22 9:43, Greg KH wrote: > > On Wed, Apr 20, 2022 at 05:43:40AM -0700, Georgi Djakov wrote: > > > From: Mike Rapoport <rppt@linux.ibm.com> > > > > > > [ Upstream commit a9c38c5d267cb94871dfa2de5539c92025c855d7 ] > > > > > > 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. > > > > > > As dma_map_resource() is the only method in DMA mapping APIs that has this > > > check, simply drop the pfn_valid() test from dma_map_resource(). > > > > > > Link: https://lore.kernel.org/all/20210824173741.GC623@arm.com/ > > > Signed-off-by: Mike Rapoport <rppt@linux.ibm.com> > > > Reviewed-by: Christoph Hellwig <hch@lst.de> > > > Acked-by: David Hildenbrand <david@redhat.com> > > > Link: https://lore.kernel.org/r/20210930013039.11260-2-rppt@kernel.org > > > Signed-off-by: Will Deacon <will@kernel.org> > > > Fixes: 859a85ddf90e ("mm: remove pfn_valid_within() and CONFIG_HOLES_IN_ZONE") > > > Link: https://lore.kernel.org/r/Yl0IZWT2nsiYtqBT@linux.ibm.com > > > Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> > > > --- > > > kernel/dma/mapping.c | 4 ---- > > > 1 file changed, 4 deletions(-) > > > > I took this, but I do not understand why patch 2/2 in this series is > > needed, as Sasha points out. Cleanups are nice, but is it necessary > > here? > > It's needed as it removes the "select HAVE_ARCH_PFN_VALID" from the arm64/Kconfig. > This will make us use the generic pfn_valid() function in mmzone.h, instead of the > arch-specific one, that we are dropping. Ah, that is not obvious at all. Ok, I'll queue this up, but you should make sure that this doesn't break anything on your systems as I thought they required this to be a function for some reason... thanks, greg k-h
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c index 8349a9f2c345..9478eccd1c8e 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -296,10 +296,6 @@ dma_addr_t dma_map_resource(struct device *dev, phys_addr_t phys_addr, if (WARN_ON_ONCE(!dev->dma_mask)) return DMA_MAPPING_ERROR; - /* Don't allow RAM to be mapped */ - if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr)))) - return DMA_MAPPING_ERROR; - if (dma_map_direct(dev, ops)) addr = dma_direct_map_resource(dev, phys_addr, size, dir, attrs); else if (ops->map_resource)