Add MSM SMMUv1 support

drivers/iommu/iommu.c +8 -0

+
+	BUG_ON(iova & (~PAGE_MASK));
+
+	return domain->ops->map_range(domain, iova, sg, len, prot);
+}
+EXPORT_SYMBOL_GPL(iommu_map_range);
+
+int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,

Mapping and unmapping are more often than not in the critical path. map_range and unmap_range allows SMMU driver implementations to optimize the process of mapping and unmapping buffers into the SMMU page tables. Instead of mapping one physical address, do TLB operation (expensive), mapping, do TLB operation, mapping, do TLB operation the driver can map a scatter-gatherlist of physically contiguous pages into one virtual address space and then at the end do one TLB operation.

Additionally, the mapping operation would be faster in general since clients does not have to keep calling map API over and over again for each physically contiguous chunk of memory that needs to be mapped to a virtually contiguous region.

-----Original Message-----
From: iommu-bounces at lists.linux-foundation.org [mailto:iommu- bounces at lists.linux-foundation.org] On Behalf Of Will Deacon
Sent: Tuesday, July 01, 2014 3:04 PM
To: Olav Haugan
Cc: linux-arm-msm at vger.kernel.org; iommu at lists.linux-foundation.org; thierry.reding at gmail.com; vgandhi at codeaurora.org; linux-arm- kernel at lists.infradead.org
Subject: Re: [RFC/PATCH 2/7] iommu-api: Add map_range/unmap_range functions

Hi Olav,

Quoted 3 lines · Olav Haugan

Mapping and unmapping are more often than not in the critical path. map_range and unmap_range allows SMMU driver implementations to optimize the process of mapping and unmapping buffers into the SMMU

page tables.

Quoted 9 lines

Instead of mapping one physical address, do TLB operation (expensive), mapping, do TLB operation, mapping, do TLB operation the driver can map a scatter-gatherlist of physically contiguous pages into one virtual address space and then at the end do one TLB operation.

Additionally, the mapping operation would be faster in general since clients does not have to keep calling map API over and over again for each physically contiguous chunk of memory that needs to be mapped to a virtually contiguous region.

I like the idea of this, although it does mean that drivers implementing the range mapping functions need more featureful page-table manipulation code than currently required.

For example, iommu_map uses iommu_pgsize to guarantee that mappings are created in blocks of the largest support page size. This can be used to simplify iterating in the SMMU driver (although the ARM SMMU driver doesn't yet make use of this, I think Varun would add this when he adds support for sections).

Mapping and unmapping are more often than not in the critical path. map_range and unmap_range allows SMMU driver implementations to optimize the process of mapping and unmapping buffers into the SMMU page tables. Instead of mapping one physical address, do TLB operation (expensive), mapping, do TLB operation, mapping, do TLB operation the driver can map a scatter-gatherlist of physically contiguous pages into one virtual address space and then at the end do one TLB operation.

Additionally, the mapping operation would be faster in general since clients does not have to keep calling map API over and over again for each physically contiguous chunk of memory that needs to be mapped to a virtually contiguous region.

Signed-off-by Olav Haugan <[email protected]>

---
 drivers/iommu/iommu.c | 24 ++++++++++++++++++++++++
 include/linux/iommu.h | 24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

drivers/iommu/iommu.c +1 -0

index e5555fc..f2a6b80 100644
@@ -898,6 +898,30 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
 EXPORT_SYMBOL_GPL(iommu_unmap);
 
 
+int iommu_map_range(struct iommu_domain *domain, unsigned int iova,

Hi Olav,

Quoted 27 lines · Olav Haugan

Mapping and unmapping are more often than not in the critical path. map_range and unmap_range allows SMMU driver implementations to optimize the process of mapping and unmapping buffers into the SMMU page tables. Instead of mapping one physical address, do TLB operation (expensive), mapping, do TLB operation, mapping, do TLB operation the driver can map a scatter-gatherlist of physically contiguous pages into one virtual address space and then at the end do one TLB operation.

Additionally, the mapping operation would be faster in general since clients does not have to keep calling map API over and over again for each physically contiguous chunk of memory that needs to be mapped to a virtually contiguous region.

Signed-off-by Olav Haugan <[email protected]>

---
 drivers/iommu/iommu.c | 24 ++++++++++++++++++++++++
 include/linux/iommu.h | 24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

drivers/iommu/iommu.c +0 -0

index e5555fc..f2a6b80 100644
@@ -898,6 +898,30 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
 EXPORT_SYMBOL_GPL(iommu_unmap);
 
 

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 911

Quoted 11 lines

drivers/iommu/iommu.c +11 -0

+int iommu_map_range(struct iommu_domain *domain, unsigned int iova,
+		    struct scatterlist *sg, unsigned int len, int prot)
+{
+	if (unlikely(domain->ops->map_range == NULL))
+		return -ENODEV;
+
+	BUG_ON(iova & (~PAGE_MASK));
+
+	return domain->ops->map_range(domain, iova, sg, len, prot);
+}
+EXPORT_SYMBOL_GPL(iommu_map_range);

We have the similar one internally, which is named, "iommu_map_sg()", called from DMA API.

Quoted 5 lines

drivers/iommu/iommu.c +5 -0

+int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,
+		      unsigned int len)
+{
+	if (unlikely(domain->ops->unmap_range == NULL))
+		return -ENODEV;

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 923

Quoted 6 lines

drivers/iommu/iommu.c +6 -0

+
+	BUG_ON(iova & (~PAGE_MASK));
+
+	return domain->ops->unmap_range(domain, iova, len);
+}
+EXPORT_SYMBOL_GPL(iommu_unmap_range);

Can the existing iommu_unmap() do the same?

Hi Hiroshi,

Quoted 31 lines · Hiroshi Doyu

Hi Olav,

Quoted 27 lines · Olav Haugan

Mapping and unmapping are more often than not in the critical path. map_range and unmap_range allows SMMU driver implementations to optimize the process of mapping and unmapping buffers into the SMMU page tables. Instead of mapping one physical address, do TLB operation (expensive), mapping, do TLB operation, mapping, do TLB operation the driver can map a scatter-gatherlist of physically contiguous pages into one virtual address space and then at the end do one TLB operation.

Additionally, the mapping operation would be faster in general since clients does not have to keep calling map API over and over again for each physically contiguous chunk of memory that needs to be mapped to a virtually contiguous region.

Signed-off-by Olav Haugan <[email protected]>

---
 drivers/iommu/iommu.c | 24 ++++++++++++++++++++++++
 include/linux/iommu.h | 24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

drivers/iommu/iommu.c +0 -0

index e5555fc..f2a6b80 100644
@@ -898,6 +898,30 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
 EXPORT_SYMBOL_GPL(iommu_unmap);
 
 

Quoted 14 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 911

Quoted 11 lines

drivers/iommu/iommu.c +11 -0

+int iommu_map_range(struct iommu_domain *domain, unsigned int iova,
+                struct scatterlist *sg, unsigned int len, int prot)
+{
+    if (unlikely(domain->ops->map_range == NULL))
+            return -ENODEV;
+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->map_range(domain, iova, sg, len, prot);
+}
+EXPORT_SYMBOL_GPL(iommu_map_range);

We have the similar one internally, which is named, "iommu_map_sg()", called from DMA API.

Great, so this new API will be useful to more people!

Quoted 5 lines

Quoted 5 lines

drivers/iommu/iommu.c +5 -0

+int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,
+                  unsigned int len)
+{
+    if (unlikely(domain->ops->unmap_range == NULL))
+            return -ENODEV;

Quoted 8 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 923

Quoted 6 lines

drivers/iommu/iommu.c +6 -0

+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->unmap_range(domain, iova, len);
+}
+EXPORT_SYMBOL_GPL(iommu_unmap_range);

