Re: [PATCH for-next] RDMA/cxgb4: use dma_mmap_coherent() for mapping non-contiguous memory

[Christoph Hellwig <hch@lst.de>]

On Sun, Jul 07, 2024 at 12:11:05PM +0300, Leon Romanovsky wrote:
> On Fri, Jul 05, 2024 at 06:47:53PM +0530, Anumula Murali Mohan Reddy wrote:
> > dma_alloc_coherent() allocates contiguous memory irrespective of
> > iommu mode, but after commit f5ff79fddf0e ("dma-mapping: remove
> > CONFIG_DMA_REMAP") if iommu is enabled in translate mode,
> > dma_alloc_coherent() may allocate non-contiguous memory.
> > Attempt to map this memory results in panic.
> > This patch fixes the issue by using dma_mmap_coherent() to map each page
> > to user space.
> 
> It is perfect time to move to use rdma_user_mmap_io(), instead of
> open-code it in the driver.

rdma_user_mmap_io does not work on dma coherent allocations.

> > Fixes: f5ff79fddf0e ("dma-mapping: remove CONFIG_DMA_REMAP")
> 
> + authors of the commit mentioned in Fixes.

If that commit triggered a bug for you it was buggy before, you
just didn't hit it.  The fixes tag needs to point to the commit
assuming trying to convert the return value from dma_alloc* into
a page/pfn/physical address.

> > +++ b/drivers/infiniband/hw/cxgb4/cq.c
> > @@ -1127,12 +1127,16 @@ int c4iw_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
> >  
> >  		mm->key = uresp.key;
> >  		mm->addr = virt_to_phys(chp->cq.queue);

... aka this one.  And it still is buggy and needs to go away.

> > +		if (vaddr && is_vmalloc_addr(vaddr)) {

And this check is broken.  The virtual address returned from
dma_alloc_coherent can also be other things than a vmalloc address.

>
>
> > +			vm_pgoff = vma->vm_pgoff;
> > +			vma->vm_pgoff = 0;
> > +			ret = dma_mmap_coherent(&rdev->lldi.pdev->dev, vma,
> > +						vaddr, dma_addr, size);
> > +			vma->vm_pgoff = vm_pgoff;

... and you thus must use this path unconditionally.

Same for the other hunks.