[RFC,1/1] ion: support multiple address space map

Commit Message

Hiroshi DOYU March 14, 2014, 7:36 a.m. UTC

Hi,

Our H/Ws(inc. GPU) can accept a dmabuf fd which is originally
allocated via ION. GPU tries to map this buffer into  GPU address
space via ION dmabuf backend(.map_dma_buf). Our Tegra IOMMU supports
multiple address space in IOMMU. Basically each device has its own
address space assigned by IOMMU. Usually DMA API hides the existance
of IOMMU from driver code. To support this multiple IOMMU address
space, I think that at least "ion_map_dma_buf" needs to create IOMMU
mapping *per* device(address space). And I made the following patch
*experimentally*.

Is this the right solution to solve this multiple IOMMU address space?

Any comment would be really appreciated.

---8<------8<------8<------8<------8<------8<------8<------8<---
From: Hiroshi Doyu <[e-mail]>

Ion doesn't support multiple address space. The recent IOMMU usually
provide multiple address spaces, where a mapping depends on an address
space. An address space is bind to a device pointer passed from
attachment. Those info needs to be stored in a buffer info. If the
same buffer needs to be mapped into the same AS, the same mapping info
needs to be passed back.

Signed-off-by: Hiroshi Doyu <[e-mail]>
---
 drivers/staging/android/ion/ion.c      | 96 +++++++++++++++++++++++++++++++++-
 drivers/staging/android/ion/ion_priv.h | 10 ++++
 2 files changed, 104 insertions(+), 2 deletions(-)

Patch

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 0836717..e260434 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -872,17 +872,109 @@  static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
 static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
 					enum dma_data_direction direction)
 {
+	int err, i, empty = -1;
+	struct dma_iommu_mapping *iommu_map;
 	struct dma_buf *dmabuf = attachment->dmabuf;
 	struct ion_buffer *buffer = dmabuf->priv;
+	unsigned int nents = buffer->sg_table->nents;
+	struct ion_mapping *map_ptr;
+	struct scatterlist *sg;
+
+	iommu_map = to_dma_iommu_mapping(attachment->dev);
+	if (!iommu_map) {
+		ion_buffer_sync_for_device(buffer, attachment->dev, direction);
+		return buffer->sg_table;
+	}
+
+	mutex_lock(&buffer->lock);
+	for (i = 0; i < ARRAY_SIZE(buffer->mapping); i++) {
+		map_ptr = &buffer->mapping[i];
+		if (!map_ptr->dev) {
+			empty = i;
+			continue;
+		}
+
+		if (to_dma_iommu_mapping(map_ptr->dev) == iommu_map) {
+			kref_get(&map_ptr->kref);
+			mutex_unlock(&buffer->lock);
+			return &map_ptr->sgt;
+		}
+	}
 
-	ion_buffer_sync_for_device(buffer, attachment->dev, direction);
-	return buffer->sg_table;
+	if (!empty) {
+		err = -ENOMEM;
+		goto err_no_space;
+	}
+
+	map_ptr = &buffer->mapping[empty];
+	err = sg_alloc_table(&map_ptr->sgt, nents, GFP_KERNEL);
+	if (err)
+		goto err_sg_alloc_table;
+
+	for_each_sg(buffer->sg_table->sgl, sg, nents, i)
+		memcpy(map_ptr->sgt.sgl + i, sg, sizeof(*sg));
+
+	nents = dma_map_sg(attachment->dev, map_ptr->sgt.sgl, nents, direction);
+	if (!nents) {
+		err = -EINVAL;
+		goto err_dma_map_sg;
+	}
+
+	kref_init(&map_ptr->kref);
+	map_ptr->dev = attachment->dev;
+	mutex_unlock(&buffer->lock);
+	return &map_ptr->sgt;
+
+err_dma_map_sg:
+	sg_free_table(&map_ptr->sgt);
+err_sg_alloc_table:
+err_no_space:
+	mutex_unlock(&buffer->lock);
+	return ERR_PTR(err);
+}
+
+static void __ion_unmap_dma_buf(struct kref *kref)
+{
+	struct ion_mapping *map_ptr;
+
+	map_ptr = container_of(kref, struct ion_mapping, kref);
+	dma_unmap_sg(map_ptr->dev, map_ptr->sgt.sgl, map_ptr->sgt.nents,
+		     DMA_BIDIRECTIONAL);
+	sg_free_table(&map_ptr->sgt);
+	memset(map_ptr, 0, sizeof(*map_ptr));
 }
 
 static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
 			      struct sg_table *table,
 			      enum dma_data_direction direction)
 {
+	int i;
+	struct dma_iommu_mapping *iommu_map;
+	struct dma_buf *dmabuf = attachment->dmabuf;
+	struct ion_buffer *buffer = dmabuf->priv;
+	struct ion_mapping *map_ptr;
+
+	iommu_map = to_dma_iommu_mapping(attachment->dev);
+	if (!iommu_map)
+		return;
+
+	mutex_lock(&buffer->lock);
+	for (i = 0; i < ARRAY_SIZE(buffer->mapping); i++) {
+		map_ptr = &buffer->mapping[i];
+		if (!map_ptr->dev)
+			continue;
+
+		if (to_dma_iommu_mapping(map_ptr->dev) == iommu_map) {
+			kref_put(&map_ptr->kref, __ion_unmap_dma_buf);
+			mutex_unlock(&buffer->lock);
+			return;
+		}
+	}
+
+	dev_warn(attachment->dev, "Not found a map(%p)\n",
+		 to_dma_iommu_mapping(attachment->dev));
+
+	mutex_unlock(&buffer->lock);
 }
 
 void ion_pages_sync_for_device(struct device *dev, struct page *page,
diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h
index 1eba3f2..441c251 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -26,11 +26,19 @@ 
 #include <linux/sched.h>
 #include <linux/shrinker.h>
 #include <linux/types.h>
+#include <linux/scatterlist.h>
 
 #include "ion.h"
 
 struct ion_buffer *ion_handle_buffer(struct ion_handle *handle);
 
+struct ion_mapping {
+	struct device *dev; /* to get a map and dma_ops */
+	struct sg_table sgt;
+	struct kref kref;
+};
+#define NUM_ION_MAPPING 5 /* FIXME: dynamically allocate more than this */
+
 /**
  * struct ion_buffer - metadata for a particular buffer
  * @ref:		refernce count
@@ -84,6 +92,8 @@  struct ion_buffer {
 	int handle_count;
 	char task_comm[TASK_COMM_LEN];
 	pid_t pid;
+
+	struct ion_mapping mapping[NUM_ION_MAPPING];
 };
 void ion_buffer_destroy(struct ion_buffer *buffer);