Re: [RFC 2/6] omap: iovmm: generic iommu api migration

2011-06-09 Thread Ohad Ben-Cohen
On Wed, Jun 8, 2011 at 1:46 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 On Tuesday 07 June 2011 15:46:26 Ohad Ben-Cohen wrote:
 Where do you take care of those potential offsets today ? Or do you
 simply ignore the offsets and map the entire page ?

 Here http://marc.info/?l=linux-omapm=130693502326513w=2 :-)

:)

Ok so it seems iovmm will take care of that for now ?

Let's get the basics working with the IOMMU API and then revise this
when we switch from iovmm to the generic dma.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 2/6] omap: iovmm: generic iommu api migration

2011-06-08 Thread Laurent Pinchart
Hi Ohad,

On Tuesday 07 June 2011 15:46:26 Ohad Ben-Cohen wrote:
 On Tue, Jun 7, 2011 at 2:26 PM, Laurent Pinchart wrote:
  Right now we have a BUG_ON if pa is unaligned, but that can be changed
  if needed (do we want it to handle offsets ?).
  
  At least for the OMAP3 ISP we need to, as video buffers don't necessarily
  start on page boundaries.
 
 Where do you take care of those potential offsets today ? Or do you
 simply ignore the offsets and map the entire page ?

Here http://marc.info/?l=linux-omapm=130693502326513w=2 :-)

 Seems like omap's iommu (mostly) rejects unaligned pa addresses, see:
 
 4abb761749abfb4ec403e4054f9dae2ee604e54f omap iommu: Reject unaligned
 addresses at setting page table entry
 
 (this doesn't seem to cover 4KB entries though, only large pages,
 sections and super sections)
 
  A separate patch is indeed needed, yes. As you're already working on
  iommu it might be simpler if you add it to your tree.
 
 Sure, i'll send it.

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 2/6] omap: iovmm: generic iommu api migration

2011-06-07 Thread Laurent Pinchart
Hi Ohad,

Thanks for the patch.

On Friday 03 June 2011 00:27:39 Ohad Ben-Cohen wrote:
 Migrate omap's iovmm (virtual memory manager) to the generic iommu api.
 
 This brings iovmm a step forward towards being completely non
 omap-specific (it's still assuming omap's iommu page sizes, and also
 maintaining state inside omap's internal iommu structure, but it no
 longer calls omap-specific iommu map/unmap api).
 
 Further generalizing of iovmm (or complete removal) should take place
 together with broader plans of providing a generic virtual memory manager
 and allocation framework (de-coupled from specific mappers).
 
 Signed-off-by: Ohad Ben-Cohen o...@wizery.com

[snip]

 diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
 index 51ef43e..80bb2b6 100644
 --- a/arch/arm/plat-omap/iovmm.c
 +++ b/arch/arm/plat-omap/iovmm.c

[snip]

 @@ -473,22 +475,22 @@ static int map_iovm_area(struct iommu *obj, struct
 iovm_struct *new, u32 pa;
   int pgsz;
   size_t bytes;
 - struct iotlb_entry e;
 
   pa = sg_phys(sg);
   bytes = sg_dma_len(sg);
 
   flags = ~IOVMF_PGSZ_MASK;
 +
   pgsz = bytes_to_iopgsz(bytes);
   if (pgsz  0)
   goto err_out;
 - flags |= pgsz;

pgsz isn't used anymore, you can remove it.

 +
 + order = get_order(bytes);

Does iommu_map() handle offsets correctly, or does it expect pa to be aligned 
to an order (or other) boundary ? Same comment for iommu_unmap() in 
unmap_iovm_area().

   pr_debug(%s: [%d] %08x %08x(%x)\n, __func__,
i, da, pa, bytes);
 
 - iotlb_init_entry(e, da, pa, flags);
 - err = iopgtable_store_entry(obj, e);
 + err = iommu_map(domain, da, pa, order, flags);
   if (err)
   goto err_out;
 
 @@ -502,9 +504,11 @@ err_out:
   for_each_sg(sgt-sgl, sg, i, j) {
   size_t bytes;
 
 - bytes = iopgtable_clear_entry(obj, da);
 + bytes = sg_dma_len(sg);

As Russell pointed out, we should use sg-length instead of sg_dma_length(sg). 
sg_dma_length(sg) is only valid after the scatter list has been DMA-mapped, 
which doesn't happen in the iovmm driver. This applies to all sg_dma_len(sg) 
calls.

 + order = get_order(bytes);
 
 - BUG_ON(!iopgsz_ok(bytes));
 + /* ignore failures.. we're already handling one */
 + iommu_unmap(domain, da, order);
 
   da += bytes;
   }

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 2/6] omap: iovmm: generic iommu api migration

