[PATCH] staging: ion: ion_cma_heap: Don't directly use dma_common_get_sgtable

Jon Medhurst (Tixy)
2015-07-17T11:01:36Z

Use dma_get_sgtable rather than dma_common_get_sgtable so a device's
dma_ops aren't bypassed. This is essential in situations where a device
uses an IOMMU and the physical memory is not contiguous (as the common
function assumes).

Signed-off-by: Jon Medhurst <tixy@linaro.org>
---

This also begs the question as to what happens if the memory region _is_
contiguous but is in highmem or an ioremapped region. Should a device
always provide dma_ops for that case? Because I believe the current
implementation of dma_common_get_sgtable won't work for those as it uses
virt_to_page.

I see that this point has been raised before [1] by Zeng Tao, and I
myself have been given a different fix to apply to a Linaro kernel tree.
However, both solutions looked wrong to me as they treat a dma_addr_t as
a physical address, so should at least be using dma_to_phys.

So, should we fix dma_common_get_sgtable or mandate that the device
has dma_ops? The latter seems to be implied by the commit message which
introduced the function:

This patch provides a generic implementation based on
virt_to_page() call. Architectures which require more
sophisticated translation might provide their own get_sgtable()
methods.

Note, I don't have a system where any of this code is used to test
things, and have never looked at this area before yesterday, so I may
have misunderstood what’s going on in the code.

[1] https://lkml.org/lkml/2014/12/1/584

drivers/staging/android/ion/ion_cma_heap.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c
index f4211f1..86b91fd 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -73,8 +73,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
if (!info->table)
goto free_mem;

- if (dma_common_get_sgtable
- (dev, info->table, info->cpu_addr, info->handle, len))
+ if (dma_get_sgtable(dev, info->table, info->cpu_addr, info->handle, len))
goto free_table;
/* keep this for memory release */
buffer->priv_virt = info;
--
2.1.4