2010-12-14 13:06:19

by Fernando Guzman Lugo

[permalink] [raw]
Subject: [PATCH 0/4] omap: iovmm - fixes for iovmm module

Version 6:
* Rebase on Russell King branch.
- for details see:
http://marc.info/?l=linux-omap&m=129228495723001&w=2

Version 5:
* Changes in "iommu: create new api to set valid da range"
- Change range variables to platform data structure.

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.

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

arch/arm/plat-omap/include/plat/iommu.h | 3 +
arch/arm/plat-omap/iommu.c | 33 ++++++++++++
arch/arm/plat-omap/iovmm.c | 84 ++++++++++++++++++-------------
3 files changed, 85 insertions(+), 35 deletions(-)

--
1.7.3.2


2010-12-14 13:06:37

by Fernando Guzman Lugo

[permalink] [raw]
Subject: [PATCHv6 1/4] omap: iovmm - no gap checking for fixed address

From: Guzman Lugo, Fernando <[email protected]>

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 <[email protected]>
Acked-by: Hiroshi DOYU <[email protected]>
---
arch/arm/plat-omap/iovmm.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
index 8ce0de2..34f0012 100644
--- a/arch/arm/plat-omap/iovmm.c
+++ b/arch/arm/plat-omap/iovmm.c
@@ -289,10 +289,10 @@ 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)
+ if (start + bytes <= tmp->da_start)
goto found;

if (flags & IOVMF_DA_ANON)
@@ -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.7.3.2

2010-12-14 13:06:50

by Fernando Guzman Lugo

[permalink] [raw]
Subject: [PATCHv6 3/4] omap: iovmm - replace __iounmap with iounmap

From: Guzman Lugo, Fernando <[email protected]>

__iounmap function is wrong for OMAP architecture,
instead use iounmap which will call to the correct function.

Signed-off-by: Fernando Guzman Lugo <[email protected]>
Acked-by: Hiroshi DOYU <[email protected]>
---
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..fa6e643 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)iounmap,
IOVMF_LINEAR | IOVMF_MMIO);
if (!sgt)
dev_dbg(obj->dev, "%s: No sgt\n", __func__);
--
1.7.3.2

2010-12-14 13:06:52

by Fernando Guzman Lugo

[permalink] [raw]
Subject: [PATCHv6 4/4] omap: iommu - create new api to set valid da range

From: Guzman Lugo, Fernando <[email protected]>

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

Signed-off-by: Fernando Guzman Lugo <[email protected]>
Acked-by: Hiroshi DOYU <[email protected]>
---
arch/arm/plat-omap/include/plat/iommu.h | 3 ++
arch/arm/plat-omap/iommu.c | 33 +++++++++++++++++++++++++++++++
arch/arm/plat-omap/iovmm.c | 18 ++++++++++------
3 files changed, 47 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..2ea8ea3 100644
--- a/arch/arm/plat-omap/include/plat/iommu.h
+++ b/arch/arm/plat-omap/include/plat/iommu.h
@@ -103,6 +103,8 @@ struct iommu_platform_data {
const char *name;
const char *clk_name;
const int nr_tlb_entries;
+ u32 da_start;
+ u32 da_end;
};

