* [PATCH v5 27/38] xen: gntdev: fix common struct sg_table related issues [not found] ` <CGME20200513133316eucas1p2ad01d27ea4388cb50424bcf112d710ef@eucas1p2.samsung.com> @ 2020-05-13 13:32 ` Marek Szyprowski 2020-05-18 5:17 ` Jürgen Groß 0 siblings, 1 reply; 3+ messages in thread From: Marek Szyprowski @ 2020-05-13 13:32 UTC (permalink / raw) To: dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Juergen Gross, Bartlomiej Zolnierkiewicz, David Airlie, Daniel Vetter, xen-devel, Boris Ostrovsky, Robin Murphy, Christoph Hellwig, linux-arm-kernel, Marek Szyprowski 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> --- For more information, see '[PATCH v5 00/38] DRM: fix struct sg_table nents vs. orig_nents misuse' thread: https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsung.com/T/ --- drivers/xen/gntdev-dmabuf.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c index 75d3bb9..ba6cad8 100644 --- a/drivers/xen/gntdev-dmabuf.c +++ b/drivers/xen/gntdev-dmabuf.c @@ -247,10 +247,9 @@ static void dmabuf_exp_ops_detach(struct dma_buf *dma_buf, if (sgt) { if (gntdev_dmabuf_attach->dir != DMA_NONE) - dma_unmap_sg_attrs(attach->dev, sgt->sgl, - sgt->nents, - gntdev_dmabuf_attach->dir, - DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(attach->dev, sgt, + gntdev_dmabuf_attach->dir, + DMA_ATTR_SKIP_CPU_SYNC); sg_free_table(sgt); } @@ -288,8 +287,8 @@ static void dmabuf_exp_ops_detach(struct dma_buf *dma_buf, sgt = dmabuf_pages_to_sgt(gntdev_dmabuf->pages, gntdev_dmabuf->nr_pages); if (!IS_ERR(sgt)) { - if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, - DMA_ATTR_SKIP_CPU_SYNC)) { + if (dma_map_sgtable(attach->dev, sgt, dir, + DMA_ATTR_SKIP_CPU_SYNC)) { sg_free_table(sgt); kfree(sgt); sgt = ERR_PTR(-ENOMEM); @@ -625,7 +624,7 @@ static struct gntdev_dmabuf *dmabuf_imp_alloc_storage(int count) /* 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) { struct page *page = sg_page_iter_page(&sg_iter); /* * Check if page is valid: this can happen if we are given -- 1.9.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v5 27/38] xen: gntdev: fix common struct sg_table related issues 2020-05-13 13:32 ` [PATCH v5 27/38] xen: gntdev: fix common struct sg_table related issues Marek Szyprowski @ 2020-05-18 5:17 ` Jürgen Groß 0 siblings, 0 replies; 3+ messages in thread From: Jürgen Groß @ 2020-05-18 5:17 UTC (permalink / raw) To: Marek Szyprowski, dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Bartlomiej Zolnierkiewicz, David Airlie, Daniel Vetter, xen-devel, Boris Ostrovsky, Robin Murphy, Christoph Hellwig, linux-arm-kernel On 13.05.20 15:32, Marek Szyprowski wrote: > 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> Juergen ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <CGME20200522125708eucas1p233b80b0741f087a84d47f24b6d91985f@eucas1p2.samsung.com>]
* [PATCH v5 39/38] drm: xen: fix common struct sg_table related issues [not found] ` <CGME20200522125708eucas1p233b80b0741f087a84d47f24b6d91985f@eucas1p2.samsung.com> @ 2020-05-22 12:56 ` Marek Szyprowski 0 siblings, 0 replies; 3+ messages in thread From: Marek Szyprowski @ 2020-05-22 12:56 UTC (permalink / raw) To: dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Bartlomiej Zolnierkiewicz, David Airlie, Oleksandr Andrushchenko, Daniel Vetter, xen-devel, Robin Murphy, Christoph Hellwig, linux-arm-kernel, Marek Szyprowski 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> --- For more information, see '[PATCH v5 00/38] DRM: fix struct sg_table nents vs. orig_nents misuse' thread: https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsung.com/T/ This patch has been resurrected on Oleksandr Andrushchenko request. The patch was a part of v2 patchset, but then I've dropped it as it only fixes the debug output, thus I didn't consider it so critical. --- drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c index f0b85e0..ba4bdc5 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c @@ -215,7 +215,7 @@ struct drm_gem_object * return ERR_PTR(ret); DRM_DEBUG("Imported buffer of size %zu with nents %u\n", - size, sgt->nents); + size, sgt->orig_nents); return &xen_obj->base; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-22 12:57 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200513132114.6046-1-m.szyprowski@samsung.com> [not found] ` <20200513133245.6408-1-m.szyprowski@samsung.com> [not found] ` <CGME20200513133316eucas1p2ad01d27ea4388cb50424bcf112d710ef@eucas1p2.samsung.com> 2020-05-13 13:32 ` [PATCH v5 27/38] xen: gntdev: fix common struct sg_table related issues Marek Szyprowski 2020-05-18 5:17 ` Jürgen Groß [not found] ` <CGME20200522125708eucas1p233b80b0741f087a84d47f24b6d91985f@eucas1p2.samsung.com> 2020-05-22 12:56 ` [PATCH v5 39/38] drm: xen: " Marek Szyprowski
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).