Can the existing iommu_unmap() do the same?

I believe iommu_unmap() behaves a bit differently because it will keep on calling domain->ops->unmap() until everything is unmapped instead of letting the iommu implementation take care of unmapping everything in one call.

I am abandoning the patch series since our driver was not accepted. However, if there are no objections I will resubmit this patch (PATCH 2/7) as an independent patch to add this new map_range API.

Thanks,

Olav Haugan

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 65 lines · Olav Haugan

Hi Hiroshi,

Quoted 31 lines · Hiroshi Doyu

Hi Olav,

Quoted 27 lines · Olav Haugan

Mapping and unmapping are more often than not in the critical path. map_range and unmap_range allows SMMU driver implementations to optimize the process of mapping and unmapping buffers into the SMMU page tables. Instead of mapping one physical address, do TLB operation (expensive), mapping, do TLB operation, mapping, do TLB operation the driver can map a scatter-gatherlist of physically contiguous pages into one virtual address space and then at the end do one TLB operation.

Additionally, the mapping operation would be faster in general since clients does not have to keep calling map API over and over again for each physically contiguous chunk of memory that needs to be mapped to a virtually contiguous region.

Signed-off-by Olav Haugan <[email protected]>

---
 drivers/iommu/iommu.c | 24 ++++++++++++++++++++++++
 include/linux/iommu.h | 24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

drivers/iommu/iommu.c +0 -0

index e5555fc..f2a6b80 100644
@@ -898,6 +898,30 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
 EXPORT_SYMBOL_GPL(iommu_unmap);
 
 

Quoted 14 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 911

Quoted 11 lines

drivers/iommu/iommu.c +11 -0

+int iommu_map_range(struct iommu_domain *domain, unsigned int iova,
+                struct scatterlist *sg, unsigned int len, int prot)
+{
+    if (unlikely(domain->ops->map_range == NULL))
+            return -ENODEV;
+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->map_range(domain, iova, sg, len, prot);
+}
+EXPORT_SYMBOL_GPL(iommu_map_range);

We have the similar one internally, which is named, "iommu_map_sg()", called from DMA API.

Great, so this new API will be useful to more people!

Quoted 5 lines

Quoted 5 lines

drivers/iommu/iommu.c +5 -0

+int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,
+                  unsigned int len)
+{
+    if (unlikely(domain->ops->unmap_range == NULL))
+            return -ENODEV;

Quoted 8 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 923

Quoted 6 lines

drivers/iommu/iommu.c +6 -0

+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->unmap_range(domain, iova, len);
+}
+EXPORT_SYMBOL_GPL(iommu_unmap_range);

Can the existing iommu_unmap() do the same?

Quoted 8 lines

I believe iommu_unmap() behaves a bit differently because it will keep on calling domain->ops->unmap() until everything is unmapped instead of letting the iommu implementation take care of unmapping everything in one call.

I am abandoning the patch series since our driver was not accepted. However, if there are no objections I will resubmit this patch (PATCH 2/7) as an independent patch to add this new map_range API.

+1 for map_range().. I've seen for gpu workloads, at least, it is the
downstream map_range() API is quite beneficial. It was worth at
least a few fps in xonotic.

And, possibly getting off the subject a bit, but I was wondering about the possibility of going one step further and batching up mapping and/or unmapping multiple buffers (ranges) at once. I have a pretty convenient sync point in drm/msm to flush out multiple mappings before kicking gpu.

Quoted 79 lines · Rob Clark

Quoted 65 lines · Olav Haugan

Hi Hiroshi,

Quoted 31 lines · Hiroshi Doyu

Hi Olav,

Quoted 27 lines · Olav Haugan

Mapping and unmapping are more often than not in the critical path. map_range and unmap_range allows SMMU driver implementations to optimize the process of mapping and unmapping buffers into the SMMU page tables. Instead of mapping one physical address, do TLB operation (expensive), mapping, do TLB operation, mapping, do TLB operation the driver can map a scatter-gatherlist of physically contiguous pages into one virtual address space and then at the end do one TLB operation.

