[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Fernando Guzman Lugo]

From: Ramesh Gupta <grgupta@ti.com>

This patch is to flush the iommu page table entries from L1 and L2
caches using dma_map_single. This also simplifies the implementation
by removing the functions  flush_iopgd_range/flush_iopte_range.

Change-Id: I19c0bf437d75c79084b2fa28c7da50a4c84b91a0
Signed-off-by: Ramesh Gupta <grgupta@ti.com>
Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
Signed-off-by: Fernando Guzman Lugo <fernando.lugo@ti.com>
---
 arch/arm/plat-omap/iommu.c |   41 ++++++++++-------------------------------
 1 files changed, 10 insertions(+), 31 deletions(-)

diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 8a51fd5..1fb5e41 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -18,6 +18,7 @@
 #include <linux/ioport.h>
 #include <linux/clk.h>
 #include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
 
 #include <asm/cacheflush.h>
 
@@ -466,29 +467,6 @@ EXPORT_SYMBOL_GPL(foreach_iommu_device);
 
 #endif /* CONFIG_OMAP_IOMMU_DEBUG_MODULE */
 
-/*
- *	H/W pagetable operations
- */
-static void flush_iopgd_range(u32 *first, u32 *last)
-{
-	/* FIXME: L2 cache should be taken care of if it exists */
-	do {
-		asm("mcr	p15, 0, %0, c7, c10, 1 @ flush_pgd"
-		    : : "r" (first));
-		first += L1_CACHE_BYTES / sizeof(*first);
-	} while (first <= last);
-}
-
-static void flush_iopte_range(u32 *first, u32 *last)
-{
-	/* FIXME: L2 cache should be taken care of if it exists */
-	do {
-		asm("mcr	p15, 0, %0, c7, c10, 1 @ flush_pte"
-		    : : "r" (first));
-		first += L1_CACHE_BYTES / sizeof(*first);
-	} while (first <= last);
-}
-
 static void iopte_free(u32 *iopte)
 {
 	/* Note: freed iopte's must be clean ready for re-use */
@@ -515,7 +493,7 @@ static u32 *iopte_alloc(struct iommu *obj, u32 *iopgd, u32 da)
 			return ERR_PTR(-ENOMEM);
 
 		*iopgd = virt_to_phys(iopte) | IOPGD_TABLE;
-		flush_iopgd_range(iopgd, iopgd);
+		dma_map_single(obj->dev, iopgd, 1, DMA_TO_DEVICE);
 
 		dev_vdbg(obj->dev, "%s: a new pte:%p\n", __func__, iopte);
 	} else {
@@ -544,7 +522,7 @@ static int iopgd_alloc_section(struct iommu *obj, u32 da, u32 pa, u32 prot)
 	}
 
 	*iopgd = (pa & IOSECTION_MASK) | prot | IOPGD_SECTION;
-	flush_iopgd_range(iopgd, iopgd);
+	dma_map_single(obj->dev, iopgd, 1, DMA_TO_DEVICE);
 	return 0;
 }
 
@@ -561,7 +539,7 @@ static int iopgd_alloc_super(struct iommu *obj, u32 da, u32 pa, u32 prot)
 
 	for (i = 0; i < 16; i++)
 		*(iopgd + i) = (pa & IOSUPER_MASK) | prot | IOPGD_SUPER;
-	flush_iopgd_range(iopgd, iopgd + 15);
+	dma_map_single(obj->dev, iopgd, 16, DMA_TO_DEVICE);
 	return 0;
 }
 
@@ -574,7 +552,7 @@ static int iopte_alloc_page(struct iommu *obj, u32 da, u32 pa, u32 prot)
 		return PTR_ERR(iopte);
 
 	*iopte = (pa & IOPAGE_MASK) | prot | IOPTE_SMALL;
-	flush_iopte_range(iopte, iopte);
+	dma_map_single(obj->dev, iopte, 1, DMA_TO_DEVICE);
 
 	dev_vdbg(obj->dev, "%s: da:%08x pa:%08x pte:%p *pte:%08x\n",
 		 __func__, da, pa, iopte, *iopte);
@@ -599,7 +577,7 @@ static int iopte_alloc_large(struct iommu *obj, u32 da, u32 pa, u32 prot)
 
 	for (i = 0; i < 16; i++)
 		*(iopte + i) = (pa & IOLARGE_MASK) | prot | IOPTE_LARGE;
-	flush_iopte_range(iopte, iopte + 15);
+	dma_map_single(obj->dev, iopte, 15, DMA_TO_DEVICE);
 	return 0;
 }
 
@@ -703,7 +681,8 @@ static size_t iopgtable_clear_entry_core(struct iommu *obj, u32 da)
 		}
 		bytes *= nent;
 		memset(iopte, 0, nent * sizeof(*iopte));
-		flush_iopte_range(iopte, iopte + (nent - 1) * sizeof(*iopte));
+		dma_map_single(obj->dev, iopte, nent * sizeof(*iopte),
+				 DMA_TO_DEVICE);
 
 		/*
 		 * do table walk to check if this table is necessary or not
@@ -725,7 +704,7 @@ static size_t iopgtable_clear_entry_core(struct iommu *obj, u32 da)
 		bytes *= nent;
 	}
 	memset(iopgd, 0, nent * sizeof(*iopgd));
-	flush_iopgd_range(iopgd, iopgd + (nent - 1) * sizeof(*iopgd));
+	dma_map_single(obj->dev, iopgd, nent * sizeof(*iopgd), DMA_TO_DEVICE);
 out:
 	return bytes;
 }
@@ -770,7 +749,7 @@ static void iopgtable_clear_entry_all(struct iommu *obj)
 			iopte_free(iopte_offset(iopgd, 0));
 
 		*iopgd = 0;
-		flush_iopgd_range(iopgd, iopgd);
+		dma_map_single(obj->dev, iopgd, 1, DMA_TO_DEVICE);
 	}
 
 	flush_iotlb_all(obj);
-- 
1.7.0.4

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Fernando Guzman Lugo]

From: Ramesh Gupta <grgupta@ti.com>

This patch is to flush the iommu page table entries from L1 and L2
caches using dma_map_single. This also simplifies the implementation
by removing the functions  flush_iopgd_range/flush_iopte_range.

Change-Id: I19c0bf437d75c79084b2fa28c7da50a4c84b91a0
Signed-off-by: Ramesh Gupta <grgupta@ti.com>
Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
Signed-off-by: Fernando Guzman Lugo <fernando.lugo@ti.com>
---
 arch/arm/plat-omap/iommu.c |   41 ++++++++++-------------------------------
 1 files changed, 10 insertions(+), 31 deletions(-)

diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 8a51fd5..1fb5e41 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -18,6 +18,7 @@
 #include <linux/ioport.h>
 #include <linux/clk.h>
 #include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
 
 #include <asm/cacheflush.h>
 
@@ -466,29 +467,6 @@ EXPORT_SYMBOL_GPL(foreach_iommu_device);
 
 #endif /* CONFIG_OMAP_IOMMU_DEBUG_MODULE */
 
-/*
- *	H/W pagetable operations
- */
-static void flush_iopgd_range(u32 *first, u32 *last)
-{
-	/* FIXME: L2 cache should be taken care of if it exists */
-	do {
-		asm("mcr	p15, 0, %0, c7, c10, 1 @ flush_pgd"
-		    : : "r" (first));
-		first += L1_CACHE_BYTES / sizeof(*first);
-	} while (first <= last);
-}
-
-static void flush_iopte_range(u32 *first, u32 *last)
-{
-	/* FIXME: L2 cache should be taken care of if it exists */
-	do {
-		asm("mcr	p15, 0, %0, c7, c10, 1 @ flush_pte"
-		    : : "r" (first));
-		first += L1_CACHE_BYTES / sizeof(*first);
-	} while (first <= last);
-}
-
 static void iopte_free(u32 *iopte)
 {
 	/* Note: freed iopte's must be clean ready for re-use */
@@ -515,7 +493,7 @@ static u32 *iopte_alloc(struct iommu *obj, u32 *iopgd, u32 da)
 			return ERR_PTR(-ENOMEM);
 
 		*iopgd = virt_to_phys(iopte) | IOPGD_TABLE;
-		flush_iopgd_range(iopgd, iopgd);
+		dma_map_single(obj->dev, iopgd, 1, DMA_TO_DEVICE);
 
 		dev_vdbg(obj->dev, "%s: a new pte:%p\n", __func__, iopte);
 	} else {
@@ -544,7 +522,7 @@ static int iopgd_alloc_section(struct iommu *obj, u32 da, u32 pa, u32 prot)
 	}
 
 	*iopgd = (pa & IOSECTION_MASK) | prot | IOPGD_SECTION;
-	flush_iopgd_range(iopgd, iopgd);
+	dma_map_single(obj->dev, iopgd, 1, DMA_TO_DEVICE);
 	return 0;
 }
 
@@ -561,7 +539,7 @@ static int iopgd_alloc_super(struct iommu *obj, u32 da, u32 pa, u32 prot)
 
 	for (i = 0; i < 16; i++)
 		*(iopgd + i) = (pa & IOSUPER_MASK) | prot | IOPGD_SUPER;
-	flush_iopgd_range(iopgd, iopgd + 15);
+	dma_map_single(obj->dev, iopgd, 16, DMA_TO_DEVICE);
 	return 0;
 }
 
@@ -574,7 +552,7 @@ static int iopte_alloc_page(struct iommu *obj, u32 da, u32 pa, u32 prot)
 		return PTR_ERR(iopte);
 
 	*iopte = (pa & IOPAGE_MASK) | prot | IOPTE_SMALL;
-	flush_iopte_range(iopte, iopte);
+	dma_map_single(obj->dev, iopte, 1, DMA_TO_DEVICE);
 
 	dev_vdbg(obj->dev, "%s: da:%08x pa:%08x pte:%p *pte:%08x\n",
 		 __func__, da, pa, iopte, *iopte);
@@ -599,7 +577,7 @@ static int iopte_alloc_large(struct iommu *obj, u32 da, u32 pa, u32 prot)
 
 	for (i = 0; i < 16; i++)
 		*(iopte + i) = (pa & IOLARGE_MASK) | prot | IOPTE_LARGE;
-	flush_iopte_range(iopte, iopte + 15);
+	dma_map_single(obj->dev, iopte, 15, DMA_TO_DEVICE);
 	return 0;
 }
 
@@ -703,7 +681,8 @@ static size_t iopgtable_clear_entry_core(struct iommu *obj, u32 da)
 		}
 		bytes *= nent;
 		memset(iopte, 0, nent * sizeof(*iopte));
-		flush_iopte_range(iopte, iopte + (nent - 1) * sizeof(*iopte));
+		dma_map_single(obj->dev, iopte, nent * sizeof(*iopte),
+				 DMA_TO_DEVICE);
 
 		/*
 		 * do table walk to check if this table is necessary or not
@@ -725,7 +704,7 @@ static size_t iopgtable_clear_entry_core(struct iommu *obj, u32 da)
 		bytes *= nent;
 	}
 	memset(iopgd, 0, nent * sizeof(*iopgd));
-	flush_iopgd_range(iopgd, iopgd + (nent - 1) * sizeof(*iopgd));
+	dma_map_single(obj->dev, iopgd, nent * sizeof(*iopgd), DMA_TO_DEVICE);
 out:
 	return bytes;
 }
@@ -770,7 +749,7 @@ static void iopgtable_clear_entry_all(struct iommu *obj)
 			iopte_free(iopte_offset(iopgd, 0));
 
 		*iopgd = 0;
-		flush_iopgd_range(iopgd, iopgd);
+		dma_map_single(obj->dev, iopgd, 1, DMA_TO_DEVICE);
 	}
 
 	flush_iotlb_all(obj);
-- 
1.7.0.4

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Fernando Guzman Lugo]

From: Ramesh Gupta <grgupta@ti.com>

This patch is to flush the iommu page table entries from L1 and L2
caches using dma_map_single. This also simplifies the implementation
by removing the functions  flush_iopgd_range/flush_iopte_range.

Change-Id: I19c0bf437d75c79084b2fa28c7da50a4c84b91a0
Signed-off-by: Ramesh Gupta <grgupta@ti.com>
Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
Signed-off-by: Fernando Guzman Lugo <fernando.lugo@ti.com>
---
 arch/arm/plat-omap/iommu.c |   41 ++++++++++-------------------------------
 1 files changed, 10 insertions(+), 31 deletions(-)

diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 8a51fd5..1fb5e41 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -18,6 +18,7 @@
 #include <linux/ioport.h>
 #include <linux/clk.h>
 #include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
 
 #include <asm/cacheflush.h>
 
@@ -466,29 +467,6 @@ EXPORT_SYMBOL_GPL(foreach_iommu_device);
 
 #endif /* CONFIG_OMAP_IOMMU_DEBUG_MODULE */
 
