staging tidspbridge: iommu migration

drivers/staging/tidspbridge/core/io_sm.c +1 -1

index 5718645..842b8db 100644
@@ -291,6 +291,7 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
        struct cod_manager *cod_man;
        struct chnl_mgr *hchnl_mgr;
        struct msg_mgr *hmsg_mgr;
+       struct iommu *mmu;
        u32 ul_shm_base;
        u32 ul_shm_base_offset;
        u32 ul_shm_limit;

@@ -313,7 +314,6 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
        struct bridge_ioctl_extproc ae_proc[BRDIOCTL_NUMOFMMUTLB];
        struct cfg_hostres *host_res;
        struct bridge_dev_context *pbridge_context;
-       u32 map_attrs;
        u32 shm0_end;
        u32 ul_dyn_ext_base;
        u32 ul_seg1_size = 0;

drivers/staging/tidspbridge/core/io_sm.c +11 -68

@@ -1122,217 +1081,81 @@ static int bridge_brd_mem_write(struct bridge_dev_context *dev_ctxt,
  *
  *  TODO: Disable MMU while updating the page tables (but that'll stall DSP)
  */
-static int bridge_brd_mem_map(struct bridge_dev_context *dev_ctxt,
-                                 u32 ul_mpu_addr, u32 virt_addr,
-                                 u32 ul_num_bytes, u32 ul_map_attr,
-                                 struct page **mapped_pages)
+static int bridge_brd_mem_map(struct bridge_dev_context *dev_ctx,
+                       u32 uva, u32 da, u32 size, u32 attr,
+                       struct page **usr_pgs)
+
 {
-       u32 attrs;
-       int status = 0;
-       struct bridge_dev_context *dev_context = dev_ctxt;
-       struct hw_mmu_map_attrs_t hw_attrs;
+       int res, w;
+       unsigned pages, i;
+       struct iommu *mmu = dev_ctx->dsp_mmu;
        struct vm_area_struct *vma;
        struct mm_struct *mm = current->mm;
-       u32 write = 0;
-       u32 num_usr_pgs = 0;
-       struct page *mapped_page, *pg;
-       s32 pg_num;
-       u32 va = virt_addr;
-       struct task_struct *curr_task = current;
-       u32 pg_i = 0;
-       u32 mpu_addr, pa;
-
-       dev_dbg(bridge,
-               "%s hDevCtxt %p, pa %x, va %x, size %x, ul_map_attr %x\n",
-               __func__, dev_ctxt, ul_mpu_addr, virt_addr, ul_num_bytes,
-               ul_map_attr);
-       if (ul_num_bytes == 0)
-               return -EINVAL;
+       struct sg_table *sgt;
+       struct scatterlist *sg;
 
-       if (ul_map_attr & DSP_MAP_DIR_MASK) {
-               attrs = ul_map_attr;
-       } else {
-               /* Assign default attributes */
-               attrs = ul_map_attr | (DSP_MAPVIRTUALADDR | DSP_MAPELEMSIZE16);
-       }
-       /* Take mapping properties */
-       if (attrs & DSP_MAPBIGENDIAN)
-               hw_attrs.endianism = HW_BIG_ENDIAN;
-       else
-               hw_attrs.endianism = HW_LITTLE_ENDIAN;
-
-       hw_attrs.mixed_size = (enum hw_mmu_mixed_size_t)
-           ((attrs & DSP_MAPMIXEDELEMSIZE) >> 2);
-       /* Ignore element_size if mixed_size is enabled */
-       if (hw_attrs.mixed_size == 0) {
-               if (attrs & DSP_MAPELEMSIZE8) {
-                       /* Size is 8 bit */
-                       hw_attrs.element_size = HW_ELEM_SIZE8BIT;
-               } else if (attrs & DSP_MAPELEMSIZE16) {
-                       /* Size is 16 bit */
-                       hw_attrs.element_size = HW_ELEM_SIZE16BIT;
-               } else if (attrs & DSP_MAPELEMSIZE32) {
-                       /* Size is 32 bit */
-                       hw_attrs.element_size = HW_ELEM_SIZE32BIT;
-               } else if (attrs & DSP_MAPELEMSIZE64) {
-                       /* Size is 64 bit */
-                       hw_attrs.element_size = HW_ELEM_SIZE64BIT;
-               } else {
-                       /*
-                        * Mixedsize isn't enabled, so size can't be
-                        * zero here
-                        */
-                       return -EINVAL;
-               }
-       }
-       if (attrs & DSP_MAPDONOTLOCK)
-               hw_attrs.donotlockmpupage = 1;
-       else
-               hw_attrs.donotlockmpupage = 0;
+       if (!size || !usr_pgs)
+               return -EINVAL;
 
-       if (attrs & DSP_MAPVMALLOCADDR) {
-               return mem_map_vmalloc(dev_ctxt, ul_mpu_addr, virt_addr,
-                                      ul_num_bytes, &hw_attrs);
-       }
-       /*

drivers/staging/tidspbridge/core/io_sm.c +27 -118

-       /*
-        * Important Note: ul_mpu_addr is mapped from user application process
-        * to current process - it must lie completely within the current
-        * virtual memory address space in order to be of use to us here!
-        */
        down_read(&mm->mmap_sem);
-       vma = find_vma(mm, ul_mpu_addr);
-       if (vma)
-               dev_dbg(bridge,
-                       "VMAfor UserBuf: ul_mpu_addr=%x, ul_num_bytes=%x, "
-                       "vm_start=%lx, vm_end=%lx, vm_flags=%lx\n", ul_mpu_addr,
-                       ul_num_bytes, vma->vm_start, vma->vm_end,
-                       vma->vm_flags);
-
-       /*
-        * It is observed that under some circumstances, the user buffer is
-        * spread across several VMAs. So loop through and check if the entire
-        * user buffer is covered
-        */
-       while ((vma) && (ul_mpu_addr + ul_num_bytes > vma->vm_end)) {
-               /* jump to the next VMA region */
+       vma = find_vma(mm, uva);
+       while (vma && (uva + size > vma->vm_end))
                vma = find_vma(mm, vma->vm_end + 1);
-               dev_dbg(bridge,
-                       "VMA for UserBuf ul_mpu_addr=%x ul_num_bytes=%x, "
-                       "vm_start=%lx, vm_end=%lx, vm_flags=%lx\n", ul_mpu_addr,
-                       ul_num_bytes, vma->vm_start, vma->vm_end,
-                       vma->vm_flags);
-       }
+
        if (!vma) {
                pr_err("%s: Failed to get VMA region for 0x%x (%d)\n",
-                      __func__, ul_mpu_addr, ul_num_bytes);
-               status = -EINVAL;
+                                               __func__, uva, size);
                up_read(&mm->mmap_sem);
-               goto func_cont;
+               return -EINVAL;
        }
+       if (vma->vm_flags & (VM_WRITE | VM_MAYWRITE))
+               w = 1;
 
-       if (vma->vm_flags & VM_IO) {
-               num_usr_pgs = ul_num_bytes / PG_SIZE4K;
-               mpu_addr = ul_mpu_addr;
-
-               /* Get the physical addresses for user buffer */
-               for (pg_i = 0; pg_i < num_usr_pgs; pg_i++) {
-                       pa = user_va2_pa(mm, mpu_addr);
-                       if (!pa) {
-                               status = -EPERM;
-                               pr_err("DSPBRIDGE: VM_IO mapping physical"
-                                      "address is invalid\n");
-                               break;
-                       }
-                       if (pfn_valid(__phys_to_pfn(pa))) {
-                               pg = PHYS_TO_PAGE(pa);
-                               get_page(pg);
-                               if (page_count(pg) < 1) {
-                                       pr_err("Bad page in VM_IO buffer\n");
-                                       bad_page_dump(pa, pg);
-                               }
-                       }
-                       status = pte_set(dev_context->pt_attrs, pa,
-                                        va, HW_PAGE_SIZE4KB, &hw_attrs);
-                       if (status)
-                               break;
+       if (vma->vm_flags & VM_IO)
+               i = get_io_pages(mm, uva, pages, usr_pgs);
+       else
+               i = get_user_pages(current, mm, uva, pages, w, 1,
+                                                       usr_pgs, NULL);
+       up_read(&mm->mmap_sem);
 
-                       va += HW_PAGE_SIZE4KB;
-                       mpu_addr += HW_PAGE_SIZE4KB;
-                       pa += HW_PAGE_SIZE4KB;
-               }
-       } else {
-               num_usr_pgs = ul_num_bytes / PG_SIZE4K;
-               if (vma->vm_flags & (VM_WRITE | VM_MAYWRITE))
-                       write = 1;
-
-               for (pg_i = 0; pg_i < num_usr_pgs; pg_i++) {
-                       pg_num = get_user_pages(curr_task, mm, ul_mpu_addr, 1,
-                                               write, 1, &mapped_page, NULL);
-                       if (pg_num > 0) {
-                               if (page_count(mapped_page) < 1) {
-                                       pr_err("Bad page count after doing"
-                                              "get_user_pages on"
-                                              "user buffer\n");
-                                       bad_page_dump(page_to_phys(mapped_page),
-                                                     mapped_page);
-                               }
-                               status = pte_set(dev_context->pt_attrs,
-                                                page_to_phys(mapped_page), va,
-                                                HW_PAGE_SIZE4KB, &hw_attrs);
-                               if (status)
-                                       break;
-
-                               if (mapped_pages)
-                                       mapped_pages[pg_i] = mapped_page;
-
-                               va += HW_PAGE_SIZE4KB;
-                               ul_mpu_addr += HW_PAGE_SIZE4KB;
-                       } else {
-                               pr_err("DSPBRIDGE: get_user_pages FAILED,"
-                                      "MPU addr = 0x%x,"
-                                      "vma->vm_flags = 0x%lx,"
-                                      "get_user_pages Err"
-                                      "Value = %d, Buffer"
-                                      "size=0x%x\n", ul_mpu_addr,
-                                      vma->vm_flags, pg_num, ul_num_bytes);
-                               status = -EPERM;
-                               break;
-                       }
-               }
+       if (i < 0)
+               return i;
+
+       if (i < pages) {
+               res = -EFAULT;
+               goto err_pages;
        }
-       up_read(&mm->mmap_sem);
-func_cont:
-       if (status) {
-               /*
-                * Roll out the mapped pages incase it failed in middle of
-                * mapping
-                */
-               if (pg_i) {
-                       bridge_brd_mem_un_map(dev_context, virt_addr,
-                                          (pg_i * PG_SIZE4K));
-               }
-               status = -EPERM;
+
+       sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
+       if (!sgt) {
+               res = -ENOMEM;
+               goto err_pages;
        }
-       /*
-        * In any case, flush the TLB
-        * This is called from here instead from pte_update to avoid unnecessary
-        * repetition while mapping non-contiguous physical regions of a virtual
-        * region
-        */
-       flush_all(dev_context);
-       dev_dbg(bridge, "%s status %x\n", __func__, status);
-       return status;
+
+       res = sg_alloc_table(sgt, pages, GFP_KERNEL);
+

drivers/staging/tidspbridge/core/io_sm.c +10 -0

+       res = (int)da;
+
+       sg_free_table(sgt);
+err_sg:
+       kfree(sgt);
+       i = pages;
+err_pages:
+       while (i--)
+               put_page(usr_pgs[i]);
+       return res;
 }
 

-----Original Message-----
From: David Cohen [mailto:[email protected]]
Sent: Wednesday, October 06, 2010 12:33 PM
To: Guzman Lugo, Fernando
Cc: [email protected]; Contreras Felipe (Nokia-MS/Helsinki); Palande Ameya (Nokia-MS/Helsinki); Menon, Nishanth; Doyu Hiroshi (Nokia-MS/Espoo); [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCHv3 01/11] staging: tidspbridge: replace iommu custom for opensource implementation

Hi Fernando,

I have few comments below.

Quoted 27 lines

drivers/staging/tidspbridge/core/io_sm.c +2 -1

index 5718645..842b8db 100644
@@ -291,6 +291,7 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
        struct cod_manager *cod_man;
        struct chnl_mgr *hchnl_mgr;
        struct msg_mgr *hmsg_mgr;
+       struct iommu *mmu;
        u32 ul_shm_base;
        u32 ul_shm_base_offset;
        u32 ul_shm_limit;

@@ -313,7 +314,6 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
        struct bridge_ioctl_extproc ae_proc[BRDIOCTL_NUMOFMMUTLB];
        struct cfg_hostres *host_res;
        struct bridge_dev_context *pbridge_context;
-       u32 map_attrs;
        u32 shm0_end;
        u32 ul_dyn_ext_base;
        u32 ul_seg1_size = 0;

@@ -337,6 +337,20 @@ int bridge_io_on_loaded(struct io_mgr *hio_mgr)
                status = -EFAULT;
                goto func_end;
        }
+       mmu = pbridge_context->dsp_mmu;

drivers/staging/tidspbridge/core/io_sm.c int bridge_io_on_loaded(struct io_mgr *hio_mgr) · line 346

Quoted 6 lines

drivers/staging/tidspbridge/core/io_sm.c +6 -0

+
+       if (mmu)
+               iommu_put(mmu);
+       mmu = iommu_get("iva2");
+
+       if (IS_ERR_OR_NULL(mmu)) {

You can use IS_ERR() instead.

[snip]

drivers/staging/tidspbridge/core/io_sm.c static int

Quoted 1 line

drivers/staging/tidspbridge/core/io_sm.c +0 -0

@@ -1122,217 +1081,81 @@ static int

bridge_brd_mem_write(struct bridge_dev_context *dev_ctxt,

drivers/staging/tidspbridge/core/io_sm.c static int · line 1082

Quoted 2 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -0

  *
  *  TODO: Disable MMU while updating the page tables (but

that'll stall DSP)

Quoted 20 lines

drivers/staging/tidspbridge/core/io_sm.c +7 -9

  */
-static int bridge_brd_mem_map(struct bridge_dev_context *dev_ctxt,
-                                 u32 ul_mpu_addr, u32 virt_addr,
-                                 u32 ul_num_bytes, u32 ul_map_attr,
-                                 struct page **mapped_pages)
+static int bridge_brd_mem_map(struct bridge_dev_context *dev_ctx,
+                       u32 uva, u32 da, u32 size, u32 attr,
+                       struct page **usr_pgs)
+
 {
-       u32 attrs;
-       int status = 0;
-       struct bridge_dev_context *dev_context = dev_ctxt;
-       struct hw_mmu_map_attrs_t hw_attrs;
+       int res, w;
+       unsigned pages, i;
+       struct iommu *mmu = dev_ctx->dsp_mmu;
        struct vm_area_struct *vma;
        struct mm_struct *mm = current->mm;
-       u32 write = 0;

drivers/staging/tidspbridge/core/io_sm.c static int · line 1146

Quoted 10 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -10

-       u32 num_usr_pgs = 0;
-       struct page *mapped_page, *pg;
-       s32 pg_num;
-       u32 va = virt_addr;
-       struct task_struct *curr_task = current;
-       u32 pg_i = 0;
-       u32 mpu_addr, pa;
-
-       dev_dbg(bridge,
-               "%s hDevCtxt %p, pa %x, va %x, size %x,

ul_map_attr %x\n",

drivers/staging/tidspbridge/core/io_sm.c static int · line 1147

Quoted 1 line

drivers/staging/tidspbridge/core/io_sm.c +0 -1

-               __func__, dev_ctxt, ul_mpu_addr, virt_addr,

ul_num_bytes,

drivers/staging/tidspbridge/core/io_sm.c static int · line 1156

Quoted 11 lines

drivers/staging/tidspbridge/core/io_sm.c +2 -8

-               ul_map_attr);
-       if (ul_num_bytes == 0)
-               return -EINVAL;
+       struct sg_table *sgt;
+       struct scatterlist *sg;
 
-       if (ul_map_attr & DSP_MAP_DIR_MASK) {
-               attrs = ul_map_attr;
-       } else {
-               /* Assign default attributes */
-               attrs = ul_map_attr | (DSP_MAPVIRTUALADDR |

DSP_MAPELEMSIZE16);

Quoted 30 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -30

-       }
-       /* Take mapping properties */
-       if (attrs & DSP_MAPBIGENDIAN)
-               hw_attrs.endianism = HW_BIG_ENDIAN;
-       else
-               hw_attrs.endianism = HW_LITTLE_ENDIAN;
-
-       hw_attrs.mixed_size = (enum hw_mmu_mixed_size_t)
-           ((attrs & DSP_MAPMIXEDELEMSIZE) >> 2);
-       /* Ignore element_size if mixed_size is enabled */
-       if (hw_attrs.mixed_size == 0) {
-               if (attrs & DSP_MAPELEMSIZE8) {
-                       /* Size is 8 bit */
-                       hw_attrs.element_size = HW_ELEM_SIZE8BIT;
-               } else if (attrs & DSP_MAPELEMSIZE16) {
-                       /* Size is 16 bit */
-                       hw_attrs.element_size = HW_ELEM_SIZE16BIT;
-               } else if (attrs & DSP_MAPELEMSIZE32) {
-                       /* Size is 32 bit */
-                       hw_attrs.element_size = HW_ELEM_SIZE32BIT;
-               } else if (attrs & DSP_MAPELEMSIZE64) {
-                       /* Size is 64 bit */
-                       hw_attrs.element_size = HW_ELEM_SIZE64BIT;
-               } else {
-                       /*
-                        * Mixedsize isn't enabled, so size can't be
-                        * zero here
-                        */
-                       return -EINVAL;
-               }

drivers/staging/tidspbridge/core/io_sm.c static int · line 1194

Quoted 10 lines

drivers/staging/tidspbridge/core/io_sm.c +2 -7

-       }
-       if (attrs & DSP_MAPDONOTLOCK)
-               hw_attrs.donotlockmpupage = 1;
-       else
-               hw_attrs.donotlockmpupage = 0;
+       if (!size || !usr_pgs)
+               return -EINVAL;
 
-       if (attrs & DSP_MAPVMALLOCADDR) {
-               return mem_map_vmalloc(dev_ctxt,

ul_mpu_addr, virt_addr,

drivers/staging/tidspbridge/core/io_sm.c static int · line 1203

Quoted 9 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -9

-                                      ul_num_bytes, &hw_attrs);
-       }
-       /*
-        * Do OS-specific user-va to pa translation.
-        * Combine physically contiguous regions to reduce TLBs.
-        * Pass the translated pa to pte_update.
-        */
-       if ((attrs & DSP_MAPPHYSICALADDR)) {
-               status = pte_update(dev_context,

ul_mpu_addr, virt_addr,

drivers/staging/tidspbridge/core/io_sm.c static int · line 1100

Quoted 4 lines

drivers/staging/tidspbridge/core/io_sm.c +1 -3

-                                   ul_num_bytes, &hw_attrs);
-               goto func_cont;
-       }
+       pages = size / PG_SIZE4K;

Can you ensure 'size' is always PG_SIZE4K aligned? I don't have so much knowledge about the dsp bridge implementation, but you're testing only if size == 0.

drivers/staging/tidspbridge/core/io_sm.c static int · line 1208

Quoted 4 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -2

-       /*
-        * Important Note: ul_mpu_addr is mapped from user

application process

drivers/staging/tidspbridge/core/io_sm.c static int · line 1209

Quoted 1 line

drivers/staging/tidspbridge/core/io_sm.c +0 -1

-        * to current process - it must lie completely

within the current

drivers/staging/tidspbridge/core/io_sm.c static int · line 1210

Quoted 1 line

drivers/staging/tidspbridge/core/io_sm.c +0 -1

-        * virtual memory address space in order to be of

use to us here!

drivers/staging/tidspbridge/core/io_sm.c static int · line 1216

Quoted 6 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -5

-        */
        down_read(&mm->mmap_sem);
-       vma = find_vma(mm, ul_mpu_addr);
-       if (vma)
-               dev_dbg(bridge,
-                       "VMAfor UserBuf: ul_mpu_addr=%x,

ul_num_bytes=%x, "

drivers/staging/tidspbridge/core/io_sm.c static int · line 1217

Quoted 1 line

drivers/staging/tidspbridge/core/io_sm.c +0 -1

-                       "vm_start=%lx, vm_end=%lx,

vm_flags=%lx\n", ul_mpu_addr,

drivers/staging/tidspbridge/core/io_sm.c static int · line 1222

Quoted 5 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -5

-                       ul_num_bytes, vma->vm_start, vma->vm_end,
-                       vma->vm_flags);
-
-       /*
-        * It is observed that under some circumstances,

the user buffer is

drivers/staging/tidspbridge/core/io_sm.c static int · line 1223

Quoted 1 line

drivers/staging/tidspbridge/core/io_sm.c +0 -1

-        * spread across several VMAs. So loop through and

check if the entire

drivers/staging/tidspbridge/core/io_sm.c static int · line 1226

Quoted 3 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -3

-        * user buffer is covered
-        */
-       while ((vma) && (ul_mpu_addr + ul_num_bytes >

vma->vm_end)) {

drivers/staging/tidspbridge/core/io_sm.c static int · line 1230

Quoted 6 lines

drivers/staging/tidspbridge/core/io_sm.c +2 -3

-               /* jump to the next VMA region */
+       vma = find_vma(mm, uva);
+       while (vma && (uva + size > vma->vm_end))
                vma = find_vma(mm, vma->vm_end + 1);
-               dev_dbg(bridge,
-                       "VMA for UserBuf ul_mpu_addr=%x

ul_num_bytes=%x, "

drivers/staging/tidspbridge/core/io_sm.c static int · line 1231

Quoted 1 line

drivers/staging/tidspbridge/core/io_sm.c +0 -1

-                       "vm_start=%lx, vm_end=%lx,

vm_flags=%lx\n", ul_mpu_addr,

drivers/staging/tidspbridge/core/io_sm.c static int · line 1107

Quoted 6 lines

drivers/staging/tidspbridge/core/io_sm.c +1 -3

-                       ul_num_bytes, vma->vm_start, vma->vm_end,
-                       vma->vm_flags);
-       }
+
        if (!vma) {
                pr_err("%s: Failed to get VMA region for

0x%x (%d)\n",

drivers/staging/tidspbridge/core/io_sm.c static int · line 1108

Quoted 3 lines

drivers/staging/tidspbridge/core/io_sm.c +1 -2

-                      __func__, ul_mpu_addr, ul_num_bytes);
-               status = -EINVAL;
+                                               __func__,

uva, size);

Quoted 7 lines

drivers/staging/tidspbridge/core/io_sm.c +3 -1

                up_read(&mm->mmap_sem);
-               goto func_cont;
+               return -EINVAL;
        }
+       if (vma->vm_flags & (VM_WRITE | VM_MAYWRITE))
+               w = 1;
 

drivers/staging/tidspbridge/core/io_sm.c static int · line 1252

Quoted 10 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -10

-       if (vma->vm_flags & VM_IO) {
-               num_usr_pgs = ul_num_bytes / PG_SIZE4K;
-               mpu_addr = ul_mpu_addr;
-
-               /* Get the physical addresses for user buffer */
-               for (pg_i = 0; pg_i < num_usr_pgs; pg_i++) {
-                       pa = user_va2_pa(mm, mpu_addr);
-                       if (!pa) {
-                               status = -EPERM;
-                               pr_err("DSPBRIDGE: VM_IO

mapping physical"

drivers/staging/tidspbridge/core/io_sm.c static int · line 1260

Quoted 8 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -8

-                                      "address is invalid\n");
-                               break;
-                       }
-                       if (pfn_valid(__phys_to_pfn(pa))) {
-                               pg = PHYS_TO_PAGE(pa);
-                               get_page(pg);
-                               if (page_count(pg) < 1) {
-                                       pr_err("Bad page in

VM_IO buffer\n");

drivers/staging/tidspbridge/core/io_sm.c static int · line 1265

Quoted 5 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -5

-                                       bad_page_dump(pa, pg);
-                               }
-                       }
-                       status = pte_set(dev_context->pt_attrs, pa,
-                                        va,

HW_PAGE_SIZE4KB, &hw_attrs);

drivers/staging/tidspbridge/core/io_sm.c static int · line 1119

Quoted 7 lines

drivers/staging/tidspbridge/core/io_sm.c +5 -2

-                       if (status)
-                               break;
+       if (vma->vm_flags & VM_IO)
+               i = get_io_pages(mm, uva, pages, usr_pgs);
+       else
+               i = get_user_pages(current, mm, uva, pages, w, 1,
+

usr_pgs, NULL);

drivers/staging/tidspbridge/core/io_sm.c static int · line 1279

Quoted 13 lines

drivers/staging/tidspbridge/core/io_sm.c +1 -11

+       up_read(&mm->mmap_sem);
 
-                       va += HW_PAGE_SIZE4KB;
-                       mpu_addr += HW_PAGE_SIZE4KB;
-                       pa += HW_PAGE_SIZE4KB;
-               }
-       } else {
-               num_usr_pgs = ul_num_bytes / PG_SIZE4K;
-               if (vma->vm_flags & (VM_WRITE | VM_MAYWRITE))
-                       write = 1;
-
-               for (pg_i = 0; pg_i < num_usr_pgs; pg_i++) {
-                       pg_num = get_user_pages(curr_task,

mm, ul_mpu_addr, 1,

drivers/staging/tidspbridge/core/io_sm.c static int · line 1280

Quoted 1 line

drivers/staging/tidspbridge/core/io_sm.c +0 -1

-                                               write, 1,

&mapped_page, NULL);

drivers/staging/tidspbridge/core/io_sm.c static int · line 1283

Quoted 3 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -3

-                       if (pg_num > 0) {
-                               if (page_count(mapped_page) < 1) {
-                                       pr_err("Bad page

count after doing"

drivers/staging/tidspbridge/core/io_sm.c static int · line 1286

Quoted 3 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -3

-                                              "get_user_pages on"
-                                              "user buffer\n");
-

bad_page_dump(page_to_phys(mapped_page),

drivers/staging/tidspbridge/core/io_sm.c static int · line 1289

Quoted 3 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -3

-                                                     mapped_page);
-                               }
-                               status =

pte_set(dev_context->pt_attrs,

drivers/staging/tidspbridge/core/io_sm.c static int · line 1290

Quoted 1 line

drivers/staging/tidspbridge/core/io_sm.c +0 -1

-

page_to_phys(mapped_page), va,

drivers/staging/tidspbridge/core/io_sm.c static int · line 1291

Quoted 1 line

drivers/staging/tidspbridge/core/io_sm.c +0 -1

-

HW_PAGE_SIZE4KB, &hw_attrs);

drivers/staging/tidspbridge/core/io_sm.c static int · line 1296

Quoted 5 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -5

-                               if (status)
-                                       break;
-
-                               if (mapped_pages)
-                                       mapped_pages[pg_i]

= mapped_page;

drivers/staging/tidspbridge/core/io_sm.c static int · line 1301

Quoted 5 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -5

-
-                               va += HW_PAGE_SIZE4KB;
-                               ul_mpu_addr += HW_PAGE_SIZE4KB;
-                       } else {
-                               pr_err("DSPBRIDGE:

get_user_pages FAILED,"

drivers/staging/tidspbridge/core/io_sm.c static int · line 1307

Quoted 6 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -6

-                                      "MPU addr = 0x%x,"
-                                      "vma->vm_flags = 0x%lx,"
-                                      "get_user_pages Err"
-                                      "Value = %d, Buffer"
-                                      "size=0x%x\n", ul_mpu_addr,
-                                      vma->vm_flags,

pg_num, ul_num_bytes);

Quoted 6 lines

drivers/staging/tidspbridge/core/io_sm.c +2 -4

-                               status = -EPERM;
-                               break;
-                       }
-               }
+       if (i < 0)
+               return i;

drivers/staging/tidspbridge/core/io_sm.c static int · line 1317

Quoted 10 lines

drivers/staging/tidspbridge/core/io_sm.c +4 -5

+
+       if (i < pages) {
+               res = -EFAULT;
+               goto err_pages;
        }
-       up_read(&mm->mmap_sem);
-func_cont:
-       if (status) {
-               /*
-                * Roll out the mapped pages incase it

failed in middle of

drivers/staging/tidspbridge/core/io_sm.c static int · line 1321

Quoted 4 lines

drivers/staging/tidspbridge/core/io_sm.c +0 -4

-                * mapping
-                */
-               if (pg_i) {
-                       bridge_brd_mem_un_map(dev_context,

virt_addr,

drivers/staging/tidspbridge/core/io_sm.c static int · line 1328

Quoted 12 lines

drivers/staging/tidspbridge/core/io_sm.c +5 -6

-                                          (pg_i * PG_SIZE4K));
-               }
-               status = -EPERM;
+
+       sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
+       if (!sgt) {
+               res = -ENOMEM;
+               goto err_pages;
        }
-       /*
-        * In any case, flush the TLB
-        * This is called from here instead from pte_update

to avoid unnecessary

drivers/staging/tidspbridge/core/io_sm.c static int · line 1329

Quoted 1 line

drivers/staging/tidspbridge/core/io_sm.c +0 -1

-        * repetition while mapping non-contiguous physical

regions of a virtual

Quoted 16 lines

drivers/staging/tidspbridge/core/io_sm.c +11 -5

-        * region
-        */
-       flush_all(dev_context);
-       dev_dbg(bridge, "%s status %x\n", __func__, status);
-       return status;
+
+       res = sg_alloc_table(sgt, pages, GFP_KERNEL);
+
+       if (res < 0)
+               goto err_sg;
+
+       for_each_sg(sgt->sgl, sg, sgt->nents, i)
+               sg_set_page(sg, usr_pgs[i], PAGE_SIZE, 0);
+
+       da = iommu_vmap(mmu, da, sgt, IOVMF_ENDIAN_LITTLE |
+ IOVMF_ELSZ_32);

drivers/staging/tidspbridge/core/io_sm.c static int · line 1148

Quoted 3 lines

drivers/staging/tidspbridge/core/io_sm.c +3 -0

+
+       if (!IS_ERR_VALUE(da))
+               return 0;

It might be a matter of taste, but you could unconditionally return 0 and add a goto in case of error like you're doing in the rest of the function. IMO it would improve readability to point out the code for non-error condition has ended here.

Br,

David

Quoted 12 lines

drivers/staging/tidspbridge/core/io_sm.c +10 -0

+       res = (int)da;
+
+       sg_free_table(sgt);
+err_sg:
+       kfree(sgt);
+       i = pages;
+err_pages:
+       while (i--)
+               put_page(usr_pgs[i]);
+       return res;
 }
 

Now the tidspbridge uses the API's from
iovmm module.

Signed-off-by Fernando Guzman Lugo <[email protected]>

Quoted 4 lines · Fernando Guzman Lugo

Now the tidspbridge uses the API's from
iovmm module.

Signed-off-by Fernando Guzman Lugo <[email protected]>

NAK. This patch doesn't work... I guess it's supposed to.

So far I've found these errors, but it still doesn't work, maybe we should start thinking of reverting the whole iommu stuff:

Quoted 10 lines · Felipe Contreras

Quoted 4 lines · Fernando Guzman Lugo

Now the tidspbridge uses the API's from
iovmm module.

Signed-off-by Fernando Guzman Lugo <[email protected]>

NAK. This patch doesn't work... I guess it's supposed to.

So far I've found these errors, but it still doesn't work, maybe we should start thinking of reverting the whole iommu stuff:

I also tested it with the userspace-dspbridge tools and it doesn't work for me either.

Maybe it's better to revert this patch series for now and re-submit it when all necessary patches get merged and it actually works on top of the staging-next tree?

This set of patches remove the dspbridge custom mmu implementation and use iommu module instead.

-----Original Message-----
From: Felipe Contreras [mailto:[email protected]]
Sent: Sunday, October 10, 2010 12:33 PM
To: Guzman Lugo, Fernando
Cc: [email protected]; [email protected]; [email protected]; Menon, Nishanth; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCHv3 00/11] staging tidspbridge: iommu migration

Quoted 1 line · Fernando Guzman Lugo

This set of patches remove the dspbridge custom mmu

implementation and

Quoted 1 line

use iommu module instead.

I have tried this, it works for simple tests, but not real use-cases. I applied all your iommu patches. How did you test this?

--
Felipe Contreras

Quoted 8 lines

Quoted 1 line · Fernando Guzman Lugo

This set of patches remove the dspbridge custom mmu

implementation and

Quoted 1 line

use iommu module instead.

I have tried this, it works for simple tests, but not real use-cases. I applied all your iommu patches. How did you test this?

Have you applied:

- "scatterlist: define SG chain for arm architecture"
- "iovmm: replace __iounmap with omap_iounmap"
- "iovmm: add superpages support to fixed da address"
- "iovmm: IVA2 MMU range is from 0x11000000 to 0xFFFFFFFF"
- "iovmm: no gap checking for fixed address"

Also make sure your baseline have this patch:

- "omap:iommu-load cam register before flushing the entry"

What kind of error are you getting?

I don't have a complete framework to test MM testcases at this moment

-----Original Message-----
From: Felipe Contreras [mailto:[email protected]]
Sent: Tuesday, October 12, 2010 6:21 AM
To: Guzman Lugo, Fernando
Cc: [email protected]; [email protected]; [email protected]; Menon, Nishanth; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCHv3 00/11] staging tidspbridge: iommu migration

Quoted 7 lines · Fernando

Quoted 7 lines

Quoted 1 line · Fernando Guzman Lugo

This set of patches remove the dspbridge custom mmu

implementation and

Quoted 1 line

use iommu module instead.

I have tried this, it works for simple tests, but not real

use-cases.

Quoted 9 lines

Quoted 1 line

I applied all your iommu patches. How did you test this?

Have you applied:

- "scatterlist: define SG chain for arm architecture"
- "iovmm: replace __iounmap with omap_iounmap"
- "iovmm: add superpages support to fixed da address"
- "iovmm: IVA2 MMU range is from 0x11000000 to 0xFFFFFFFF"
- "iovmm: no gap checking for fixed address"

Yes.

Quoted 3 lines

Also make sure your baseline have this patch:

- "omap:iommu-load cam register before flushing the entry"

Huh? That's not even in v2.6.36-rc7, in which baseline is this supposed to be in? Anyway, I'll try adding that.

Quoted 2 lines

What kind of error are you getting?

Node allocation failing IIRC.

Quoted 2 lines

I don't have a complete framework to test MM testcases at

this moment

See:
http://felipec.wordpress.com/2010/10/08/my-arm-development-notes/

I even prepared a tarball so you just need to extract it on your device. It's not difficult to test this with GStreamer, and I don't see how you can be confident that they indeed work without testing some real use-cases. Anyway, I'll try that missing patch.

Cheers.

--
Felipe Contreras

Quoted 24 lines

Quoted 7 lines · Fernando

Quoted 7 lines

Quoted 1 line · Fernando Guzman Lugo

This set of patches remove the dspbridge custom mmu

implementation and

Quoted 1 line

use iommu module instead.

I have tried this, it works for simple tests, but not real

use-cases.

Quoted 9 lines

Quoted 1 line

I applied all your iommu patches. How did you test this?

Have you applied:

- "scatterlist: define SG chain for arm architecture"
- "iovmm: replace __iounmap with omap_iounmap"
- "iovmm: add superpages support to fixed da address"
- "iovmm: IVA2 MMU range is from 0x11000000 to 0xFFFFFFFF"
- "iovmm: no gap checking for fixed address"

Yes.

Quoted 2 lines

Also make sure your baseline have this patch:

Quoted 4 lines

Quoted 1 line

- "omap:iommu-load cam register before flushing the entry"

Huh? That's not even in v2.6.36-rc7, in which baseline is this supposed to be in? Anyway, I'll try adding that.

That's is in latest Hiroshi's tree and it is really needed, Otherwise You will have wrong traslations which can cause unexpected behavior.

Quoted 3 lines

Quoted 1 line

What kind of error are you getting?

Node allocation failing IIRC.

Is it falling to map the Heap??
I mean you see this trace?

       if (status)
               pr_err("%s: Failed to map memory for Heap: 0x%x\n", __func__, status);

Otherwise, I don't see how that fail is related with iommu changes.

Quoted 11 lines

Quoted 1 line

I don't have a complete framework to test MM testcases at

this moment

See:
http://felipec.wordpress.com/2010/10/08/my-arm-development-notes/

I even prepared a tarball so you just need to extract it on your device. It's not difficult to test this with GStreamer, and I don't see how you can be confident that they indeed work without testing some real use-cases. Anyway, I'll try that missing patch.

Most of time real use-cases are not so stressing like testcases We can make to test under real stress in order to find out corner cases. However when I test it was pretty stable and just few erros because staging Does not have latest mailbox patches. Also I test in a .35 version of staging. So now I am using a branch with all new patches and I will recheck and test Again any possible issue. Also I will look at your gstreamer fail too.

-----Original Message-----
From: Felipe Contreras [mailto:[email protected]]
Sent: Thursday, October 14, 2010 7:27 AM
To: Guzman Lugo, Fernando
Cc: [email protected]; [email protected]; [email protected]; Menon, Nishanth; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCHv3 00/11] staging tidspbridge: iommu migration

Quoted 22 lines · Fernando

Quoted 22 lines

Quoted 7 lines · Fernando

Quoted 7 lines

Quoted 1 line · Fernando Guzman Lugo

This set of patches remove the dspbridge custom mmu

implementation and

Quoted 1 line

use iommu module instead.

I have tried this, it works for simple tests, but not real

use-cases.

Quoted 9 lines

Quoted 1 line

I applied all your iommu patches. How did you test this?

Have you applied:

- "scatterlist: define SG chain for arm architecture"
- "iovmm: replace __iounmap with omap_iounmap"
- "iovmm: add superpages support to fixed da address"
- "iovmm: IVA2 MMU range is from 0x11000000 to 0xFFFFFFFF"
- "iovmm: no gap checking for fixed address"

Yes.

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

Also make sure your baseline have this patch:

- "omap:iommu-load cam register before flushing the entry"

Huh? That's not even in v2.6.36-rc7, in which baseline is this supposed to be in? Anyway, I'll try adding that.

That's is in latest Hiroshi's tree and it is really needed,

Otherwise

Quoted 1 line

You will have wrong traslations which can cause unexpected behavior.

Now I applied that, still fails.

Quoted 4 lines

Quoted 3 lines

Quoted 1 line

What kind of error are you getting?

Node allocation failing IIRC.

Quoted 8 lines

Is it falling to map the Heap??
I mean you see this trace?

       if (status)
               pr_err("%s: Failed to map memory for Heap: 0x%x\n", __func__, status);

Otherwise, I don't see how that fail is related with iommu changes.

Nope.

Quoted 8 lines

Quoted 8 lines

Quoted 1 line

I don't have a complete framework to test MM testcases at

this moment

See:
http://felipec.wordpress.com/2010/10/08/my-arm-development-notes/

I even prepared a tarball so you just need to extract it on your device. It's not difficult to test this with GStreamer,

and I don't

Quoted 4 lines

Quoted 2 lines

see how you can be confident that they indeed work without testing some real use-cases. Anyway, I'll try that missing patch.

Most of time real use-cases are not so stressing like

testcases We can

Quoted 3 lines

make to test under real stress in order to find out corner cases. However when I test it was pretty stable and just few erros because staging Does not have latest mailbox patches. Also I test

in a .35 version of staging.

Quoted 1 line

So now I am using a branch with all new patches and I will

recheck and

Quoted 1 line

test Again any possible issue. Also I will look at your

gstreamer fail too.

Well, in my experience it's the other way around, the stress test-cases don't catch the errors that happen on real use-case scenarios, no matter how extensive they are. This is a good example.

Cheers.

--
Felipe Contreras
--

Quoted 4 lines

Well, in my experience it's the other way around, the stress test-cases don't catch the errors that happen on real use-case scenarios, no matter how extensive they are. This is a good example.

I am facing some unstability with the latest bridge merge. I am looing into That once it is stable I wil check you testcase too to confirm everything is Working fine.

-----Original Message-----
From: Felipe Contreras [mailto:[email protected]]
Sent: Friday, October 15, 2010 11:28 AM
To: Guzman Lugo, Fernando
Cc: [email protected]; [email protected]; [email protected]; Menon, Nishanth; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCHv3 00/11] staging tidspbridge: iommu migration

Quoted 3 lines · Fernando

Quoted 3 lines

Well, in my experience it's the other way around, the stress test-cases don't catch the errors that happen on real use-case scenarios, no matter how extensive they are. This is a

good example.

Quoted 1 line

Quoted 1 line

I am facing some unstability with the latest bridge merge.

I am looing

Quoted 2 lines

into That once it is stable I wil check you testcase too to confirm everything is Working fine.

If you want I can provide my working branch on top of .36-rc8 with next-staging merged. If I revert your patches, it works fine.

--
Felipe Contreras

Quoted 2 lines

If you want I can provide my working branch on top of .36-rc8 with next-staging merged. If I revert your patches, it works fine.

That could be interested. Can you send me the link?

Quoted 4 lines · Fernando

Quoted 2 lines

If you want I can provide my working branch on top of .36-rc8 with next-staging merged. If I revert your patches, it works fine.

That could be interested. Can you send me the link?

There you go:
git://gitorious.org/~felipec/linux-omap/felipec.git (fc-dsp-work)

HEAD works, HEAD^ doesn't.

Quoted 10 lines · Felipe Contreras

Quoted 4 lines · Fernando

Quoted 2 lines

If you want I can provide my working branch on top of .36-rc8 with next-staging merged. If I revert your patches, it works fine.

That could be interested. Can you send me the link?

There you go:
git://gitorious.org/~felipec/linux-omap/felipec.git (fc-dsp-work)

Quoted 1 line

HEAD works, HEAD^ doesn't.

Great, can you please also let us know what if anything should be still queued for the merge window that's about to open?

I've lost track of the dspbridge mutations by now..