Chris,
> -----Original Message-----
> From: Chris Leech <cleech@xxxxxxxxxx>
> Sent: Friday, January 5, 2024 1:50 AM
> To: Nilesh Javali <njavali@xxxxxxxxxxx>
> Cc: martin.petersen@xxxxxxxxxx; lduncan@xxxxxxxx; linux-
> scsi@xxxxxxxxxxxxxxx; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-
> Upstream@xxxxxxxxxxx>; jmeneghi@xxxxxxxxxx
> Subject: [EXT] Re: [PATCH v2 1/3] uio: introduce
> UIO_MEM_DMA_COHERENT type
>
> External Email
>
> ----------------------------------------------------------------------
> On Wed, Jan 03, 2024 at 02:41:35PM +0530, Nilesh Javali wrote:
> > From: Chris Leech <cleech@xxxxxxxxxx>
> >
> > Add a UIO memtype specifically for sharing dma_alloc_coherent
> > memory with userspace, backed by dma_mmap_coherent.
> >
> > Signed-off-by: Nilesh Javali <njavali@xxxxxxxxxxx>
> > Signed-off-by: Chris Leech <cleech@xxxxxxxxxx>
> > ---
> > v2:
> > - expose only the dma_addr within uio_mem
> > - Cleanup newly added unions comprising virtual_addr
> > and struct device
>
> Nilesh, thanks for taking another look at these changes. If we're taking
> another shot at getting uio changes accepted, Greg KH is going to have
> to be part of that conversation.
>
> Removing the unions looks good, but I do have some concerns with your
> modifications.
>
> On Wed, Jan 03, 2024 at 02:41:35PM +0530, Nilesh Javali wrote:
>
> > +static int uio_mmap_dma_coherent(struct vm_area_struct *vma)
> > +{
> > + struct uio_device *idev = vma->vm_private_data;
> > + struct uio_mem *mem;
> > + int ret = 0;
> > + int mi;
> > +
> > + mi = uio_find_mem_index(vma);
> > + if (mi < 0)
> > + return -EINVAL;
> > +
> > + mem = idev->info->mem + mi;
> > +
> > + if (mem->dma_addr & ~PAGE_MASK)
> > + return -ENODEV;
> > + if (vma->vm_end - vma->vm_start > mem->size)
> > + return -EINVAL;
> > +
> > + /*
> > + * UIO uses offset to index into the maps for a device.
> > + * We need to clear vm_pgoff for dma_mmap_coherent.
> > + */
> > + vma->vm_pgoff = 0;
> > +
> > + ret = dma_mmap_coherent(&idev->dev,
> ~~~~~~~~~~
> This doesn't seem right. You've plugged a struct device into the call,
> but it's not the same device used when calling dma_alloc_coherent. I
> don't see a way around needing a way to access the correct device in
> uio_mmap_dma_coherent, or a way to do that without attaching it to the
> uio_device.
While the cnic loads, it registers the cnic_uio_dev->pdev->dev with uio and the uio
attaches its device to cnic device as it's parent. So uio and cnic are attached to the same PCI device.
I will post the v3 of the patch set shortly.
Thanks,
Nilesh