Additionally, the mapping operation would be faster in general since clients does not have to keep calling map API over and over again for each physically contiguous chunk of memory that needs to be mapped to a virtually contiguous region.

Signed-off-by Olav Haugan <[email protected]>

---
 drivers/iommu/iommu.c | 24 ++++++++++++++++++++++++
 include/linux/iommu.h | 24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

drivers/iommu/iommu.c +0 -0

index e5555fc..f2a6b80 100644
@@ -898,6 +898,30 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
 EXPORT_SYMBOL_GPL(iommu_unmap);
 
 

Quoted 14 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 911

Quoted 11 lines

drivers/iommu/iommu.c +11 -0

+int iommu_map_range(struct iommu_domain *domain, unsigned int iova,
+                struct scatterlist *sg, unsigned int len, int prot)
+{
+    if (unlikely(domain->ops->map_range == NULL))
+            return -ENODEV;
+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->map_range(domain, iova, sg, len, prot);
+}
+EXPORT_SYMBOL_GPL(iommu_map_range);

We have the similar one internally, which is named, "iommu_map_sg()", called from DMA API.

Great, so this new API will be useful to more people!

Quoted 5 lines

Quoted 5 lines

drivers/iommu/iommu.c +5 -0

+int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,
+                  unsigned int len)
+{
+    if (unlikely(domain->ops->unmap_range == NULL))
+            return -ENODEV;

Quoted 8 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 923

Quoted 6 lines

drivers/iommu/iommu.c +6 -0

+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->unmap_range(domain, iova, len);
+}
+EXPORT_SYMBOL_GPL(iommu_unmap_range);

Can the existing iommu_unmap() do the same?

Quoted 8 lines

I believe iommu_unmap() behaves a bit differently because it will keep on calling domain->ops->unmap() until everything is unmapped instead of letting the iommu implementation take care of unmapping everything in one call.

I am abandoning the patch series since our driver was not accepted. However, if there are no objections I will resubmit this patch (PATCH 2/7) as an independent patch to add this new map_range API.

+1 for map_range().. I've seen for gpu workloads, at least, it is the
downstream map_range() API is quite beneficial. It was worth at
least a few fps in xonotic.

Quoted 5 lines

And, possibly getting off the subject a bit, but I was wondering about the possibility of going one step further and batching up mapping and/or unmapping multiple buffers (ranges) at once. I have a pretty convenient sync point in drm/msm to flush out multiple mappings before kicking gpu.

I think you should be able to do that with this API already - at least the mapping part since we are passing in a sg list (this could be a chained sglist).

Thanks,

Olav

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Quoted 86 lines · Olav Haugan

Quoted 79 lines · Rob Clark

Quoted 65 lines · Olav Haugan

Hi Hiroshi,

Quoted 31 lines · Hiroshi Doyu

Hi Olav,

Quoted 27 lines · Olav Haugan

Mapping and unmapping are more often than not in the critical path. map_range and unmap_range allows SMMU driver implementations to optimize the process of mapping and unmapping buffers into the SMMU page tables. Instead of mapping one physical address, do TLB operation (expensive), mapping, do TLB operation, mapping, do TLB operation the driver can map a scatter-gatherlist of physically contiguous pages into one virtual address space and then at the end do one TLB operation.

Additionally, the mapping operation would be faster in general since clients does not have to keep calling map API over and over again for each physically contiguous chunk of memory that needs to be mapped to a virtually contiguous region.

Signed-off-by Olav Haugan <[email protected]>

---
 drivers/iommu/iommu.c | 24 ++++++++++++++++++++++++
 include/linux/iommu.h | 24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

drivers/iommu/iommu.c +0 -0

index e5555fc..f2a6b80 100644
@@ -898,6 +898,30 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
 EXPORT_SYMBOL_GPL(iommu_unmap);
 
 

Quoted 14 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 911

Quoted 11 lines

drivers/iommu/iommu.c +11 -0

+int iommu_map_range(struct iommu_domain *domain, unsigned int iova,
+                struct scatterlist *sg, unsigned int len, int prot)
+{
+    if (unlikely(domain->ops->map_range == NULL))
+            return -ENODEV;
+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->map_range(domain, iova, sg, len, prot);
+}
+EXPORT_SYMBOL_GPL(iommu_map_range);

We have the similar one internally, which is named, "iommu_map_sg()", called from DMA API.

Great, so this new API will be useful to more people!

Quoted 5 lines

Quoted 5 lines

drivers/iommu/iommu.c +5 -0

+int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,
+                  unsigned int len)
+{
+    if (unlikely(domain->ops->unmap_range == NULL))
+            return -ENODEV;

Quoted 8 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 923

Quoted 6 lines

drivers/iommu/iommu.c +6 -0

+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->unmap_range(domain, iova, len);
+}
+EXPORT_SYMBOL_GPL(iommu_unmap_range);

Can the existing iommu_unmap() do the same?

Quoted 8 lines

I believe iommu_unmap() behaves a bit differently because it will keep on calling domain->ops->unmap() until everything is unmapped instead of letting the iommu implementation take care of unmapping everything in one call.

I am abandoning the patch series since our driver was not accepted. However, if there are no objections I will resubmit this patch (PATCH 2/7) as an independent patch to add this new map_range API.

+1 for map_range().. I've seen for gpu workloads, at least, it is the
downstream map_range() API is quite beneficial. It was worth at
least a few fps in xonotic.

Quoted 5 lines

And, possibly getting off the subject a bit, but I was wondering about the possibility of going one step further and batching up mapping and/or unmapping multiple buffers (ranges) at once. I have a pretty convenient sync point in drm/msm to flush out multiple mappings before kicking gpu.

Quoted 3 lines

I think you should be able to do that with this API already - at least the mapping part since we are passing in a sg list (this could be a chained sglist).