-/*
- *	H/W pagetable operations
- */
-static void flush_iopgd_range(u32 *first, u32 *last)
-{
-	/* FIXME: L2 cache should be taken care of if it exists */
-	do {
-		asm("mcr	p15, 0, %0, c7, c10, 1 @ flush_pgd"
-		    : : "r" (first));
-		first += L1_CACHE_BYTES / sizeof(*first);
-	} while (first <= last);
-}
-
-static void flush_iopte_range(u32 *first, u32 *last)
-{
-	/* FIXME: L2 cache should be taken care of if it exists */
-	do {
-		asm("mcr	p15, 0, %0, c7, c10, 1 @ flush_pte"
-		    : : "r" (first));
-		first += L1_CACHE_BYTES / sizeof(*first);
-	} while (first <= last);
-}
-
 static void iopte_free(u32 *iopte)
 {
 	/* Note: freed iopte's must be clean ready for re-use */
@@ -515,7 +493,7 @@ static u32 *iopte_alloc(struct iommu *obj, u32 *iopgd, u32 da)
 			return ERR_PTR(-ENOMEM);
 
 		*iopgd = virt_to_phys(iopte) | IOPGD_TABLE;
-		flush_iopgd_range(iopgd, iopgd);
+		dma_map_single(obj->dev, iopgd, 1, DMA_TO_DEVICE);
 
 		dev_vdbg(obj->dev, "%s: a new pte:%p\n", __func__, iopte);
 	} else {
@@ -544,7 +522,7 @@ static int iopgd_alloc_section(struct iommu *obj, u32 da, u32 pa, u32 prot)
 	}
 
 	*iopgd = (pa & IOSECTION_MASK) | prot | IOPGD_SECTION;
-	flush_iopgd_range(iopgd, iopgd);
+	dma_map_single(obj->dev, iopgd, 1, DMA_TO_DEVICE);
 	return 0;
 }
 
@@ -561,7 +539,7 @@ static int iopgd_alloc_super(struct iommu *obj, u32 da, u32 pa, u32 prot)
 
 	for (i = 0; i < 16; i++)
 		*(iopgd + i) = (pa & IOSUPER_MASK) | prot | IOPGD_SUPER;
-	flush_iopgd_range(iopgd, iopgd + 15);
+	dma_map_single(obj->dev, iopgd, 16, DMA_TO_DEVICE);
 	return 0;
 }
 
@@ -574,7 +552,7 @@ static int iopte_alloc_page(struct iommu *obj, u32 da, u32 pa, u32 prot)
 		return PTR_ERR(iopte);
 
 	*iopte = (pa & IOPAGE_MASK) | prot | IOPTE_SMALL;
-	flush_iopte_range(iopte, iopte);
+	dma_map_single(obj->dev, iopte, 1, DMA_TO_DEVICE);
 
 	dev_vdbg(obj->dev, "%s: da:%08x pa:%08x pte:%p *pte:%08x\n",
 		 __func__, da, pa, iopte, *iopte);
@@ -599,7 +577,7 @@ static int iopte_alloc_large(struct iommu *obj, u32 da, u32 pa, u32 prot)
 
 	for (i = 0; i < 16; i++)
 		*(iopte + i) = (pa & IOLARGE_MASK) | prot | IOPTE_LARGE;
-	flush_iopte_range(iopte, iopte + 15);
+	dma_map_single(obj->dev, iopte, 15, DMA_TO_DEVICE);
 	return 0;
 }
 
@@ -703,7 +681,8 @@ static size_t iopgtable_clear_entry_core(struct iommu *obj, u32 da)
 		}
 		bytes *= nent;
 		memset(iopte, 0, nent * sizeof(*iopte));
