From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:40:24
Dear All,
During the Exynos DRM GEM rework and fixing the issues in the.
drm_prime_sg_to_page_addr_arrays() function [1] I've noticed that most
drivers in DRM framework incorrectly use nents and orig_nents entries of
the struct sg_table.
In case of the most DMA-mapping implementations exchanging those two
entries or using nents for all loops on the scatterlist is harmless,
because they both have the same value. There exists however a DMA-mapping
implementations, for which such incorrect usage breaks things. The nents
returned by dma_map_sg() might be lower than the nents passed as its
parameter and this is perfectly fine. DMA framework or IOMMU is allowed
to join consecutive chunks while mapping if such operation is supported
by the underlying HW (bus, bridge, IOMMU, etc). Example of the case
where dma_map_sg() might return 1 'DMA' chunk for the 4 'physical' pages
is described here [2]
The DMA-mapping framework documentation [3] states that dma_map_sg()
returns the numer of the created entries in the DMA address space.
However the subsequent calls to dma_sync_sg_for_{device,cpu} and
dma_unmap_sg must be called with the original number of entries passed to
dma_map_sg. The common pattern in DRM drivers were to assign the
dma_map_sg() return value to sg_table->nents and use that value for
the subsequent calls to dma_sync_sg_* or dma_unmap_sg functions. Also
the code iterated over nents times to access the pages stored in the
processed scatterlist, while it should use orig_nents as the numer of
the page entries.
I've tried to identify all such incorrect usage of sg_table->nents and
this is a result of my research. It looks that the incorrect pattern has
been copied over the many drivers mainly in the DRM subsystem. Too bad in
most cases it even worked correctly if the system used a simple, linear
DMA-mapping implementation, for which swapping nents and orig_nents
doesn't make any difference. To avoid similar issues in the future, I've
introduced a common wrappers for DMA-mapping calls, which operate directly
on the sg_table objects. I've also added wrappers for iterating over the
scatterlists stored in the sg_table objects and applied them where
possible. This, together with some common DRM prime helpers, allowed me
to almost get rid of all nents/orig_nents usage in the drivers. I hope
that such change makes the code robust, easier to follow and copy/paste
safe.
The biggest TODO is DRM/i915 driver and I don't feel brave enough to fix
it fully. The driver creatively uses sg_table->orig_nents to store the
size of the allocate scatterlist and ignores the number of the entries
returned by dma_map_sg function. In this patchset I only fixed the
sg_table objects exported by dmabuf related functions. I hope that I
didn't break anything there.
Patches are based on top of Linux next-20200618. The required changes to
DMA-mapping framework has been already merged to v5.8-rc1.
If possible I would like ask for merging most of the patches via DRM
tree.
Best regards,
Marek Szyprowski
References:
[1] https://lkml.org/lkml/2020/3/27/555
[2] https://lkml.org/lkml/2020/3/29/65
[3] Documentation/DMA-API-HOWTO.txt
[4] https://lore.kernel.org/linux-iommu/20200512121931.GD20393@lst.de/T/#ma18c958a48c3b241d5409517fa7d192eef87459b
Changelog:
v6:
- rebased onto Linux next-20200618, which is based on v5.8-rc1; fixed conflicts
v5: https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsung.com/T/
- fixed some minor style issues and typos
- fixed lack of the attrs argument in ion, dmabuf, rapidio, fastrpc and
vfio patches
v4: https://lore.kernel.org/linux-iommu/20200512121931.GD20393@lst.de/T/
- added for_each_sgtable_* wrappers and applied where possible
- added drm_prime_get_contiguous_size() and applied where possible
- applied drm_prime_sg_to_page_addr_arrays() where possible to remove page
extraction from sg_table objects
- added documentation for the introduced wrappers
- improved patches description a bit
v3: https://lore.kernel.org/dri-devel/20200505083926.28503-1-m.szyprowski@samsung.com/
- introduce dma_*_sgtable_* wrappers and use them in all patches
v2: https://lore.kernel.org/linux-iommu/c01c9766-9778-fd1f-f36e-2dc7bd376ba4@arm.com/T/
- dropped most of the changes to drm/i915
- added fixes for rcar-du, xen, media and ion
- fixed a few issues pointed by kbuild test robot
- added wide cc: list for each patch
v1: https://lore.kernel.org/linux-iommu/c01c9766-9778-fd1f-f36e-2dc7bd376ba4@arm.com/T/
- initial version
Patch summary:
Marek Szyprowski (36):
drm: prime: add common helper to check scatterlist contiguity
drm: prime: use sgtable iterators in
drm_prime_sg_to_page_addr_arrays()
drm: core: fix common struct sg_table related issues
drm: amdgpu: fix common struct sg_table related issues
drm: armada: fix common struct sg_table related issues
drm: etnaviv: fix common struct sg_table related issues
drm: exynos: use common helper for a scatterlist contiguity check
drm: exynos: fix common struct sg_table related issues
drm: i915: fix common struct sg_table related issues
drm: lima: fix common struct sg_table related issues
drm: mediatek: use common helper for a scatterlist contiguity check
drm: mediatek: use common helper for extracting pages array
drm: msm: fix common struct sg_table related issues
drm: omapdrm: use common helper for extracting pages array
drm: omapdrm: fix common struct sg_table related issues
drm: panfrost: fix common struct sg_table related issues
drm: radeon: fix common struct sg_table related issues
drm: rockchip: use common helper for a scatterlist contiguity check
drm: rockchip: fix common struct sg_table related issues
drm: tegra: fix common struct sg_table related issues
drm: v3d: fix common struct sg_table related issues
drm: virtio: fix common struct sg_table related issues
drm: vmwgfx: fix common struct sg_table related issues
xen: gntdev: fix common struct sg_table related issues
drm: host1x: fix common struct sg_table related issues
drm: rcar-du: fix common struct sg_table related issues
dmabuf: fix common struct sg_table related issues
staging: ion: remove dead code
staging: ion: fix common struct sg_table related issues
staging: tegra-vde: fix common struct sg_table related issues
misc: fastrpc: fix common struct sg_table related issues
rapidio: fix common struct sg_table related issues
samples: vfio-mdev/mbochs: fix common struct sg_table related issues
media: pci: fix common ALSA DMA-mapping related codes
videobuf2: use sgtable-based scatterlist wrappers
drm: xen: fix common struct sg_table related issues
drivers/dma-buf/heaps/heap-helpers.c | 13 ++-
drivers/dma-buf/udmabuf.c | 7 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 9 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 8 +-
drivers/gpu/drm/armada/armada_gem.c | 12 +--
drivers/gpu/drm/drm_cache.c | 2 +-
drivers/gpu/drm/drm_gem_cma_helper.c | 23 +----
drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +--
drivers/gpu/drm/drm_prime.c | 86 ++++++++++---------
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 12 ++-
drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 13 +--
drivers/gpu/drm/exynos/exynos_drm_g2d.c | 10 +--
drivers/gpu/drm/exynos/exynos_drm_gem.c | 23 +----
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 11 +--
.../gpu/drm/i915/gem/selftests/mock_dmabuf.c | 7 +-
drivers/gpu/drm/lima/lima_gem.c | 11 ++-
drivers/gpu/drm/lima/lima_vm.c | 5 +-
drivers/gpu/drm/mediatek/mtk_drm_gem.c | 37 ++------
drivers/gpu/drm/msm/msm_gem.c | 13 ++-
drivers/gpu/drm/msm/msm_gpummu.c | 14 ++-
drivers/gpu/drm/msm/msm_iommu.c | 2 +-
drivers/gpu/drm/omapdrm/omap_gem.c | 20 ++---
drivers/gpu/drm/panfrost/panfrost_gem.c | 4 +-
drivers/gpu/drm/panfrost/panfrost_mmu.c | 7 +-
drivers/gpu/drm/radeon/radeon_ttm.c | 11 ++-
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 +-
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 42 +++------
drivers/gpu/drm/tegra/gem.c | 27 +++---
drivers/gpu/drm/tegra/plane.c | 15 ++--
drivers/gpu/drm/v3d/v3d_mmu.c | 17 ++--
drivers/gpu/drm/virtio/virtgpu_object.c | 36 ++++----
drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 17 +---
drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +-
drivers/gpu/host1x/job.c | 22 ++---
.../common/videobuf2/videobuf2-dma-contig.c | 41 ++++-----
.../media/common/videobuf2/videobuf2-dma-sg.c | 32 +++----
.../common/videobuf2/videobuf2-vmalloc.c | 12 +--
drivers/media/pci/cx23885/cx23885-alsa.c | 2 +-
drivers/media/pci/cx25821/cx25821-alsa.c | 2 +-
drivers/media/pci/cx88/cx88-alsa.c | 2 +-
drivers/media/pci/saa7134/saa7134-alsa.c | 2 +-
drivers/media/platform/vsp1/vsp1_drm.c | 8 +-
drivers/misc/fastrpc.c | 4 +-
drivers/rapidio/devices/rio_mport_cdev.c | 8 +-
drivers/staging/android/ion/ion.c | 25 +++---
drivers/staging/android/ion/ion.h | 1 -
drivers/staging/android/ion/ion_heap.c | 53 +++---------
drivers/staging/android/ion/ion_system_heap.c | 2 +-
drivers/staging/media/tegra-vde/iommu.c | 4 +-
drivers/xen/gntdev-dmabuf.c | 13 ++-
include/drm/drm_prime.h | 2 +
samples/vfio-mdev/mbochs.c | 3 +-
54 files changed, 311 insertions(+), 478 deletions(-)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:40:21
It is a common operation done by DRM drivers to check the contiguity
of the DMA-mapped buffer described by a scatterlist in the
sg_table object. Let's add a common helper for this operation.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/drm_gem_cma_helper.c | 23 +++--------------------
drivers/gpu/drm/drm_prime.c | 26 ++++++++++++++++++++++++++
include/drm/drm_prime.h | 2 ++
3 files changed, 31 insertions(+), 20 deletions(-)
@@ -471,26 +471,9 @@ drm_gem_cma_prime_import_sg_table(struct drm_device *dev,{structdrm_gem_cma_object*cma_obj;-if(sgt->nents!=1){-/* check if the entries in the sg_table are contiguous */-dma_addr_tnext_addr=sg_dma_address(sgt->sgl);-structscatterlist*s;-unsignedinti;--for_each_sg(sgt->sgl,s,sgt->nents,i){-/*-*sg_dma_address(s)isonlyvalidforentries-*thathavesg_dma_len(s)!=0-*/-if(!sg_dma_len(s))-continue;--if(sg_dma_address(s)!=next_addr)-returnERR_PTR(-EINVAL);--next_addr=sg_dma_address(s)+sg_dma_len(s);-}-}+/* check if the entries in the sg_table are contiguous */+if(drm_prime_get_contiguous_size(sgt)<attach->dmabuf->size)+returnERR_PTR(-EINVAL);/* Create a CMA GEM buffer. */cma_obj=__drm_gem_cma_create(dev,attach->dmabuf->size);
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:40:29
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: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 6 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 9 +++------
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 8 ++++----
3 files changed, 10 insertions(+), 13 deletions(-)
@@ -1025,7 +1025,6 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm){structamdgpu_device*adev=amdgpu_ttm_adev(ttm->bdev);structamdgpu_ttm_tt*gtt=(void*)ttm;-unsignednents;intr;intwrite=!(gtt->userflags&AMDGPU_GEM_USERPTR_READONLY);
@@ -1040,9 +1039,8 @@ static int amdgpu_ttm_tt_pin_userptr(struct ttm_tt *ttm)gotorelease_sg;/* Map SG to device */-r=-ENOMEM;-nents=dma_map_sg(adev->dev,ttm->sg->sgl,ttm->sg->nents,direction);-if(nents==0)+r=dma_map_sgtable(adev->dev,ttm->sg,direction,0);+if(r)gotorelease_sg;/* convert SG to linear array of pages and dma addresses */
@@ -1073,8 +1071,7 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_tt *ttm)return;/* unmap the pages mapped to the device */-dma_unmap_sg(adev->dev,ttm->sg->sgl,ttm->sg->nents,direction);-+dma_unmap_sgtable(adev->dev,ttm->sg,direction,0);sg_free_table(ttm->sg);#if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:40:32
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>
---
drivers/gpu/drm/armada/armada_gem.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:40:41
Use common helper for checking the contiguity of the imported dma-buf and
do this check before allocating resources, so the error path is simpler.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/mediatek/mtk_drm_gem.c | 28 ++++++--------------------
1 file changed, 6 insertions(+), 22 deletions(-)
@@ -212,37 +212,21 @@ struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device *dev,structdma_buf_attachment*attach,structsg_table*sg){structmtk_drm_gem_obj*mtk_gem;-intret;-structscatterlist*s;-unsignedinti;-dma_addr_texpected;-mtk_gem=mtk_drm_gem_init(dev,attach->dmabuf->size);+/* check if the entries in the sg_table are contiguous */+if(drm_prime_get_contiguous_size(sg)<attach->dmabuf->size){+DRM_ERROR("sg_table is not contiguous");+returnERR_PTR(-EINVAL);+}+mtk_gem=mtk_drm_gem_init(dev,attach->dmabuf->size);if(IS_ERR(mtk_gem))returnERR_CAST(mtk_gem);-expected=sg_dma_address(sg->sgl);-for_each_sg(sg->sgl,s,sg->nents,i){-if(!sg_dma_len(s))-break;--if(sg_dma_address(s)!=expected){-DRM_ERROR("sg_table is not contiguous");-ret=-EINVAL;-gotoerr_gem_free;-}-expected=sg_dma_address(s)+sg_dma_len(s);-}-mtk_gem->dma_addr=sg_dma_address(sg->sgl);mtk_gem->sg=sg;return&mtk_gem->base;--err_gem_free:-kfree(mtk_gem);-returnERR_PTR(ret);}void*mtk_drm_gem_prime_vmap(structdrm_gem_object*obj)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:40:44
Use common helper for converting a sg_table object into struct
page pointer array.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/omapdrm/omap_gem.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:40:46
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: Steven Price <steven.price@arm.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
drivers/gpu/drm/panfrost/panfrost_gem.c | 4 ++--
drivers/gpu/drm/panfrost/panfrost_mmu.c | 7 +++----
2 files changed, 5 insertions(+), 6 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:40:51
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>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_object.c | 36 ++++++++++++++-----------
drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++++-----
2 files changed, 26 insertions(+), 22 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:40:57
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>
Acked-by: Juergen Gross <jgross@suse.com>
---
drivers/xen/gntdev-dmabuf.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
@@ -625,7 +624,7 @@ dmabuf_imp_to_refs(struct gntdev_dmabuf_priv *priv, struct device *dev,/* Now convert sgt to array of pages and check for page validity. */i=0;-for_each_sg_page(sgt->sgl,&sg_iter,sgt->nents,0){+for_each_sgtable_page(sgt,&sg_iter,0){structpage*page=sg_page_iter_page(&sg_iter);/**Checkifpageisvalid:thiscanhappenifwearegiven
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:41:00
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>
Acked-by: Roland Scheidegger <redacted>
---
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:41:03
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: Eric Anholt <redacted>
---
drivers/gpu/drm/v3d/v3d_mmu.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:41:13
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>
---
drivers/gpu/drm/tegra/gem.c | 27 ++++++++++-----------------
drivers/gpu/drm/tegra/plane.c | 15 +++++----------
2 files changed, 15 insertions(+), 27 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:41:18
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.
Fix the code to refer to proper nents or orig_nents entries. This driver
checks for a buffer contiguity in DMA address space, so it should test
sg_table->nents entry.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/omapdrm/omap_gem.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
@@ -1279,7 +1279,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,unionomap_gem_sizegsize;/* Without a DMM only physically contiguous buffers can be supported. */-if(sgt->orig_nents!=1&&!priv->has_dmm)+if(sgt->nents!=1&&!priv->has_dmm)returnERR_PTR(-EINVAL);gsize.bytes=PAGE_ALIGN(size);
@@ -1293,7 +1293,7 @@ struct drm_gem_object *omap_gem_new_dmabuf(struct drm_device *dev, size_t size,omap_obj->sgt=sgt;-if(sgt->orig_nents==1){+if(sgt->nents==1){omap_obj->dma_addr=sg_dma_address(sgt->sgl);}else{/* Create pages list from sgt */
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:41:24
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>
---
drivers/gpu/drm/msm/msm_gem.c | 13 +++++--------
drivers/gpu/drm/msm/msm_gpummu.c | 14 ++++++--------
drivers/gpu/drm/msm/msm_iommu.c | 2 +-
3 files changed, 12 insertions(+), 17 deletions(-)
@@ -30,21 +30,19 @@ static int msm_gpummu_map(struct msm_mmu *mmu, uint64_t iova,{structmsm_gpummu*gpummu=to_msm_gpummu(mmu);unsignedidx=(iova-GPUMMU_VA_START)/GPUMMU_PAGE_SIZE;-structscatterlist*sg;+structsg_dma_page_iterdma_iter;unsignedprot_bits=0;-unsignedi,j;if(prot&IOMMU_WRITE)prot_bits|=1;if(prot&IOMMU_READ)prot_bits|=2;-for_each_sg(sgt->sgl,sg,sgt->nents,i){-dma_addr_taddr=sg->dma_address;-for(j=0;j<sg->length/GPUMMU_PAGE_SIZE;j++,idx++){-gpummu->table[idx]=addr|prot_bits;-addr+=GPUMMU_PAGE_SIZE;-}+for_each_sgtable_dma_page(sgt,&dma_iter,0){+dma_addr_taddr=sg_page_iter_dma_address(&dma_iter);++BUILD_BUG_ON(GPUMMU_PAGE_SIZE!=PAGE_SIZE);+gpummu->table[idx++]=addr|prot_bits;}/* we can improve by deferring flush for multiple map() */
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:41:29
Use recently introduced common wrappers operating directly on the struct
sg_table objects and scatterlist page iterators to make the code a bit
more compact, robust, easier to follow and copy/paste safe.
No functional change, because the code already properly did all the
scaterlist related calls.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
.../common/videobuf2/videobuf2-dma-contig.c | 41 ++++++++-----------
.../media/common/videobuf2/videobuf2-dma-sg.c | 32 ++++++---------
.../common/videobuf2/videobuf2-vmalloc.c | 12 ++----
3 files changed, 34 insertions(+), 51 deletions(-)
@@ -436,15 +431,12 @@ static struct sg_table *vb2_dma_sg_dmabuf_ops_map(/* release any previous cache */if(attach->dma_dir!=DMA_NONE){-dma_unmap_sg(db_attach->dev,sgt->sgl,sgt->orig_nents,-attach->dma_dir);+dma_unmap_sgtable(db_attach->dev,sgt,attach->dma_dir);attach->dma_dir=DMA_NONE;}/* mapping to the client with new direction */-sgt->nents=dma_map_sg(db_attach->dev,sgt->sgl,sgt->orig_nents,-dma_dir);-if(!sgt->nents){+if(dma_map_sgtable(db_attach->dev,sgt,dma_dir,0)){pr_err("failed to map scatterlist\n");mutex_unlock(lock);returnERR_PTR(-EIO);
@@ -285,15 +284,12 @@ static struct sg_table *vb2_vmalloc_dmabuf_ops_map(/* release any previous cache */if(attach->dma_dir!=DMA_NONE){-dma_unmap_sg(db_attach->dev,sgt->sgl,sgt->orig_nents,-attach->dma_dir);+dma_unmap_sgtable(db_attach->dev,sgt,attach->dma_dir,0);attach->dma_dir=DMA_NONE;}/* mapping to the client with new direction */-sgt->nents=dma_map_sg(db_attach->dev,sgt->sgl,sgt->orig_nents,-dma_dir);-if(!sgt->nents){+if(dma_map_sgtable(db_attach->dev,sgt,dma_dir,0)){pr_err("failed to map scatterlist\n");mutex_unlock(lock);returnERR_PTR(-EIO);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:41:41
The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the
numer of the created entries in the DMA address space. However the
subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be
called with the original number of entries passed to dma_map_sg. The
sg_table->nents in turn holds the result of the dma_map_sg call as stated
in include/linux/scatterlist.h. Adapt the code to obey those rules.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/media/pci/cx23885/cx23885-alsa.c | 2 +-
drivers/media/pci/cx25821/cx25821-alsa.c | 2 +-
drivers/media/pci/cx88/cx88-alsa.c | 2 +-
drivers/media/pci/saa7134/saa7134-alsa.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:41:45
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.
While touching this code, also add missing call to dma_unmap_sgtable.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
samples/vfio-mdev/mbochs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:41:51
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>
---
drivers/rapidio/devices/rio_mport_cdev.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:42:25
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>
---
drivers/staging/android/ion/ion.c | 25 +++++------
drivers/staging/android/ion/ion_heap.c | 44 ++++++-------------
drivers/staging/android/ion/ion_system_heap.c | 2 +-
3 files changed, 25 insertions(+), 46 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:42:27
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>
---
drivers/misc/fastrpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:42:31
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(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:42:33
ion_heap_pages_zero() function is not used at all, so remove it to
simplify the ion_heap_sglist_zero() function later.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/staging/android/ion/ion.h | 1 -
drivers/staging/android/ion/ion_heap.c | 9 ---------
2 files changed, 10 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:42:46
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>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
drivers/dma-buf/heaps/heap-helpers.c | 13 ++++++-------
drivers/dma-buf/udmabuf.c | 7 +++----
2 files changed, 9 insertions(+), 11 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:42:50
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.
dma_map_sgtable() function returns zero or an error code, so adjust the
return value check for the vsp1_du_map_sg() function.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 +--
drivers/media/platform/vsp1/vsp1_drm.c | 8 ++++----
2 files changed, 5 insertions(+), 6 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:42:53
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>
---
drivers/gpu/host1x/job.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:42:55
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: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/radeon/radeon_ttm.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:43:14
Use common helper for converting a sg_table object into struct
page pointer array.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/mediatek/mtk_drm_gem.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:43:16
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>
---
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 23 +++++++++------------
1 file changed, 10 insertions(+), 13 deletions(-)
@@ -476,15 +475,13 @@ rockchip_gem_dma_map_sg(struct drm_device *drm,structsg_table*sg,structrockchip_gem_object*rk_obj){-intcount=dma_map_sg(drm->dev,sg->sgl,sg->nents,-DMA_BIDIRECTIONAL);-if(!count)-return-EINVAL;+interr=dma_map_sgtable(drm->dev,sg,DMA_BIDIRECTIONAL,0);+if(err)+returnerr;if(drm_prime_get_contiguous_size(sg)<attach->dmabuf->size){DRM_ERROR("failed to map sg_table to contiguous linear address.\n");-dma_unmap_sg(drm->dev,sg->sgl,sg->nents,-DMA_BIDIRECTIONAL);+dma_unmap_sgtable(drm->dev,sg,DMA_BIDIRECTIONAL,0);return-EINVAL;}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:43:18
Use common helper for checking the contiguity of the imported dma-buf.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
@@ -498,7 +481,7 @@ rockchip_gem_dma_map_sg(struct drm_device *drm,if(!count)return-EINVAL;-if(rockchip_sg_get_contiguous_size(sg,count)<attach->dmabuf->size){+if(drm_prime_get_contiguous_size(sg)<attach->dmabuf->size){DRM_ERROR("failed to map sg_table to contiguous linear address.\n");dma_unmap_sg(drm->dev,sg->sgl,sg->nents,DMA_BIDIRECTIONAL);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:43:23
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: Qiang Yu <yuq825@gmail.com>
---
drivers/gpu/drm/lima/lima_gem.c | 11 ++++++++---
drivers/gpu/drm/lima/lima_vm.c | 5 ++---
2 files changed, 10 insertions(+), 6 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:43:30
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.
This driver creatively uses sg_table->orig_nents to store the size of the
allocated scatterlist and ignores the number of the entries returned by
dma_map_sg function. The sg_table->orig_nents is (mis)used to properly
free the (over)allocated scatterlist.
This patch only introduces the common DMA-mapping wrappers operating
directly on the struct sg_table objects to the dmabuf related functions,
so the other drivers, which might share buffers with i915 could rely on
the properly set nents and orig_nents values.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 11 +++--------
drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c | 7 +++----
2 files changed, 6 insertions(+), 12 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:43:34
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>
---
drivers/gpu/drm/exynos/exynos_drm_g2d.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:43:35
Use common helper for checking the contiguity of the imported dma-buf.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_gem.c | 23 +++--------------------
1 file changed, 3 insertions(+), 20 deletions(-)
@@ -431,27 +431,10 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,{structexynos_drm_gem*exynos_gem;-if(sgt->nents<1)+/* check if the entries in the sg_table are contiguous */+if(drm_prime_get_contiguous_size(sgt)<attach->dmabuf->size){+DRM_ERROR("buffer chunks must be mapped contiguously");returnERR_PTR(-EINVAL);--/*-*Checkiftheprovidedbufferhasbeenmappedascontiguous-*intoDMAaddressspace.-*/-if(sgt->nents>1){-dma_addr_tnext_addr=sg_dma_address(sgt->sgl);-structscatterlist*s;-unsignedinti;--for_each_sg(sgt->sgl,s,sgt->nents,i){-if(!sg_dma_len(s))-break;-if(sg_dma_address(s)!=next_addr){-DRM_ERROR("buffer chunks must be mapped contiguously");-returnERR_PTR(-EINVAL);-}-next_addr=sg_dma_address(s)+sg_dma_len(s);-}}exynos_gem=exynos_drm_gem_init(dev,attach->dmabuf->size);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:43:41
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>
---
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 12 +++++-------
drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 13 +++----------
2 files changed, 8 insertions(+), 17 deletions(-)
@@ -51,7 +51,7 @@ static void etnaviv_gem_scatterlist_unmap(struct etnaviv_gem_object *etnaviv_obj*discardthosewrites.*/if(etnaviv_obj->flags&ETNA_BO_CACHE_MASK)-dma_unmap_sg(dev->dev,sgt->sgl,sgt->nents,DMA_BIDIRECTIONAL);+dma_unmap_sgtable(dev->dev,sgt,DMA_BIDIRECTIONAL,0);}/* called with etnaviv_obj->lock held */
@@ -404,9 +404,8 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,}if(etnaviv_obj->flags&ETNA_BO_CACHED){-dma_sync_sg_for_cpu(dev->dev,etnaviv_obj->sgt->sgl,-etnaviv_obj->sgt->nents,-etnaviv_op_to_dma_dir(op));+dma_sync_sgtable_for_cpu(dev->dev,etnaviv_obj->sgt,+etnaviv_op_to_dma_dir(op));etnaviv_obj->last_cpu_prep_op=op;}
@@ -421,8 +420,7 @@ int etnaviv_gem_cpu_fini(struct drm_gem_object *obj)if(etnaviv_obj->flags&ETNA_BO_CACHED){/* fini without a prep is almost certainly a userspace error */WARN_ON(etnaviv_obj->last_cpu_prep_op==0);-dma_sync_sg_for_device(dev->dev,etnaviv_obj->sgt->sgl,-etnaviv_obj->sgt->nents,+dma_sync_sgtable_for_device(dev->dev,etnaviv_obj->sgt,etnaviv_op_to_dma_dir(etnaviv_obj->last_cpu_prep_op));etnaviv_obj->last_cpu_prep_op=0;}
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:43:48
Replace the current hand-crafted code for extracting pages and DMA
addresses from the given scatterlist by the much more robust
code based on the generic scatterlist iterators and recently
introduced sg_table-based wrappers. The resulting code is simple and
easy to understand, so the comment describing the old code is no
longer needed.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/drm_prime.c | 47 +++++++++++--------------------------
1 file changed, 14 insertions(+), 33 deletions(-)
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 15:43:53
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>
---
drivers/gpu/drm/drm_cache.c | 2 +-
drivers/gpu/drm/drm_gem_shmem_helper.c | 14 +++++++++-----
drivers/gpu/drm/drm_prime.c | 11 ++++++-----
3 files changed, 16 insertions(+), 11 deletions(-)
@@ -127,7 +127,7 @@ drm_clflush_sg(struct sg_table *st)structsg_page_itersg_iter;mb();/*CLFLUSH is ordered only by using memory barriers*/-for_each_sg_page(st->sgl,&sg_iter,st->nents,0)+for_each_sgtable_page(st,&sg_iter,0)drm_clflush_page(sg_page_iter_page(&sg_iter));mb();/*Make sure that all cache line entry is flushed*/
@@ -697,12 +696,17 @@ struct sg_table *drm_gem_shmem_get_pages_sgt(struct drm_gem_object *obj)gotoerr_put_pages;}/* Map the pages for use by the h/w. */-dma_map_sg(obj->dev->dev,sgt->sgl,sgt->nents,DMA_BIDIRECTIONAL);+ret=dma_map_sgtable(obj->dev->dev,sgt,DMA_BIDIRECTIONAL,0);+if(ret)+gotoerr_free_sgt;shmem->sgt=sgt;returnsgt;+err_free_sgt:+sg_free_table(sgt);+kfree(sgt);err_put_pages:drm_gem_shmem_put_pages(shmem);returnERR_PTR(ret);
From: Marek Szyprowski <m.szyprowski@samsung.com> Date: 2020-06-18 16:01:25
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.
Fix the code to refer to proper nents or orig_nents entries. This driver
reports the number of the pages in the imported scatterlist, so it should
refer to sg_table->orig_nents entry.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2020-06-18 16:08:32
On 2020-06-18 16:39, Marek Szyprowski wrote:
quoted hunk
It is a common operation done by DRM drivers to check the contiguity
of the DMA-mapped buffer described by a scatterlist in the
sg_table object. Let's add a common helper for this operation.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/drm_gem_cma_helper.c | 23 +++--------------------
drivers/gpu/drm/drm_prime.c | 26 ++++++++++++++++++++++++++
include/drm/drm_prime.h | 2 ++
3 files changed, 31 insertions(+), 20 deletions(-)
@@ -471,26 +471,9 @@ drm_gem_cma_prime_import_sg_table(struct drm_device *dev,{structdrm_gem_cma_object*cma_obj;-if(sgt->nents!=1){-/* check if the entries in the sg_table are contiguous */-dma_addr_tnext_addr=sg_dma_address(sgt->sgl);-structscatterlist*s;-unsignedinti;--for_each_sg(sgt->sgl,s,sgt->nents,i){-/*-*sg_dma_address(s)isonlyvalidforentries-*thathavesg_dma_len(s)!=0-*/-if(!sg_dma_len(s))-continue;--if(sg_dma_address(s)!=next_addr)-returnERR_PTR(-EINVAL);--next_addr=sg_dma_address(s)+sg_dma_len(s);-}-}+/* check if the entries in the sg_table are contiguous */+if(drm_prime_get_contiguous_size(sgt)<attach->dmabuf->size)+returnERR_PTR(-EINVAL);/* Create a CMA GEM buffer. */cma_obj=__drm_gem_cma_create(dev,attach->dmabuf->size);
Hmm, in many cases this is likely to be far less efficient than simply
using for_each_sgtable_dma() and sg_dma_len() equivalent to the original
implementation, and there doesn't seem to be any good reason for that.
Plus AFAICS it could potentially let false-positives through if someone
were to pass in a table with non-page-aligned lengths (I assume that's
expected never to happen, but still...)
Robin.
quoted hunk
+ }
+ return size;
+}
+EXPORT_SYMBOL(drm_prime_get_contiguous_size);
+
/**
* drm_gem_prime_export - helper library implementation of the export callback
* @obj: GEM object to export
From: kernel test robot <hidden> Date: 2020-06-18 22:28:26
Hi Marek,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20200618]
[also build test WARNING on v5.8-rc1]
[cannot apply to linuxtv-media/master staging/staging-testing drm-exynos/exynos-drm-next drm-intel/for-linux-next linus/master v5.8-rc1 v5.7 v5.7-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Marek-Szyprowski/DRM-fix-struct-sg_table-nents-vs-orig_nents-misuse/20200619-000417
base: ce2cc8efd7a40cbd17841add878cb691d0ce0bba
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
In file included from include/linux/kernel.h:11,
from include/linux/delay.h:22,
from drivers/gpu/drm/v3d/v3d_drv.h:4,
from drivers/gpu/drm/v3d/v3d_mmu.c:21:
drivers/gpu/drm/v3d/v3d_mmu.c: In function 'v3d_mmu_insert_ptes':
include/linux/compiler.h:339:38: error: call to '__compiletime_assert_254' declared with attribute error: BUILD_BUG_ON failed: V3D_MMU_PAGE_SHIFT != PAGE_SIZE
339 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
include/linux/compiler.h:320:4: note: in definition of macro '__compiletime_assert'
320 | prefix ## suffix(); | ^~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
339 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:50:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
| ^~~~~~~~~~~~~~~~
quoted
drivers/gpu/drm/v3d/v3d_mmu.c:100:3: note: in expansion of macro 'BUILD_BUG_ON'
From: kernel test robot <hidden> Date: 2020-06-18 22:29:48
Hi Marek,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20200618]
[also build test WARNING on v5.8-rc1]
[cannot apply to linuxtv-media/master staging/staging-testing drm-exynos/exynos-drm-next drm-intel/for-linux-next linus/master v5.8-rc1 v5.7 v5.7-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Marek-Szyprowski/DRM-fix-struct-sg_table-nents-vs-orig_nents-misuse/20200619-000417
base: ce2cc8efd7a40cbd17841add878cb691d0ce0bba
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
In file included from include/linux/dma-mapping.h:11,
from drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:25:
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c: In function 'amdgpu_vram_mgr_alloc_sgt':
include/linux/scatterlist.h:158:17: error: '*sgt' is a pointer; did you mean to use '->'?
158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i)
| ^~
include/linux/scatterlist.h:152:22: note: in definition of macro 'for_each_sg'
152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
| ^~~~~~
quoted
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:480:2: note: in expansion of macro 'for_each_sgtable_sg'
480 | for_each_sgtable_sg(*sgt, sg, i)
| ^~~~~~~~~~~~~~~~~~~
include/linux/scatterlist.h:158:31: error: '*sgt' is a pointer; did you mean to use '->'?
158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i)
| ^~
include/linux/scatterlist.h:152:38: note: in definition of macro 'for_each_sg'
152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
| ^~
quoted
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:480:2: note: in expansion of macro 'for_each_sgtable_sg'
480 | for_each_sgtable_sg(*sgt, sg, i)
| ^~~~~~~~~~~~~~~~~~~
include/linux/scatterlist.h:158:17: error: '*sgt' is a pointer; did you mean to use '->'?
158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i)
| ^~
include/linux/scatterlist.h:152:22: note: in definition of macro 'for_each_sg'
152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
| ^~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:484:2: note: in expansion of macro 'for_each_sgtable_sg'
484 | for_each_sgtable_sg(*sgt, sg, i) {
| ^~~~~~~~~~~~~~~~~~~
include/linux/scatterlist.h:158:31: error: '*sgt' is a pointer; did you mean to use '->'?
158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i)
| ^~
include/linux/scatterlist.h:152:38: note: in definition of macro 'for_each_sg'
152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
| ^~
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:484:2: note: in expansion of macro 'for_each_sgtable_sg'
484 | for_each_sgtable_sg(*sgt, sg, i) {
| ^~~~~~~~~~~~~~~~~~~
include/linux/scatterlist.h:158:17: error: '*sgt' is a pointer; did you mean to use '->'?
158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i)
| ^~
include/linux/scatterlist.h:152:22: note: in definition of macro 'for_each_sg'
152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
| ^~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:504:2: note: in expansion of macro 'for_each_sgtable_sg'
504 | for_each_sgtable_sg(*sgt, sg, i) {
| ^~~~~~~~~~~~~~~~~~~
include/linux/scatterlist.h:158:31: error: '*sgt' is a pointer; did you mean to use '->'?
158 | for_each_sg(sgt->sgl, sg, sgt->orig_nents, i)
| ^~
include/linux/scatterlist.h:152:38: note: in definition of macro 'for_each_sg'
152 | for (__i = 0, sg = (sglist); __i < (nr); __i++, sg = sg_next(sg))
| ^~
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:504:2: note: in expansion of macro 'for_each_sgtable_sg'
504 | for_each_sgtable_sg(*sgt, sg, i) {
| ^~~~~~~~~~~~~~~~~~~
In file included from drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:26:
At top level:
drivers/gpu/drm/amd/amdgpu/amdgpu.h:190:18: warning: 'sched_policy' defined but not used [-Wunused-const-variable=]
190 | static const int sched_policy = KFD_SCHED_POLICY_HWS;
| ^~~~~~~~~~~~
In file included from drivers/gpu/drm/amd/amdgpu/../display/dc/dc_types.h:33,
from drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:30,
from drivers/gpu/drm/amd/amdgpu/../include/dm_pp_interface.h:26,
from drivers/gpu/drm/amd/amdgpu/amdgpu.h:65,
from drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:26:
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:76:32: warning: 'dc_fixpt_ln2_div_2' defined but not used [-Wunused-const-variable=]
76 | static const struct fixed31_32 dc_fixpt_ln2_div_2 = { 1488522236LL };
| ^~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:75:32: warning: 'dc_fixpt_ln2' defined but not used [-Wunused-const-variable=]
75 | static const struct fixed31_32 dc_fixpt_ln2 = { 2977044471LL };
| ^~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:74:32: warning: 'dc_fixpt_e' defined but not used [-Wunused-const-variable=]
74 | static const struct fixed31_32 dc_fixpt_e = { 11674931555LL };
| ^~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:73:32: warning: 'dc_fixpt_two_pi' defined but not used [-Wunused-const-variable=]
73 | static const struct fixed31_32 dc_fixpt_two_pi = { 26986075409LL };
| ^~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:72:32: warning: 'dc_fixpt_pi' defined but not used [-Wunused-const-variable=]
72 | static const struct fixed31_32 dc_fixpt_pi = { 13493037705LL };
| ^~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:67:32: warning: 'dc_fixpt_zero' defined but not used [-Wunused-const-variable=]
67 | static const struct fixed31_32 dc_fixpt_zero = { 0 };
| ^~~~~~~~~~~~~
vim +/for_each_sgtable_sg +480 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
444
445 /**
446 * amdgpu_vram_mgr_alloc_sgt - allocate and fill a sg table
447 *
448 * @adev: amdgpu device pointer
449 * @mem: TTM memory object
450 * @dev: the other device
451 * @dir: dma direction
452 * @sgt: resulting sg table
453 *
454 * Allocate and fill a sg table from a VRAM allocation.
455 */
456 int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
457 struct ttm_mem_reg *mem,
458 struct device *dev,
459 enum dma_data_direction dir,
460 struct sg_table **sgt)
461 {
462 struct drm_mm_node *node;
463 struct scatterlist *sg;
464 int num_entries = 0;
465 unsigned int pages;
466 int i, r;
467
468 *sgt = kmalloc(sizeof(*sg), GFP_KERNEL);
469 if (!*sgt)
470 return -ENOMEM;
471
472 for (pages = mem->num_pages, node = mem->mm_node;
473 pages; pages -= node->size, ++node)
474 ++num_entries;
475
476 r = sg_alloc_table(*sgt, num_entries, GFP_KERNEL);
477 if (r)
478 goto error_free;
479
> 480 for_each_sgtable_sg(*sgt, sg, i)
481 sg->length = 0;
482
483 node = mem->mm_node;
484 for_each_sgtable_sg(*sgt, sg, i) {
485 phys_addr_t phys = (node->start << PAGE_SHIFT) +
486 adev->gmc.aper_base;
487 size_t size = node->size << PAGE_SHIFT;
488 dma_addr_t addr;
489
490 ++node;
491 addr = dma_map_resource(dev, phys, size, dir,
492 DMA_ATTR_SKIP_CPU_SYNC);
493 r = dma_mapping_error(dev, addr);
494 if (r)
495 goto error_unmap;
496
497 sg_set_page(sg, NULL, size, 0);
498 sg_dma_address(sg) = addr;
499 sg_dma_len(sg) = size;
500 }
501 return 0;
502
503 error_unmap:
504 for_each_sgtable_sg(*sgt, sg, i) {
505 if (!sg->length)
506 continue;
507
508 dma_unmap_resource(dev, sg->dma_address,
509 sg->length, dir,
510 DMA_ATTR_SKIP_CPU_SYNC);
511 }
512 sg_free_table(*sgt);
513
514 error_free:
515 kfree(*sgt);
516 return r;
517 }
518
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
From: Robin Murphy <robin.murphy@arm.com> Date: 2020-06-19 09:43:54
On 2020-06-18 16:39, Marek Szyprowski wrote:
quoted hunk
Use recently introduced common wrappers operating directly on the struct
sg_table objects and scatterlist page iterators to make the code a bit
more compact, robust, easier to follow and copy/paste safe.
No functional change, because the code already properly did all the
scaterlist related calls.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
.../common/videobuf2/videobuf2-dma-contig.c | 41 ++++++++-----------
.../media/common/videobuf2/videobuf2-dma-sg.c | 32 ++++++---------
.../common/videobuf2/videobuf2-vmalloc.c | 12 ++----
3 files changed, 34 insertions(+), 51 deletions(-)
Same comment as for the DRM version. In fact, given that it's the same
function with the same purpose, might it be worth hoisting out as a
generic helper for the sg_table API itself?
@@ -258,9 +256,8 @@ static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr, * No need to sync to the device, this will happen later when the * prepare() memop is called. */- sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,- buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);- if (!sgt->nents)+ if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir,+ DMA_ATTR_SKIP_CPU_SYNC)) {