staging: ion: fix common struct sg_table related issues

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>
Link: https://lore.kernel.org/r/20200619103636.11974-31-m.szyprowski@samsung.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 38b51eace4f95ca7284166007d91ae1863ae7018..3c9f09506ffa2284733d31b322262837c6167051 100644 (file)
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -147,14 +147,14 @@ static struct sg_table *dup_sg_table(struct sg_table *table)
        if (!new_table)
                return ERR_PTR(-ENOMEM);
 
-       ret = sg_alloc_table(new_table, table->nents, GFP_KERNEL);
+       ret = sg_alloc_table(new_table, table->orig_nents, GFP_KERNEL);
        if (ret) {
                kfree(new_table);
                return ERR_PTR(-ENOMEM);
        }
 
        new_sg = new_table->sgl;
-       for_each_sg(table->sgl, sg, table->nents, i) {
+       for_each_sgtable_sg(table, sg, i) {
                memcpy(new_sg, sg, sizeof(*sg));
                new_sg->dma_address = 0;
                new_sg = sg_next(new_sg);
@@ -224,12 +224,13 @@ static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
 {
        struct ion_dma_buf_attachment *a = attachment->priv;
        struct sg_table *table;
+       int ret;
 
        table = a->table;
 
-       if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
-                       direction))
-               return ERR_PTR(-ENOMEM);
+       ret = dma_map_sgtable(attachment->dev, table, direction, 0);
+       if (ret)
+               return ERR_PTR(ret);
 
        return table;
 }
@@ -238,7 +239,7 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
                              struct sg_table *table,
                              enum dma_data_direction direction)
 {
-       dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
+       dma_unmap_sgtable(attachment->dev, table, direction, 0);
 }
 
 static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
@@ -296,10 +297,8 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
        }
 
        mutex_lock(&buffer->lock);
-       list_for_each_entry(a, &buffer->attachments, list) {
-               dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
-                                   direction);
-       }
+       list_for_each_entry(a, &buffer->attachments, list)
+               dma_sync_sgtable_for_cpu(a->dev, a->table, direction);
 
 unlock:
        mutex_unlock(&buffer->lock);
@@ -319,10 +318,8 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
        }
 
        mutex_lock(&buffer->lock);
-       list_for_each_entry(a, &buffer->attachments, list) {
-               dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents,
-                                      direction);
-       }
+       list_for_each_entry(a, &buffer->attachments, list)
+               dma_sync_sgtable_for_device(a->dev, a->table, direction);
        mutex_unlock(&buffer->lock);
 
        return 0;

diff --git a/drivers/staging/android/ion/ion_heap.c b/drivers/staging/android/ion/ion_heap.c
index 9c23b2382a1e6a5dbf8a31f88f0ab255b1941e00..79f27949edda6d5d9db54c9bf672e9a9e1a4631e 100644 (file)
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -20,8 +20,7 @@
 void *ion_heap_map_kernel(struct ion_heap *heap,
                          struct ion_buffer *buffer)
 {
-       struct scatterlist *sg;
-       int i, j;
+       struct sg_page_iter piter;
        void *vaddr;
        pgprot_t pgprot;
        struct sg_table *table = buffer->sg_table;
@@ -38,14 +37,11 @@ void *ion_heap_map_kernel(struct ion_heap *heap,
        else
                pgprot = pgprot_writecombine(PAGE_KERNEL);
 
-       for_each_sg(table->sgl, sg, table->nents, i) {
-               int npages_this_entry = PAGE_ALIGN(sg->length) / PAGE_SIZE;
-               struct page *page = sg_page(sg);
-
-               BUG_ON(i >= npages);
-               for (j = 0; j < npages_this_entry; j++)
-                       *(tmp++) = page++;
+       for_each_sgtable_page(table, &piter, 0) {
+               BUG_ON(tmp - pages >= npages);
+               *tmp++ = sg_page_iter_page(&piter);
        }
+
        vaddr = vmap(pages, npages, VM_MAP, pgprot);
        vfree(pages);
 
@@ -64,32 +60,19 @@ void ion_heap_unmap_kernel(struct ion_heap *heap,
 int ion_heap_map_user(struct ion_heap *heap, struct ion_buffer *buffer,
                      struct vm_area_struct *vma)
 {
+       struct sg_page_iter piter;
        struct sg_table *table = buffer->sg_table;
        unsigned long addr = vma->vm_start;
-       unsigned long offset = vma->vm_pgoff * PAGE_SIZE;
-       struct scatterlist *sg;
-       int i;
        int ret;
 
-       for_each_sg(table->sgl, sg, table->nents, i) {
-               struct page *page = sg_page(sg);
-               unsigned long remainder = vma->vm_end - addr;
-               unsigned long len = sg->length;
+       for_each_sgtable_page(table, &piter, vma->vm_pgoff) {
+               struct page *page = sg_page_iter_page(&piter);
 
-               if (offset >= sg->length) {
-                       offset -= sg->length;
-                       continue;
-               } else if (offset) {
-                       page += offset / PAGE_SIZE;
-                       len = sg->length - offset;
-                       offset = 0;
-               }
-               len = min(len, remainder);
-               ret = remap_pfn_range(vma, addr, page_to_pfn(page), len,
+               ret = remap_pfn_range(vma, addr, page_to_pfn(page), PAGE_SIZE,
                                      vma->vm_page_prot);
                if (ret)
                        return ret;
-               addr += len;
+               addr += PAGE_SIZE;
                if (addr >= vma->vm_end)
                        return 0;
        }
@@ -109,15 +92,14 @@ static int ion_heap_clear_pages(struct page **pages, int num, pgprot_t pgprot)
        return 0;
 }
 
-static int ion_heap_sglist_zero(struct scatterlist *sgl, unsigned int nents,
-                               pgprot_t pgprot)
+static int ion_heap_sglist_zero(struct sg_table *sgt, pgprot_t pgprot)
 {
        int p = 0;
        int ret = 0;
        struct sg_page_iter piter;
        struct page *pages[32];
 
-       for_each_sg_page(sgl, &piter, nents, 0) {
+       for_each_sgtable_page(sgt, &piter, 0) {
                pages[p++] = sg_page_iter_page(&piter);
                if (p == ARRAY_SIZE(pages)) {
                        ret = ion_heap_clear_pages(pages, p, pgprot);
@@ -142,7 +124,7 @@ int ion_heap_buffer_zero(struct ion_buffer *buffer)
        else
                pgprot = pgprot_writecombine(PAGE_KERNEL);
 
-       return ion_heap_sglist_zero(table->sgl, table->nents, pgprot);
+       return ion_heap_sglist_zero(table, pgprot);
 }
 
 void ion_heap_freelist_add(struct ion_heap *heap, struct ion_buffer *buffer)

diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
index b83a1d16bd8983a3620ee526cbde3de2e84592cb..eac0632ab4e8ca40a212a2d2db5baf56ae15ee12 100644 (file)
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -162,7 +162,7 @@ static void ion_system_heap_free(struct ion_buffer *buffer)
        if (!(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE))
                ion_heap_buffer_zero(buffer);
 
-       for_each_sg(table->sgl, sg, table->nents, i)
+       for_each_sgtable_sg(table, sg, i)
                free_buffer_page(sys_heap, buffer, sg_page(sg));
        sg_free_table(table);
        kfree(table);