2011-06-07 Thread Ohad Ben-Cohen
Hi Laurent,

On Tue, Jun 7, 2011 at 12:05 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 pgsz isn't used anymore, you can remove it.

Ok.

 +             order = get_order(bytes);

 Does iommu_map() handle offsets correctly, or does it expect pa to be aligned
 to an order (or other) boundary ?

Right now we have a BUG_ON if pa is unaligned, but that can be changed
if needed (do we want it to handle offsets ?).

 As Russell pointed out, we should use sg-length instead of sg_dma_length(sg).
 sg_dma_length(sg) is only valid after the scatter list has been DMA-mapped,
 which doesn't happen in the iovmm driver. This applies to all sg_dma_len(sg)
 calls.

I'll make sure I don't introduce such calls, but it sounds like a
separate patch should take care of the existing ones; pls tell me if
you want me to send one.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 2/6] omap: iovmm: generic iommu api migration

2011-06-07 Thread Laurent Pinchart
Hi Ohad,

On Tuesday 07 June 2011 12:28:53 Ohad Ben-Cohen wrote:
 On Tue, Jun 7, 2011 at 12:05 PM, Laurent Pinchart wrote:
  pgsz isn't used anymore, you can remove it.
 
 Ok.
 
  + order = get_order(bytes);
  
  Does iommu_map() handle offsets correctly, or does it expect pa to be
  aligned to an order (or other) boundary ?
 
 Right now we have a BUG_ON if pa is unaligned, but that can be changed
 if needed (do we want it to handle offsets ?).

At least for the OMAP3 ISP we need to, as video buffers don't necessarily 
start on page boundaries.

  As Russell pointed out, we should use sg-length instead of
  sg_dma_length(sg). sg_dma_length(sg) is only valid after the scatter
  list has been DMA-mapped, which doesn't happen in the iovmm driver. This
  applies to all sg_dma_len(sg) calls.
 
 I'll make sure I don't introduce such calls, but it sounds like a
 separate patch should take care of the existing ones; pls tell me if
 you want me to send one.

A separate patch is indeed needed, yes. As you're already working on iommu it 
might be simpler if you add it to your tree. Otherwise I can send it.

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 2/6] omap: iovmm: generic iommu api migration

2011-06-07 Thread Ohad Ben-Cohen
Hi Laurent,

On Tue, Jun 7, 2011 at 2:26 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Right now we have a BUG_ON if pa is unaligned, but that can be changed
 if needed (do we want it to handle offsets ?).

 At least for the OMAP3 ISP we need to, as video buffers don't necessarily
 start on page boundaries.

Where do you take care of those potential offsets today ? Or do you
simply ignore the offsets and map the entire page ?

Seems like omap's iommu (mostly) rejects unaligned pa addresses, see:

4abb761749abfb4ec403e4054f9dae2ee604e54f omap iommu: Reject unaligned
addresses at setting page table entry

(this doesn't seem to cover 4KB entries though, only large pages,
sections and super sections)

 A separate patch is indeed needed, yes. As you're already working on iommu it
 might be simpler if you add it to your tree.

Sure, i'll send it.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC 2/6] omap: iovmm: generic iommu api migration

2011-06-02 Thread Ohad Ben-Cohen
Migrate omap's iovmm (virtual memory manager) to the generic iommu api.

This brings iovmm a step forward towards being completely non
omap-specific (it's still assuming omap's iommu page sizes, and also
maintaining state inside omap's internal iommu structure, but it no
longer calls omap-specific iommu map/unmap api).

Further generalizing of iovmm (or complete removal) should take place
together with broader plans of providing a generic virtual memory manager
and allocation framework (de-coupled from specific mappers).

Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
 arch/arm/plat-omap/include/plat/iovmm.h |   27 +---
 arch/arm/plat-omap/iovmm.c  |  111 ++-
 2 files changed, 82 insertions(+), 56 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/iovmm.h 
b/arch/arm/plat-omap/include/plat/iovmm.h
index 32a2f6c..56ee2b9 100644
--- a/arch/arm/plat-omap/include/plat/iovmm.h
+++ b/arch/arm/plat-omap/include/plat/iovmm.h
@@ -13,6 +13,8 @@
 #ifndef __IOMMU_MMAP_H
 #define __IOMMU_MMAP_H
 
