* [PATCH v2 0/3] omap: iovmm: Fix IOVMM check for fixed 'da'
@ 2011-03-08 20:15 David Cohen
2011-03-08 20:15 ` [PATCH v2 1/3] omap: iovmm: disallow mapping NULL address when IOVMF_DA_ANON is set David Cohen
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: David Cohen @ 2011-03-08 20:15 UTC (permalink / raw)
To: Hiroshi.DOYU
Cc: linux-omap, fernando.lugo, linux-media, laurent.pinchart,
sakari.ailus, David Cohen
IOVMM driver checks input 'da == 0' when mapping address to determine whether
user wants fixed 'da' or not. At the same time, it doesn't disallow address
0x0 to be used, what creates an ambiguous situation. This patch set moves
fixed 'da' check to the input flags.
It also fixes da_start value for ISP IOMMU instance.
Br,
David Cohen
---
David Cohen (2):
omap3: change ISP's IOMMU da_start address
omap: iovmm: don't check 'da' to set IOVMF_DA_FIXED flag
Michael Jones (1):
omap: iovmm: disallow mapping NULL address when IOVMF_DA_ANON is set
arch/arm/mach-omap2/omap-iommu.c | 2 +-
arch/arm/plat-omap/include/plat/iovmm.h | 2 --
arch/arm/plat-omap/iovmm.c | 30 +++++++++++++++---------------
3 files changed, 16 insertions(+), 18 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v2 1/3] omap: iovmm: disallow mapping NULL address when IOVMF_DA_ANON is set 2011-03-08 20:15 [PATCH v2 0/3] omap: iovmm: Fix IOVMM check for fixed 'da' David Cohen @ 2011-03-08 20:15 ` David Cohen 2011-03-08 20:31 ` Guzman Lugo, Fernando 2011-03-08 20:15 ` [PATCH v2 2/3] omap3: change ISP's IOMMU da_start address David Cohen 2011-03-08 20:15 ` [PATCH v2 3/3] omap: iovmm: don't check 'da' to set IOVMF_DA_FIXED flag David Cohen 2 siblings, 1 reply; 9+ messages in thread From: David Cohen @ 2011-03-08 20:15 UTC (permalink / raw) To: Hiroshi.DOYU Cc: linux-omap, fernando.lugo, linux-media, laurent.pinchart, sakari.ailus, Michael Jones From: Michael Jones <michael.jones@matrix-vision.de> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping the NULL address if da_start==0, which would then not get unmapped. Disallow this again if IOVMF_DA_ANON is set. And spell variable 'alignment' correctly. Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> --- arch/arm/plat-omap/iovmm.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-) diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c index 6dc1296..e5f8341 100644 --- a/arch/arm/plat-omap/iovmm.c +++ b/arch/arm/plat-omap/iovmm.c @@ -271,20 +271,24 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da, size_t bytes, u32 flags) { struct iovm_struct *new, *tmp; - u32 start, prev_end, alignement; + u32 start, prev_end, alignment; if (!obj || !bytes) return ERR_PTR(-EINVAL); start = da; - alignement = PAGE_SIZE; + alignment = PAGE_SIZE; if (flags & IOVMF_DA_ANON) { - start = obj->da_start; + /* Don't map address 0 */ + if (obj->da_start) + start = obj->da_start; + else + start = alignment; if (flags & IOVMF_LINEAR) - alignement = iopgsz_max(bytes); - start = roundup(start, alignement); + alignment = iopgsz_max(bytes); + start = roundup(start, alignment); } else if (start < obj->da_start || start > obj->da_end || obj->da_end - start < bytes) { return ERR_PTR(-EINVAL); @@ -304,7 +308,7 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da, goto found; if (tmp->da_end >= start && flags & IOVMF_DA_ANON) - start = roundup(tmp->da_end + 1, alignement); + start = roundup(tmp->da_end + 1, alignment); prev_end = tmp->da_end; } -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] omap: iovmm: disallow mapping NULL address when IOVMF_DA_ANON is set 2011-03-08 20:15 ` [PATCH v2 1/3] omap: iovmm: disallow mapping NULL address when IOVMF_DA_ANON is set David Cohen @ 2011-03-08 20:31 ` Guzman Lugo, Fernando 2011-03-09 7:56 ` Michael Jones 0 siblings, 1 reply; 9+ messages in thread From: Guzman Lugo, Fernando @ 2011-03-08 20:31 UTC (permalink / raw) To: David Cohen Cc: Hiroshi.DOYU, linux-omap, linux-media, laurent.pinchart, sakari.ailus, Michael Jones On Tue, Mar 8, 2011 at 2:15 PM, David Cohen <dacohen@gmail.com> wrote: > From: Michael Jones <michael.jones@matrix-vision.de> > > commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping the NULL > address if da_start==0, which would then not get unmapped. Disallow > this again if IOVMF_DA_ANON is set. And spell variable 'alignment' > correctly. > > Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> > --- > arch/arm/plat-omap/iovmm.c | 16 ++++++++++------ > 1 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c > index 6dc1296..e5f8341 100644 > --- a/arch/arm/plat-omap/iovmm.c > +++ b/arch/arm/plat-omap/iovmm.c > @@ -271,20 +271,24 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da, > size_t bytes, u32 flags) > { > struct iovm_struct *new, *tmp; > - u32 start, prev_end, alignement; > + u32 start, prev_end, alignment; > > if (!obj || !bytes) > return ERR_PTR(-EINVAL); > > start = da; > - alignement = PAGE_SIZE; > + alignment = PAGE_SIZE; > > if (flags & IOVMF_DA_ANON) { > - start = obj->da_start; > + /* Don't map address 0 */ > + if (obj->da_start) > + start = obj->da_start; > + else > + start = alignment; looks good to me, just a nitpick comment, that would look better start = (obj->da_start) ? obj->da_start : alignment; Regards, Fernando. > > if (flags & IOVMF_LINEAR) > - alignement = iopgsz_max(bytes); > - start = roundup(start, alignement); > + alignment = iopgsz_max(bytes); > + start = roundup(start, alignment); > } else if (start < obj->da_start || start > obj->da_end || > obj->da_end - start < bytes) { > return ERR_PTR(-EINVAL); > @@ -304,7 +308,7 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da, > goto found; > > if (tmp->da_end >= start && flags & IOVMF_DA_ANON) > - start = roundup(tmp->da_end + 1, alignement); > + start = roundup(tmp->da_end + 1, alignment); > > prev_end = tmp->da_end; > } > -- > 1.7.0.4 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] omap: iovmm: disallow mapping NULL address when IOVMF_DA_ANON is set 2011-03-08 20:31 ` Guzman Lugo, Fernando @ 2011-03-09 7:56 ` Michael Jones 0 siblings, 0 replies; 9+ messages in thread From: Michael Jones @ 2011-03-09 7:56 UTC (permalink / raw) To: David Cohen Cc: Guzman Lugo, Fernando, Hiroshi.DOYU, linux-omap, linux-media, laurent.pinchart, sakari.ailus On 03/08/2011 09:31 PM, Guzman Lugo, Fernando wrote: > On Tue, Mar 8, 2011 at 2:15 PM, David Cohen <dacohen@gmail.com> wrote: >> From: Michael Jones <michael.jones@matrix-vision.de> >> >> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping the NULL >> address if da_start==0, which would then not get unmapped. Disallow >> this again if IOVMF_DA_ANON is set. And spell variable 'alignment' >> correctly. >> >> Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> >> --- >> arch/arm/plat-omap/iovmm.c | 16 ++++++++++------ >> 1 files changed, 10 insertions(+), 6 deletions(-) >> >> diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c >> index 6dc1296..e5f8341 100644 >> --- a/arch/arm/plat-omap/iovmm.c >> +++ b/arch/arm/plat-omap/iovmm.c >> @@ -271,20 +271,24 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da, >> size_t bytes, u32 flags) >> { >> struct iovm_struct *new, *tmp; >> - u32 start, prev_end, alignement; >> + u32 start, prev_end, alignment; >> >> if (!obj || !bytes) >> return ERR_PTR(-EINVAL); >> >> start = da; >> - alignement = PAGE_SIZE; >> + alignment = PAGE_SIZE; >> >> if (flags & IOVMF_DA_ANON) { >> - start = obj->da_start; >> + /* Don't map address 0 */ >> + if (obj->da_start) >> + start = obj->da_start; >> + else >> + start = alignment; > > looks good to me, just a nitpick comment, that would look better > > start = (obj->da_start) ? obj->da_start : alignment; > > Regards, > Fernando. > >> >> if (flags & IOVMF_LINEAR) >> - alignement = iopgsz_max(bytes); >> - start = roundup(start, alignement); >> + alignment = iopgsz_max(bytes); >> + start = roundup(start, alignment); >> } else if (start < obj->da_start || start > obj->da_end || >> obj->da_end - start < bytes) { >> return ERR_PTR(-EINVAL); >> @@ -304,7 +308,7 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da, >> goto found; >> >> if (tmp->da_end >= start && flags & IOVMF_DA_ANON) >> - start = roundup(tmp->da_end + 1, alignement); >> + start = roundup(tmp->da_end + 1, alignment); >> >> prev_end = tmp->da_end; >> } >> -- >> 1.7.0.4 >> Hi David, These changes to my patch are fine with me. If you want to incorporate Fernando's recommendation above, too, go ahead. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] omap3: change ISP's IOMMU da_start address 2011-03-08 20:15 [PATCH v2 0/3] omap: iovmm: Fix IOVMM check for fixed 'da' David Cohen 2011-03-08 20:15 ` [PATCH v2 1/3] omap: iovmm: disallow mapping NULL address when IOVMF_DA_ANON is set David Cohen @ 2011-03-08 20:15 ` David Cohen 2011-03-09 7:43 ` Sakari Ailus 2011-03-08 20:15 ` [PATCH v2 3/3] omap: iovmm: don't check 'da' to set IOVMF_DA_FIXED flag David Cohen 2 siblings, 1 reply; 9+ messages in thread From: David Cohen @ 2011-03-08 20:15 UTC (permalink / raw) To: Hiroshi.DOYU Cc: linux-omap, fernando.lugo, linux-media, laurent.pinchart, sakari.ailus, David Cohen ISP doesn't consider 0x0 as a valid address, so it should explicitly exclude first page from allowed 'da' range. Signed-off-by: David Cohen <dacohen@gmail.com> --- arch/arm/mach-omap2/omap-iommu.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c index 3fc5dc7..3bea489 100644 --- a/arch/arm/mach-omap2/omap-iommu.c +++ b/arch/arm/mach-omap2/omap-iommu.c @@ -33,7 +33,7 @@ static struct iommu_device omap3_devices[] = { .name = "isp", .nr_tlb_entries = 8, .clk_name = "cam_ick", - .da_start = 0x0, + .da_start = 0x1000, .da_end = 0xFFFFF000, }, }, -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] omap3: change ISP's IOMMU da_start address 2011-03-08 20:15 ` [PATCH v2 2/3] omap3: change ISP's IOMMU da_start address David Cohen @ 2011-03-09 7:43 ` Sakari Ailus 2011-03-09 8:19 ` David Cohen 0 siblings, 1 reply; 9+ messages in thread From: Sakari Ailus @ 2011-03-09 7:43 UTC (permalink / raw) To: David Cohen Cc: Hiroshi.DOYU, linux-omap, fernando.lugo, linux-media, laurent.pinchart David Cohen wrote: > ISP doesn't consider 0x0 as a valid address, so it should explicitly > exclude first page from allowed 'da' range. > > Signed-off-by: David Cohen <dacohen@gmail.com> > --- > arch/arm/mach-omap2/omap-iommu.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c > index 3fc5dc7..3bea489 100644 > --- a/arch/arm/mach-omap2/omap-iommu.c > +++ b/arch/arm/mach-omap2/omap-iommu.c > @@ -33,7 +33,7 @@ static struct iommu_device omap3_devices[] = { > .name = "isp", > .nr_tlb_entries = 8, > .clk_name = "cam_ick", > - .da_start = 0x0, > + .da_start = 0x1000, > .da_end = 0xFFFFF000, > }, > }, Hi David! Thanks for the patch. My question is once again: is this necessary? My understanding is that the IOMMU allows mapping the NULL address if the user wishes to map it explicitly. da_end specifies the real hardware limit for the mapped top address, da_start should do the same for bottom. I think that the IOMMU users should be either able to rely that they get no NULL allocated automatically for them. Do we want or not want it to be part of the API? I don't think the ISP driver is a special case of all the possible drivers using the IOMMU. On the other hand, probably there will be an API change at some point for the IOMMU since as far as I remember, there are somewhat established APIs for IOMMUs in existence. Regards, -- Sakari Ailus sakari.ailus@maxwell.research.nokia.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] omap3: change ISP's IOMMU da_start address 2011-03-09 7:43 ` Sakari Ailus @ 2011-03-09 8:19 ` David Cohen 0 siblings, 0 replies; 9+ messages in thread From: David Cohen @ 2011-03-09 8:19 UTC (permalink / raw) To: Sakari Ailus Cc: Hiroshi.DOYU, linux-omap, fernando.lugo, linux-media, laurent.pinchart On Wed, Mar 9, 2011 at 9:43 AM, Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> wrote: > David Cohen wrote: >> ISP doesn't consider 0x0 as a valid address, so it should explicitly >> exclude first page from allowed 'da' range. >> >> Signed-off-by: David Cohen <dacohen@gmail.com> >> --- >> arch/arm/mach-omap2/omap-iommu.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c >> index 3fc5dc7..3bea489 100644 >> --- a/arch/arm/mach-omap2/omap-iommu.c >> +++ b/arch/arm/mach-omap2/omap-iommu.c >> @@ -33,7 +33,7 @@ static struct iommu_device omap3_devices[] = { >> .name = "isp", >> .nr_tlb_entries = 8, >> .clk_name = "cam_ick", >> - .da_start = 0x0, >> + .da_start = 0x1000, >> .da_end = 0xFFFFF000, >> }, >> }, > > Hi David! Hi Sakari, > > Thanks for the patch. And thanks for the comments. :) > > My question is once again: is this necessary? My understanding is that > the IOMMU allows mapping the NULL address if the user wishes to map it > explicitly. da_end specifies the real hardware limit for the mapped top > address, da_start should do the same for bottom. Hm. da_start/da_end in this case belong to OMAP3 IOMMU ISP VM. It means they're related to OMAP3 ISP only. But they do not reflect the hw limit, as the range limit is anything which fits in u32. They say what's the range OMAP3 ISP is expecting to have mapped. And the answer to this question is the first page is not expected. Then, why say the opposite in da_start? > > I think that the IOMMU users should be either able to rely that they get > no NULL allocated automatically for them. Do we want or not want it to > be part of the API? I don't think the ISP driver is a special case of > all the possible drivers using the IOMMU. My understanding after this discussion is address 0x0 should be allowed (what is done by patch 3/3 of this set). But as a safer choice, it won't be returned without explicitly request (what is done in path 1/3). Because of that, I'm OK in drop this patch 2/3. But then there's one question which is the motivation for this change: If the current OMAP3 ISP driver doesn't want the first page, (1) should we pass a generic information and expect IOVMM driver to correct it to ISP need or (2) should we pass the information which reflects the real ISP driver's need? My understanding is (2). But I'm fine with (1) as it will lead to the same result. > > On the other hand, probably there will be an API change at some point > for the IOMMU since as far as I remember, there are somewhat > established APIs for IOMMUs in existence. At some point we need to find a standard for many IOMMU drivers. But for now we need to stick with what we have. :) Regards, David Cohen ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] omap: iovmm: don't check 'da' to set IOVMF_DA_FIXED flag 2011-03-08 20:15 [PATCH v2 0/3] omap: iovmm: Fix IOVMM check for fixed 'da' David Cohen 2011-03-08 20:15 ` [PATCH v2 1/3] omap: iovmm: disallow mapping NULL address when IOVMF_DA_ANON is set David Cohen 2011-03-08 20:15 ` [PATCH v2 2/3] omap3: change ISP's IOMMU da_start address David Cohen @ 2011-03-08 20:15 ` David Cohen 2011-03-08 20:42 ` Guzman Lugo, Fernando 2 siblings, 1 reply; 9+ messages in thread From: David Cohen @ 2011-03-08 20:15 UTC (permalink / raw) To: Hiroshi.DOYU Cc: linux-omap, fernando.lugo, linux-media, laurent.pinchart, sakari.ailus, David Cohen Currently IOVMM driver sets IOVMF_DA_FIXED/IOVMF_DA_ANON flags according to input 'da' address when mapping memory: da == 0: IOVMF_DA_ANON da != 0: IOVMF_DA_FIXED It prevents IOMMU to map first page with fixed 'da'. To avoid such issue, IOVMM will not automatically set IOVMF_DA_FIXED. It should now come from the user throught 'flags' parameter when mapping memory. As IOVMF_DA_ANON and IOVMF_DA_FIXED are mutually exclusive, IOVMF_DA_ANON can be removed. The driver will now check internally if IOVMF_DA_FIXED is set or not. Signed-off-by: David Cohen <dacohen@gmail.com> --- arch/arm/plat-omap/include/plat/iovmm.h | 2 -- arch/arm/plat-omap/iovmm.c | 14 +++++--------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/arch/arm/plat-omap/include/plat/iovmm.h b/arch/arm/plat-omap/include/plat/iovmm.h index bdc7ce5..32a2f6c 100644 --- a/arch/arm/plat-omap/include/plat/iovmm.h +++ b/arch/arm/plat-omap/include/plat/iovmm.h @@ -71,8 +71,6 @@ struct iovm_struct { #define IOVMF_LINEAR_MASK (3 << (2 + IOVMF_SW_SHIFT)) #define IOVMF_DA_FIXED (1 << (4 + IOVMF_SW_SHIFT)) -#define IOVMF_DA_ANON (2 << (4 + IOVMF_SW_SHIFT)) -#define IOVMF_DA_MASK (3 << (4 + IOVMF_SW_SHIFT)) extern struct iovm_struct *find_iovm_area(struct iommu *obj, u32 da); diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c index e5f8341..894489c 100644 --- a/arch/arm/plat-omap/iovmm.c +++ b/arch/arm/plat-omap/iovmm.c @@ -279,7 +279,7 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da, start = da; alignment = PAGE_SIZE; - if (flags & IOVMF_DA_ANON) { + if (~flags & IOVMF_DA_FIXED) { /* Don't map address 0 */ if (obj->da_start) start = obj->da_start; @@ -307,7 +307,7 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da, if (tmp->da_start > start && (tmp->da_start - start) >= bytes) goto found; - if (tmp->da_end >= start && flags & IOVMF_DA_ANON) + if (tmp->da_end >= start && ~flags & IOVMF_DA_FIXED) start = roundup(tmp->da_end + 1, alignment); prev_end = tmp->da_end; @@ -654,7 +654,6 @@ u32 iommu_vmap(struct iommu *obj, u32 da, const struct sg_table *sgt, flags &= IOVMF_HW_MASK; flags |= IOVMF_DISCONT; flags |= IOVMF_MMIO; - flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON); da = __iommu_vmap(obj, da, sgt, va, bytes, flags); if (IS_ERR_VALUE(da)) @@ -694,7 +693,7 @@ EXPORT_SYMBOL_GPL(iommu_vunmap); * @flags: iovma and page property * * Allocate @bytes linearly and creates 1-n-1 mapping and returns - * @da again, which might be adjusted if 'IOVMF_DA_ANON' is set. + * @da again, which might be adjusted if 'IOVMF_DA_FIXED' is not set. */ u32 iommu_vmalloc(struct iommu *obj, u32 da, size_t bytes, u32 flags) { @@ -713,7 +712,6 @@ u32 iommu_vmalloc(struct iommu *obj, u32 da, size_t bytes, u32 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)) { @@ -784,7 +782,7 @@ static u32 __iommu_kmap(struct iommu *obj, u32 da, u32 pa, void *va, * @flags: iovma and page property * * Creates 1-1-1 mapping and returns @da again, which can be - * adjusted if 'IOVMF_DA_ANON' is set. + * adjusted if 'IOVMF_DA_FIXED' is not set. */ u32 iommu_kmap(struct iommu *obj, u32 da, u32 pa, size_t bytes, u32 flags) @@ -803,7 +801,6 @@ u32 iommu_kmap(struct iommu *obj, u32 da, u32 pa, size_t bytes, flags &= IOVMF_HW_MASK; flags |= IOVMF_LINEAR; flags |= IOVMF_MMIO; - flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON); da = __iommu_kmap(obj, da, pa, va, bytes, flags); if (IS_ERR_VALUE(da)) @@ -842,7 +839,7 @@ EXPORT_SYMBOL_GPL(iommu_kunmap); * @flags: iovma and page property * * Allocate @bytes linearly and creates 1-1-1 mapping and returns - * @da again, which might be adjusted if 'IOVMF_DA_ANON' is set. + * @da again, which might be adjusted if 'IOVMF_DA_FIXED' is not set. */ u32 iommu_kmalloc(struct iommu *obj, u32 da, size_t bytes, u32 flags) { @@ -862,7 +859,6 @@ u32 iommu_kmalloc(struct iommu *obj, u32 da, size_t bytes, u32 flags) flags &= IOVMF_HW_MASK; flags |= IOVMF_LINEAR; flags |= IOVMF_ALLOC; - flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON); da = __iommu_kmap(obj, da, pa, va, bytes, flags); if (IS_ERR_VALUE(da)) -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] omap: iovmm: don't check 'da' to set IOVMF_DA_FIXED flag 2011-03-08 20:15 ` [PATCH v2 3/3] omap: iovmm: don't check 'da' to set IOVMF_DA_FIXED flag David Cohen @ 2011-03-08 20:42 ` Guzman Lugo, Fernando 0 siblings, 0 replies; 9+ messages in thread From: Guzman Lugo, Fernando @ 2011-03-08 20:42 UTC (permalink / raw) To: David Cohen Cc: Hiroshi.DOYU, linux-omap, linux-media, laurent.pinchart, sakari.ailus On Tue, Mar 8, 2011 at 2:15 PM, David Cohen <dacohen@gmail.com> wrote: > Currently IOVMM driver sets IOVMF_DA_FIXED/IOVMF_DA_ANON flags according > to input 'da' address when mapping memory: > da == 0: IOVMF_DA_ANON > da != 0: IOVMF_DA_FIXED > > It prevents IOMMU to map first page with fixed 'da'. To avoid such > issue, IOVMM will not automatically set IOVMF_DA_FIXED. It should now > come from the user throught 'flags' parameter when mapping memory. > As IOVMF_DA_ANON and IOVMF_DA_FIXED are mutually exclusive, IOVMF_DA_ANON > can be removed. The driver will now check internally if IOVMF_DA_FIXED > is set or not. Looks good to me. Regards, Fernando. > > Signed-off-by: David Cohen <dacohen@gmail.com> > --- > arch/arm/plat-omap/include/plat/iovmm.h | 2 -- > arch/arm/plat-omap/iovmm.c | 14 +++++--------- > 2 files changed, 5 insertions(+), 11 deletions(-) > > diff --git a/arch/arm/plat-omap/include/plat/iovmm.h b/arch/arm/plat-omap/include/plat/iovmm.h > index bdc7ce5..32a2f6c 100644 > --- a/arch/arm/plat-omap/include/plat/iovmm.h > +++ b/arch/arm/plat-omap/include/plat/iovmm.h > @@ -71,8 +71,6 @@ struct iovm_struct { > #define IOVMF_LINEAR_MASK (3 << (2 + IOVMF_SW_SHIFT)) > > #define IOVMF_DA_FIXED (1 << (4 + IOVMF_SW_SHIFT)) > -#define IOVMF_DA_ANON (2 << (4 + IOVMF_SW_SHIFT)) > -#define IOVMF_DA_MASK (3 << (4 + IOVMF_SW_SHIFT)) > > > extern struct iovm_struct *find_iovm_area(struct iommu *obj, u32 da); > diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c > index e5f8341..894489c 100644 > --- a/arch/arm/plat-omap/iovmm.c > +++ b/arch/arm/plat-omap/iovmm.c > @@ -279,7 +279,7 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da, > start = da; > alignment = PAGE_SIZE; > > - if (flags & IOVMF_DA_ANON) { > + if (~flags & IOVMF_DA_FIXED) { > /* Don't map address 0 */ > if (obj->da_start) > start = obj->da_start; > @@ -307,7 +307,7 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da, > if (tmp->da_start > start && (tmp->da_start - start) >= bytes) > goto found; > > - if (tmp->da_end >= start && flags & IOVMF_DA_ANON) > + if (tmp->da_end >= start && ~flags & IOVMF_DA_FIXED) > start = roundup(tmp->da_end + 1, alignment); > > prev_end = tmp->da_end; > @@ -654,7 +654,6 @@ u32 iommu_vmap(struct iommu *obj, u32 da, const struct sg_table *sgt, > flags &= IOVMF_HW_MASK; > flags |= IOVMF_DISCONT; > flags |= IOVMF_MMIO; > - flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON); > > da = __iommu_vmap(obj, da, sgt, va, bytes, flags); > if (IS_ERR_VALUE(da)) > @@ -694,7 +693,7 @@ EXPORT_SYMBOL_GPL(iommu_vunmap); > * @flags: iovma and page property > * > * Allocate @bytes linearly and creates 1-n-1 mapping and returns > - * @da again, which might be adjusted if 'IOVMF_DA_ANON' is set. > + * @da again, which might be adjusted if 'IOVMF_DA_FIXED' is not set. > */ > u32 iommu_vmalloc(struct iommu *obj, u32 da, size_t bytes, u32 flags) > { > @@ -713,7 +712,6 @@ u32 iommu_vmalloc(struct iommu *obj, u32 da, size_t bytes, u32 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)) { > @@ -784,7 +782,7 @@ static u32 __iommu_kmap(struct iommu *obj, u32 da, u32 pa, void *va, > * @flags: iovma and page property > * > * Creates 1-1-1 mapping and returns @da again, which can be > - * adjusted if 'IOVMF_DA_ANON' is set. > + * adjusted if 'IOVMF_DA_FIXED' is not set. > */ > u32 iommu_kmap(struct iommu *obj, u32 da, u32 pa, size_t bytes, > u32 flags) > @@ -803,7 +801,6 @@ u32 iommu_kmap(struct iommu *obj, u32 da, u32 pa, size_t bytes, > flags &= IOVMF_HW_MASK; > flags |= IOVMF_LINEAR; > flags |= IOVMF_MMIO; > - flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON); > > da = __iommu_kmap(obj, da, pa, va, bytes, flags); > if (IS_ERR_VALUE(da)) > @@ -842,7 +839,7 @@ EXPORT_SYMBOL_GPL(iommu_kunmap); > * @flags: iovma and page property > * > * Allocate @bytes linearly and creates 1-1-1 mapping and returns > - * @da again, which might be adjusted if 'IOVMF_DA_ANON' is set. > + * @da again, which might be adjusted if 'IOVMF_DA_FIXED' is not set. > */ > u32 iommu_kmalloc(struct iommu *obj, u32 da, size_t bytes, u32 flags) > { > @@ -862,7 +859,6 @@ u32 iommu_kmalloc(struct iommu *obj, u32 da, size_t bytes, u32 flags) > flags &= IOVMF_HW_MASK; > flags |= IOVMF_LINEAR; > flags |= IOVMF_ALLOC; > - flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON); > > da = __iommu_kmap(obj, da, pa, va, bytes, flags); > if (IS_ERR_VALUE(da)) > -- > 1.7.0.4 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-03-09 8:19 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-08 20:15 [PATCH v2 0/3] omap: iovmm: Fix IOVMM check for fixed 'da' David Cohen 2011-03-08 20:15 ` [PATCH v2 1/3] omap: iovmm: disallow mapping NULL address when IOVMF_DA_ANON is set David Cohen 2011-03-08 20:31 ` Guzman Lugo, Fernando 2011-03-09 7:56 ` Michael Jones 2011-03-08 20:15 ` [PATCH v2 2/3] omap3: change ISP's IOMMU da_start address David Cohen 2011-03-09 7:43 ` Sakari Ailus 2011-03-09 8:19 ` David Cohen 2011-03-08 20:15 ` [PATCH v2 3/3] omap: iovmm: don't check 'da' to set IOVMF_DA_FIXED flag David Cohen 2011-03-08 20:42 ` Guzman Lugo, Fernando
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox