Hi Greg, I'd like to ask you to reconsider accepting UIO API changes for mapping DMA coherent allocations. The bnx2i iSCSI driver is currently broken due to incorrect (but previously functioning) uio use in the "cnic" module. I believe that the most direct way to fix it is to specifically handle DMA coherent allocations in the UIO API backed by dma_mmap_coherent. After my original posting of these patches[1], Nilesh Javali had made an attempt to change the cnic allocations[2]. Those were rejected, and Nilesh has returned to cleaning up the UIO_MEM_DMA_COHERENT patches. [1] https://lore.kernel.org/all/20230929170023.1020032-1-cleech@xxxxxxxxxx/ [2] https://lore.kernel.org/linux-scsi/20231219055514.12324-1-njavali@xxxxxxxxxxx/ While I understand the complaints about how these drivers have been structured, I also don't like letting support bitrot when there's a reasonable alternative to re-architecting an existing driver. There are two other uio drivers which are mmaping dma_alloc_coherent memory as UIO_MEM_PHYS, uio_dmem_genirq and uio_pruss. While a conversion to use dma_mmap_coherent might be more correct for these as well, I have no way of testing them and assume that this just hasn't been an issue for the platforms in question. Nilesh has kindly removed the unions I had in the original posting, and done additional testing on the changes. Although I do believe that we need to go back to somehow storing the device struct associated with the PCI device, and not using idev->dev here. > + ret = dma_mmap_coherent(&idev->dev, > + vma, > + addr, > + mem->dma_addr, > + vma->vm_end - vma->vm_start); Thank you, - Chris Leech On Tue, Jan 09, 2024 at 05:44:56PM +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. > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > Closes: https://lore.kernel.org/oe-kbuild-all/202401042222.J9GOUiYL-lkp@xxxxxxxxx/ > Signed-off-by: Nilesh Javali <njavali@xxxxxxxxxxx> > Signed-off-by: Chris Leech <cleech@xxxxxxxxxx> > --- > v3: > - fix warnings reported by kernel test robot > v2: > - expose only the dma_addr within uio_mem > - Cleanup newly added unions comprising virtual_addr > and struct device > drivers/uio/uio.c | 40 ++++++++++++++++++++++++++++++++++++++ > include/linux/uio_driver.h | 2 ++ > 2 files changed, 42 insertions(+) > > diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c > index 62082d64ece0..01d83728b513 100644 > --- a/drivers/uio/uio.c > +++ b/drivers/uio/uio.c > @@ -24,6 +24,7 @@ > #include <linux/kobject.h> > #include <linux/cdev.h> > #include <linux/uio_driver.h> > +#include <linux/dma-mapping.h> > > #define UIO_MAX_DEVICES (1U << MINORBITS) > > @@ -759,6 +760,42 @@ static int uio_mmap_physical(struct vm_area_struct *vma) > vma->vm_page_prot); > } > > +static int uio_mmap_dma_coherent(struct vm_area_struct *vma) > +{ > + struct uio_device *idev = vma->vm_private_data; > + struct uio_mem *mem; > + void *addr; > + 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; > + > + addr = (void *)mem->addr; > + ret = dma_mmap_coherent(&idev->dev, > + vma, > + addr, > + mem->dma_addr, > + vma->vm_end - vma->vm_start); > + vma->vm_pgoff = mi; > + > + return ret; > +} > + > static int uio_mmap(struct file *filep, struct vm_area_struct *vma) > { > struct uio_listener *listener = filep->private_data; > @@ -806,6 +843,9 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma) > case UIO_MEM_VIRTUAL: > ret = uio_mmap_logical(vma); > break; > + case UIO_MEM_DMA_COHERENT: > + ret = uio_mmap_dma_coherent(vma); > + break; > default: > ret = -EINVAL; > } > diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h > index 47c5962b876b..7efa81497183 100644 > --- a/include/linux/uio_driver.h > +++ b/include/linux/uio_driver.h > @@ -37,6 +37,7 @@ struct uio_map; > struct uio_mem { > const char *name; > phys_addr_t addr; > + dma_addr_t dma_addr; > unsigned long offs; > resource_size_t size; > int memtype; > @@ -158,6 +159,7 @@ extern int __must_check > #define UIO_MEM_LOGICAL 2 > #define UIO_MEM_VIRTUAL 3 > #define UIO_MEM_IOVA 4 > +#define UIO_MEM_DMA_COHERENT 5 > > /* defines for uio_port->porttype */ > #define UIO_PORT_NONE 0 > -- > 2.23.1 >
