[PATCHv4 0/4] iovmm: fixes for iovmm module

[PATCHv4 0/4] iovmm: fixes for iovmm module

[Fernando Guzman Lugo]

Version 4:
* Changes in "iommu: create new api to set valid da range"
  - Validate range for fixed address.
  - Change way of change boundaries to avoid possible overflow
    instead of style :
       start + bytes >= end     which start + end can overflow
    use style:
       end - start < bytes

Version 3:
* change patch 2 base on Felipe Contreras' comments,
  now it uses min_t and I deleted some blank lines.
* patch "create new api to set valid da range" is base on
  "iovmm: IVA2 MMU range is from 0x11000000 to 0xFFFFFFFF"
  patch and on Hiroshi's comments and now it is added to
  this set.

Version 2:
* Removed "iovmm: fixes for iovmm module" that patch was already
  sent.
* Modified "iovmm: fix roundup for next area and end check for the
  last area" patch, base on Davin Cohen's comments and rename it
  to a proper name that describes what it is doing now.

Fernando Guzman Lugo (4):
  iovmm: no gap checking for fixed address
  iovmm: add superpages support to fixed da address
  iovmm: replace __iounmap with omap_iounmap
  iommu: create new api to set valid da range

 arch/arm/plat-omap/include/plat/iommu.h |    3 +
 arch/arm/plat-omap/iommu.c              |   29 +++++++++++
 arch/arm/plat-omap/iovmm.c              |   81 +++++++++++++++++-------------
 3 files changed, 78 insertions(+), 35 deletions(-)

[PATCHv4 1/4] iovmm: no gap checking for fixed address

[Fernando Guzman Lugo]

If some fixed da address is wanted to be mapped and the page
is freed but it is used as gap, the mapping will fail.
This patch is fixing that and olny keeps the gap for
not fixed address.

Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
---
 arch/arm/plat-omap/iovmm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
index 24ca9c4..34f0012 100644
--- a/arch/arm/plat-omap/iovmm.c
+++ b/arch/arm/plat-omap/iovmm.c
@@ -289,7 +289,7 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da,
 	prev_end = 0;
 	list_for_each_entry(tmp, &obj->mmap, list) {
 
-		if (prev_end >= start)
+		if (prev_end > start)
 			break;
 
 		if (start + bytes <= tmp->da_start)
@@ -301,7 +301,7 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da,
 		prev_end = tmp->da_end;
 	}
 
-	if ((start > prev_end) && (ULONG_MAX - start >= bytes))
+	if ((start >= prev_end) && (ULONG_MAX - start + 1 >= bytes))
 		goto found;
 
 	dev_dbg(obj->dev, "%s: no space to fit %08x(%x) flags: %08x\n",
-- 
1.6.3.3

[PATCHv4 2/4] iovmm: add superpages support to fixed da address

[Fernando Guzman Lugo]

This patch adds superpages support to fixed ad address
inside iommu_kmap function.

Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
---
 arch/arm/plat-omap/iovmm.c |   62 +++++++++++++++++++++++++------------------
 1 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
index 34f0012..93a34d9 100644
--- a/arch/arm/plat-omap/iovmm.c
+++ b/arch/arm/plat-omap/iovmm.c
@@ -87,35 +87,43 @@ static size_t sgtable_len(const struct sg_table *sgt)
 }
 #define sgtable_ok(x)	(!!sgtable_len(x))
 
+static unsigned max_alignment(u32 addr)
+{
+	int i;
+	unsigned pagesize[] = { SZ_16M, SZ_1M, SZ_64K, SZ_4K, };
+	for (i = 0; i < ARRAY_SIZE(pagesize) && addr & (pagesize[i] - 1); i++)
+		;
+	return (i < ARRAY_SIZE(pagesize)) ? pagesize[i] : 0;
+}
+
 /*
  * calculate the optimal number sg elements from total bytes based on
  * iommu superpages
  */