#if defined(CONFIG_ARCH_OMAP1)
@@ -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..b3846bd 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,31 @@ 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)
+{
+ struct iommu_platform_data *pdata;
+
+ if (!obj)
+ return -EFAULT;
+
+ if (end < start || !PAGE_ALIGN(start | end))
+ return -EINVAL;
+
+ pdata = obj->dev->platform_data;
+
+ pdata->da_start = start;
+ pdata->da_end = end;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iommu_set_da_range);
+
+/**
* iommu_get - Get iommu handler
* @name: target iommu name
**/
@@ -853,6 +884,8 @@ struct iommu *iommu_get(const char *name)
if (err)
goto err_enable;
flush_iotlb_all(obj);
+ iommu_set_da_range(obj, IOMMU_DEFAULT_DA_START,
+ 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 fa6e643..ee3ca20 100644
--- a/arch/arm/plat-omap/iovmm.c
+++ b/arch/arm/plat-omap/iovmm.c
@@ -272,21 +272,25 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da,
{
struct iovm_struct *new, *tmp;
u32 start, prev_end, alignement;
+ struct iommu_platform_data *pdata;

if (!obj || !bytes)
return ERR_PTR(-EINVAL);

+ pdata = obj->dev->platform_data;
+
start = da;
alignement = PAGE_SIZE;

if (flags & IOVMF_DA_ANON) {
- /*
- * Reserve the first page for NULL
- */
- start = PAGE_SIZE;
+ start = pdata->da_start;
+
if (flags & IOVMF_LINEAR)
alignement = iopgsz_max(bytes);
start = roundup(start, alignement);
+ } else if (start < pdata->da_start || start > pdata->da_end ||
+ pdata->da_end - start < bytes) {
+ return ERR_PTR(-EINVAL);
}

tmp = NULL;
@@ -299,16 +303,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) && (pdata->da_end - start >= bytes))
goto found;

