From: Leon Romanovsky [mailto:leon@xxxxxxxxxx] Sent: Monday, May 14, 2018 9:37 AM > > On Sun, May 13, 2018 at 09:07:07PM +0300, Michal Kalderon wrote: > > Each user_context receives a separate dpi value and thus a different > > address on the doorbell bar. The qedr_mmap function needs to validate > > the address and map the doorbell bar accordingly. > > The current implementation always checked against dpi=0 doorbell range > > leading to a wrong mapping for doorbell bar. (It entered an else case > > that mapped the address differently). qedr_mmap should only be used > > for doorbells, so the else was actually wrong in the first place. > > This only has an affect on arm architecture and not an issue on a > > x86 based architecture. > > This lead to doorbells not occurring on arm based systems and left > > applications that use more than one dpi (or several applications run > > simultaneously ) to hang. > > > > Fixes: ac1b36e55a51 ("qedr: Add support for user context verbs") > > Signed-off-by: Ariel Elior <Ariel.Elior@xxxxxxxxxx> > > Signed-off-by: Michal Kalderon <Michal.Kalderon@xxxxxxxxxx> > > --- > > drivers/infiniband/hw/qedr/verbs.c | 59 > > ++++++++++++++++++-------------------- > > 1 file changed, 28 insertions(+), 31 deletions(-) > > Please don't leak kernel pointers [1], and use %pK instead of %x [2]. > > [1] https://lwn.net/Articles/735589/ > [2] Documentation/core-api/printk-formats.rst > > Thanks Thanks for pointing these out. I was not familiar with this convention. However, the addresses Which are printed below are the physical doorbell addresses which don't Seem to fall into the category of the kernel pointers described in the article as the ones that should Be protected. Using the %pk doesn't print the value correctly. Thanks, > > > > > diff --git a/drivers/infiniband/hw/qedr/verbs.c > > b/drivers/infiniband/hw/qedr/verbs.c > > index 7d3763b..27c0f00 100644 > > --- a/drivers/infiniband/hw/qedr/verbs.c > > +++ b/drivers/infiniband/hw/qedr/verbs.c > > @@ -401,49 +401,46 @@ int qedr_mmap(struct ib_ucontext *context, > > struct vm_area_struct *vma) { > > struct qedr_ucontext *ucontext = get_qedr_ucontext(context); > > struct qedr_dev *dev = get_qedr_dev(context->device); > > - unsigned long vm_page = vma->vm_pgoff << PAGE_SHIFT; > > - u64 unmapped_db = dev->db_phys_addr; > > + unsigned long phys_addr = vma->vm_pgoff << PAGE_SHIFT; > > unsigned long len = (vma->vm_end - vma->vm_start); > > - int rc = 0; > > - bool found; > > + unsigned long dpi_start; > > + > > + dpi_start = dev->db_phys_addr + (ucontext->dpi * > > +ucontext->dpi_size); > > > > DP_DEBUG(dev, QEDR_MSG_INIT, > > - "qedr_mmap called vm_page=0x%lx vm_pgoff=0x%lx > unmapped_db=0x%llx db_size=%x, len=%lx\n", > > - vm_page, vma->vm_pgoff, unmapped_db, dev->db_size, > len); > > - if (vma->vm_start & (PAGE_SIZE - 1)) { > > - DP_ERR(dev, "Vma_start not page aligned = %ld\n", > > - vma->vm_start); > > + "mmap invoked with vm_start=0x%lx, > vm_end=0x%lx,vm_pgoff=0x%lx; dpi_start=0x%lx dpi_size=0x%x\n", > > + vma->vm_start, vma->vm_end, vma->vm_pgoff, dpi_start, > > + ucontext->dpi_size); > > + > > + if ((vma->vm_start & (PAGE_SIZE - 1)) || (len & (PAGE_SIZE - 1))) { > > + DP_ERR(dev, > > + "failed mmap, adrresses must be page aligned: > start=0x%lx, end=0x%lx\n", > > + vma->vm_start, vma->vm_end); > > return -EINVAL; > > } > > > > - found = qedr_search_mmap(ucontext, vm_page, len); > > - if (!found) { > > - DP_ERR(dev, "Vma_pgoff not found in mapped array = > %ld\n", > > + if (!qedr_search_mmap(ucontext, phys_addr, len)) { > > + DP_ERR(dev, "failed mmap, vm_pgoff=0x%lx is not > authorized\n", > > vma->vm_pgoff); > > return -EINVAL; > > } > > > > - DP_DEBUG(dev, QEDR_MSG_INIT, "Mapping doorbell bar\n"); > > - > > - if ((vm_page >= unmapped_db) && (vm_page <= (unmapped_db + > > - dev->db_size))) { > > - DP_DEBUG(dev, QEDR_MSG_INIT, "Mapping doorbell > bar\n"); > > - if (vma->vm_flags & VM_READ) { > > - DP_ERR(dev, "Trying to map doorbell bar for > read\n"); > > - return -EPERM; > > - } > > - > > - vma->vm_page_prot = pgprot_writecombine(vma- > >vm_page_prot); > > + if (phys_addr < dpi_start || > > + ((phys_addr + len) > (dpi_start + ucontext->dpi_size))) { > > + DP_ERR(dev, > > + "failed mmap, pages are outside of dpi; page > address=0x%lx, dpi_start=0x%lx, dpi_size=0x%x\n", > > + phys_addr, dpi_start, ucontext->dpi_size); > > + return -EINVAL; > > + } > > > > - rc = io_remap_pfn_range(vma, vma->vm_start, vma- > >vm_pgoff, > > - PAGE_SIZE, vma->vm_page_prot); > > - } else { > > - DP_DEBUG(dev, QEDR_MSG_INIT, "Mapping chains\n"); > > - rc = remap_pfn_range(vma, vma->vm_start, > > - vma->vm_pgoff, len, vma- > >vm_page_prot); > > + if (vma->vm_flags & VM_READ) { > > + DP_ERR(dev, "failed mmap, cannot map doorbell bar for > read\n"); > > + return -EINVAL; > > } > > - DP_DEBUG(dev, QEDR_MSG_INIT, "qedr_mmap return code: > %d\n", rc); > > - return rc; > > + > > + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); > > + return io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff, > len, > > + vma->vm_page_prot); > > } > > > > struct ib_pd *qedr_alloc_pd(struct ib_device *ibdev, > > -- > > 2.9.5 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" > > in the body of a message to majordomo@xxxxxxxxxxxxxxx More > majordomo > > info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