-static unsigned int sgtable_nents(size_t bytes)
+static unsigned sgtable_nents(size_t bytes, u32 da, u32 pa)
 {
-	int i;
-	unsigned int nr_entries;
-	const unsigned long pagesize[] = { SZ_16M, SZ_1M, SZ_64K, SZ_4K, };
+	unsigned nr_entries = 0, ent_sz;
 
 	if (!IS_ALIGNED(bytes, PAGE_SIZE)) {
 		pr_err("%s: wrong size %08x\n", __func__, bytes);
 		return 0;
 	}
 
-	nr_entries = 0;
-	for (i = 0; i < ARRAY_SIZE(pagesize); i++) {
-		if (bytes >= pagesize[i]) {
-			nr_entries += (bytes / pagesize[i]);
-			bytes %= pagesize[i];
-		}
+	while (bytes) {
+		ent_sz = max_alignment(da | pa);
+		ent_sz = min_t(unsigned, ent_sz, iopgsz_max(bytes));
+		nr_entries++;
+		da += ent_sz;
+		pa += ent_sz;
+		bytes -= ent_sz;
 	}
-	BUG_ON(bytes);
 
 	return nr_entries;
 }
 
 /* allocate and initialize sg_table header(a kind of 'superblock') */
-static struct sg_table *sgtable_alloc(const size_t bytes, u32 flags)
+static struct sg_table *sgtable_alloc(const size_t bytes, u32 flags,
+							u32 da, u32 pa)
 {
 	unsigned int nr_entries;
 	int err;
@@ -127,9 +135,8 @@ static struct sg_table *sgtable_alloc(const size_t bytes, u32 flags)
 	if (!IS_ALIGNED(bytes, PAGE_SIZE))
 		return ERR_PTR(-EINVAL);
 
-	/* FIXME: IOVMF_DA_FIXED should support 'superpages' */
-	if ((flags & IOVMF_LINEAR) && (flags & IOVMF_DA_ANON)) {
-		nr_entries = sgtable_nents(bytes);
+	if (flags & IOVMF_LINEAR) {
+		nr_entries = sgtable_nents(bytes, da, pa);
 		if (!nr_entries)
 			return ERR_PTR(-EINVAL);
 	} else
@@ -409,7 +416,8 @@ static inline void sgtable_drain_vmalloc(struct sg_table *sgt)
 	BUG_ON(!sgt);
 }
 
-static void sgtable_fill_kmalloc(struct sg_table *sgt, u32 pa, size_t len)
+static void sgtable_fill_kmalloc(struct sg_table *sgt, u32 pa, u32 da,
+								size_t len)
 {
 	unsigned int i;
 	struct scatterlist *sg;
@@ -418,9 +426,10 @@ static void sgtable_fill_kmalloc(struct sg_table *sgt, u32 pa, size_t len)
 	va = phys_to_virt(pa);
 
 	for_each_sg(sgt->sgl, sg, sgt->nents, i) {
-		size_t bytes;
+		unsigned bytes;
 
-		bytes = iopgsz_max(len);
+		bytes = max_alignment(da | pa);
+		bytes = min_t(unsigned, bytes, iopgsz_max(len));
 
 		BUG_ON(!iopgsz_ok(bytes));
 
@@ -429,6 +438,7 @@ static void sgtable_fill_kmalloc(struct sg_table *sgt, u32 pa, size_t len)
 		 * 'pa' is cotinuous(linear).
 		 */
 		pa += bytes;
+		da += bytes;
 		len -= bytes;
 	}
 	BUG_ON(len);
@@ -695,18 +705,18 @@ u32 iommu_vmalloc(struct iommu *obj, u32 da, size_t bytes, u32 flags)
 	if (!va)
 		return -ENOMEM;
 
-	sgt = sgtable_alloc(bytes, flags);
+	flags &= IOVMF_HW_MASK;
+	flags |= IOVMF_DISCONT;
+	flags |= IOVMF_ALLOC;
+	flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON);
+
+	sgt = sgtable_alloc(bytes, flags, da, 0);
 	if (IS_ERR(sgt)) {
 		da = PTR_ERR(sgt);
 		goto err_sgt_alloc;
 	}
 	sgtable_fill_vmalloc(sgt, va);
 
-	flags &= IOVMF_HW_MASK;
-	flags |= IOVMF_DISCONT;
-	flags |= IOVMF_ALLOC;
-	flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON);
-
 	da = __iommu_vmap(obj, da, sgt, va, bytes, flags);
 	if (IS_ERR_VALUE(da))
 		goto err_iommu_vmap;
@@ -746,11 +756,11 @@ static u32 __iommu_kmap(struct iommu *obj, u32 da, u32 pa, void *va,
 {
 	struct sg_table *sgt;
 
-	sgt = sgtable_alloc(bytes, flags);
+	sgt = sgtable_alloc(bytes, flags, da, pa);
 	if (IS_ERR(sgt))
 		return PTR_ERR(sgt);
 
-	sgtable_fill_kmalloc(sgt, pa, bytes);
+	sgtable_fill_kmalloc(sgt, pa, da, bytes);
 
 	da = map_iommu_region(obj, da, sgt, va, bytes, flags);
 	if (IS_ERR_VALUE(da)) {
-- 
1.6.3.3

[PATCHv4 3/4] iovmm: replace __iounmap with omap_iounmap

[Fernando Guzman Lugo]

Omap platform is omap_iounmap function.

Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
---
 arch/arm/plat-omap/iovmm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
index 93a34d9..5489ca9 100644
--- a/arch/arm/plat-omap/iovmm.c
+++ b/arch/arm/plat-omap/iovmm.c
@@ -821,7 +821,7 @@ void iommu_kunmap(struct iommu *obj, u32 da)
 	struct sg_table *sgt;
 	typedef void (*func_t)(const void *);
 
-	sgt = unmap_vm_area(obj, da, (func_t)__iounmap,
+	sgt = unmap_vm_area(obj, da, (func_t)omap_iounmap,
 			    IOVMF_LINEAR | IOVMF_MMIO);
 	if (!sgt)
 		dev_dbg(obj->dev, "%s: No sgt\n", __func__);
-- 
1.6.3.3

[PATCHv4 4/4] iommu: create new api to set valid da range

[Fernando Guzman Lugo]

Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF range.
With this new API the valid range can be set.

Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
---
 arch/arm/plat-omap/include/plat/iommu.h |    3 +++
 arch/arm/plat-omap/iommu.c              |   29 +++++++++++++++++++++++++++++
 arch/arm/plat-omap/iovmm.c              |   15 ++++++++-------
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h
index 33c7d41..aea01b1 100644
--- a/arch/arm/plat-omap/include/plat/iommu.h
+++ b/arch/arm/plat-omap/include/plat/iommu.h
@@ -46,6 +46,8 @@ struct iommu {
 
 	struct list_head	mmap;
 	struct mutex		mmap_lock; /* protect mmap */
+	u32		da_start;
+	u32		da_end;
 
 	int (*isr)(struct iommu *obj);
 
@@ -152,6 +154,7 @@ extern void flush_iotlb_all(struct iommu *obj);
 extern int iopgtable_store_entry(struct iommu *obj, struct iotlb_entry *e);
 extern size_t iopgtable_clear_entry(struct iommu *obj, u32 iova);
 
+extern int iommu_set_da_range(struct iommu *obj, u32 start, u32 end);
 extern struct iommu *iommu_get(const char *name);
 extern void iommu_put(struct iommu *obj);
 
diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 6cd151b..e70e76b 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -25,6 +25,12 @@
 
 #include "iopgtable.h"
 
+/* Reserve the first page for NULL */
+#define IOMMU_DEFAULT_DA_START	PAGE_SIZE
+/* 0xFFFFFFFF not allowed because it is not page aligned */
+#define IOMMU_DEFAULT_DA_END	0xFFFFF000;
+
+
 #define for_each_iotlb_cr(obj, n, __i, cr)				\
 	for (__i = 0;							\
 	     (__i < (n)) && (cr = __iotlb_read_cr((obj), __i), true);	\
@@ -830,6 +836,27 @@ static int device_match_by_alias(struct device *dev, void *data)
 }
 
 /**
+ * iommu_set_da_range - Set a valid device address range
+ * @obj:		target iommu
+ * @start		Start of valid range
+ * @end			End of valid range
+ **/
+int iommu_set_da_range(struct iommu *obj, u32 start, u32 end)
+{
+	if (!obj)
+		return -EFAULT;
+
+	if (end < start || !PAGE_ALIGN(start | end))
+		return -EINVAL;
+
+	obj->da_start = start;
+	obj->da_end = end;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(iommu_set_da_range);
+
+/**
  * iommu_get - Get iommu handler
  * @name:	target iommu name
  **/
@@ -853,6 +880,8 @@ struct iommu *iommu_get(const char *name)
 		if (err)
 			goto err_enable;
 		flush_iotlb_all(obj);
+		obj->da_start = IOMMU_DEFAULT_DA_START;
+		obj->da_end = IOMMU_DEFAULT_DA_END;
 	}
 
 	if (!try_module_get(obj->owner))
diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
index 5489ca9..cff382f 100644
--- a/arch/arm/plat-omap/iovmm.c
+++ b/arch/arm/plat-omap/iovmm.c
@@ -280,13 +280,14 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da,
 	alignement = PAGE_SIZE;
 
 	if (flags & IOVMF_DA_ANON) {
-		/*
-		 * Reserve the first page for NULL
-		 */
-		start = PAGE_SIZE;
+		start = obj->da_start;
+
 		if (flags & IOVMF_LINEAR)
 			alignement = iopgsz_max(bytes);
 		start = roundup(start, alignement);
+	} else if (start < obj->da_start || start > obj->da_end ||
+					obj->da_end - start < bytes) {
+		return ERR_PTR(-EINVAL);
 	}
 
 	tmp = NULL;
@@ -299,16 +300,16 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da,
 		if (prev_end > start)
 			break;
 
-		if (start + bytes <= tmp->da_start)
+		if (tmp->da_start > start && (tmp->da_start - start) >= bytes)
 			goto found;
 
-		if (flags & IOVMF_DA_ANON)
+		if (tmp->da_end >= start && flags & IOVMF_DA_ANON)
 			start = roundup(tmp->da_end + 1, alignement);
 
 		prev_end = tmp->da_end;
 	}
 
-	if ((start >= prev_end) && (ULONG_MAX - start + 1 >= bytes))
+	if ((start >= prev_end) && (obj->da_end - start >= bytes))
 		goto found;
 
 	dev_dbg(obj->dev, "%s: no space to fit %08x(%x) flags: %08x\n",
-- 
1.6.3.3

Re: [PATCHv4 4/4] iommu: create new api to set valid da range

[felipe.contreras]

On Wed, Oct 20, 2010 at 5:48 AM, Fernando Guzman Lugo <x0095840@ti.com> wrote:
> Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF range.
> With this new API the valid range can be set.
>
> Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>

I don't see the point in having an API. It could be done through
platform data, and the usage of 0xFFFFF000 instead of ULONG_MAX is
independent of this.

diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
index f5a1aad..db919254 100644
--- a/arch/arm/mach-omap2/omap-iommu.c
+++ b/arch/arm/mach-omap2/omap-iommu.c
@@ -43,6 +43,7 @@ static struct iommu_device omap3_devices[] = {
 			.name = "iva2",
 			.nr_tlb_entries = 32,
 			.clk_name = "iva2_ck",
+			.start_addr = 0x11000000,
 		},
 	},
 #endif
diff --git a/arch/arm/plat-omap/include/plat/iommu.h
b/arch/arm/plat-omap/include/plat/iommu.h
index 33c7d41..94f3a9a 100644
--- a/arch/arm/plat-omap/include/plat/iommu.h
+++ b/arch/arm/plat-omap/include/plat/iommu.h
@@ -50,6 +50,8 @@ struct iommu {
 	int (*isr)(struct iommu *obj);

 	void *ctx; /* iommu context: registres saved area */
+
+	u32		start_addr;
 };

 struct cr_regs {
@@ -103,6 +105,7 @@ struct iommu_platform_data {
 	const char *name;
 	const char *clk_name;
 	const int nr_tlb_entries;
+	u32 start_addr;
 };

 #if defined(CONFIG_ARCH_OMAP1)
diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 6336ae2..3023d0b 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -926,6 +926,10 @@ static int __devinit omap_iommu_probe(struct
platform_device *pdev)
 	obj->name = pdata->name;
 	obj->dev = &pdev->dev;
 	obj->ctx = (void *)obj + sizeof(*obj);
+	obj->start_addr = pdata->start_addr;
+
+	if (!obj->start_addr)
+		obj->start_addr = PAGE_SIZE;

 	mutex_init(&obj->iommu_lock);
 	mutex_init(&obj->mmap_lock);
diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
index f318476..fd0c93f 100644
--- a/arch/arm/plat-omap/iovmm.c
+++ b/arch/arm/plat-omap/iovmm.c
@@ -284,7 +284,7 @@ static struct iovm_struct *alloc_iovm_area(struct
iommu *obj, u32 da,
 		/*
 		 * Reserve the first page for NULL
 		 */
-		start = PAGE_SIZE;
+		start = obj->start_addr;
 		if (flags & IOVMF_LINEAR)
 			alignement = iopgsz_max(bytes);
 		start = roundup(start, alignement);

-- 
Felipe Contreras

Re: [PATCHv4 4/4] iommu: create new api to set valid da range

[Hiroshi DOYU]

From: "ext felipe.contreras@gmail.com" <felipe.contreras@gmail.com>
Subject: Re: [PATCHv4 4/4] iommu: create new api to set valid da range
Date: Wed, 20 Oct 2010 10:35:50 +0200

> On Wed, Oct 20, 2010 at 5:48 AM, Fernando Guzman Lugo <x0095840@ti.com> wrote:
>> Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF range.
>> With this new API the valid range can be set.
>>
>> Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
> 
> I don't see the point in having an API. It could be done through
> platform data, and the usage of 0xFFFFF000 instead of ULONG_MAX is
> independent of this.

Using platform data is better, but why dropping da_end?

Re: [PATCHv4 4/4] iommu: create new api to set valid da range

[Felipe Contreras]

On Wed, Oct 20, 2010 at 11:45 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> From: "ext felipe.contreras@gmail.com" <felipe.contreras@gmail.com>
> Subject: Re: [PATCHv4 4/4] iommu: create new api to set valid da range
> Date: Wed, 20 Oct 2010 10:35:50 +0200
>
>> On Wed, Oct 20, 2010 at 5:48 AM, Fernando Guzman Lugo <x0095840@ti.com> wrote:
>>> Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF range.
>>> With this new API the valid range can be set.
>>>
>>> Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
>>
>> I don't see the point in having an API. It could be done through
>> platform data, and the usage of 0xFFFFF000 instead of ULONG_MAX is
>> independent of this.
>
> Using platform data is better, but why dropping da_end?

Because it's always 0xFFFFF000. Do you know of any instance where it's not?

-- 
Felipe Contreras

Re: [PATCHv4 4/4] iommu: create new api to set valid da range

[Hiroshi DOYU]

From: ext Felipe Contreras <felipe.contreras@gmail.com>
Subject: Re: [PATCHv4 4/4] iommu: create new api to set valid da range
Date: Wed, 20 Oct 2010 10:58:16 +0200

> On Wed, Oct 20, 2010 at 11:45 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
>> From: "ext felipe.contreras@gmail.com" <felipe.contreras@gmail.com>
>> Subject: Re: [PATCHv4 4/4] iommu: create new api to set valid da range
>> Date: Wed, 20 Oct 2010 10:35:50 +0200
>>
>>> On Wed, Oct 20, 2010 at 5:48 AM, Fernando Guzman Lugo <x0095840@ti.com> wrote:
>>>> Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF range.
>>>> With this new API the valid range can be set.
>>>>
>>>> Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
>>>
>>> I don't see the point in having an API. It could be done through
>>> platform data, and the usage of 0xFFFFF000 instead of ULONG_MAX is
>>> independent of this.
>>
>> Using platform data is better, but why dropping da_end?
> 
> Because it's always 0xFFFFF000. Do you know of any instance where
>it's not?

We should try to avoid forcing unnecessary limitations and
dependencies. The usage of device virtual address space range depends
on dsp(coprocessor) side S/W implimentation or configuration.

RE: [PATCHv4 4/4] iommu: create new api to set valid da range

[Guzman Lugo, Fernando]

 

> -----Original Message-----
> From: Hiroshi DOYU [mailto:Hiroshi.DOYU@nokia.com] 
> Sent: Wednesday, October 20, 2010 4:27 AM
> To: felipe.contreras@gmail.com
> Cc: Guzman Lugo, Fernando; felipe.contreras@nokia.com; 
> david.cohen@nokia.com; linux-kernel@vger.kernel.org; 
> linux-omap@vger.kernel.org
> Subject: Re: [PATCHv4 4/4] iommu: create new api to set valid da range
> 
> From: ext Felipe Contreras <felipe.contreras@gmail.com>
> Subject: Re: [PATCHv4 4/4] iommu: create new api to set valid da range
> Date: Wed, 20 Oct 2010 10:58:16 +0200
> 
> > On Wed, Oct 20, 2010 at 11:45 AM, Hiroshi DOYU 
> <Hiroshi.DOYU@nokia.com> wrote:
> >> From: "ext felipe.contreras@gmail.com" <felipe.contreras@gmail.com>
> >> Subject: Re: [PATCHv4 4/4] iommu: create new api to set valid da 
> >> range
> >> Date: Wed, 20 Oct 2010 10:35:50 +0200
> >>
> >>> On Wed, Oct 20, 2010 at 5:48 AM, Fernando Guzman Lugo 
> <x0095840@ti.com> wrote:
> >>>> Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF range.
> >>>> With this new API the valid range can be set.
> >>>>
> >>>> Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
> >>>
> >>> I don't see the point in having an API. It could be done through 
> >>> platform data, and the usage of 0xFFFFF000 instead of 
> ULONG_MAX is 
> >>> independent of this.
> >>
> >> Using platform data is better, but why dropping da_end?
> > 
> > Because it's always 0xFFFFF000. Do you know of any instance 
> where it's 
> >not?
> 
> We should try to avoid forcing unnecessary limitations and 
> dependencies. The usage of device virtual address space range 
> depends on dsp(coprocessor) side S/W implimentation or configuration.

I am agree. The valid range depends on the user (dsp or something else)
The only limitation of see in the iommu is that range has to be
Page aligned and that should be the only limitation.

Regards,
Fernando.

> 

RE: [PATCHv4 4/4] iommu: create new api to set valid da range

[Guzman Lugo, Fernando]

 

> -----Original Message-----
> From: felipe.contreras@gmail.com [mailto:felipe.contreras@gmail.com] 
> Sent: Wednesday, October 20, 2010 3:36 AM
> To: Guzman Lugo, Fernando
> Cc: Hiroshi.DOYU@nokia.com; felipe.contreras@nokia.com; 
> david.cohen@nokia.com; linux-kernel@vger.kernel.org; 
> linux-omap@vger.kernel.org
> Subject: Re: [PATCHv4 4/4] iommu: create new api to set valid da range
> 
> On Wed, Oct 20, 2010 at 5:48 AM, Fernando Guzman Lugo 
> <x0095840@ti.com> wrote:
> > Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF range.
> > With this new API the valid range can be set.
> >
> > Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
> 
> I don't see the point in having an API. It could be done 

Thanks to this api we can make this:


A new kconfig parameter for DMM size is added. Also DMM is
allocated in the last part of the IVA MMU capable address.
So that DMM address are far away of shared memory address
reducing the probability of shared memory corruption.

Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
---
 drivers/staging/tidspbridge/Kconfig                |    8 ++++++++
 drivers/staging/tidspbridge/core/tiomap3430.c      |   18 ++++++++++++++++--
 .../tidspbridge/include/dspbridge/dsp-mmu.h        |    3 +++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig
index 672008f..37e47f5 100644
--- a/drivers/staging/tidspbridge/Kconfig
+++ b/drivers/staging/tidspbridge/Kconfig
@@ -42,6 +42,14 @@ config TIDSPBRIDGE_MEMPOOL_SIZE
 	  Allocate specified size of memory at booting time to avoid allocation
 	  failure under heavy memory fragmentation after some use time.
 
+config TIDSPBRIDGE_DMM_SIZE
+	hex "DMM capable memory size (Byte)"
+	depends on TIDSPBRIDGE
+	default "0x10000000"
+	help
+	  Memory size of DSP virtural address for Dynamic Memory Mapping.
+	  Please make sure the size is 4K aligned.
+
 config TIDSPBRIDGE_DEBUG
 	bool "Debug Support"
 	depends on TIDSPBRIDGE
diff --git a/drivers/staging/tidspbridge/core/tiomap3430.c b/drivers/staging/tidspbridge/core/tiomap3430.c
index c28dcf1..31f1dd6 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430.c
@@ -345,7 +345,6 @@ static int bridge_brd_start(struct bridge_dev_context *dev_ctxt,
 					OMAP343X_CONTROL_IVA2_BOOTMOD));
 		}
 	}
-
 	if (!status) {
 		(*pdata->dsp_prm_rmw_bits)(OMAP3430_RST2_IVA2_MASK, 0,
 					OMAP3430_IVA2_MOD, OMAP2_RM_RSTCTRL);
@@ -362,6 +361,11 @@ static int bridge_brd_start(struct bridge_dev_context *dev_ctxt,
 	if (!status) {
 		dev_context->dsp_mmu = mmu;
 		sm_sg = &dev_context->sh_s;
+		/* Set valid range to map shared memory */
+		status = iommu_set_da_range(mmu, sm_sg->seg0_da,
+					sm_sg->seg1_da + sm_sg->seg1_size);
+	}
+	if (!status) {
 		sg0_da = iommu_kmap(mmu, sm_sg->seg0_da, sm_sg->seg0_pa,
 			sm_sg->seg0_size, IOVMF_ENDIAN_LITTLE | IOVMF_ELSZ_32);
 		if (IS_ERR_VALUE(sg0_da)) {
@@ -411,7 +415,17 @@ static int bridge_brd_start(struct bridge_dev_context *dev_ctxt,
 			l4_i++;
 		}
 	}
-
+	if (!status) {
+		/* Set valid range for DMM mapings */
+		if (MAX_DSP_MMU_DA - CONFIG_TIDSPBRIDGE_DMM_SIZE <
+					sm_sg->seg1_da + sm_sg->seg1_size) {
+			dev_err(bridge, "DMM size too big!\n");
+			status = -ENOMEM;
+		} else {
+			status = iommu_set_da_range(mmu, MAX_DSP_MMU_DA -
+				CONFIG_TIDSPBRIDGE_DMM_SIZE, MAX_DSP_MMU_DA);
+		}
+	}
 	/* Lock the above TLB entries and get the BIOS and load monitor timer
 	 * information */
 	if (!status) {
diff --git a/drivers/staging/tidspbridge/include/dspbridge/dsp-mmu.h b/drivers/staging/tidspbridge/include/dspbridge/dsp-mmu.h
index cb38d4c..bbbe9e6 100644
--- a/drivers/staging/tidspbridge/include/dspbridge/dsp-mmu.h
+++ b/drivers/staging/tidspbridge/include/dspbridge/dsp-mmu.h
@@ -22,6 +22,9 @@
 #include <plat/iommu.h>
 #include <plat/iovmm.h>
 
+/* Last patch is not mapped to detect buffer overflow in DSP side */
+#define MAX_DSP_MMU_DA 0xFFFFF000
+
 /**
  * dsp_mmu_init() - initialize dsp_mmu module and returns a handle
  *
-- 
1.6.3.3

With this patch we ara mapping SHM (shared memory) at the beginning of 
Iva2 iommu capabile address, it is from 0x11000000 to 0x203f0000 which 
Include some parts not used. And we can mapped the DMM at the end of
The Iva2 iommu capable address it is from 0xFFFFF000 - DMMSIZE to 0xFFFFF000.

This way we have separated the DMM part from SHM range and DMM buffer can not
Be allocated near to SHM segments or in the part not used between SHM the segments.
With this we make more unlikely an unintentional corruption of SHM memory by
Some node running on the DSP side. Also we avoid an underflow of some buffer below
0xFFFFF000 - DMMSIZE because that part would be always unmapped and would trigger
A mmufualt. Aso the overflow of the last buffer is avoid too, because it will 
trigger a mmufault when it reach last page (0xFFFFF000) because it is never mapped.
Moreover avoid and address overflow (0xFFFFFFFF + 0x1) which would write DSP memory 
Area (0x0 to 0x10FFFFFF).

So I have some advantages having that control. Please let me know what you think.
I can move the elements to platform data if you want, I am agree with that change.

The patch I showed above would be sent if this patch is accepted and after merging it.

Regards,
Fernando.

> through platform data, and the usage of 0xFFFFF000 instead of 
> ULONG_MAX is independent of this.
> 



[snip]
> Felipe Contreras
> 

Re: [PATCHv4 4/4] iommu: create new api to set valid da range

[Felipe Contreras]

On Wed, Oct 20, 2010 at 6:22 PM, Guzman Lugo, Fernando
<fernando.lugo@ti.com> wrote:
>> On Wed, Oct 20, 2010 at 5:48 AM, Fernando Guzman Lugo
>> <x0095840@ti.com> wrote:
>> > Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF range.
>> > With this new API the valid range can be set.
>> >
>> > Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
>>
>> I don't see the point in having an API. It could be done
>
> Thanks to this api we can make this:
>
> A new kconfig parameter for DMM size is added. Also DMM is
> allocated in the last part of the IVA MMU capable address.
> So that DMM address are far away of shared memory address
> reducing the probability of shared memory corruption.

Ok, that's why it's usually a good idea to send a patch that makes use
of the new API.

-- 
Felipe Contreras

Re: [PATCHv4 4/4] iommu: create new api to set valid da range

[Felipe Contreras]

On Wed, Oct 20, 2010 at 7:26 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Wed, Oct 20, 2010 at 6:22 PM, Guzman Lugo, Fernando
> <fernando.lugo@ti.com> wrote:
>>> On Wed, Oct 20, 2010 at 5:48 AM, Fernando Guzman Lugo
>>> <x0095840@ti.com> wrote:
>>> > Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF range.
>>> > With this new API the valid range can be set.
>>> >
>>> > Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
>>>
>>> I don't see the point in having an API. It could be done
>>
>> Thanks to this api we can make this:
>>
>> A new kconfig parameter for DMM size is added. Also DMM is
>> allocated in the last part of the IVA MMU capable address.
>> So that DMM address are far away of shared memory address
>> reducing the probability of shared memory corruption.
>
> Ok, that's why it's usually a good idea to send a patch that makes use
> of the new API.

Oh, and some more comments: Hiroshi wanted the end address too, but
also on the platform data.

And please use a sensible prefix, like "omap: iommu: ", if you only do
"iommu: " it can be confused with other parts of the system, specially
if you send to lkml.

-- 
Felipe Contreras

RE: [PATCHv4 4/4] iommu: create new api to set valid da range

[Guzman Lugo, Fernando]

 

> -----Original Message-----
> From: Felipe Contreras [mailto:felipe.contreras@gmail.com] 
> Sent: Friday, October 22, 2010 4:59 AM
> To: Guzman Lugo, Fernando
> Cc: Hiroshi.DOYU@nokia.com; felipe.contreras@nokia.com; 
> david.cohen@nokia.com; linux-kernel@vger.kernel.org; 
> linux-omap@vger.kernel.org
> Subject: Re: [PATCHv4 4/4] iommu: create new api to set valid da range
> 
> On Wed, Oct 20, 2010 at 7:26 PM, Felipe Contreras 
> <felipe.contreras@gmail.com> wrote:
> > On Wed, Oct 20, 2010 at 6:22 PM, Guzman Lugo, Fernando 
> > <fernando.lugo@ti.com> wrote:
> >>> On Wed, Oct 20, 2010 at 5:48 AM, Fernando Guzman Lugo 
> >>> <x0095840@ti.com> wrote:
> >>> > Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF range.
> >>> > With this new API the valid range can be set.
> >>> >
> >>> > Signed-off-by: Fernando Guzman Lugo <x0095840@ti.com>
> >>>
> >>> I don't see the point in having an API. It could be done
> >>
> >> Thanks to this api we can make this:
> >>
> >> A new kconfig parameter for DMM size is added. Also DMM is 
> allocated 
> >> in the last part of the IVA MMU capable address.
> >> So that DMM address are far away of shared memory address reducing 
> >> the probability of shared memory corruption.
> >
> > Ok, that's why it's usually a good idea to send a patch 
> that makes use 
> > of the new API.
> 
> Oh, and some more comments: Hiroshi wanted the end address 
> too, but also on the platform data.

I will try that.

> 
> And please use a sensible prefix, like "omap: iommu: ", if you only do
> "iommu: " it can be confused with other parts of the system, 
> specially if you send to lkml.

Sure, I wil take care for that.

Thanks and regards,
Fernando.

> 
> --
> Felipe Contreras
>