Thread (2 messages) 2 messages, 2 authors, 2020-07-01

Re: [PATCH v7 31/36] staging: tegra-vde: fix common struct sg_table related issues

From: Dmitry Osipenko <digetx@gmail.com>
Date: 2020-07-01 01:45:53
Also in: dri-devel, linux-iommu, linux-media, linux-tegra, lkml

30.06.2020 13:07, Marek Szyprowski пишет:
On 21.06.2020 06:00, Dmitry Osipenko wrote:
quoted
В Fri, 19 Jun 2020 12:36:31 +0200
Marek Szyprowski [off-list ref] пишет:
quoted
The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg()
function returns the number of the created entries in the DMA address
space. However the subsequent calls to the
dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with
the original number of the entries passed to the dma_map_sg().

struct sg_table is a common structure used for describing a
non-contiguous memory buffer, used commonly in the DRM and graphics
subsystems. It consists of a scatterlist with memory pages and DMA
addresses (sgl entry), as well as the number of scatterlist entries:
CPU pages (orig_nents entry) and DMA mapped pages (nents entry).

It turned out that it was a common mistake to misuse nents and
orig_nents entries, calling DMA-mapping functions with a wrong number
of entries or ignoring the number of mapped entries returned by the
dma_map_sg() function.

To avoid such issues, lets use a common dma-mapping wrappers operating
directly on the struct sg_table objects and use scatterlist page
iterators where possible. This, almost always, hides references to the
nents and orig_nents entries, making the code robust, easier to follow
and copy/paste safe.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
---
  drivers/staging/media/tegra-vde/iommu.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/tegra-vde/iommu.c
b/drivers/staging/media/tegra-vde/iommu.c index
6af863d92123..adf8dc7ee25c 100644 ---
a/drivers/staging/media/tegra-vde/iommu.c +++
b/drivers/staging/media/tegra-vde/iommu.c @@ -36,8 +36,8 @@ int
tegra_vde_iommu_map(struct tegra_vde *vde,
  	addr = iova_dma_addr(&vde->iova, iova);
  
-	size = iommu_map_sg(vde->domain, addr, sgt->sgl, sgt->nents,
-			    IOMMU_READ | IOMMU_WRITE);
+	size = iommu_map_sgtable(vde->domain, addr, sgt,
+				 IOMMU_READ | IOMMU_WRITE);
  	if (!size) {
  		__free_iova(&vde->iova, iova);
  		return -ENXIO;
Ahh, I saw the build failure report. You're changing the DMA API in
this series, while DMA API isn't used by this driver, it uses IOMMU
API. Hence there is no need to touch this code. Similar problem in the
host1x driver patch.
The issue is caused by the lack of iommu_map_sgtable() stub when no 
IOMMU support is configured. I've posted a patch for this:

https://lore.kernel.org/lkml/20200630081756.18526-1-m.szyprowski@samsung.com/ (local)

The patch for this driver is fine, we have to wait until the above fix 
gets merged and then it can be applied during the next release cycle.
Thank you for the clarification!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help