What I mean by batching up is mapping and unmapping multiple sglists each at different iova's with minmal cpu cache and iommu tlb flushes..

Ideally we'd let the IOMMU driver be clever and build out all 2nd level tables before inserting into first level tables (to minimize cpu cache flushing).. also, there is probably a reasonable chance that we'd be mapping a new buffer into existing location, so there might be some potential to reuse existing 2nd level tables (and save a tiny bit of free/alloc). I've not thought too much about how that would look in code.. might be kinda, umm, fun..

But at an API level, we should be able to do a bunch of map/unmap_range's with one flush.

Maybe it could look like a sequence of iommu_{map,unmap}_range() followed by iommu_flush()?

Quoted 105 lines · Rob Clark

Quoted 86 lines · Olav Haugan

Quoted 79 lines · Rob Clark

Quoted 65 lines · Olav Haugan

Hi Hiroshi,

Quoted 31 lines · Hiroshi Doyu

Hi Olav,

Quoted 27 lines · Olav Haugan

Mapping and unmapping are more often than not in the critical path. map_range and unmap_range allows SMMU driver implementations to optimize the process of mapping and unmapping buffers into the SMMU page tables. Instead of mapping one physical address, do TLB operation (expensive), mapping, do TLB operation, mapping, do TLB operation the driver can map a scatter-gatherlist of physically contiguous pages into one virtual address space and then at the end do one TLB operation.

Additionally, the mapping operation would be faster in general since clients does not have to keep calling map API over and over again for each physically contiguous chunk of memory that needs to be mapped to a virtually contiguous region.

Signed-off-by Olav Haugan <[email protected]>

---
 drivers/iommu/iommu.c | 24 ++++++++++++++++++++++++
 include/linux/iommu.h | 24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

drivers/iommu/iommu.c +0 -0

index e5555fc..f2a6b80 100644
@@ -898,6 +898,30 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
 EXPORT_SYMBOL_GPL(iommu_unmap);
 
 

Quoted 14 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 911

Quoted 11 lines

drivers/iommu/iommu.c +11 -0

+int iommu_map_range(struct iommu_domain *domain, unsigned int iova,
+                struct scatterlist *sg, unsigned int len, int prot)
+{
+    if (unlikely(domain->ops->map_range == NULL))
+            return -ENODEV;
+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->map_range(domain, iova, sg, len, prot);
+}
+EXPORT_SYMBOL_GPL(iommu_map_range);

We have the similar one internally, which is named, "iommu_map_sg()", called from DMA API.

Great, so this new API will be useful to more people!

Quoted 5 lines

Quoted 5 lines

drivers/iommu/iommu.c +5 -0

+int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,
+                  unsigned int len)
+{
+    if (unlikely(domain->ops->unmap_range == NULL))
+            return -ENODEV;

Quoted 8 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 922

Quoted 6 lines

drivers/iommu/iommu.c +6 -0

+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->unmap_range(domain, iova, len);
+}
+EXPORT_SYMBOL_GPL(iommu_unmap_range);

Can the existing iommu_unmap() do the same?

Quoted 8 lines

I believe iommu_unmap() behaves a bit differently because it will keep on calling domain->ops->unmap() until everything is unmapped instead of letting the iommu implementation take care of unmapping everything in one call.

I am abandoning the patch series since our driver was not accepted. However, if there are no objections I will resubmit this patch (PATCH 2/7) as an independent patch to add this new map_range API.

+1 for map_range().. I've seen for gpu workloads, at least, it is the
downstream map_range() API is quite beneficial. It was worth at
least a few fps in xonotic.

Quoted 5 lines

And, possibly getting off the subject a bit, but I was wondering about the possibility of going one step further and batching up mapping and/or unmapping multiple buffers (ranges) at once. I have a pretty convenient sync point in drm/msm to flush out multiple mappings before kicking gpu.

Quoted 3 lines

I think you should be able to do that with this API already - at least the mapping part since we are passing in a sg list (this could be a chained sglist).

What I mean by batching up is mapping and unmapping multiple sglists each at different iova's with minmal cpu cache and iommu tlb flushes..

Ideally we'd let the IOMMU driver be clever and build out all 2nd level tables before inserting into first level tables (to minimize cpu cache flushing).. also, there is probably a reasonable chance that we'd be mapping a new buffer into existing location, so there might be some potential to reuse existing 2nd level tables (and save a tiny bit of free/alloc). I've not thought too much about how that would look in code.. might be kinda, umm, fun..

But at an API level, we should be able to do a bunch of map/unmap_range's with one flush.

Quoted 2 lines

Maybe it could look like a sequence of iommu_{map,unmap}_range() followed by iommu_flush()?

Doesn't that mean that the IOMMU driver would have to keep track of all mappings until it sees an iommu_flush()? That sounds like it could be a lot of work and complicated code.

Thierry

Quoted 86 lines · Olav Haugan

Quoted 79 lines · Rob Clark

Quoted 65 lines · Olav Haugan

Hi Hiroshi,

Quoted 31 lines · Hiroshi Doyu

Hi Olav,

Quoted 27 lines · Olav Haugan

Mapping and unmapping are more often than not in the critical path. map_range and unmap_range allows SMMU driver implementations to optimize the process of mapping and unmapping buffers into the SMMU page tables. Instead of mapping one physical address, do TLB operation (expensive), mapping, do TLB operation, mapping, do TLB operation the driver can map a scatter-gatherlist of physically contiguous pages into one virtual address space and then at the end do one TLB operation.

Additionally, the mapping operation would be faster in general since clients does not have to keep calling map API over and over again for each physically contiguous chunk of memory that needs to be mapped to a virtually contiguous region.

Signed-off-by Olav Haugan <[email protected]>

---
 drivers/iommu/iommu.c | 24 ++++++++++++++++++++++++
 include/linux/iommu.h | 24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

drivers/iommu/iommu.c +0 -0