dev_dbg(obj->dev, "%s: no space to fit %08x(%x) flags: %08x\n",
--
1.7.3.2

2010-12-14 13:06:46

by Fernando Guzman Lugo

[permalink] [raw]
Subject: [PATCHv6 2/4] omap: iovmm - add superpages support to fixed da address

From: Guzman Lugo, Fernando <[email protected]>

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

Signed-off-by: Fernando Guzman Lugo <[email protected]>
Acked-by: Hiroshi DOYU <[email protected]>
---
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.7.3.2

2010-12-14 15:08:46

by Felipe Contreras

[permalink] [raw]
Subject: Re: [PATCHv6 4/4] omap: iommu - create new api to set valid da range

On Tue, Dec 14, 2010 at 3:07 PM, Fernando Guzman Lugo
<[email protected]> wrote:
> From: Guzman Lugo, Fernando <[email protected]>
>
> Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF rage.
> With this new API the valid range can be set.
>
> Signed-off-by: Fernando Guzman Lugo <[email protected]>
> Acked-by: Hiroshi DOYU <[email protected]>

I still don't see where this API is being used.

Also, the point of using platform data was to add it to omap3_devices
in omap-iommu.c, see[1]. I thought the platform data start/end would
move to struct iommu, and iommu_set_da_range would change the
start/end on struct iommu, but that would only be necessary if the
platform data doesn't match that.

If there's no sensible platform data default, then there's no point in
using platform data, as the platform is not the limiting factor, but
the DSP SW.

[1] http://article.gmane.org/gmane.linux.kernel/1051268

--
Felipe Contreras

2010-12-14 15:11:03

by Felipe Contreras

[permalink] [raw]
Subject: Re: [PATCH 0/4] omap: iovmm - fixes for iovmm module

On Tue, Dec 14, 2010 at 3:07 PM, Fernando Guzman Lugo
<[email protected]> wrote:

You are missing a description here.

> Version 6:
> * Rebase on Russell King branch.
>  - for details see:
>  http://marc.info/?l=linux-omap&m=129228495723001&w=2

Even if these patches are applied, iommu would still not be usable
form tidspbridge, you need at least this patch:
http://article.gmane.org/gmane.linux.ports.arm.omap/44753

Cheers.

--
Felipe Contreras

2010-12-14 18:09:32

by Fernando Guzman Lugo

[permalink] [raw]
Subject: Re: [PATCH 0/4] omap: iovmm - fixes for iovmm module

On Tue, Dec 14, 2010 at 9:10 AM, Felipe Contreras
<[email protected]> wrote:
> On Tue, Dec 14, 2010 at 3:07 PM, Fernando Guzman Lugo
> <[email protected]> wrote:
>
> You are missing a description here.

Yes, I forget the description but it is pretty much the same than the subject.

>
>> Version 6:
>> * Rebase on Russell King branch.
>> ?- for details see:
>> ?http://marc.info/?l=linux-omap&m=129228495723001&w=2
>
> Even if these patches are applied, iommu would still not be usable
> form tidspbridge, you need at least this patch:
> http://article.gmane.org/gmane.linux.ports.arm.omap/44753

I think Hari can take that patch too.

Regards,
Fernando.

>
> Cheers.
>
> --
> Felipe Contreras
>

2010-12-14 19:07:04

by Fernando Guzman Lugo

[permalink] [raw]
Subject: Re: [PATCHv6 4/4] omap: iommu - create new api to set valid da range

On Tue, Dec 14, 2010 at 9:08 AM, Felipe Contreras
<[email protected]> wrote:
> On Tue, Dec 14, 2010 at 3:07 PM, Fernando Guzman Lugo
> <[email protected]> wrote:
>> From: Guzman Lugo, Fernando <[email protected]>
>>
>> Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF rage.
>> With this new API the valid range can be set.
>>
>> Signed-off-by: Fernando Guzman Lugo <[email protected]>
>> Acked-by: Hiroshi DOYU <[email protected]>
>
> I still don't see where this API is being used.

you can find it here:

http://marc.info/?l=linux-kernel&m=128805403014740&w=2

I will add it to the iommu migration set once all the dependencies are merged.

>
> Also, the point of using platform data was to add it to omap3_devices
> in omap-iommu.c, see[1]. I thought the platform data start/end would
> move to struct iommu, and iommu_set_da_range would change the
> start/end on struct iommu, but that would only be necessary if the
> platform data doesn't match that.

it is not clear for me. do you mean having default start/end in
platform data then when iommu_get is call pass them to start/end in
struct iommu and the new api change start/end in struct iommu?

Please let me know if that is correct.

Regards,
Fernando.

>
> If there's no sensible platform data default, then there's no point in
> using platform data, as the platform is not the limiting factor, but
> the DSP SW.
>
> [1] http://article.gmane.org/gmane.linux.kernel/1051268
>
> --
> Felipe Contreras
>

2010-12-14 21:11:37

by Felipe Contreras

[permalink] [raw]
Subject: Re: [PATCHv6 4/4] omap: iommu - create new api to set valid da range

On Tue, Dec 14, 2010 at 8:59 PM, Guzman Lugo, Fernando
<[email protected]> wrote:
> On Tue, Dec 14, 2010 at 9:08 AM, Felipe Contreras
> <[email protected]> wrote:
>> On Tue, Dec 14, 2010 at 3:07 PM, Fernando Guzman Lugo
>> <[email protected]> wrote:
>>> From: Guzman Lugo, Fernando <[email protected]>
>>>
>>> Some IOMMUs cannot use the whole 0x0 - 0xFFFFFFFF rage.
>>> With this new API the valid range can be set.
>>>
>>> Signed-off-by: Fernando Guzman Lugo <[email protected]>
>>> Acked-by: Hiroshi DOYU <[email protected]>
>>
>> I still don't see where this API is being used.
>
> you can find it here:
>
> http://marc.info/?l=linux-kernel&m=128805403014740&w=2

Oh, ok, thanks. somehow I missed it.

>> Also, the point of using platform data was to add it to omap3_devices
>> in omap-iommu.c, see[1]. I thought the platform data start/end would
>> move to struct iommu, and iommu_set_da_range would change the
>> start/end on struct iommu, but that would only be necessary if the
>> platform data doesn't match that.
>
> it is not clear for me. do you mean having default start/end in
> platform data then when iommu_get is call pass them to start/end in
> struct iommu and the new api change start/end in struct iommu?
>
> Please let me know if that is correct.

Yeah, if you are not going to add it in the platform data
(mach-omap2/omap-iommu.c), then it doesn't make sense to use platform
data for that.

Cheers.

--
Felipe Contreras