* fix a layering violation in videobuf2 and improve dma_map_resource v2 @ 2019-01-18 11:37 ` Christoph Hellwig 2019-01-18 11:37 ` [PATCH 1/3] dma-mapping: remove the default map_resource implementation Christoph Hellwig ` (3 more replies) 0 siblings, 4 replies; 8+ messages in thread From: Christoph Hellwig @ 2019-01-18 11:37 UTC (permalink / raw) To: Pawel Osciak, Marek Szyprowski, Kyungmin Park, Niklas Söderlund Cc: Russell King, linux-kernel, iommu, Mauro Carvalho Chehab, linuxppc-dev, linux-arm-kernel, linux-media Hi all, this series fixes a rather gross layering violation in videobuf2, which pokes into arm DMA mapping internals to get a DMA address for memory that does not have a page structure, and to do so fixes up the dma_map_resource implementation to not provide a somewhat dangerous default and improve the error handling. Changes since v1: - don't apply bus offsets in dma_direct_map_resource ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] dma-mapping: remove the default map_resource implementation 2019-01-18 11:37 ` fix a layering violation in videobuf2 and improve dma_map_resource v2 Christoph Hellwig @ 2019-01-18 11:37 ` Christoph Hellwig 2019-01-18 11:37 ` [PATCH 2/3] dma-mapping: don't BUG when calling dma_map_resource on RAM Christoph Hellwig ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Christoph Hellwig @ 2019-01-18 11:37 UTC (permalink / raw) To: Pawel Osciak, Marek Szyprowski, Kyungmin Park, Niklas Söderlund Cc: Russell King, linux-kernel, iommu, Mauro Carvalho Chehab, linuxppc-dev, linux-arm-kernel, linux-media Instead provide a proper implementation in the direct mapping code, and also wire it up for arm and powerpc, leaving an error return for all the IOMMU or virtual mapping instances for which we'd have to wire up an actual implementation Signed-off-by: Christoph Hellwig <hch@lst.de> --- arch/arm/mm/dma-mapping.c | 2 ++ arch/powerpc/kernel/dma-swiotlb.c | 1 + arch/powerpc/kernel/dma.c | 1 + include/linux/dma-mapping.h | 12 +++++++----- kernel/dma/direct.c | 14 ++++++++++++++ 5 files changed, 25 insertions(+), 5 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index f1e2922e447c..3c8534904209 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -188,6 +188,7 @@ const struct dma_map_ops arm_dma_ops = { .unmap_page = arm_dma_unmap_page, .map_sg = arm_dma_map_sg, .unmap_sg = arm_dma_unmap_sg, + .map_resource = dma_direct_map_resource, .sync_single_for_cpu = arm_dma_sync_single_for_cpu, .sync_single_for_device = arm_dma_sync_single_for_device, .sync_sg_for_cpu = arm_dma_sync_sg_for_cpu, @@ -211,6 +212,7 @@ const struct dma_map_ops arm_coherent_dma_ops = { .get_sgtable = arm_dma_get_sgtable, .map_page = arm_coherent_dma_map_page, .map_sg = arm_dma_map_sg, + .map_resource = dma_direct_map_resource, .dma_supported = arm_dma_supported, }; EXPORT_SYMBOL(arm_coherent_dma_ops); diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-swiotlb.c index 7d5fc9751622..fbb2506a414e 100644 --- a/arch/powerpc/kernel/dma-swiotlb.c +++ b/arch/powerpc/kernel/dma-swiotlb.c @@ -55,6 +55,7 @@ const struct dma_map_ops powerpc_swiotlb_dma_ops = { .dma_supported = swiotlb_dma_supported, .map_page = dma_direct_map_page, .unmap_page = dma_direct_unmap_page, + .map_resource = dma_direct_map_resource, .sync_single_for_cpu = dma_direct_sync_single_for_cpu, .sync_single_for_device = dma_direct_sync_single_for_device, .sync_sg_for_cpu = dma_direct_sync_sg_for_cpu, diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c index b1903ebb2e9c..258b9e8ebb99 100644 --- a/arch/powerpc/kernel/dma.c +++ b/arch/powerpc/kernel/dma.c @@ -273,6 +273,7 @@ const struct dma_map_ops dma_nommu_ops = { .dma_supported = dma_nommu_dma_supported, .map_page = dma_nommu_map_page, .unmap_page = dma_nommu_unmap_page, + .map_resource = dma_direct_map_resource, .get_required_mask = dma_nommu_get_required_mask, #ifdef CONFIG_NOT_COHERENT_CACHE .sync_single_for_cpu = dma_nommu_sync_single, diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index f6ded992c183..9842085e6774 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -208,6 +208,8 @@ dma_addr_t dma_direct_map_page(struct device *dev, struct page *page, unsigned long attrs); int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents, enum dma_data_direction dir, unsigned long attrs); +dma_addr_t dma_direct_map_resource(struct device *dev, phys_addr_t paddr, + size_t size, enum dma_data_direction dir, unsigned long attrs); #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \ defined(CONFIG_SWIOTLB) @@ -346,19 +348,19 @@ static inline dma_addr_t dma_map_resource(struct device *dev, unsigned long attrs) { const struct dma_map_ops *ops = get_dma_ops(dev); - dma_addr_t addr; + dma_addr_t addr = DMA_MAPPING_ERROR; BUG_ON(!valid_dma_direction(dir)); /* Don't allow RAM to be mapped */ BUG_ON(pfn_valid(PHYS_PFN(phys_addr))); - addr = phys_addr; - if (ops && ops->map_resource) + if (dma_is_direct(ops)) + addr = dma_direct_map_resource(dev, phys_addr, size, dir, attrs); + else if (ops->map_resource) addr = ops->map_resource(dev, phys_addr, size, dir, attrs); debug_dma_map_resource(dev, phys_addr, size, dir, addr); - return addr; } @@ -369,7 +371,7 @@ static inline void dma_unmap_resource(struct device *dev, dma_addr_t addr, const struct dma_map_ops *ops = get_dma_ops(dev); BUG_ON(!valid_dma_direction(dir)); - if (ops && ops->unmap_resource) + if (!dma_is_direct(ops) && ops->unmap_resource) ops->unmap_resource(dev, addr, size, dir, attrs); debug_dma_unmap_resource(dev, addr, size, dir); } diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c index 355d16acee6d..25bd19974223 100644 --- a/kernel/dma/direct.c +++ b/kernel/dma/direct.c @@ -356,6 +356,20 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents, } EXPORT_SYMBOL(dma_direct_map_sg); +dma_addr_t dma_direct_map_resource(struct device *dev, phys_addr_t paddr, + size_t size, enum dma_data_direction dir, unsigned long attrs) +{ + dma_addr_t dma_addr = paddr; + + if (unlikely(!dma_direct_possible(dev, dma_addr, size))) { + report_addr(dev, dma_addr, size); + return DMA_MAPPING_ERROR; + } + + return dma_addr; +} +EXPORT_SYMBOL(dma_direct_map_resource); + /* * Because 32-bit DMA masks are so common we expect every architecture to be * able to satisfy them - either by not supporting more physical memory, or by -- 2.20.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] dma-mapping: don't BUG when calling dma_map_resource on RAM 2019-01-18 11:37 ` fix a layering violation in videobuf2 and improve dma_map_resource v2 Christoph Hellwig 2019-01-18 11:37 ` [PATCH 1/3] dma-mapping: remove the default map_resource implementation Christoph Hellwig @ 2019-01-18 11:37 ` Christoph Hellwig 2019-01-18 11:37 ` [PATCH 3/3] videobuf2: replace a layering violation with dma_map_resource Christoph Hellwig 2019-02-01 7:05 ` fix a layering violation in videobuf2 and improve dma_map_resource v2 Marek Szyprowski 3 siblings, 0 replies; 8+ messages in thread From: Christoph Hellwig @ 2019-01-18 11:37 UTC (permalink / raw) To: Pawel Osciak, Marek Szyprowski, Kyungmin Park, Niklas Söderlund Cc: Robin Murphy, Russell King, linux-kernel, iommu, Mauro Carvalho Chehab, linuxppc-dev, linux-arm-kernel, linux-media Use WARN_ON_ONCE to print a stack trace and return a proper error code instead. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Robin Murphy <robin.murphy@arm.com> --- include/linux/dma-mapping.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index 9842085e6774..b904d55247ab 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -353,7 +353,8 @@ static inline dma_addr_t dma_map_resource(struct device *dev, BUG_ON(!valid_dma_direction(dir)); /* Don't allow RAM to be mapped */ - BUG_ON(pfn_valid(PHYS_PFN(phys_addr))); + if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr)))) + return DMA_MAPPING_ERROR; if (dma_is_direct(ops)) addr = dma_direct_map_resource(dev, phys_addr, size, dir, attrs); -- 2.20.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] videobuf2: replace a layering violation with dma_map_resource 2019-01-18 11:37 ` fix a layering violation in videobuf2 and improve dma_map_resource v2 Christoph Hellwig 2019-01-18 11:37 ` [PATCH 1/3] dma-mapping: remove the default map_resource implementation Christoph Hellwig 2019-01-18 11:37 ` [PATCH 2/3] dma-mapping: don't BUG when calling dma_map_resource on RAM Christoph Hellwig @ 2019-01-18 11:37 ` Christoph Hellwig 2019-02-01 7:05 ` fix a layering violation in videobuf2 and improve dma_map_resource v2 Marek Szyprowski 3 siblings, 0 replies; 8+ messages in thread From: Christoph Hellwig @ 2019-01-18 11:37 UTC (permalink / raw) To: Pawel Osciak, Marek Szyprowski, Kyungmin Park, Niklas Söderlund Cc: Russell King, linux-kernel, iommu, Mauro Carvalho Chehab, Mauro Carvalho Chehab, linuxppc-dev, linux-arm-kernel, linux-media vb2_dc_get_userptr pokes into arm direct mapping details to get the resemblance of a dma address for a a physical address that does is not backed by a page struct. Not only is this not portable to other architectures with dma direct mapping offsets, but also not to uses of IOMMUs of any kind. Switch to the proper dma_map_resource / dma_unmap_resource interface instead. Signed-off-by: Christoph Hellwig <hch@lst.de> Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> --- .../common/videobuf2/videobuf2-dma-contig.c | 41 ++++--------------- 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c index aff0ab7bf83d..82389aead6ed 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c @@ -439,42 +439,14 @@ static void vb2_dc_put_userptr(void *buf_priv) set_page_dirty_lock(pages[i]); sg_free_table(sgt); kfree(sgt); + } else { + dma_unmap_resource(buf->dev, buf->dma_addr, buf->size, + buf->dma_dir, 0); } vb2_destroy_framevec(buf->vec); kfree(buf); } -/* - * For some kind of reserved memory there might be no struct page available, - * so all that can be done to support such 'pages' is to try to convert - * pfn to dma address or at the last resort just assume that - * dma address == physical address (like it has been assumed in earlier version - * of videobuf2-dma-contig - */ - -#ifdef __arch_pfn_to_dma -static inline dma_addr_t vb2_dc_pfn_to_dma(struct device *dev, unsigned long pfn) -{ - return (dma_addr_t)__arch_pfn_to_dma(dev, pfn); -} -#elif defined(__pfn_to_bus) -static inline dma_addr_t vb2_dc_pfn_to_dma(struct device *dev, unsigned long pfn) -{ - return (dma_addr_t)__pfn_to_bus(pfn); -} -#elif defined(__pfn_to_phys) -static inline dma_addr_t vb2_dc_pfn_to_dma(struct device *dev, unsigned long pfn) -{ - return (dma_addr_t)__pfn_to_phys(pfn); -} -#else -static inline dma_addr_t vb2_dc_pfn_to_dma(struct device *dev, unsigned long pfn) -{ - /* really, we cannot do anything better at this point */ - return (dma_addr_t)(pfn) << PAGE_SHIFT; -} -#endif - static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr, unsigned long size, enum dma_data_direction dma_dir) { @@ -528,7 +500,12 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr, for (i = 1; i < n_pages; i++) if (nums[i-1] + 1 != nums[i]) goto fail_pfnvec; - buf->dma_addr = vb2_dc_pfn_to_dma(buf->dev, nums[0]); + buf->dma_addr = dma_map_resource(buf->dev, + __pfn_to_phys(nums[0]), size, buf->dma_dir, 0); + if (dma_mapping_error(buf->dev, buf->dma_addr)) { + ret = -ENOMEM; + goto fail_pfnvec; + } goto out; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: fix a layering violation in videobuf2 and improve dma_map_resource v2 2019-01-18 11:37 ` fix a layering violation in videobuf2 and improve dma_map_resource v2 Christoph Hellwig ` (2 preceding siblings ...) 2019-01-18 11:37 ` [PATCH 3/3] videobuf2: replace a layering violation with dma_map_resource Christoph Hellwig @ 2019-02-01 7:05 ` Marek Szyprowski 2019-02-01 9:07 ` Christoph Hellwig 3 siblings, 1 reply; 8+ messages in thread From: Marek Szyprowski @ 2019-02-01 7:05 UTC (permalink / raw) To: Christoph Hellwig, Pawel Osciak, Kyungmin Park, Niklas Söderlund Cc: Russell King, linux-kernel, iommu, Mauro Carvalho Chehab, linuxppc-dev, linux-arm-kernel, linux-media Hi All, On 2019-01-18 12:37, Christoph Hellwig wrote: > Hi all, > > this series fixes a rather gross layering violation in videobuf2, which > pokes into arm DMA mapping internals to get a DMA address for memory that > does not have a page structure, and to do so fixes up the dma_map_resource > implementation to not provide a somewhat dangerous default and improve > the error handling. > > Changes since v1: > - don't apply bus offsets in dma_direct_map_resource Works fine on older Exynos based boards with IOMMU and CMA disabled. Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: fix a layering violation in videobuf2 and improve dma_map_resource v2 2019-02-01 7:05 ` fix a layering violation in videobuf2 and improve dma_map_resource v2 Marek Szyprowski @ 2019-02-01 9:07 ` Christoph Hellwig 0 siblings, 0 replies; 8+ messages in thread From: Christoph Hellwig @ 2019-02-01 9:07 UTC (permalink / raw) To: Marek Szyprowski Cc: Niklas Söderlund, Pawel Osciak, Russell King, iommu, linux-kernel, Kyungmin Park, Mauro Carvalho Chehab, linuxppc-dev, Christoph Hellwig, linux-arm-kernel, linux-media On Fri, Feb 01, 2019 at 08:05:21AM +0100, Marek Szyprowski wrote: > Works fine on older Exynos based boards with IOMMU and CMA disabled. > > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> Thanks. I've merged the ѕeries into the dma-mapping tree, and I've also made a stable branch available at: git://git.infradead.org/users/hch/dma-mapping.git videobuf-map-resource in case it needs to be pulled into the media tree. ^ permalink raw reply [flat|nested] 8+ messages in thread
* fix a layering violation in videobuf2 and improve dma_map_resource @ 2019-01-11 18:17 Christoph Hellwig 2019-01-11 18:17 ` [PATCH 2/3] dma-mapping: don't BUG when calling dma_map_resource on RAM Christoph Hellwig 0 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2019-01-11 18:17 UTC (permalink / raw) To: Pawel Osciak, Marek Szyprowski, Kyungmin Park, Niklas Söderlund Cc: Russell King, linux-kernel, iommu, Mauro Carvalho Chehab, linuxppc-dev, linux-arm-kernel, linux-media Hi all, this series fixes a rather gross layering violation in videobuf2, which pokes into arm DMA mapping internals to get a DMA address for memory that does not have a page structure, and to do so fixes up the dma_map_resource implementation to be practically useful. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] dma-mapping: don't BUG when calling dma_map_resource on RAM 2019-01-11 18:17 fix a layering violation in videobuf2 and improve dma_map_resource Christoph Hellwig @ 2019-01-11 18:17 ` Christoph Hellwig 2019-01-14 14:16 ` Robin Murphy 0 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2019-01-11 18:17 UTC (permalink / raw) To: Pawel Osciak, Marek Szyprowski, Kyungmin Park, Niklas Söderlund Cc: Russell King, linux-kernel, iommu, Mauro Carvalho Chehab, linuxppc-dev, linux-arm-kernel, linux-media Use WARN_ON_ONCE to print a stack trace and return a proper error code instead. Signed-off-by: Christoph Hellwig <hch@lst.de> --- include/linux/dma-mapping.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index d3087829a6df..91add0751aa5 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -353,7 +353,8 @@ static inline dma_addr_t dma_map_resource(struct device *dev, BUG_ON(!valid_dma_direction(dir)); /* Don't allow RAM to be mapped */ - BUG_ON(pfn_valid(PHYS_PFN(phys_addr))); + if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr)))) + return DMA_MAPPING_ERROR; if (dma_is_direct(ops)) addr = dma_direct_map_resource(dev, phys_addr, size, dir, attrs); -- 2.20.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] dma-mapping: don't BUG when calling dma_map_resource on RAM 2019-01-11 18:17 ` [PATCH 2/3] dma-mapping: don't BUG when calling dma_map_resource on RAM Christoph Hellwig @ 2019-01-14 14:16 ` Robin Murphy 0 siblings, 0 replies; 8+ messages in thread From: Robin Murphy @ 2019-01-14 14:16 UTC (permalink / raw) To: Christoph Hellwig, Pawel Osciak, Marek Szyprowski, Kyungmin Park, Niklas Söderlund Cc: Russell King, linux-kernel, iommu, Mauro Carvalho Chehab, linuxppc-dev, linux-arm-kernel, linux-media On 11/01/2019 18:17, Christoph Hellwig wrote: > Use WARN_ON_ONCE to print a stack trace and return a proper error > code instead. I was racking my brain to remember the reasoning behind BUG_ON() being the only viable way to prevent errors getting through unhandled, but of course that was before we had a standardised DMA_MAPPING_ERROR that would work across all implementations. > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > include/linux/dma-mapping.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h > index d3087829a6df..91add0751aa5 100644 > --- a/include/linux/dma-mapping.h > +++ b/include/linux/dma-mapping.h > @@ -353,7 +353,8 @@ static inline dma_addr_t dma_map_resource(struct device *dev, > BUG_ON(!valid_dma_direction(dir)); > > /* Don't allow RAM to be mapped */ Ugh, I'm pretty sure that that "pfn_valid means RAM" misunderstanding originally came from me - it might be nice to have a less-misleading comment here, but off-hand I can't think of a succinct way to say "only for 'DMA' access to MMIO registers/SRAMs/etc. and not for anything the kernel knows as actual system/device memory" to better explain the WARN... Either way, though, Reviewed-by: Robin Murphy <robin.murphy@arm.com> > - BUG_ON(pfn_valid(PHYS_PFN(phys_addr))); > + if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr)))) > + return DMA_MAPPING_ERROR; > > if (dma_is_direct(ops)) > addr = dma_direct_map_resource(dev, phys_addr, size, dir, attrs); > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-02-01 9:09 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20190118113751epcas2p2d7d678dcf247806a119316aabb4dde21@epcas2p2.samsung.com>
2019-01-18 11:37 ` fix a layering violation in videobuf2 and improve dma_map_resource v2 Christoph Hellwig
2019-01-18 11:37 ` [PATCH 1/3] dma-mapping: remove the default map_resource implementation Christoph Hellwig
2019-01-18 11:37 ` [PATCH 2/3] dma-mapping: don't BUG when calling dma_map_resource on RAM Christoph Hellwig
2019-01-18 11:37 ` [PATCH 3/3] videobuf2: replace a layering violation with dma_map_resource Christoph Hellwig
2019-02-01 7:05 ` fix a layering violation in videobuf2 and improve dma_map_resource v2 Marek Szyprowski
2019-02-01 9:07 ` Christoph Hellwig
2019-01-11 18:17 fix a layering violation in videobuf2 and improve dma_map_resource Christoph Hellwig
2019-01-11 18:17 ` [PATCH 2/3] dma-mapping: don't BUG when calling dma_map_resource on RAM Christoph Hellwig
2019-01-14 14:16 ` Robin Murphy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).