index e5555fc..f2a6b80 100644
@@ -898,6 +898,30 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
 EXPORT_SYMBOL_GPL(iommu_unmap);
 
 

Quoted 14 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 911

Quoted 11 lines

drivers/iommu/iommu.c +11 -0

+int iommu_map_range(struct iommu_domain *domain, unsigned int iova,
+                struct scatterlist *sg, unsigned int len, int prot)
+{
+    if (unlikely(domain->ops->map_range == NULL))
+            return -ENODEV;
+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->map_range(domain, iova, sg, len, prot);
+}
+EXPORT_SYMBOL_GPL(iommu_map_range);

We have the similar one internally, which is named, "iommu_map_sg()", called from DMA API.

Great, so this new API will be useful to more people!

Quoted 5 lines

Quoted 5 lines

drivers/iommu/iommu.c +5 -0

+int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,
+                  unsigned int len)
+{
+    if (unlikely(domain->ops->unmap_range == NULL))
+            return -ENODEV;

Quoted 8 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 923

Quoted 6 lines

drivers/iommu/iommu.c +6 -0

+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->unmap_range(domain, iova, len);
+}
+EXPORT_SYMBOL_GPL(iommu_unmap_range);

Can the existing iommu_unmap() do the same?

Quoted 8 lines

I believe iommu_unmap() behaves a bit differently because it will keep on calling domain->ops->unmap() until everything is unmapped instead of letting the iommu implementation take care of unmapping everything in one call.

I am abandoning the patch series since our driver was not accepted. However, if there are no objections I will resubmit this patch (PATCH 2/7) as an independent patch to add this new map_range API.

+1 for map_range().. I've seen for gpu workloads, at least, it is the
downstream map_range() API is quite beneficial. It was worth at
least a few fps in xonotic.

Quoted 5 lines

And, possibly getting off the subject a bit, but I was wondering about the possibility of going one step further and batching up mapping and/or unmapping multiple buffers (ranges) at once. I have a pretty convenient sync point in drm/msm to flush out multiple mappings before kicking gpu.

Quoted 3 lines

I think you should be able to do that with this API already - at least the mapping part since we are passing in a sg list (this could be a chained sglist).

What I mean by batching up is mapping and unmapping multiple sglists each at different iova's with minmal cpu cache and iommu tlb flushes..

Ideally we'd let the IOMMU driver be clever and build out all 2nd level tables before inserting into first level tables (to minimize cpu cache flushing).. also, there is probably a reasonable chance that we'd be mapping a new buffer into existing location, so there might be some potential to reuse existing 2nd level tables (and save a tiny bit of free/alloc). I've not thought too much about how that would look in code.. might be kinda, umm, fun..

But at an API level, we should be able to do a bunch of map/unmap_range's with one flush.

Maybe it could look like a sequence of iommu_{map,unmap}_range() followed by iommu_flush()?

Quoted 105 lines · Rob Clark

Quoted 86 lines · Olav Haugan

Quoted 79 lines · Rob Clark

Quoted 65 lines · Olav Haugan

Hi Hiroshi,

Quoted 31 lines · Hiroshi Doyu

Hi Olav,

Quoted 27 lines · Olav Haugan

Mapping and unmapping are more often than not in the critical path. map_range and unmap_range allows SMMU driver implementations to optimize the process of mapping and unmapping buffers into the SMMU page tables. Instead of mapping one physical address, do TLB operation (expensive), mapping, do TLB operation, mapping, do TLB operation the driver can map a scatter-gatherlist of physically contiguous pages into one virtual address space and then at the end do one TLB operation.

Additionally, the mapping operation would be faster in general since clients does not have to keep calling map API over and over again for each physically contiguous chunk of memory that needs to be mapped to a virtually contiguous region.

Signed-off-by Olav Haugan <[email protected]>

---
 drivers/iommu/iommu.c | 24 ++++++++++++++++++++++++
 include/linux/iommu.h | 24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

drivers/iommu/iommu.c +0 -0

index e5555fc..f2a6b80 100644
@@ -898,6 +898,30 @@ size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size)
 EXPORT_SYMBOL_GPL(iommu_unmap);
 
 

Quoted 14 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 911

Quoted 11 lines

drivers/iommu/iommu.c +11 -0

+int iommu_map_range(struct iommu_domain *domain, unsigned int iova,
+                struct scatterlist *sg, unsigned int len, int prot)
+{
+    if (unlikely(domain->ops->map_range == NULL))
+            return -ENODEV;
+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->map_range(domain, iova, sg, len, prot);
+}
+EXPORT_SYMBOL_GPL(iommu_map_range);

We have the similar one internally, which is named, "iommu_map_sg()", called from DMA API.

Great, so this new API will be useful to more people!

Quoted 5 lines

Quoted 5 lines

drivers/iommu/iommu.c +5 -0

+int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,
+                  unsigned int len)
+{
+    if (unlikely(domain->ops->unmap_range == NULL))
+            return -ENODEV;

Quoted 8 lines

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 922

Quoted 6 lines

drivers/iommu/iommu.c +6 -0

+
+    BUG_ON(iova & (~PAGE_MASK));
+
+    return domain->ops->unmap_range(domain, iova, len);
+}
+EXPORT_SYMBOL_GPL(iommu_unmap_range);

Can the existing iommu_unmap() do the same?

Quoted 8 lines

I believe iommu_unmap() behaves a bit differently because it will keep on calling domain->ops->unmap() until everything is unmapped instead of letting the iommu implementation take care of unmapping everything in one call.

I am abandoning the patch series since our driver was not accepted. However, if there are no objections I will resubmit this patch (PATCH 2/7) as an independent patch to add this new map_range API.

+1 for map_range().. I've seen for gpu workloads, at least, it is the
downstream map_range() API is quite beneficial. It was worth at
least a few fps in xonotic.

Quoted 5 lines