+#include linux/iommu.h
+
 struct iovm_struct {
struct iommu*iommu; /* iommu object which this belongs to */
u32 da_start; /* area definition */
@@ -74,18 +76,21 @@ struct iovm_struct {
 
 
 extern struct iovm_struct *find_iovm_area(struct iommu *obj, u32 da);
-extern u32 iommu_vmap(struct iommu *obj, u32 da,
+extern u32 iommu_vmap(struct iommu_domain *domain, struct iommu *obj, u32 da,
const struct sg_table *sgt, u32 flags);
-extern struct sg_table *iommu_vunmap(struct iommu *obj, u32 da);
-extern u32 iommu_vmalloc(struct iommu *obj, u32 da, size_t bytes,
-  u32 flags);
-extern void iommu_vfree(struct iommu *obj, const u32 da);
-extern u32 iommu_kmap(struct iommu *obj, u32 da, u32 pa, size_t bytes,
-   u32 flags);
-extern void iommu_kunmap(struct iommu *obj, u32 da);
-extern u32 iommu_kmalloc(struct iommu *obj, u32 da, size_t bytes,
-  u32 flags);
-extern void iommu_kfree(struct iommu *obj, u32 da);
+extern struct sg_table *iommu_vunmap(struct iommu_domain *domain,
+   struct iommu *obj, u32 da);
+extern u32 iommu_vmalloc(struct iommu_domain *domain, struct iommu *obj,
+   u32 da, size_t bytes, u32 flags);
+extern void iommu_vfree(struct iommu_domain *domain, struct iommu *obj,
+   const u32 da);
+extern u32 iommu_kmap(struct iommu_domain *domain, struct iommu *obj, u32 da,
+   u32 pa, size_t bytes, u32 flags);
+extern void iommu_kunmap(struct iommu_domain *domain, struct iommu *obj,
+   u32 da);
+extern u32 iommu_kmalloc(struct iommu_domain *domain, struct iommu *obj,
+   u32 da, size_t bytes, u32 flags);
+extern void iommu_kfree(struct iommu_domain *domain, struct iommu *obj, u32 
da);
 
 extern void *da_to_va(struct iommu *obj, u32 da);
 
diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
index 51ef43e..80bb2b6 100644
--- a/arch/arm/plat-omap/iovmm.c
+++ b/arch/arm/plat-omap/iovmm.c
@@ -15,6 +15,7 @@
 #include linux/vmalloc.h
 #include linux/device.h
 #include linux/scatterlist.h
+#include linux/iommu.h
 
 #include asm/cacheflush.h
 #include asm/mach/map.h
@@ -456,15 +457,16 @@ static inline void sgtable_drain_kmalloc(struct sg_table 
*sgt)
 }
 
 /* create 'da' - 'pa' mapping from 'sgt' */
-static int map_iovm_area(struct iommu *obj, struct iovm_struct *new,
-const struct sg_table *sgt, u32 flags)
+static int map_iovm_area(struct iommu_domain *domain, struct iovm_struct *new,
+   const struct sg_table *sgt, u32 flags)
 {
int err;
unsigned int i, j;
struct scatterlist *sg;
u32 da = new-da_start;
+   int order;
 
-   if (!obj || !sgt)
+   if (!domain || !sgt)
return -EINVAL;
 
BUG_ON(!sgtable_ok(sgt));
@@ -473,22 +475,22 @@ static int map_iovm_area(struct iommu *obj, struct 
iovm_struct *new,
u32 pa;
int pgsz;
size_t bytes;
-   struct iotlb_entry e;
 
pa = sg_phys(sg);
bytes = sg_dma_len(sg);
 
flags = ~IOVMF_PGSZ_MASK;
+
pgsz = bytes_to_iopgsz(bytes);
if (pgsz  0)
goto err_out;
-   flags |= pgsz;
+
+   order = get_order(bytes);
 
pr_debug(%s: [%d] %08x %08x(%x)\n, __func__,
 i, da, pa, bytes);
 
-   iotlb_init_entry(e, da, pa, flags);
-   err = iopgtable_store_entry(obj, e);
+   err = iommu_map(domain, da, pa, order, flags);
if (err)
goto err_out;
 
@@ -502,9 +504,11 @@ err_out:
for_each_sg(sgt-sgl, sg, i, j) {
size_t