-		flush_iopte_range(iopte, iopte + (nent - 1) * sizeof(*iopte));
+		dma_map_single(obj->dev, iopte, nent * sizeof(*iopte),
+				 DMA_TO_DEVICE);
 
 		/*
 		 * do table walk to check if this table is necessary or not
@@ -725,7 +704,7 @@ static size_t iopgtable_clear_entry_core(struct iommu *obj, u32 da)
 		bytes *= nent;
 	}
 	memset(iopgd, 0, nent * sizeof(*iopgd));
-	flush_iopgd_range(iopgd, iopgd + (nent - 1) * sizeof(*iopgd));
+	dma_map_single(obj->dev, iopgd, nent * sizeof(*iopgd), DMA_TO_DEVICE);
 out:
 	return bytes;
 }
@@ -770,7 +749,7 @@ static void iopgtable_clear_entry_all(struct iommu *obj)
 			iopte_free(iopte_offset(iopgd, 0));
 
 		*iopgd = 0;
-		flush_iopgd_range(iopgd, iopgd);
+		dma_map_single(obj->dev, iopgd, 1, DMA_TO_DEVICE);
 	}
 
 	flush_iotlb_all(obj);
-- 
1.7.0.4

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Russell King - ARM Linux]

On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> From: Ramesh Gupta <grgupta@ti.com>
> 
> This patch is to flush the iommu page table entries from L1 and L2
> caches using dma_map_single. This also simplifies the implementation
> by removing the functions  flush_iopgd_range/flush_iopte_range.

No.  This usage is just wrong.  If you're going to use the DMA API then
unmap it, otherwise the DMA API debugging will go awol.

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Russell King - ARM Linux]

On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> From: Ramesh Gupta <grgupta@ti.com>
> 
> This patch is to flush the iommu page table entries from L1 and L2
> caches using dma_map_single. This also simplifies the implementation
> by removing the functions  flush_iopgd_range/flush_iopte_range.

No.  This usage is just wrong.  If you're going to use the DMA API then
unmap it, otherwise the DMA API debugging will go awol.

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[KyongHo Cho]

Hi, Russell.

I think we need cache maintenance operations that effect on inner and
outer caches at the same time.

Even though the DMA APIs are not for cache maintenance but for IO mapping,
they are useful for cache maint'
because they operate on inner and outer caches.

As you know, inner cache of Cortex-A is logical cache and outer cache
is physical cache in the programmer's point of view.
We need logical address and physical address at the same time to clean
or invalidate inner and outer cache.
That means we need to translate logical to physical address and it is
sometimes not trivial.

Finally, the kernel will contain many similar routines that do same thing.

On Fri, Apr 15, 2011 at 7:30 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> From: Ramesh Gupta <grgupta@ti.com>
>>
>> This patch is to flush the iommu page table entries from L1 and L2
>> caches using dma_map_single. This also simplifies the implementation
>> by removing the functions  flush_iopgd_range/flush_iopte_range.
>
> No.  This usage is just wrong.  If you're going to use the DMA API then
> unmap it, otherwise the DMA API debugging will go awol.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[KyongHo Cho]

Hi, Russell.

I think we need cache maintenance operations that effect on inner and
outer caches at the same time.

Even though the DMA APIs are not for cache maintenance but for IO mapping,
they are useful for cache maint'
because they operate on inner and outer caches.

As you know, inner cache of Cortex-A is logical cache and outer cache
is physical cache in the programmer's point of view.
We need logical address and physical address at the same time to clean
or invalidate inner and outer cache.
That means we need to translate logical to physical address and it is
sometimes not trivial.

Finally, the kernel will contain many similar routines that do same thing.

On Fri, Apr 15, 2011 at 7:30 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> From: Ramesh Gupta <grgupta@ti.com>
>>
>> This patch is to flush the iommu page table entries from L1 and L2
>> caches using dma_map_single. This also simplifies the implementation
>> by removing the functions ?flush_iopgd_range/flush_iopte_range.
>
> No. ?This usage is just wrong. ?If you're going to use the DMA API then
> unmap it, otherwise the DMA API debugging will go awol.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Russell King - ARM Linux]

On Fri, Apr 15, 2011 at 11:24:16AM +0900, KyongHo Cho wrote:
> That means we need to translate logical to physical address and it is
> sometimes not trivial.

What do you mean "sometimes not trivial" ?  The DMA does nothing more
than virt_to_phys(virt) to get the physical address.  It's _that_ simple.

If virt_to_phys(virt) is likely to fail, there's protection in the DMA API
to BUG_ON() in that case.

> Finally, the kernel will contain many similar routines that do same thing.

So when we get coherent DMA, you won't care that the DMA API functions
start doing nothing with caches?

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Russell King - ARM Linux]

On Fri, Apr 15, 2011 at 11:24:16AM +0900, KyongHo Cho wrote:
> That means we need to translate logical to physical address and it is
> sometimes not trivial.

What do you mean "sometimes not trivial" ?  The DMA does nothing more
than virt_to_phys(virt) to get the physical address.  It's _that_ simple.

If virt_to_phys(virt) is likely to fail, there's protection in the DMA API
to BUG_ON() in that case.

> Finally, the kernel will contain many similar routines that do same thing.

So when we get coherent DMA, you won't care that the DMA API functions
start doing nothing with caches?

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Gupta, Ramesh]

Russell,

On Thu, Apr 14, 2011 at 5:30 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> From: Ramesh Gupta <grgupta@ti.com>
>>
>> This patch is to flush the iommu page table entries from L1 and L2
>> caches using dma_map_single. This also simplifies the implementation
>> by removing the functions  flush_iopgd_range/flush_iopte_range.
>
> No.  This usage is just wrong.  If you're going to use the DMA API then
> unmap it, otherwise the DMA API debugging will go awol.
>

Thank you for the comments, this particular memory is always a write
from the A9 for MMU programming and
only read from the slave processor, that is the reason for not calling
the unmap. I can re-look into the changes to call
unmap in a proper way as this impacts the DMA API.
Are there any other ways to perform only flush the memory from L1/L2 caches?

 regards
Ramesh Gupta G

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Gupta, Ramesh]

Russell,

On Thu, Apr 14, 2011 at 5:30 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> From: Ramesh Gupta <grgupta@ti.com>
>>
>> This patch is to flush the iommu page table entries from L1 and L2
>> caches using dma_map_single. This also simplifies the implementation
>> by removing the functions ?flush_iopgd_range/flush_iopte_range.
>
> No. ?This usage is just wrong. ?If you're going to use the DMA API then
> unmap it, otherwise the DMA API debugging will go awol.
>

Thank you for the comments, this particular memory is always a write
from the A9 for MMU programming and
only read from the slave processor, that is the reason for not calling
the unmap. I can re-look into the changes to call
unmap in a proper way as this impacts the DMA API.
Are there any other ways to perform only flush the memory from L1/L2 caches?

 regards
Ramesh Gupta G

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Arnd Bergmann]

On Friday 15 April 2011, Russell King - ARM Linux wrote:
> On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> > From: Ramesh Gupta <grgupta@ti.com>
> > 
> > This patch is to flush the iommu page table entries from L1 and L2
> > caches using dma_map_single. This also simplifies the implementation
> > by removing the functions  flush_iopgd_range/flush_iopte_range.
> 
> No.  This usage is just wrong.  If you're going to use the DMA API then
> unmap it, otherwise the DMA API debugging will go awol.


It's also completely upside-down: The iommu support should provide interfaces
using the dma-mapping API, not use that API to provide a machine specific
version of the generic interface.

As far as I can tell, nothing actually uses these drivers, maybe we should just
remove them before we get any code in the mainline kernel that depends on it.

	Arnd

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Arnd Bergmann]

On Friday 15 April 2011, Russell King - ARM Linux wrote:
> On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> > From: Ramesh Gupta <grgupta@ti.com>
> > 
> > This patch is to flush the iommu page table entries from L1 and L2
> > caches using dma_map_single. This also simplifies the implementation
> > by removing the functions  flush_iopgd_range/flush_iopte_range.
> 
> No.  This usage is just wrong.  If you're going to use the DMA API then
> unmap it, otherwise the DMA API debugging will go awol.


It's also completely upside-down: The iommu support should provide interfaces
using the dma-mapping API, not use that API to provide a machine specific
version of the generic interface.

As far as I can tell, nothing actually uses these drivers, maybe we should just
remove them before we get any code in the mainline kernel that depends on it.

	Arnd

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Tony Lindgren]

* Arnd Bergmann <arnd@arndb.de> [110418 10:26]:
> On Friday 15 April 2011, Russell King - ARM Linux wrote:
> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> > > From: Ramesh Gupta <grgupta@ti.com>
> > > 
> > > This patch is to flush the iommu page table entries from L1 and L2
> > > caches using dma_map_single. This also simplifies the implementation
> > > by removing the functions  flush_iopgd_range/flush_iopte_range.
> > 
> > No.  This usage is just wrong.  If you're going to use the DMA API then
> > unmap it, otherwise the DMA API debugging will go awol.
> 
> 
> It's also completely upside-down: The iommu support should provide interfaces
> using the dma-mapping API, not use that API to provide a machine specific
> version of the generic interface.
> 
> As far as I can tell, nothing actually uses these drivers, maybe we should just
> remove them before we get any code in the mainline kernel that depends on it.

There is drivers/media/video/omap3isp/isp.c. But if we now
have a generic replacement for this code we should start using it.

Hiroshi, any comments on that?

Regards,

Tony

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Tony Lindgren]

* Arnd Bergmann <arnd@arndb.de> [110418 10:26]:
> On Friday 15 April 2011, Russell King - ARM Linux wrote:
> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> > > From: Ramesh Gupta <grgupta@ti.com>
> > > 
> > > This patch is to flush the iommu page table entries from L1 and L2
> > > caches using dma_map_single. This also simplifies the implementation
> > > by removing the functions  flush_iopgd_range/flush_iopte_range.
> > 
> > No.  This usage is just wrong.  If you're going to use the DMA API then
> > unmap it, otherwise the DMA API debugging will go awol.
> 
> 
> It's also completely upside-down: The iommu support should provide interfaces
> using the dma-mapping API, not use that API to provide a machine specific
> version of the generic interface.
> 
> As far as I can tell, nothing actually uses these drivers, maybe we should just
> remove them before we get any code in the mainline kernel that depends on it.

There is drivers/media/video/omap3isp/isp.c. But if we now
have a generic replacement for this code we should start using it.

Hiroshi, any comments on that?

Regards,

Tony

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Hiroshi DOYU]

From: ext Tony Lindgren <tony@atomide.com>
Subject: Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache
Date: Mon, 18 Apr 2011 14:05:08 +0300

> * Arnd Bergmann <arnd@arndb.de> [110418 10:26]:
>> On Friday 15 April 2011, Russell King - ARM Linux wrote:
>> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> > > From: Ramesh Gupta <grgupta@ti.com>
>> > > 
>> > > This patch is to flush the iommu page table entries from L1 and L2
>> > > caches using dma_map_single. This also simplifies the implementation
>> > > by removing the functions  flush_iopgd_range/flush_iopte_range.
>> > 
>> > No.  This usage is just wrong.  If you're going to use the DMA API then
>> > unmap it, otherwise the DMA API debugging will go awol.
>> 
>> 
>> It's also completely upside-down: The iommu support should provide interfaces
>> using the dma-mapping API, not use that API to provide a machine specific
>> version of the generic interface.
>> 
>> As far as I can tell, nothing actually uses these drivers, maybe we should just
>> remove them before we get any code in the mainline kernel that depends on it.
> 
> There is drivers/media/video/omap3isp/isp.c. But if we now

Yes, and "dspbridge" has introduced this too, IIRC.

> have a generic replacement for this code we should start using it.

I'm afraid that there's no general IOMMU APIs yet, or already? If
there is, migrating to those general IOMMU API is the way, but still
SoC dependent parts remain, anyway. I guess that more or less general
IOMMU API is composed of common set of client APIs(like IOVMM) and the
registration of H/W dependent functions(like omap iommu), I guess.

> Hiroshi, any comments on that?

This patch is not about (1)general buffer handling between CPU cores
but just about (2)page table entry coherency. (2) is quite same to
what ARM does for its pagetable in cpu_*_set_pte_ext. I think that
using dma api to make pte entry coherent may make sense for general
solution, as Russell suggested.

For (1), I agree that IOVMM layer should be refactored by introducing
dma-mapping API in any case. Any patches are appreciated.

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Hiroshi DOYU]

From: ext Tony Lindgren <tony@atomide.com>
Subject: Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache
Date: Mon, 18 Apr 2011 14:05:08 +0300

> * Arnd Bergmann <arnd@arndb.de> [110418 10:26]:
>> On Friday 15 April 2011, Russell King - ARM Linux wrote:
>> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> > > From: Ramesh Gupta <grgupta@ti.com>
>> > > 
>> > > This patch is to flush the iommu page table entries from L1 and L2
>> > > caches using dma_map_single. This also simplifies the implementation
>> > > by removing the functions  flush_iopgd_range/flush_iopte_range.
>> > 
>> > No.  This usage is just wrong.  If you're going to use the DMA API then
>> > unmap it, otherwise the DMA API debugging will go awol.
>> 
>> 
>> It's also completely upside-down: The iommu support should provide interfaces
>> using the dma-mapping API, not use that API to provide a machine specific
>> version of the generic interface.
>> 
>> As far as I can tell, nothing actually uses these drivers, maybe we should just
>> remove them before we get any code in the mainline kernel that depends on it.
> 
> There is drivers/media/video/omap3isp/isp.c. But if we now

Yes, and "dspbridge" has introduced this too, IIRC.

> have a generic replacement for this code we should start using it.

I'm afraid that there's no general IOMMU APIs yet, or already? If
there is, migrating to those general IOMMU API is the way, but still
SoC dependent parts remain, anyway. I guess that more or less general
IOMMU API is composed of common set of client APIs(like IOVMM) and the
registration of H/W dependent functions(like omap iommu), I guess.

> Hiroshi, any comments on that?

This patch is not about (1)general buffer handling between CPU cores
but just about (2)page table entry coherency. (2) is quite same to
what ARM does for its pagetable in cpu_*_set_pte_ext. I think that
using dma api to make pte entry coherent may make sense for general
solution, as Russell suggested.

For (1), I agree that IOVMM layer should be refactored by introducing
dma-mapping API in any case. Any patches are appreciated.

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Arnd Bergmann]

On Monday 18 April 2011, Tony Lindgren wrote:
> * Arnd Bergmann <arnd@arndb.de> [110418 10:26]:
> > On Friday 15 April 2011, Russell King - ARM Linux wrote:
> > > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> > > > From: Ramesh Gupta <grgupta@ti.com>
> > > > 
> > > > This patch is to flush the iommu page table entries from L1 and L2
> > > > caches using dma_map_single. This also simplifies the implementation
> > > > by removing the functions  flush_iopgd_range/flush_iopte_range.
> > > 
> > > No.  This usage is just wrong.  If you're going to use the DMA API then
> > > unmap it, otherwise the DMA API debugging will go awol.
> > 
> > 
> > It's also completely upside-down: The iommu support should provide interfaces
> > using the dma-mapping API, not use that API to provide a machine specific
> > version of the generic interface.
> > 
> > As far as I can tell, nothing actually uses these drivers, maybe we should just
> > remove them before we get any code in the mainline kernel that depends on it.
> 
> There is drivers/media/video/omap3isp/isp.c.

Ah, I didn't see that, was looking at an older version.

> But if we now have a generic replacement for this code we should start
> using it.

To give some background:

Historically, the dma-mapping API has been used for all IOMMUs on
architectures that need it. This interface is rather flexible,
but ARM currently only uses it for managing static mappings.
One thing that it cannot do is mapping memory to an arbitrary
(driver-chosen) bus address. The generic iommu API was added in order
to do that, and is mostly used for virtual machines with KVM.

The MSM platform in ARM also added support for the generic iommu
API, and now the exynos4 is gaining support for it as well.

One missing piece is still a way for a platform to provide both
the iommu and the dma-mapping API in a unified driver. Right now,
you have to export both interface for a generic solution.

On ARM, we don't yet use include/asm-generic/dma-mapping-common.h,
which allows overriding the dma-mapping API per device. This would
have to change if you want to export the IOMMU functionality using
the dma-mapping API, but that would also allow abstracting the
various ways we currently have (dmabounce, swiotlb, linear map,
custom __arch_page_to_dma, iommu, coherent or noncoherent DMA)
in a nicer way per device.

	Arnd

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Arnd Bergmann]

On Monday 18 April 2011, Tony Lindgren wrote:
> * Arnd Bergmann <arnd@arndb.de> [110418 10:26]:
> > On Friday 15 April 2011, Russell King - ARM Linux wrote:
> > > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> > > > From: Ramesh Gupta <grgupta@ti.com>
> > > > 
> > > > This patch is to flush the iommu page table entries from L1 and L2
> > > > caches using dma_map_single. This also simplifies the implementation
> > > > by removing the functions  flush_iopgd_range/flush_iopte_range.
> > > 
> > > No.  This usage is just wrong.  If you're going to use the DMA API then
> > > unmap it, otherwise the DMA API debugging will go awol.
> > 
> > 
> > It's also completely upside-down: The iommu support should provide interfaces
> > using the dma-mapping API, not use that API to provide a machine specific
> > version of the generic interface.
> > 
> > As far as I can tell, nothing actually uses these drivers, maybe we should just
> > remove them before we get any code in the mainline kernel that depends on it.
> 
> There is drivers/media/video/omap3isp/isp.c.

Ah, I didn't see that, was looking at an older version.

> But if we now have a generic replacement for this code we should start
> using it.

To give some background:

Historically, the dma-mapping API has been used for all IOMMUs on
architectures that need it. This interface is rather flexible,
but ARM currently only uses it for managing static mappings.
One thing that it cannot do is mapping memory to an arbitrary
(driver-chosen) bus address. The generic iommu API was added in order
to do that, and is mostly used for virtual machines with KVM.

The MSM platform in ARM also added support for the generic iommu
API, and now the exynos4 is gaining support for it as well.

One missing piece is still a way for a platform to provide both
the iommu and the dma-mapping API in a unified driver. Right now,
you have to export both interface for a generic solution.

On ARM, we don't yet use include/asm-generic/dma-mapping-common.h,
which allows overriding the dma-mapping API per device. This would
have to change if you want to export the IOMMU functionality using
the dma-mapping API, but that would also allow abstracting the
various ways we currently have (dmabounce, swiotlb, linear map,
custom __arch_page_to_dma, iommu, coherent or noncoherent DMA)
in a nicer way per device.

	Arnd

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Kyungmin Park]

On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 18 April 2011, Tony Lindgren wrote:
>> * Arnd Bergmann <arnd@arndb.de> [110418 10:26]:
>> > On Friday 15 April 2011, Russell King - ARM Linux wrote:
>> > > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> > > > From: Ramesh Gupta <grgupta@ti.com>
>> > > >
>> > > > This patch is to flush the iommu page table entries from L1 and L2
>> > > > caches using dma_map_single. This also simplifies the implementation
>> > > > by removing the functions  flush_iopgd_range/flush_iopte_range.
>> > >
>> > > No.  This usage is just wrong.  If you're going to use the DMA API then
>> > > unmap it, otherwise the DMA API debugging will go awol.
>> >
>> >
>> > It's also completely upside-down: The iommu support should provide interfaces
>> > using the dma-mapping API, not use that API to provide a machine specific
>> > version of the generic interface.
>> >
>> > As far as I can tell, nothing actually uses these drivers, maybe we should just
>> > remove them before we get any code in the mainline kernel that depends on it.
>>
>> There is drivers/media/video/omap3isp/isp.c.
>
> Ah, I didn't see that, was looking at an older version.
>
>> But if we now have a generic replacement for this code we should start
>> using it.
>
> To give some background:
>
> Historically, the dma-mapping API has been used for all IOMMUs on
> architectures that need it. This interface is rather flexible,
> but ARM currently only uses it for managing static mappings.
> One thing that it cannot do is mapping memory to an arbitrary
> (driver-chosen) bus address. The generic iommu API was added in order
> to do that, and is mostly used for virtual machines with KVM.
>
> The MSM platform in ARM also added support for the generic iommu
> API, and now the exynos4 is gaining support for it as well.

You can find a exynos4 patches.
http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/31613

>
> One missing piece is still a way for a platform to provide both
> the iommu and the dma-mapping API in a unified driver. Right now,
> you have to export both interface for a generic solution.

Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
implementation into mm, but MM did't accept it.
So now it's implemented at each SoCs. I think it's good chance to make
a generic IOMMU feature for ARM consolidation.

>
> On ARM, we don't yet use include/asm-generic/dma-mapping-common.h,
> which allows overriding the dma-mapping API per device. This would
> have to change if you want to export the IOMMU functionality using
> the dma-mapping API, but that would also allow abstracting the
> various ways we currently have (dmabounce, swiotlb, linear map,
> custom __arch_page_to_dma, iommu, coherent or noncoherent DMA)
> in a nicer way per device.

Before this idea, can you review our implementation at above URL?

Thank you,
Kyungmin Park
>
>        Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Kyungmin Park]

On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 18 April 2011, Tony Lindgren wrote:
>> * Arnd Bergmann <arnd@arndb.de> [110418 10:26]:
>> > On Friday 15 April 2011, Russell King - ARM Linux wrote:
>> > > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> > > > From: Ramesh Gupta <grgupta@ti.com>
>> > > >
>> > > > This patch is to flush the iommu page table entries from L1 and L2
>> > > > caches using dma_map_single. This also simplifies the implementation
>> > > > by removing the functions  flush_iopgd_range/flush_iopte_range.
>> > >
>> > > No.  This usage is just wrong.  If you're going to use the DMA API then
>> > > unmap it, otherwise the DMA API debugging will go awol.
>> >
>> >
>> > It's also completely upside-down: The iommu support should provide interfaces
>> > using the dma-mapping API, not use that API to provide a machine specific
>> > version of the generic interface.
>> >
>> > As far as I can tell, nothing actually uses these drivers, maybe we should just
>> > remove them before we get any code in the mainline kernel that depends on it.
>>
>> There is drivers/media/video/omap3isp/isp.c.
>
> Ah, I didn't see that, was looking at an older version.
>
>> But if we now have a generic replacement for this code we should start
>> using it.
>
> To give some background:
>
> Historically, the dma-mapping API has been used for all IOMMUs on
> architectures that need it. This interface is rather flexible,
> but ARM currently only uses it for managing static mappings.
> One thing that it cannot do is mapping memory to an arbitrary
> (driver-chosen) bus address. The generic iommu API was added in order
> to do that, and is mostly used for virtual machines with KVM.
>
> The MSM platform in ARM also added support for the generic iommu
> API, and now the exynos4 is gaining support for it as well.

You can find a exynos4 patches.
http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/31613

>
> One missing piece is still a way for a platform to provide both
> the iommu and the dma-mapping API in a unified driver. Right now,
> you have to export both interface for a generic solution.

Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
implementation into mm, but MM did't accept it.
So now it's implemented at each SoCs. I think it's good chance to make
a generic IOMMU feature for ARM consolidation.

>
> On ARM, we don't yet use include/asm-generic/dma-mapping-common.h,
> which allows overriding the dma-mapping API per device. This would
> have to change if you want to export the IOMMU functionality using
> the dma-mapping API, but that would also allow abstracting the
> various ways we currently have (dmabounce, swiotlb, linear map,
> custom __arch_page_to_dma, iommu, coherent or noncoherent DMA)
> in a nicer way per device.

Before this idea, can you review our implementation at above URL?

Thank you,
Kyungmin Park
>
>        Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Kyungmin Park]

On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 18 April 2011, Tony Lindgren wrote:
>> * Arnd Bergmann <arnd@arndb.de> [110418 10:26]:
>> > On Friday 15 April 2011, Russell King - ARM Linux wrote:
>> > > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> > > > From: Ramesh Gupta <grgupta@ti.com>
>> > > >
>> > > > This patch is to flush the iommu page table entries from L1 and L2
>> > > > caches using dma_map_single. This also simplifies the implementation
>> > > > by removing the functions ?flush_iopgd_range/flush_iopte_range.
>> > >
>> > > No. ?This usage is just wrong. ?If you're going to use the DMA API then
>> > > unmap it, otherwise the DMA API debugging will go awol.
>> >
>> >
>> > It's also completely upside-down: The iommu support should provide interfaces
>> > using the dma-mapping API, not use that API to provide a machine specific
>> > version of the generic interface.
>> >
>> > As far as I can tell, nothing actually uses these drivers, maybe we should just
>> > remove them before we get any code in the mainline kernel that depends on it.
>>
>> There is drivers/media/video/omap3isp/isp.c.
>
> Ah, I didn't see that, was looking at an older version.
>
>> But if we now have a generic replacement for this code we should start
>> using it.
>
> To give some background:
>
> Historically, the dma-mapping API has been used for all IOMMUs on
> architectures that need it. This interface is rather flexible,
> but ARM currently only uses it for managing static mappings.
> One thing that it cannot do is mapping memory to an arbitrary
> (driver-chosen) bus address. The generic iommu API was added in order
> to do that, and is mostly used for virtual machines with KVM.
>
> The MSM platform in ARM also added support for the generic iommu
> API, and now the exynos4 is gaining support for it as well.

You can find a exynos4 patches.
http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/31613

>
> One missing piece is still a way for a platform to provide both
> the iommu and the dma-mapping API in a unified driver. Right now,
> you have to export both interface for a generic solution.

Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
implementation into mm, but MM did't accept it.
So now it's implemented at each SoCs. I think it's good chance to make
a generic IOMMU feature for ARM consolidation.

>
> On ARM, we don't yet use include/asm-generic/dma-mapping-common.h,
> which allows overriding the dma-mapping API per device. This would
> have to change if you want to export the IOMMU functionality using
> the dma-mapping API, but that would also allow abstracting the
> various ways we currently have (dmabounce, swiotlb, linear map,
> custom __arch_page_to_dma, iommu, coherent or noncoherent DMA)
> in a nicer way per device.

Before this idea, can you review our implementation at above URL?

Thank you,
Kyungmin Park
>
> ? ? ? ?Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Arnd Bergmann]

On Monday 18 April 2011, Hiroshi DOYU wrote:
> From: ext Tony Lindgren <tony@atomide.com>

> > have a generic replacement for this code we should start using it.
> 
> I'm afraid that there's no general IOMMU APIs yet, or already? If
> there is, migrating to those general IOMMU API is the way, but still
> SoC dependent parts remain, anyway. I guess that more or less general
> IOMMU API is composed of common set of client APIs(like IOVMM) and the
> registration of H/W dependent functions(like omap iommu), I guess.

As I explained, we have a surplus of generic iommu APIs
already, see include/linux/dma-mapping.h and include/linux/iommu.h
 
The dma-mapping interface handles IOMMUs on all other architectures
that have them.

	Arnd

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Arnd Bergmann]

On Monday 18 April 2011, Hiroshi DOYU wrote:
> From: ext Tony Lindgren <tony@atomide.com>

> > have a generic replacement for this code we should start using it.
> 
> I'm afraid that there's no general IOMMU APIs yet, or already? If
> there is, migrating to those general IOMMU API is the way, but still
> SoC dependent parts remain, anyway. I guess that more or less general
> IOMMU API is composed of common set of client APIs(like IOVMM) and the
> registration of H/W dependent functions(like omap iommu), I guess.

As I explained, we have a surplus of generic iommu APIs
already, see include/linux/dma-mapping.h and include/linux/iommu.h
 
The dma-mapping interface handles IOMMUs on all other architectures
that have them.

	Arnd

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Arnd Bergmann]

On Monday 18 April 2011, Kyungmin Park wrote:
> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > One missing piece is still a way for a platform to provide both
> > the iommu and the dma-mapping API in a unified driver. Right now,
> > you have to export both interface for a generic solution.
> 
> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
> implementation into mm, but MM did't accept it.

I'm confused. What do you mean with MM?

> So now it's implemented at each SoCs. I think it's good chance to make
> a generic IOMMU feature for ARM consolidation.
>
> Before this idea, can you review our implementation at above URL?

I've commented on the main implementation for the IOMMU now.

	Arnd

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Arnd Bergmann]

On Monday 18 April 2011, Kyungmin Park wrote:
> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > One missing piece is still a way for a platform to provide both
> > the iommu and the dma-mapping API in a unified driver. Right now,
> > you have to export both interface for a generic solution.
> 
> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
> implementation into mm, but MM did't accept it.

I'm confused. What do you mean with MM?

> So now it's implemented at each SoCs. I think it's good chance to make
> a generic IOMMU feature for ARM consolidation.
>
> Before this idea, can you review our implementation at above URL?

I've commented on the main implementation for the IOMMU now.

	Arnd

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Kyungmin Park]

On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 18 April 2011, Kyungmin Park wrote:
>> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> >
>> > One missing piece is still a way for a platform to provide both
>> > the iommu and the dma-mapping API in a unified driver. Right now,
>> > you have to export both interface for a generic solution.
>>
>> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
>> implementation into mm, but MM did't accept it.
>
> I'm confused. What do you mean with MM?
linux/mm, Memory Management.

Thank you,
Kyungmin Park
>
>> So now it's implemented at each SoCs. I think it's good chance to make
>> a generic IOMMU feature for ARM consolidation.
>>
>> Before this idea, can you review our implementation at above URL?
>
> I've commented on the main implementation for the IOMMU now.
>
>        Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Kyungmin Park]

On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 18 April 2011, Kyungmin Park wrote:
>> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> >
>> > One missing piece is still a way for a platform to provide both
>> > the iommu and the dma-mapping API in a unified driver. Right now,
>> > you have to export both interface for a generic solution.
>>
>> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
>> implementation into mm, but MM did't accept it.
>
> I'm confused. What do you mean with MM?
linux/mm, Memory Management.

Thank you,
Kyungmin Park
>
>> So now it's implemented at each SoCs. I think it's good chance to make
>> a generic IOMMU feature for ARM consolidation.
>>
>> Before this idea, can you review our implementation at above URL?
>
> I've commented on the main implementation for the IOMMU now.
>
> ? ? ? ?Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Arnd Bergmann]

On Tuesday 19 April 2011, Kyungmin Park wrote:
> On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday 18 April 2011, Kyungmin Park wrote:
> >> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> >
> >> > One missing piece is still a way for a platform to provide both
> >> > the iommu and the dma-mapping API in a unified driver. Right now,
> >> > you have to export both interface for a generic solution.
> >>
> >> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
> >> implementation into mm, but MM did't accept it.
> >
> > I'm confused. What do you mean with MM?
> linux/mm, Memory Management.

I'm still confused. What were you suggesting to merge in there?
Do you have a link to a mailing list discussion?

	Arnd

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Arnd Bergmann]

On Tuesday 19 April 2011, Kyungmin Park wrote:
> On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday 18 April 2011, Kyungmin Park wrote:
> >> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> >
> >> > One missing piece is still a way for a platform to provide both
> >> > the iommu and the dma-mapping API in a unified driver. Right now,
> >> > you have to export both interface for a generic solution.
> >>
> >> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
> >> implementation into mm, but MM did't accept it.
> >
> > I'm confused. What do you mean with MM?
> linux/mm, Memory Management.

I'm still confused. What were you suggesting to merge in there?
Do you have a link to a mailing list discussion?

	Arnd

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Kyungmin Park]

On Tue, Apr 19, 2011 at 9:01 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 19 April 2011, Kyungmin Park wrote:
>> On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Monday 18 April 2011, Kyungmin Park wrote:
>> >> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> >> >
>> >> > One missing piece is still a way for a platform to provide both
>> >> > the iommu and the dma-mapping API in a unified driver. Right now,
>> >> > you have to export both interface for a generic solution.
>> >>
>> >> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
>> >> implementation into mm, but MM did't accept it.
>> >
>> > I'm confused. What do you mean with MM?
>> linux/mm, Memory Management.
>
> I'm still confused. What were you suggesting to merge in there?
> Do you have a link to a mailing list discussion?

First, Zach Pfeffer <zpfeffer@codeaurora.org> sent the patch
https://patchwork.kernel.org/patch/108736/

Second, Michal tried it.
http://lkml.org/lkml/2010/9/6/41

http://lwn.net/Articles/403643/
https://patchwork.kernel.org/patch/157451/

Thank you,
Kyungmin Park

>
>        Arnd
>

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Kyungmin Park]

On Tue, Apr 19, 2011 at 9:01 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 19 April 2011, Kyungmin Park wrote:
>> On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Monday 18 April 2011, Kyungmin Park wrote:
>> >> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> >> >
>> >> > One missing piece is still a way for a platform to provide both
>> >> > the iommu and the dma-mapping API in a unified driver. Right now,
>> >> > you have to export both interface for a generic solution.
>> >>
>> >> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
>> >> implementation into mm, but MM did't accept it.
>> >
>> > I'm confused. What do you mean with MM?
>> linux/mm, Memory Management.
>
> I'm still confused. What were you suggesting to merge in there?
> Do you have a link to a mailing list discussion?

First, Zach Pfeffer <zpfeffer@codeaurora.org> sent the patch
https://patchwork.kernel.org/patch/108736/

Second, Michal tried it.
http://lkml.org/lkml/2010/9/6/41

http://lwn.net/Articles/403643/
https://patchwork.kernel.org/patch/157451/

Thank you,
Kyungmin Park

>
> ? ? ? ?Arnd
>

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Russell King - ARM Linux]

On Tue, Apr 19, 2011 at 09:35:56PM +0900, Kyungmin Park wrote:
> On Tue, Apr 19, 2011 at 9:01 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 19 April 2011, Kyungmin Park wrote:
> >> On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Monday 18 April 2011, Kyungmin Park wrote:
> >> >> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> >> >
> >> >> > One missing piece is still a way for a platform to provide both
> >> >> > the iommu and the dma-mapping API in a unified driver. Right now,
> >> >> > you have to export both interface for a generic solution.
> >> >>
> >> >> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
> >> >> implementation into mm, but MM did't accept it.
> >> >
> >> > I'm confused. What do you mean with MM?
> >> linux/mm, Memory Management.
> >
> > I'm still confused. What were you suggesting to merge in there?
> > Do you have a link to a mailing list discussion?
> 
> First, Zach Pfeffer <zpfeffer@codeaurora.org> sent the patch
> https://patchwork.kernel.org/patch/108736/
> 
> Second, Michal tried it.
> http://lkml.org/lkml/2010/9/6/41
> 
> http://lwn.net/Articles/403643/
> https://patchwork.kernel.org/patch/157451/

This shows why ARM is in such a problem.  People love to create new
frameworks and infrastructure where they find existing stuff lacking,
rather than looking at what other architectures do and adapt.

As I'm sure has already mentioned, the kernel already has support for
IOMMUs in the DMA path with several non-ARM architectures, and this
support is managed through the existing DMA API.

When you want a device behind an IOMMU to perform DMA, the driver
uses the DMA API in the standard way as if there was no IOMMU there.
As part of the DMA API, particularly the SG list part, the DMA API
is allowed to coalesce the SG entries together if it can arrange
the IOMMU mappings to achieve a continguous mapping for the device.
Remember, dma_map_sg() is allowed to return _fewer_ entries in the
scatterlist than was passed to it for this very purpose.

In order to transition ARM over to this, we need to modify the
scatterlist structure by enabling CONFIG_NEED_SG_DMA_LENGTH.  We
then need to arrange for the DMA API to have the hooks _both_ into
the DMA cache coherency code (to ensure that the data hits memory)
and the IOMMU code (this part is missing from ARM.)  The current
abstractions in the generic header files don't allow for this.

I don't believe there's any need for any major new framework - and I
don't think I'm alone in thinking that, especially as this point has
been raised more than once when this framework has been submitted.

I see no reason why ARM should be any different from other architectures
which have IOMMUs, and I don't see why ARM should have to invent a whole
new framework to handle IOMMUs.  And I see no explanation why the
existing hooks are unsuitable - at least as the _initial_ starting point.

So, can you please explain why your IOMMU code can not be hidden behind
the DMA API.  (Please omit any complaints about the mechanics of hooking
it in there, such things are solvable.)

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Russell King - ARM Linux]

On Tue, Apr 19, 2011 at 09:35:56PM +0900, Kyungmin Park wrote:
> On Tue, Apr 19, 2011 at 9:01 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 19 April 2011, Kyungmin Park wrote:
> >> On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Monday 18 April 2011, Kyungmin Park wrote:
> >> >> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> >> >
> >> >> > One missing piece is still a way for a platform to provide both
> >> >> > the iommu and the dma-mapping API in a unified driver. Right now,
> >> >> > you have to export both interface for a generic solution.
> >> >>
> >> >> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
> >> >> implementation into mm, but MM did't accept it.
> >> >
> >> > I'm confused. What do you mean with MM?
> >> linux/mm, Memory Management.
> >
> > I'm still confused. What were you suggesting to merge in there?
> > Do you have a link to a mailing list discussion?
> 
> First, Zach Pfeffer <zpfeffer@codeaurora.org> sent the patch
> https://patchwork.kernel.org/patch/108736/
> 
> Second, Michal tried it.
> http://lkml.org/lkml/2010/9/6/41
> 
> http://lwn.net/Articles/403643/
> https://patchwork.kernel.org/patch/157451/

This shows why ARM is in such a problem.  People love to create new
frameworks and infrastructure where they find existing stuff lacking,
rather than looking at what other architectures do and adapt.

As I'm sure has already mentioned, the kernel already has support for
IOMMUs in the DMA path with several non-ARM architectures, and this
support is managed through the existing DMA API.

When you want a device behind an IOMMU to perform DMA, the driver
uses the DMA API in the standard way as if there was no IOMMU there.
As part of the DMA API, particularly the SG list part, the DMA API
is allowed to coalesce the SG entries together if it can arrange
the IOMMU mappings to achieve a continguous mapping for the device.
Remember, dma_map_sg() is allowed to return _fewer_ entries in the
scatterlist than was passed to it for this very purpose.

In order to transition ARM over to this, we need to modify the
scatterlist structure by enabling CONFIG_NEED_SG_DMA_LENGTH.  We
then need to arrange for the DMA API to have the hooks _both_ into
the DMA cache coherency code (to ensure that the data hits memory)
and the IOMMU code (this part is missing from ARM.)  The current
abstractions in the generic header files don't allow for this.

I don't believe there's any need for any major new framework - and I
don't think I'm alone in thinking that, especially as this point has
been raised more than once when this framework has been submitted.

I see no reason why ARM should be any different from other architectures
which have IOMMUs, and I don't see why ARM should have to invent a whole
new framework to handle IOMMUs.  And I see no explanation why the
existing hooks are unsuitable - at least as the _initial_ starting point.

So, can you please explain why your IOMMU code can not be hidden behind
the DMA API.  (Please omit any complaints about the mechanics of hooking
it in there, such things are solvable.)

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Arnd Bergmann]

On Tuesday 19 April 2011, Kyungmin Park wrote:
> On Tue, Apr 19, 2011 at 9:01 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 19 April 2011, Kyungmin Park wrote:
> >> On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Monday 18 April 2011, Kyungmin Park wrote:
> >> >> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> >> >
> >> >> > One missing piece is still a way for a platform to provide both
> >> >> > the iommu and the dma-mapping API in a unified driver. Right now,
> >> >> > you have to export both interface for a generic solution.
> >> >>
> >> >> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
> >> >> implementation into mm, but MM did't accept it.
> >> >
> >> > I'm confused. What do you mean with MM?
> >> linux/mm, Memory Management.
> >
> > I'm still confused. What were you suggesting to merge in there?
> > Do you have a link to a mailing list discussion?
> 
> First, Zach Pfeffer <zpfeffer@codeaurora.org> sent the patch
> https://patchwork.kernel.org/patch/108736/
> 
> Second, Michal tried it.
> http://lkml.org/lkml/2010/9/6/41
> 
> http://lwn.net/Articles/403643/
> https://patchwork.kernel.org/patch/157451/

Ah, I did not realize you were talking about VCMM. I believe the main
reason what that patch was not received well is that it tried to add
yet another abstraction for IOMMUs when we already have too many of them.

It is essentially doing the opposite of what I was referring to above:
If we had added VCMM, each platform that has IOMMUs would now have to 
implement three interfaces: dma-mapping.h (struct dma_map_ops),
iommu.h (struct iommu_ops) and and vcm_driver.h (struct vcm_driver).

What I really meant was to unify the two we already have, so that
a platform only needs to implement e.g. iommu_ops and get the dma_mapping.h
interfaces for free.

	Arnd

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Arnd Bergmann]

On Tuesday 19 April 2011, Kyungmin Park wrote:
> On Tue, Apr 19, 2011 at 9:01 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 19 April 2011, Kyungmin Park wrote:
> >> On Mon, Apr 18, 2011 at 11:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Monday 18 April 2011, Kyungmin Park wrote:
> >> >> On Mon, Apr 18, 2011 at 8:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> >> >
> >> >> > One missing piece is still a way for a platform to provide both
> >> >> > the iommu and the dma-mapping API in a unified driver. Right now,
> >> >> > you have to export both interface for a generic solution.
> >> >>
> >> >> Actually MSM and we (Michal, Marek) tried to merge the generic IOMMU
> >> >> implementation into mm, but MM did't accept it.
> >> >
> >> > I'm confused. What do you mean with MM?
> >> linux/mm, Memory Management.
> >
> > I'm still confused. What were you suggesting to merge in there?
> > Do you have a link to a mailing list discussion?
> 
> First, Zach Pfeffer <zpfeffer@codeaurora.org> sent the patch
> https://patchwork.kernel.org/patch/108736/
> 
> Second, Michal tried it.
> http://lkml.org/lkml/2010/9/6/41
> 
> http://lwn.net/Articles/403643/
> https://patchwork.kernel.org/patch/157451/

Ah, I did not realize you were talking about VCMM. I believe the main
reason what that patch was not received well is that it tried to add
yet another abstraction for IOMMUs when we already have too many of them.

It is essentially doing the opposite of what I was referring to above:
If we had added VCMM, each platform that has IOMMUs would now have to 
implement three interfaces: dma-mapping.h (struct dma_map_ops),
iommu.h (struct iommu_ops) and and vcm_driver.h (struct vcm_driver).

What I really meant was to unify the two we already have, so that
a platform only needs to implement e.g. iommu_ops and get the dma_mapping.h
interfaces for free.

	Arnd

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Russell King - ARM Linux]

On Fri, Apr 15, 2011 at 06:26:40AM -0500, Gupta, Ramesh wrote:
> Russell,
> 
> On Thu, Apr 14, 2011 at 5:30 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> >> From: Ramesh Gupta <grgupta@ti.com>
> >>
> >> This patch is to flush the iommu page table entries from L1 and L2
> >> caches using dma_map_single. This also simplifies the implementation
> >> by removing the functions  flush_iopgd_range/flush_iopte_range.
> >
> > No.  This usage is just wrong.  If you're going to use the DMA API then
> > unmap it, otherwise the DMA API debugging will go awol.
> >
> 
> Thank you for the comments, this particular memory is always a write
> from the A9 for MMU programming and
> only read from the slave processor, that is the reason for not calling
> the unmap. I can re-look into the changes to call
> unmap in a proper way as this impacts the DMA API.
> Are there any other ways to perform only flush the memory from L1/L2 caches?

We _could_ invent a new API to deal with this, which is probably going
to be far better in the longer term for page table based iommus.  That's
going to need some thought - eg, do we need to pass a struct device
argument for the iommu cache flushing so we know whether we need to flush
or not (eg, if we have cache coherent iommus)...

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Russell King - ARM Linux]

On Fri, Apr 15, 2011 at 06:26:40AM -0500, Gupta, Ramesh wrote:
> Russell,
> 
> On Thu, Apr 14, 2011 at 5:30 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> >> From: Ramesh Gupta <grgupta@ti.com>
> >>
> >> This patch is to flush the iommu page table entries from L1 and L2
> >> caches using dma_map_single. This also simplifies the implementation
> >> by removing the functions  flush_iopgd_range/flush_iopte_range.
> >
> > No.  This usage is just wrong.  If you're going to use the DMA API then
> > unmap it, otherwise the DMA API debugging will go awol.
> >
> 
> Thank you for the comments, this particular memory is always a write
> from the A9 for MMU programming and
> only read from the slave processor, that is the reason for not calling
> the unmap. I can re-look into the changes to call
> unmap in a proper way as this impacts the DMA API.
> Are there any other ways to perform only flush the memory from L1/L2 caches?

We _could_ invent a new API to deal with this, which is probably going
to be far better in the longer term for page table based iommus.  That's
going to need some thought - eg, do we need to pass a struct device
argument for the iommu cache flushing so we know whether we need to flush
or not (eg, if we have cache coherent iommus)...
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Russell King - ARM Linux]

On Fri, Apr 15, 2011 at 06:26:40AM -0500, Gupta, Ramesh wrote:
> Russell,
> 
> On Thu, Apr 14, 2011 at 5:30 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
> >> From: Ramesh Gupta <grgupta@ti.com>
> >>
> >> This patch is to flush the iommu page table entries from L1 and L2
> >> caches using dma_map_single. This also simplifies the implementation
> >> by removing the functions ?flush_iopgd_range/flush_iopte_range.
> >
> > No. ?This usage is just wrong. ?If you're going to use the DMA API then
> > unmap it, otherwise the DMA API debugging will go awol.
> >
> 
> Thank you for the comments, this particular memory is always a write
> from the A9 for MMU programming and
> only read from the slave processor, that is the reason for not calling
> the unmap. I can re-look into the changes to call
> unmap in a proper way as this impacts the DMA API.
> Are there any other ways to perform only flush the memory from L1/L2 caches?

We _could_ invent a new API to deal with this, which is probably going
to be far better in the longer term for page table based iommus.  That's
going to need some thought - eg, do we need to pass a struct device
argument for the iommu cache flushing so we know whether we need to flush
or not (eg, if we have cache coherent iommus)...

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Gupta, Ramesh]

Hi Russel,

On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Apr 15, 2011 at 06:26:40AM -0500, Gupta, Ramesh wrote:
>> Russell,
>>
>> On Thu, Apr 14, 2011 at 5:30 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> >> From: Ramesh Gupta <grgupta@ti.com>
>> >>
>> >> This patch is to flush the iommu page table entries from L1 and L2
>> >> caches using dma_map_single. This also simplifies the implementation
>> >> by removing the functions  flush_iopgd_range/flush_iopte_range.
>> >
>> > No.  This usage is just wrong.  If you're going to use the DMA API then
>> > unmap it, otherwise the DMA API debugging will go awol.
>> >
>>
>> Thank you for the comments, this particular memory is always a write
>> from the A9 for MMU programming and
>> only read from the slave processor, that is the reason for not calling
>> the unmap. I can re-look into the changes to call
>> unmap in a proper way as this impacts the DMA API.
>> Are there any other ways to perform only flush the memory from L1/L2 caches?
>
> We _could_ invent a new API to deal with this, which is probably going
> to be far better in the longer term for page table based iommus.  That's
> going to need some thought - eg, do we need to pass a struct device
> argument for the iommu cache flushing so we know whether we need to flush
> or not (eg, if we have cache coherent iommus)...

I agree with you, right now these functions are taking only start and
end arguments but
we can pass a struct deivice to these functions.

thank you and regards
Ramesh Gupta G

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Gupta, Ramesh]

Hi Russel,

On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Apr 15, 2011 at 06:26:40AM -0500, Gupta, Ramesh wrote:
>> Russell,
>>
>> On Thu, Apr 14, 2011 at 5:30 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>> >> From: Ramesh Gupta <grgupta@ti.com>
>> >>
>> >> This patch is to flush the iommu page table entries from L1 and L2
>> >> caches using dma_map_single. This also simplifies the implementation
>> >> by removing the functions ?flush_iopgd_range/flush_iopte_range.
>> >
>> > No. ?This usage is just wrong. ?If you're going to use the DMA API then
>> > unmap it, otherwise the DMA API debugging will go awol.
>> >
>>
>> Thank you for the comments, this particular memory is always a write
>> from the A9 for MMU programming and
>> only read from the slave processor, that is the reason for not calling
>> the unmap. I can re-look into the changes to call
>> unmap in a proper way as this impacts the DMA API.
>> Are there any other ways to perform only flush the memory from L1/L2 caches?
>
> We _could_ invent a new API to deal with this, which is probably going
> to be far better in the longer term for page table based iommus. ?That's
> going to need some thought - eg, do we need to pass a struct device
> argument for the iommu cache flushing so we know whether we need to flush
> or not (eg, if we have cache coherent iommus)...

I agree with you, right now these functions are taking only start and
end arguments but
we can pass a struct deivice to these functions.

thank you and regards
Ramesh Gupta G

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Gupta, Ramesh]

Hi Russel,


On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
> Hi Russel,
>
> On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Fri, Apr 15, 2011 at 06:26:40AM -0500, Gupta, Ramesh wrote:
>>> Russell,
>>>
>>> On Thu, Apr 14, 2011 at 5:30 PM, Russell King - ARM Linux
>>> <linux@arm.linux.org.uk> wrote:
>>> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>>> >> From: Ramesh Gupta <grgupta@ti.com>
>>> >>
>>> >> This patch is to flush the iommu page table entries from L1 and L2
>>> >> caches using dma_map_single. This also simplifies the implementation
>>> >> by removing the functions  flush_iopgd_range/flush_iopte_range.
>>> >
>>> > No.  This usage is just wrong.  If you're going to use the DMA API then
>>> > unmap it, otherwise the DMA API debugging will go awol.
>>> >
>>>
>>> Thank you for the comments, this particular memory is always a write
>>> from the A9 for MMU programming and
>>> only read from the slave processor, that is the reason for not calling
>>> the unmap. I can re-look into the changes to call
>>> unmap in a proper way as this impacts the DMA API.
>>> Are there any other ways to perform only flush the memory from L1/L2 caches?
>>
>> We _could_ invent a new API to deal with this, which is probably going
>> to be far better in the longer term for page table based iommus.  That's
>> going to need some thought - eg, do we need to pass a struct device
>> argument for the iommu cache flushing so we know whether we need to flush
>> or not (eg, if we have cache coherent iommus)...

my apologies for a late mail on this topic.

do you think of any other requirements for this new API?

Could we use the existing dmac_flush_range(), outer_flush_range()
for this purpose instead of a new API?

I see a comment in the arch/arm/include/asm/cacheflush.h
for  _not_ to use these APIs directly, but I am not really understand
the reason for that.

I would appreciate any inputs on this.

thank you and regards
Ramesh Gupta G

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Gupta, Ramesh]

Hi Russel,


On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
> Hi Russel,
>
> On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Fri, Apr 15, 2011 at 06:26:40AM -0500, Gupta, Ramesh wrote:
>>> Russell,
>>>
>>> On Thu, Apr 14, 2011 at 5:30 PM, Russell King - ARM Linux
>>> <linux@arm.linux.org.uk> wrote:
>>> > On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote:
>>> >> From: Ramesh Gupta <grgupta@ti.com>
>>> >>
>>> >> This patch is to flush the iommu page table entries from L1 and L2
>>> >> caches using dma_map_single. This also simplifies the implementation
>>> >> by removing the functions ?flush_iopgd_range/flush_iopte_range.
>>> >
>>> > No. ?This usage is just wrong. ?If you're going to use the DMA API then
>>> > unmap it, otherwise the DMA API debugging will go awol.
>>> >
>>>
>>> Thank you for the comments, this particular memory is always a write
>>> from the A9 for MMU programming and
>>> only read from the slave processor, that is the reason for not calling
>>> the unmap. I can re-look into the changes to call
>>> unmap in a proper way as this impacts the DMA API.
>>> Are there any other ways to perform only flush the memory from L1/L2 caches?
>>
>> We _could_ invent a new API to deal with this, which is probably going
>> to be far better in the longer term for page table based iommus. ?That's
>> going to need some thought - eg, do we need to pass a struct device
>> argument for the iommu cache flushing so we know whether we need to flush
>> or not (eg, if we have cache coherent iommus)...

my apologies for a late mail on this topic.

do you think of any other requirements for this new API?

Could we use the existing dmac_flush_range(), outer_flush_range()
for this purpose instead of a new API?

I see a comment in the arch/arm/include/asm/cacheflush.h
for  _not_ to use these APIs directly, but I am not really understand
the reason for that.

I would appreciate any inputs on this.

thank you and regards
Ramesh Gupta G

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Russell King - ARM Linux]

On Thu, Aug 11, 2011 at 02:28:39PM -0500, Gupta, Ramesh wrote:
> Hi Russel,

grr.

> On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
> > Hi Russel,
> >
> > On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
> > <linux@arm.linux.org.uk> wrote:
> >> We _could_ invent a new API to deal with this, which is probably going
> >> to be far better in the longer term for page table based iommus.  That's
> >> going to need some thought - eg, do we need to pass a struct device
> >> argument for the iommu cache flushing so we know whether we need to flush
> >> or not (eg, if we have cache coherent iommus)...
> 
> my apologies for a late mail on this topic.
> 
> do you think of any other requirements for this new API?
> 
> Could we use the existing dmac_flush_range(), outer_flush_range()
> for this purpose instead of a new API?
> 
> I see a comment in the arch/arm/include/asm/cacheflush.h
> for  _not_ to use these APIs directly, but I am not really understand
> the reason for that.

When I create APIs, I create them to solve a _purpose_ to code which wants
to do something.  They're not created to provide some facility which can
be re-used for unrelated stuff.

This has been proven many times to be the correct approach.  Over time,
things change.

Let's say for arguments sake that you decide to use the DMA API stuff
to achieve your goals.  Then, lets say that ARM DMA becomes fully
cache coherent, but your IOMMU tables still need to be flushed from
the L1 caches.

Suddenly, dmac_flush_range() starts doing absolutely nothing.  Your
driver breaks.  I get whinged at because a previously working driver
stops working.  In effect, that usage _prevents_ me making the changes
necessary to keep the core architecture support moving forward as
things develop.  Or, alternatively I just ignore your driver, make the
changes anyway and leave it to rot.

So, APIs get created to provide a purpose.  Like - handling the DMA
issues when mapping a buffer to DMA.  Like handling the DMA issues
when unmapping a buffer from DMA.  If you start using those _because_
they happen to clean or invalidate the cache for you, you're really
asking for your driver to be broken at some point in the future.

What is far better to do is to ensure that we have the right APIs in
place for the purposes for which they are to be used.  So, if we need
an API to flush out the IOMMU page table entries, then that's what
we need, not some bodged lets-reuse-the-dma-flushing-functions thing.
Inside the processors implementation, yes, it may well be the same
thing, but that's a decision for the _processor_ support code to make,
not the IOMMU writer.

As to what shape a new API should be - as I said above, maybe it should
take a struct device argument, virtual base address and size.  Or maybe
if we don't have any coherent IOMMUs then just ignore the device
argument for the time being, and just pass the virtual base address and
size.

The next issue is whether it should require the virtual base address to
be in the kernel direct mapped region.  If you're touching L2, then
that's a yes, because we need to use virt_to_phys on it to get at the
phys address for the L2 operations.

So, I think: extend the cpu cache operations structure to have a method
for dealing with IOMMUs.  Add an inline function to deal with calling
that, and the L2 ops if that's what's required.

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Russell King - ARM Linux]

On Thu, Aug 11, 2011 at 02:28:39PM -0500, Gupta, Ramesh wrote:
> Hi Russel,

grr.

> On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
> > Hi Russel,
> >
> > On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
> > <linux@arm.linux.org.uk> wrote:
> >> We _could_ invent a new API to deal with this, which is probably going
> >> to be far better in the longer term for page table based iommus.  That's
> >> going to need some thought - eg, do we need to pass a struct device
> >> argument for the iommu cache flushing so we know whether we need to flush
> >> or not (eg, if we have cache coherent iommus)...
> 
> my apologies for a late mail on this topic.
> 
> do you think of any other requirements for this new API?
> 
> Could we use the existing dmac_flush_range(), outer_flush_range()
> for this purpose instead of a new API?
> 
> I see a comment in the arch/arm/include/asm/cacheflush.h
> for  _not_ to use these APIs directly, but I am not really understand
> the reason for that.

When I create APIs, I create them to solve a _purpose_ to code which wants
to do something.  They're not created to provide some facility which can
be re-used for unrelated stuff.

This has been proven many times to be the correct approach.  Over time,
things change.

Let's say for arguments sake that you decide to use the DMA API stuff
to achieve your goals.  Then, lets say that ARM DMA becomes fully
cache coherent, but your IOMMU tables still need to be flushed from
the L1 caches.

Suddenly, dmac_flush_range() starts doing absolutely nothing.  Your
driver breaks.  I get whinged at because a previously working driver
stops working.  In effect, that usage _prevents_ me making the changes
necessary to keep the core architecture support moving forward as
things develop.  Or, alternatively I just ignore your driver, make the
changes anyway and leave it to rot.

So, APIs get created to provide a purpose.  Like - handling the DMA
issues when mapping a buffer to DMA.  Like handling the DMA issues
when unmapping a buffer from DMA.  If you start using those _because_
they happen to clean or invalidate the cache for you, you're really
asking for your driver to be broken at some point in the future.

What is far better to do is to ensure that we have the right APIs in
place for the purposes for which they are to be used.  So, if we need
an API to flush out the IOMMU page table entries, then that's what
we need, not some bodged lets-reuse-the-dma-flushing-functions thing.
Inside the processors implementation, yes, it may well be the same
thing, but that's a decision for the _processor_ support code to make,
not the IOMMU writer.

As to what shape a new API should be - as I said above, maybe it should
take a struct device argument, virtual base address and size.  Or maybe
if we don't have any coherent IOMMUs then just ignore the device
argument for the time being, and just pass the virtual base address and
size.

The next issue is whether it should require the virtual base address to
be in the kernel direct mapped region.  If you're touching L2, then
that's a yes, because we need to use virt_to_phys on it to get at the
phys address for the L2 operations.

So, I think: extend the cpu cache operations structure to have a method
for dealing with IOMMUs.  Add an inline function to deal with calling
that, and the L2 ops if that's what's required.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Russell King - ARM Linux]

On Thu, Aug 11, 2011 at 02:28:39PM -0500, Gupta, Ramesh wrote:
> Hi Russel,

grr.

> On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
> > Hi Russel,
> >
> > On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
> > <linux@arm.linux.org.uk> wrote:
> >> We _could_ invent a new API to deal with this, which is probably going
> >> to be far better in the longer term for page table based iommus. ?That's
> >> going to need some thought - eg, do we need to pass a struct device
> >> argument for the iommu cache flushing so we know whether we need to flush
> >> or not (eg, if we have cache coherent iommus)...
> 
> my apologies for a late mail on this topic.
> 
> do you think of any other requirements for this new API?
> 
> Could we use the existing dmac_flush_range(), outer_flush_range()
> for this purpose instead of a new API?
> 
> I see a comment in the arch/arm/include/asm/cacheflush.h
> for  _not_ to use these APIs directly, but I am not really understand
> the reason for that.

When I create APIs, I create them to solve a _purpose_ to code which wants
to do something.  They're not created to provide some facility which can
be re-used for unrelated stuff.

This has been proven many times to be the correct approach.  Over time,
things change.

Let's say for arguments sake that you decide to use the DMA API stuff
to achieve your goals.  Then, lets say that ARM DMA becomes fully
cache coherent, but your IOMMU tables still need to be flushed from
the L1 caches.

Suddenly, dmac_flush_range() starts doing absolutely nothing.  Your
driver breaks.  I get whinged at because a previously working driver
stops working.  In effect, that usage _prevents_ me making the changes
necessary to keep the core architecture support moving forward as
things develop.  Or, alternatively I just ignore your driver, make the
changes anyway and leave it to rot.

So, APIs get created to provide a purpose.  Like - handling the DMA
issues when mapping a buffer to DMA.  Like handling the DMA issues
when unmapping a buffer from DMA.  If you start using those _because_
they happen to clean or invalidate the cache for you, you're really
asking for your driver to be broken at some point in the future.

What is far better to do is to ensure that we have the right APIs in
place for the purposes for which they are to be used.  So, if we need
an API to flush out the IOMMU page table entries, then that's what
we need, not some bodged lets-reuse-the-dma-flushing-functions thing.
Inside the processors implementation, yes, it may well be the same
thing, but that's a decision for the _processor_ support code to make,
not the IOMMU writer.

As to what shape a new API should be - as I said above, maybe it should
take a struct device argument, virtual base address and size.  Or maybe
if we don't have any coherent IOMMUs then just ignore the device
argument for the time being, and just pass the virtual base address and
size.

The next issue is whether it should require the virtual base address to
be in the kernel direct mapped region.  If you're touching L2, then
that's a yes, because we need to use virt_to_phys on it to get at the
phys address for the L2 operations.

So, I think: extend the cpu cache operations structure to have a method
for dealing with IOMMUs.  Add an inline function to deal with calling
that, and the L2 ops if that's what's required.

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Gupta, Ramesh]

On Thu, Aug 11, 2011 at 5:29 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Aug 11, 2011 at 02:28:39PM -0500, Gupta, Ramesh wrote:
>> Hi Russel,
>
> grr.

Sorry, typo.

>
>> On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
>> > Hi Russel,
>> >
>> > On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
>> > <linux@arm.linux.org.uk> wrote:
>> >> We _could_ invent a new API to deal with this, which is probably going
>> >> to be far better in the longer term for page table based iommus.  That's
>> >> going to need some thought - eg, do we need to pass a struct device
>> >> argument for the iommu cache flushing so we know whether we need to flush
>> >> or not (eg, if we have cache coherent iommus)...
>>
>> my apologies for a late mail on this topic.
>>
>> do you think of any other requirements for this new API?
>>
>> Could we use the existing dmac_flush_range(), outer_flush_range()
>> for this purpose instead of a new API?
>>
>> I see a comment in the arch/arm/include/asm/cacheflush.h
>> for  _not_ to use these APIs directly, but I am not really understand
>> the reason for that.
>
> When I create APIs, I create them to solve a _purpose_ to code which wants
> to do something.  They're not created to provide some facility which can
> be re-used for unrelated stuff.
>
> This has been proven many times to be the correct approach.  Over time,
> things change.
>

I agree.

> Let's say for arguments sake that you decide to use the DMA API stuff
> to achieve your goals.  Then, lets say that ARM DMA becomes fully
> cache coherent, but your IOMMU tables still need to be flushed from
> the L1 caches.
>
> Suddenly, dmac_flush_range() starts doing absolutely nothing.  Your
> driver breaks.  I get whinged at because a previously working driver
> stops working.  In effect, that usage _prevents_ me making the changes
> necessary to keep the core architecture support moving forward as
> things develop.  Or, alternatively I just ignore your driver, make the
> changes anyway and leave it to rot.
>
> So, APIs get created to provide a purpose.  Like - handling the DMA
> issues when mapping a buffer to DMA.  Like handling the DMA issues
> when unmapping a buffer from DMA.  If you start using those _because_
> they happen to clean or invalidate the cache for you, you're really
> asking for your driver to be broken at some point in the future.
>
> What is far better to do is to ensure that we have the right APIs in
> place for the purposes for which they are to be used.  So, if we need
> an API to flush out the IOMMU page table entries, then that's what
> we need, not some bodged lets-reuse-the-dma-flushing-functions thing.
> Inside the processors implementation, yes, it may well be the same
> thing, but that's a decision for the _processor_ support code to make,
> not the IOMMU writer.

I completely agree, thank you for explaining the need to use the correct API.


> As to what shape a new API should be - as I said above, maybe it should
> take a struct device argument, virtual base address and size.  Or maybe
> if we don't have any coherent IOMMUs then just ignore the device
> argument for the time being, and just pass the virtual base address and
> size.
>
> The next issue is whether it should require the virtual base address to
> be in the kernel direct mapped region.  If you're touching L2, then
> that's a yes, because we need to use virt_to_phys on it to get at the
> phys address for the L2 operations.
>
> So, I think: extend the cpu cache operations structure to have a method
> for dealing with IOMMUs.  Add an inline function to deal with calling
> that, and the L2 ops if that's what's required.

Once again thanks for the API requirements, I will work on this and send
the patches for review.

Regards
Ramesh Gupta G

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Gupta, Ramesh]

On Thu, Aug 11, 2011 at 5:29 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Aug 11, 2011 at 02:28:39PM -0500, Gupta, Ramesh wrote:
>> Hi Russel,
>
> grr.

Sorry, typo.

>
>> On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
>> > Hi Russel,
>> >
>> > On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
>> > <linux@arm.linux.org.uk> wrote:
>> >> We _could_ invent a new API to deal with this, which is probably going
>> >> to be far better in the longer term for page table based iommus.  That's
>> >> going to need some thought - eg, do we need to pass a struct device
>> >> argument for the iommu cache flushing so we know whether we need to flush
>> >> or not (eg, if we have cache coherent iommus)...
>>
>> my apologies for a late mail on this topic.
>>
>> do you think of any other requirements for this new API?
>>
>> Could we use the existing dmac_flush_range(), outer_flush_range()
>> for this purpose instead of a new API?
>>
>> I see a comment in the arch/arm/include/asm/cacheflush.h
>> for  _not_ to use these APIs directly, but I am not really understand
>> the reason for that.
>
> When I create APIs, I create them to solve a _purpose_ to code which wants
> to do something.  They're not created to provide some facility which can
> be re-used for unrelated stuff.
>
> This has been proven many times to be the correct approach.  Over time,
> things change.
>

I agree.

> Let's say for arguments sake that you decide to use the DMA API stuff
> to achieve your goals.  Then, lets say that ARM DMA becomes fully
> cache coherent, but your IOMMU tables still need to be flushed from
> the L1 caches.
>
> Suddenly, dmac_flush_range() starts doing absolutely nothing.  Your
> driver breaks.  I get whinged at because a previously working driver
> stops working.  In effect, that usage _prevents_ me making the changes
> necessary to keep the core architecture support moving forward as
> things develop.  Or, alternatively I just ignore your driver, make the
> changes anyway and leave it to rot.
>
> So, APIs get created to provide a purpose.  Like - handling the DMA
> issues when mapping a buffer to DMA.  Like handling the DMA issues
> when unmapping a buffer from DMA.  If you start using those _because_
> they happen to clean or invalidate the cache for you, you're really
> asking for your driver to be broken at some point in the future.
>
> What is far better to do is to ensure that we have the right APIs in
> place for the purposes for which they are to be used.  So, if we need
> an API to flush out the IOMMU page table entries, then that's what
> we need, not some bodged lets-reuse-the-dma-flushing-functions thing.
> Inside the processors implementation, yes, it may well be the same
> thing, but that's a decision for the _processor_ support code to make,
> not the IOMMU writer.

I completely agree, thank you for explaining the need to use the correct API.


> As to what shape a new API should be - as I said above, maybe it should
> take a struct device argument, virtual base address and size.  Or maybe
> if we don't have any coherent IOMMUs then just ignore the device
> argument for the time being, and just pass the virtual base address and
> size.
>
> The next issue is whether it should require the virtual base address to
> be in the kernel direct mapped region.  If you're touching L2, then
> that's a yes, because we need to use virt_to_phys on it to get at the
> phys address for the L2 operations.
>
> So, I think: extend the cpu cache operations structure to have a method
> for dealing with IOMMUs.  Add an inline function to deal with calling
> that, and the L2 ops if that's what's required.

Once again thanks for the API requirements, I will work on this and send
the patches for review.

Regards
Ramesh Gupta G
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Gupta, Ramesh]

On Thu, Aug 11, 2011 at 5:29 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Aug 11, 2011 at 02:28:39PM -0500, Gupta, Ramesh wrote:
>> Hi Russel,
>
> grr.

Sorry, typo.

>
>> On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
>> > Hi Russel,
>> >
>> > On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
>> > <linux@arm.linux.org.uk> wrote:
>> >> We _could_ invent a new API to deal with this, which is probably going
>> >> to be far better in the longer term for page table based iommus. ?That's
>> >> going to need some thought - eg, do we need to pass a struct device
>> >> argument for the iommu cache flushing so we know whether we need to flush
>> >> or not (eg, if we have cache coherent iommus)...
>>
>> my apologies for a late mail on this topic.
>>
>> do you think of any other requirements for this new API?
>>
>> Could we use the existing dmac_flush_range(), outer_flush_range()
>> for this purpose instead of a new API?
>>
>> I see a comment in the arch/arm/include/asm/cacheflush.h
>> for ?_not_ to use these APIs directly, but I am not really understand
>> the reason for that.
>
> When I create APIs, I create them to solve a _purpose_ to code which wants
> to do something. ?They're not created to provide some facility which can
> be re-used for unrelated stuff.
>
> This has been proven many times to be the correct approach. ?Over time,
> things change.
>

I agree.

> Let's say for arguments sake that you decide to use the DMA API stuff
> to achieve your goals. ?Then, lets say that ARM DMA becomes fully
> cache coherent, but your IOMMU tables still need to be flushed from
> the L1 caches.
>
> Suddenly, dmac_flush_range() starts doing absolutely nothing. ?Your
> driver breaks. ?I get whinged at because a previously working driver
> stops working. ?In effect, that usage _prevents_ me making the changes
> necessary to keep the core architecture support moving forward as
> things develop. ?Or, alternatively I just ignore your driver, make the
> changes anyway and leave it to rot.
>
> So, APIs get created to provide a purpose. ?Like - handling the DMA
> issues when mapping a buffer to DMA. ?Like handling the DMA issues
> when unmapping a buffer from DMA. ?If you start using those _because_
> they happen to clean or invalidate the cache for you, you're really
> asking for your driver to be broken at some point in the future.
>
> What is far better to do is to ensure that we have the right APIs in
> place for the purposes for which they are to be used. ?So, if we need
> an API to flush out the IOMMU page table entries, then that's what
> we need, not some bodged lets-reuse-the-dma-flushing-functions thing.
> Inside the processors implementation, yes, it may well be the same
> thing, but that's a decision for the _processor_ support code to make,
> not the IOMMU writer.

I completely agree, thank you for explaining the need to use the correct API.


> As to what shape a new API should be - as I said above, maybe it should
> take a struct device argument, virtual base address and size. ?Or maybe
> if we don't have any coherent IOMMUs then just ignore the device
> argument for the time being, and just pass the virtual base address and
> size.
>
> The next issue is whether it should require the virtual base address to
> be in the kernel direct mapped region. ?If you're touching L2, then
> that's a yes, because we need to use virt_to_phys on it to get at the
> phys address for the L2 operations.
>
> So, I think: extend the cpu cache operations structure to have a method
> for dealing with IOMMUs. ?Add an inline function to deal with calling
> that, and the L2 ops if that's what's required.

Once again thanks for the API requirements, I will work on this and send
the patches for review.

Regards
Ramesh Gupta G

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[C.A, Subramaniam]

Hi Russell,

On Fri, Aug 12, 2011 at 11:05 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
>
> On Thu, Aug 11, 2011 at 5:29 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Thu, Aug 11, 2011 at 02:28:39PM -0500, Gupta, Ramesh wrote:
> >> Hi Russel,
> >
> > grr.
>
> Sorry, typo.
>
> >
> >> On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
> >> > Hi Russel,
> >> >
> >> > On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
> >> > <linux@arm.linux.org.uk> wrote:
> >> >> We _could_ invent a new API to deal with this, which is probably going
> >> >> to be far better in the longer term for page table based iommus.  That's
> >> >> going to need some thought - eg, do we need to pass a struct device
> >> >> argument for the iommu cache flushing so we know whether we need to flush
> >> >> or not (eg, if we have cache coherent iommus)...
> >>
> >> my apologies for a late mail on this topic.
> >>
> >> do you think of any other requirements for this new API?
> >>
> >> Could we use the existing dmac_flush_range(), outer_flush_range()
> >> for this purpose instead of a new API?
> >>
> >> I see a comment in the arch/arm/include/asm/cacheflush.h
> >> for  _not_ to use these APIs directly, but I am not really understand
> >> the reason for that.
> >
> > When I create APIs, I create them to solve a _purpose_ to code which wants
> > to do something.  They're not created to provide some facility which can
> > be re-used for unrelated stuff.
> >
> > This has been proven many times to be the correct approach.  Over time,
> > things change.
> >
>
> I agree.
>
> > Let's say for arguments sake that you decide to use the DMA API stuff
> > to achieve your goals.  Then, lets say that ARM DMA becomes fully
> > cache coherent, but your IOMMU tables still need to be flushed from
> > the L1 caches.
> >
> > Suddenly, dmac_flush_range() starts doing absolutely nothing.  Your
> > driver breaks.  I get whinged at because a previously working driver
> > stops working.  In effect, that usage _prevents_ me making the changes
> > necessary to keep the core architecture support moving forward as
> > things develop.  Or, alternatively I just ignore your driver, make the
> > changes anyway and leave it to rot.
> >
> > So, APIs get created to provide a purpose.  Like - handling the DMA
> > issues when mapping a buffer to DMA.  Like handling the DMA issues
> > when unmapping a buffer from DMA.  If you start using those _because_
> > they happen to clean or invalidate the cache for you, you're really
> > asking for your driver to be broken at some point in the future.
> >
> > What is far better to do is to ensure that we have the right APIs in
> > place for the purposes for which they are to be used.  So, if we need
> > an API to flush out the IOMMU page table entries, then that's what
> > we need, not some bodged lets-reuse-the-dma-flushing-functions thing.
> > Inside the processors implementation, yes, it may well be the same
> > thing, but that's a decision for the _processor_ support code to make,
> > not the IOMMU writer.
>
> I completely agree, thank you for explaining the need to use the correct API.
>
Following what you suggested, we looked at the cache flush APIs
available (for v7).
There is a v7_flush_kern_dcache_area that does the same job (from what
I  understand), as
the dma_inv_range. I have a patch that I have validated for OMAP4, to
use this API to flush
iommu pte/pgd entries. I would like to know your opinion before I send
out the patch.

>
> > As to what shape a new API should be - as I said above, maybe it should
> > take a struct device argument, virtual base address and size.  Or maybe
> > if we don't have any coherent IOMMUs then just ignore the device
> > argument for the time being, and just pass the virtual base address and
> > size.
> >
> > The next issue is whether it should require the virtual base address to
> > be in the kernel direct mapped region.  If you're touching L2, then
> > that's a yes, because we need to use virt_to_phys on it to get at the
> > phys address for the L2 operations.
> >
> > So, I think: extend the cpu cache operations structure to have a method
> > for dealing with IOMMUs.  Add an inline function to deal with calling
> > that, and the L2 ops if that's what's required.
>
> Once again thanks for the API requirements, I will work on this and send
> the patches for review.
>
> Regards
> Ramesh Gupta G
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



--

Thank you and Regards
Subbu

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[C.A, Subramaniam]

Hi Russell,

On Fri, Aug 12, 2011 at 11:05 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
>
> On Thu, Aug 11, 2011 at 5:29 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Thu, Aug 11, 2011 at 02:28:39PM -0500, Gupta, Ramesh wrote:
> >> Hi Russel,
> >
> > grr.
>
> Sorry, typo.
>
> >
> >> On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
> >> > Hi Russel,
> >> >
> >> > On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
> >> > <linux@arm.linux.org.uk> wrote:
> >> >> We _could_ invent a new API to deal with this, which is probably going
> >> >> to be far better in the longer term for page table based iommus. ?That's
> >> >> going to need some thought - eg, do we need to pass a struct device
> >> >> argument for the iommu cache flushing so we know whether we need to flush
> >> >> or not (eg, if we have cache coherent iommus)...
> >>
> >> my apologies for a late mail on this topic.
> >>
> >> do you think of any other requirements for this new API?
> >>
> >> Could we use the existing dmac_flush_range(), outer_flush_range()
> >> for this purpose instead of a new API?
> >>
> >> I see a comment in the arch/arm/include/asm/cacheflush.h
> >> for ?_not_ to use these APIs directly, but I am not really understand
> >> the reason for that.
> >
> > When I create APIs, I create them to solve a _purpose_ to code which wants
> > to do something. ?They're not created to provide some facility which can
> > be re-used for unrelated stuff.
> >
> > This has been proven many times to be the correct approach. ?Over time,
> > things change.
> >
>
> I agree.
>
> > Let's say for arguments sake that you decide to use the DMA API stuff
> > to achieve your goals. ?Then, lets say that ARM DMA becomes fully
> > cache coherent, but your IOMMU tables still need to be flushed from
> > the L1 caches.
> >
> > Suddenly, dmac_flush_range() starts doing absolutely nothing. ?Your
> > driver breaks. ?I get whinged at because a previously working driver
> > stops working. ?In effect, that usage _prevents_ me making the changes
> > necessary to keep the core architecture support moving forward as
> > things develop. ?Or, alternatively I just ignore your driver, make the
> > changes anyway and leave it to rot.
> >
> > So, APIs get created to provide a purpose. ?Like - handling the DMA
> > issues when mapping a buffer to DMA. ?Like handling the DMA issues
> > when unmapping a buffer from DMA. ?If you start using those _because_
> > they happen to clean or invalidate the cache for you, you're really
> > asking for your driver to be broken at some point in the future.
> >
> > What is far better to do is to ensure that we have the right APIs in
> > place for the purposes for which they are to be used. ?So, if we need
> > an API to flush out the IOMMU page table entries, then that's what
> > we need, not some bodged lets-reuse-the-dma-flushing-functions thing.
> > Inside the processors implementation, yes, it may well be the same
> > thing, but that's a decision for the _processor_ support code to make,
> > not the IOMMU writer.
>
> I completely agree, thank you for explaining the need to use the correct API.
>
Following what you suggested, we looked at the cache flush APIs
available (for v7).
There is a v7_flush_kern_dcache_area that does the same job (from what
I  understand), as
the dma_inv_range. I have a patch that I have validated for OMAP4, to
use this API to flush
iommu pte/pgd entries. I would like to know your opinion before I send
out the patch.

>
> > As to what shape a new API should be - as I said above, maybe it should
> > take a struct device argument, virtual base address and size. ?Or maybe
> > if we don't have any coherent IOMMUs then just ignore the device
> > argument for the time being, and just pass the virtual base address and
> > size.
> >
> > The next issue is whether it should require the virtual base address to
> > be in the kernel direct mapped region. ?If you're touching L2, then
> > that's a yes, because we need to use virt_to_phys on it to get at the
> > phys address for the L2 operations.
> >
> > So, I think: extend the cpu cache operations structure to have a method
> > for dealing with IOMMUs. ?Add an inline function to deal with calling
> > that, and the L2 ops if that's what's required.
>
> Once again thanks for the API requirements, I will work on this and send
> the patches for review.
>
> Regards
> Ramesh Gupta G
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html



--

Thank you and Regards
Subbu

Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Gupta, Ramesh]

Hi Russell,


On Fri, Aug 12, 2011 at 9:35 PM, Gupta, Ramesh <grgupta@ti.com> wrote:
> On Thu, Aug 11, 2011 at 5:29 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Thu, Aug 11, 2011 at 02:28:39PM -0500, Gupta, Ramesh wrote:
>>> Hi Russel,
>>
>> grr.
>
> Sorry, typo.
>
>>
>>> On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
>>> > Hi Russel,
>>> >
>>> > On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
>>> > <linux@arm.linux.org.uk> wrote:
>>> >> We _could_ invent a new API to deal with this, which is probably going
>>> >> to be far better in the longer term for page table based iommus.  That's
>>> >> going to need some thought - eg, do we need to pass a struct device
>>> >> argument for the iommu cache flushing so we know whether we need to flush
>>> >> or not (eg, if we have cache coherent iommus)...
>>>
>>> my apologies for a late mail on this topic.
>>>
>>> do you think of any other requirements for this new API?
>>>
>>> Could we use the existing dmac_flush_range(), outer_flush_range()
>>> for this purpose instead of a new API?
>>>
>>> I see a comment in the arch/arm/include/asm/cacheflush.h
>>> for  _not_ to use these APIs directly, but I am not really understand
>>> the reason for that.
>>
>> When I create APIs, I create them to solve a _purpose_ to code which wants
>> to do something.  They're not created to provide some facility which can
>> be re-used for unrelated stuff.
>>
>> This has been proven many times to be the correct approach.  Over time,
>> things change.
>>
>
> I agree.
>
>> Let's say for arguments sake that you decide to use the DMA API stuff
>> to achieve your goals.  Then, lets say that ARM DMA becomes fully
>> cache coherent, but your IOMMU tables still need to be flushed from
>> the L1 caches.
>>
>> Suddenly, dmac_flush_range() starts doing absolutely nothing.  Your
>> driver breaks.  I get whinged at because a previously working driver
>> stops working.  In effect, that usage _prevents_ me making the changes
>> necessary to keep the core architecture support moving forward as
>> things develop.  Or, alternatively I just ignore your driver, make the
>> changes anyway and leave it to rot.
>>
>> So, APIs get created to provide a purpose.  Like - handling the DMA
>> issues when mapping a buffer to DMA.  Like handling the DMA issues
>> when unmapping a buffer from DMA.  If you start using those _because_
>> they happen to clean or invalidate the cache for you, you're really
>> asking for your driver to be broken at some point in the future.
>>
>> What is far better to do is to ensure that we have the right APIs in
>> place for the purposes for which they are to be used.  So, if we need
>> an API to flush out the IOMMU page table entries, then that's what
>> we need, not some bodged lets-reuse-the-dma-flushing-functions thing.
>> Inside the processors implementation, yes, it may well be the same
>> thing, but that's a decision for the _processor_ support code to make,
>> not the IOMMU writer.
>
> I completely agree, thank you for explaining the need to use the correct API.
>
>
>> As to what shape a new API should be - as I said above, maybe it should
>> take a struct device argument, virtual base address and size.  Or maybe
>> if we don't have any coherent IOMMUs then just ignore the device
>> argument for the time being, and just pass the virtual base address and
>> size.
>>
>> The next issue is whether it should require the virtual base address to
>> be in the kernel direct mapped region.  If you're touching L2, then
>> that's a yes, because we need to use virt_to_phys on it to get at the
>> phys address for the L2 operations.
>>
>> So, I think: extend the cpu cache operations structure to have a method
>> for dealing with IOMMUs.  Add an inline function to deal with calling
>> that, and the L2 ops if that's what's required.

my apologies for a late response. I have posted a patch for a new L1
cache maintenance api for handling iommu as you suggested. I will send
iommu patches to use this api.

Thank you and regards
Ramesh Gupta G

[PATCH] OMAP: iommu flush page table entries from L1 and L2 cache

[Gupta, Ramesh]

Hi Russell,


On Fri, Aug 12, 2011 at 9:35 PM, Gupta, Ramesh <grgupta@ti.com> wrote:
> On Thu, Aug 11, 2011 at 5:29 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Thu, Aug 11, 2011 at 02:28:39PM -0500, Gupta, Ramesh wrote:
>>> Hi Russel,
>>
>> grr.
>
> Sorry, typo.
>
>>
>>> On Thu, Apr 28, 2011 at 11:48 AM, Gupta, Ramesh <grgupta@ti.com> wrote:
>>> > Hi Russel,
>>> >
>>> > On Thu, Apr 28, 2011 at 8:40 AM, Russell King - ARM Linux
>>> > <linux@arm.linux.org.uk> wrote:
>>> >> We _could_ invent a new API to deal with this, which is probably going
>>> >> to be far better in the longer term for page table based iommus. ?That's
>>> >> going to need some thought - eg, do we need to pass a struct device
>>> >> argument for the iommu cache flushing so we know whether we need to flush
>>> >> or not (eg, if we have cache coherent iommus)...
>>>
>>> my apologies for a late mail on this topic.
>>>
>>> do you think of any other requirements for this new API?
>>>
>>> Could we use the existing dmac_flush_range(), outer_flush_range()
>>> for this purpose instead of a new API?
>>>
>>> I see a comment in the arch/arm/include/asm/cacheflush.h
>>> for ?_not_ to use these APIs directly, but I am not really understand
>>> the reason for that.
>>
>> When I create APIs, I create them to solve a _purpose_ to code which wants
>> to do something. ?They're not created to provide some facility which can
>> be re-used for unrelated stuff.
>>
>> This has been proven many times to be the correct approach. ?Over time,
>> things change.
>>
>
> I agree.
>
>> Let's say for arguments sake that you decide to use the DMA API stuff
>> to achieve your goals. ?Then, lets say that ARM DMA becomes fully
>> cache coherent, but your IOMMU tables still need to be flushed from
>> the L1 caches.
>>
>> Suddenly, dmac_flush_range() starts doing absolutely nothing. ?Your
>> driver breaks. ?I get whinged at because a previously working driver
>> stops working. ?In effect, that usage _prevents_ me making the changes
>> necessary to keep the core architecture support moving forward as
>> things develop. ?Or, alternatively I just ignore your driver, make the
>> changes anyway and leave it to rot.
>>
>> So, APIs get created to provide a purpose. ?Like - handling the DMA
>> issues when mapping a buffer to DMA. ?Like handling the DMA issues
>> when unmapping a buffer from DMA. ?If you start using those _because_
>> they happen to clean or invalidate the cache for you, you're really
>> asking for your driver to be broken at some point in the future.
>>
>> What is far better to do is to ensure that we have the right APIs in
>> place for the purposes for which they are to be used. ?So, if we need
>> an API to flush out the IOMMU page table entries, then that's what
>> we need, not some bodged lets-reuse-the-dma-flushing-functions thing.
>> Inside the processors implementation, yes, it may well be the same
>> thing, but that's a decision for the _processor_ support code to make,
>> not the IOMMU writer.
>
> I completely agree, thank you for explaining the need to use the correct API.
>
>
>> As to what shape a new API should be - as I said above, maybe it should
>> take a struct device argument, virtual base address and size. ?Or maybe
>> if we don't have any coherent IOMMUs then just ignore the device
>> argument for the time being, and just pass the virtual base address and
>> size.
>>
>> The next issue is whether it should require the virtual base address to
>> be in the kernel direct mapped region. ?If you're touching L2, then
>> that's a yes, because we need to use virt_to_phys on it to get at the
>> phys address for the L2 operations.
>>
>> So, I think: extend the cpu cache operations structure to have a method
>> for dealing with IOMMUs. ?Add an inline function to deal with calling
>> that, and the L2 ops if that's what's required.

my apologies for a late response. I have posted a patch for a new L1
cache maintenance api for handling iommu as you suggested. I will send
iommu patches to use this api.

Thank you and regards
Ramesh Gupta G