And, possibly getting off the subject a bit, but I was wondering about the possibility of going one step further and batching up mapping and/or unmapping multiple buffers (ranges) at once. I have a pretty convenient sync point in drm/msm to flush out multiple mappings before kicking gpu.

Quoted 3 lines

I think you should be able to do that with this API already - at least the mapping part since we are passing in a sg list (this could be a chained sglist).

What I mean by batching up is mapping and unmapping multiple sglists each at different iova's with minmal cpu cache and iommu tlb flushes..

Ideally we'd let the IOMMU driver be clever and build out all 2nd level tables before inserting into first level tables (to minimize cpu cache flushing).. also, there is probably a reasonable chance that we'd be mapping a new buffer into existing location, so there might be some potential to reuse existing 2nd level tables (and save a tiny bit of free/alloc). I've not thought too much about how that would look in code.. might be kinda, umm, fun..

But at an API level, we should be able to do a bunch of map/unmap_range's with one flush.

Quoted 3 lines

Maybe it could look like a sequence of iommu_{map,unmap}_range() followed by iommu_flush()?

So we could add another argument ("options") in the range api that allows you to indicate whether you want to invalidate TLB or not.

Thanks,

Olav

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

drivers/iommu/iommu.c +13 -0

+int iommu_map_range(struct iommu_domain *domain, unsigned int iova,
+		    struct scatterlist *sg, unsigned int len, int prot)
+{
+	if (unlikely(domain->ops->map_range == NULL))
+		return -ENODEV;
+
+	BUG_ON(iova & (~PAGE_MASK));
+
+	return domain->ops->map_range(domain, iova, sg, len, prot);
+}
+EXPORT_SYMBOL_GPL(iommu_map_range);
+
+int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,

Quoted 13 lines · Olav Haugan

drivers/iommu/iommu.c +13 -0

+int iommu_map_range(struct iommu_domain *domain, unsigned int iova,
+		    struct scatterlist *sg, unsigned int len, int prot)
+{
+	if (unlikely(domain->ops->map_range == NULL))
+		return -ENODEV;
+
+	BUG_ON(iova & (~PAGE_MASK));
+
+	return domain->ops->map_range(domain, iova, sg, len, prot);
+}
+EXPORT_SYMBOL_GPL(iommu_map_range);
+
+int iommu_unmap_range(struct iommu_domain *domain, unsigned int iova,

drivers/iommu/iommu.c size_t iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) · line 923

Quoted 10 lines

drivers/iommu/iommu.c +10 -0

+		      unsigned int len)
+{
+	if (unlikely(domain->ops->unmap_range == NULL))
+		return -ENODEV;
+
+	BUG_ON(iova & (~PAGE_MASK));
+
+	return domain->ops->unmap_range(domain, iova, len);
+}
+EXPORT_SYMBOL_GPL(iommu_unmap_range);

Before introducing these new API functions there should be a fall-back for IOMMU drivers that do (not yet) implement the map_range and unmap_range call-backs.

The last thing we want is this kind of functional partitioning between different IOMMU drivers.

include/linux/iopoll.h +0 -0

+/**
+ * readl_poll_timeout - Periodically poll an address until a condition is met or a timeout occurs
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @sleep_us: Maximum time to sleep between reads in uS (0 tight-loops)

+ * @timeout_us: Timeout in uS, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val. Must not
+ * be called from atomic context if sleep_us or timeout_us are used.
+ */

include/linux/iopoll.h +7 -0

+#define readl_poll_timeout(addr, val, cond, sleep_us, timeout_us) \
+({ \
+	ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+	might_sleep_if(timeout_us); \
+	for (;;) { \
+		(val) = readl(addr); \
+		if (cond) \

From: Matt Wagantall <[email protected]>

It is sometimes necessary to poll a memory-mapped register until its value satisfies some condition. Introduce a family of convenience macros that do this. Tight-loop and sleeping versions are provided with and without timeouts.

+/**
+ * readl_poll_timeout - Periodically poll an address until a condition is met or a timeout occurs
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @sleep_us: Maximum time to sleep between reads in uS (0 tight-loops)
+ * @timeout_us: Timeout in uS, 0 means never timeout

+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val. Must not
+ * be called from atomic context if sleep_us or timeout_us are used.
+ */

include/linux/iopoll.h new file +14 -0

+#define readl_poll_timeout(addr, val, cond, sleep_us, timeout_us) \
+({ \
+	ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+	might_sleep_if(timeout_us); \
+	for (;;) { \
+		(val) = readl(addr); \
+		if (cond) \
+			break; \
+		if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+			(val) = readl(addr); \
+			break; \
+		} \
+		if (sleep_us) \
+			usleep_range(DIV_ROUND_UP(sleep_us, 4), sleep_us); \

+ * @time_between_us: Time to udelay() between successive reads
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout.
+ */
+#define readl_poll_timeout_noirq(addr, val, cond, max_reads, time_between_us) \

include/linux/iopoll.h new file +12 -0

+({ \
+	int count; \
+	for (count = (max_reads); count > 0; count--) { \
+		(val) = readl(addr); \
+		if (cond) \
+			break; \
+		udelay(time_between_us); \
+	} \
+	(cond) ? 0 : -ETIMEDOUT; \
+})
+
+/**

include/linux/iopoll.h new file +14 -0

+/**
+ * readl_tight_poll_timeout - Tight-loop on an address until a condition is met or a timeout occurs
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @timeout_us: Timeout in uS, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val. Must not
+ * be called from atomic context if timeout_us is used.
+ */
+#define readl_tight_poll_timeout(addr, val, cond, timeout_us) \
+	readl_poll_timeout(addr, val, cond, 0, timeout_us)
+

MSM IOMMUv1 driver supports Qualcomm SoC MSM8974 and MSM8084.

The IOMMU driver supports the following features:

- ARM V7S page table format independent of ARM CPU page table format
- 4K/64K/1M/16M mappings (V7S)
- ATOS used for unit testing of driver
- Sharing of page tables among SMMUs
- Verbose context bank fault reporting
- Verbose global fault reporting
- Support for clocks and GDSC
- map/unmap range
- Domain specific enabling of coherent Hardware Table Walk (HTW)

Signed-off-by Olav Haugan <[email protected]>

---
 .../devicetree/bindings/iommu/msm,iommu_v1.txt     |   56 +
 drivers/iommu/Kconfig                              |   36 +
 drivers/iommu/Makefile                             |    2 +
 drivers/iommu/msm_iommu-v1.c                       | 1448 +++++++++++++
 drivers/iommu/msm_iommu.c                          |  149 ++
 drivers/iommu/msm_iommu_dev-v1.c                   |  340 +++
 drivers/iommu/msm_iommu_hw-v1.h                    | 2236 ++++++++++++++++++++
 drivers/iommu/msm_iommu_pagetable.c                |  600 ++++++
 drivers/iommu/msm_iommu_pagetable.h                |   33 +
 drivers/iommu/msm_iommu_priv.h                     |   55 +
 include/linux/qcom_iommu.h                         |  221 ++
 11 files changed, 5176 insertions(+)

Hi Olav,

Quoted 16 lines · Olav Haugan

MSM IOMMUv1 driver supports Qualcomm SoC MSM8974 and MSM8084.

The IOMMU driver supports the following features:

- ARM V7S page table format independent of ARM CPU page table format
- 4K/64K/1M/16M mappings (V7S)
- ATOS used for unit testing of driver
- Sharing of page tables among SMMUs
- Verbose context bank fault reporting
- Verbose global fault reporting
- Support for clocks and GDSC
- map/unmap range
- Domain specific enabling of coherent Hardware Table Walk (HTW)

Signed-off-by Olav Haugan <[email protected]>

Quoted 13 lines

---
 .../devicetree/bindings/iommu/msm,iommu_v1.txt     |   56 +
 drivers/iommu/Kconfig                              |   36 +
 drivers/iommu/Makefile                             |    2 +
 drivers/iommu/msm_iommu-v1.c                       | 1448 +++++++++++++
 drivers/iommu/msm_iommu.c                          |  149 ++
 drivers/iommu/msm_iommu_dev-v1.c                   |  340 +++
 drivers/iommu/msm_iommu_hw-v1.h                    | 2236 ++++++++++++++++++++
 drivers/iommu/msm_iommu_pagetable.c                |  600 ++++++
 drivers/iommu/msm_iommu_pagetable.h                |   33 +
 drivers/iommu/msm_iommu_priv.h                     |   55 +
 include/linux/qcom_iommu.h                         |  221 ++
 11 files changed, 5176 insertions(+)

This patch is *huge*! It may get bounced from some lists (I think the linux-arm-kernel lists has a ~100k limit), so it might be worth trying to do this incrementally.

That said, a quick glance at your code indicates that this IOMMU is compliant with the ARM SMMU architecture, and we already have a driver for that. Please can you rework this series to build on top of the code in mainline already, rather than simply duplicating it? We need fewer IOMMU drivers, not more!

-----Original Message-----
From: iommu-bounces at lists.linux-foundation.org [mailto:iommu- bounces at lists.linux-foundation.org] On Behalf Of Olav Haugan
Sent: Monday, June 30, 2014 10:22 PM
To: linux-arm-kernel at lists.infradead.org; iommu at lists.linux- foundation.org
Cc: linux-arm-msm at vger.kernel.org; will.deacon at arm.com; thierry.reding at gmail.com; vgandhi at codeaurora.org
Subject: [RFC/PATCH 7/7] iommu-api: Add domain attribute to enable coherent HTW

Add a new iommu domain attribute that can be used to enable cache coherent hardware table walks (HTW) by the SMMU. HTW might be supported by the SMMU HW but depending on the use case and the usage of the SMMU in the SoC it might not be always beneficial to always turn on coherent HTW for all domains/iommu's.

Signed-off-by Olav Haugan <[email protected]>

---
 drivers/iommu/msm_iommu-v1.c | 16 ++++++++++++++++
 include/linux/iommu.h        |  1 +
 2 files changed, 17 insertions(+)

drivers/iommu/msm_iommu-v1.c +16 -0

index 2c574ef..e163ffc 100644
@@ -1456,8 +1456,16 @@ static int msm_domain_get_attr(struct iommu_domain
 *domain,
 			       enum iommu_attr attr, void *data)  {
 	s32 ret = 0;
+	struct msm_iommu_priv *priv = domain->priv;
 
 	switch (attr) {
+	case DOMAIN_ATTR_COHERENT_HTW:
+	{
+		s32 *int_ptr = (s32 *) data;
+
+		*int_ptr = priv->pt.redirect;
+		break;
+	}
 	default:
 		pr_err("Unsupported attribute type\n");
 		ret = -EINVAL;

@@ -1471,8 +1479,16 @@ static int msm_domain_set_attr(struct iommu_domain
 *domain,
 			       enum iommu_attr attr, void *data)  {
 	s32 ret = 0;
+	struct msm_iommu_priv *priv = domain->priv;
 
 	switch (attr) {
+	case DOMAIN_ATTR_COHERENT_HTW:
+	{
+		s32 *int_ptr = (s32 *) data;
+
+		priv->pt.redirect = *int_ptr;
+		break;
+	}
 	default:
 		pr_err("Unsupported attribute type\n");
 		ret = -EINVAL;

include/linux/iommu.h index renamed from include/linux/iommu.h +0 -0

63dca6d..6d9596d 100644

Quoted 13 lines

-----Original Message-----
From: iommu-bounces at lists.linux-foundation.org [mailto:iommu- bounces at lists.linux-foundation.org] On Behalf Of Olav Haugan
Sent: Monday, June 30, 2014 10:22 PM
To: linux-arm-kernel at lists.infradead.org; iommu at lists.linux- foundation.org
Cc: linux-arm-msm at vger.kernel.org; will.deacon at arm.com; thierry.reding at gmail.com; vgandhi at codeaurora.org
Subject: [RFC/PATCH 7/7] iommu-api: Add domain attribute to enable coherent HTW

Quoted 6 lines

Add a new iommu domain attribute that can be used to enable cache coherent hardware table walks (HTW) by the SMMU. HTW might be supported by the SMMU HW but depending on the use case and the usage of the SMMU in the SoC it might not be always beneficial to always turn on coherent HTW for all domains/iommu's.

[Sethi Varun-B16395] Why won't you want to use the coherent table walk feature?

Quoted 46 lines

Signed-off-by Olav Haugan <[email protected]>

---
 drivers/iommu/msm_iommu-v1.c | 16 ++++++++++++++++
 include/linux/iommu.h        |  1 +
 2 files changed, 17 insertions(+)

drivers/iommu/msm_iommu-v1.c +16 -0

index 2c574ef..e163ffc 100644
@@ -1456,8 +1456,16 @@ static int msm_domain_get_attr(struct iommu_domain
 *domain,
 			       enum iommu_attr attr, void *data)  {
 	s32 ret = 0;
+	struct msm_iommu_priv *priv = domain->priv;
 
 	switch (attr) {
+	case DOMAIN_ATTR_COHERENT_HTW:
+	{
+		s32 *int_ptr = (s32 *) data;
+
+		*int_ptr = priv->pt.redirect;
+		break;
+	}
 	default:
 		pr_err("Unsupported attribute type\n");
 		ret = -EINVAL;

@@ -1471,8 +1479,16 @@ static int msm_domain_set_attr(struct iommu_domain
 *domain,
 			       enum iommu_attr attr, void *data)  {
 	s32 ret = 0;
+	struct msm_iommu_priv *priv = domain->priv;
 
 	switch (attr) {
+	case DOMAIN_ATTR_COHERENT_HTW:
+	{
+		s32 *int_ptr = (s32 *) data;
+
+		priv->pt.redirect = *int_ptr;
+		break;
+	}
 	default:
 		pr_err("Unsupported attribute type\n");
 		ret = -EINVAL;

include/linux/iommu.h index renamed from include/linux/iommu.h +0 -0

63dca6d..6d9596d 100644

include/linux/iommu.h enum iommu_attr { · line 84

Quoted 7 lines

include/linux/iommu.h +1 -0

@@ -81,6 +81,7 @@ enum iommu_attr {
 	DOMAIN_ATTR_FSL_PAMU_STASH,
 	DOMAIN_ATTR_FSL_PAMU_ENABLE,
 	DOMAIN_ATTR_FSL_PAMUV1,
+	DOMAIN_ATTR_COHERENT_HTW,

[Sethi Varun-B16395] Would it make sense to represent this as DOMAIN_ATTR_SMMU_COHERENT_HTW? I believe this is specific to SMMU.

Quoted 19 lines · Varun Sethi

Quoted 19 lines

-----Original Message-----
From: iommu-bounces at lists.linux-foundation.org [mailto:iommu- bounces at lists.linux-foundation.org] On Behalf Of Olav Haugan
Sent: Monday, June 30, 2014 10:22 PM
To: linux-arm-kernel at lists.infradead.org; iommu at lists.linux- foundation.org
Cc: linux-arm-msm at vger.kernel.org; will.deacon at arm.com; thierry.reding at gmail.com; vgandhi at codeaurora.org
Subject: [RFC/PATCH 7/7] iommu-api: Add domain attribute to enable coherent HTW

Add a new iommu domain attribute that can be used to enable cache coherent hardware table walks (HTW) by the SMMU. HTW might be supported by the SMMU HW but depending on the use case and the usage of the SMMU in the SoC it might not be always beneficial to always turn on coherent HTW for all domains/iommu's.

Quoted 1 line

[Sethi Varun-B16395] Why won't you want to use the coherent table walk feature?

Very good question. We have found that turning on IOMMU coherent HTW is not always beneficial to performance (performance either the same or slightly worse in some cases). Even if the perf. is the same we would like to avoid using precious L2 cache for no benefit to the IOMMU. Although our HW supports this feature we don't always want to turn this on for a specific use case/domain (bus master).

Quoted 20 lines · Olav Haugan

Quoted 19 lines · Varun Sethi

Quoted 19 lines

-----Original Message-----
From: iommu-bounces at lists.linux-foundation.org [mailto:iommu- bounces at lists.linux-foundation.org] On Behalf Of Olav Haugan
Sent: Monday, June 30, 2014 10:22 PM
To: linux-arm-kernel at lists.infradead.org; iommu at lists.linux- foundation.org
Cc: linux-arm-msm at vger.kernel.org; will.deacon at arm.com; thierry.reding at gmail.com; vgandhi at codeaurora.org
Subject: [RFC/PATCH 7/7] iommu-api: Add domain attribute to enable coherent HTW

Add a new iommu domain attribute that can be used to enable cache coherent hardware table walks (HTW) by the SMMU. HTW might be supported by the SMMU HW but depending on the use case and the usage of the SMMU in the SoC it might not be always beneficial to always turn on coherent HTW for all domains/iommu's.

Quoted 8 lines

Quoted 1 line

[Sethi Varun-B16395] Why won't you want to use the coherent table walk feature?

Very good question. We have found that turning on IOMMU coherent HTW is not always beneficial to performance (performance either the same or slightly worse in some cases). Even if the perf. is the same we would like to avoid using precious L2 cache for no benefit to the IOMMU. Although our HW supports this feature we don't always want to turn this on for a specific use case/domain (bus master).

Could we at least invert the feature flag, please? i.e. you set an attribute to *disable* coherent walks? I'd also be interested to see some performance numbers, as the added cacheflushing overhead from non-coherent walks is going to be non-trivial.