diff mbox series

[1/2] uio: introduce UIO_MEM_DMA_COHERENT type

Message ID 20240131191732.3247996-2-cleech@redhat.com (mailing list archive)
State Superseded
Headers show
Series UIO_MEM_DMA_COHERENT for cnic/bnx2/bnx2x | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 1289 this patch: 1290
netdev/build_tools success Errors and warnings before: 1 this patch: 0
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 1065 this patch: 1065
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1306 this patch: 1306
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 77 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc fail Errors and warnings before: 0 this patch: 2
netdev/source_inline success Was 0 now: 0

Commit Message

Chris Leech Jan. 31, 2024, 7:17 p.m. UTC
Add a UIO memtype specifically for sharing dma_alloc_coherent
memory with userspace, backed by dma_mmap_coherent.

This is mainly for the bnx2/bnx2x/bnx2i "cnic" interface, although there
are a few other uio drivers which map dma_alloc_coherent memory and
could be converted to use dma_mmap_coherent as well.

Signed-off-by: Nilesh Javali <njavali@marvell.com>
Signed-off-by: Chris Leech <cleech@redhat.com>
---
 drivers/uio/uio.c          | 40 ++++++++++++++++++++++++++++++++++++++
 include/linux/uio_driver.h |  3 +++
 2 files changed, 43 insertions(+)

Comments

Greg KH Jan. 31, 2024, 9:28 p.m. UTC | #1
On Wed, Jan 31, 2024 at 11:17:31AM -0800, Chris Leech wrote:
> Add a UIO memtype specifically for sharing dma_alloc_coherent
> memory with userspace, backed by dma_mmap_coherent.
> 
> This is mainly for the bnx2/bnx2x/bnx2i "cnic" interface, although there
> are a few other uio drivers which map dma_alloc_coherent memory and
> could be converted to use dma_mmap_coherent as well.

What other drivers could use this?  Patches doing the conversion would
be welcome, otherwise, again, I am very loath to take this
one-off-change for just a single driver that shouldn't be doing this in
the first place :)

thanks,

greg k-h
Chris Leech Jan. 31, 2024, 9:44 p.m. UTC | #2
On Wed, Jan 31, 2024 at 1:29 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jan 31, 2024 at 11:17:31AM -0800, Chris Leech wrote:
> > Add a UIO memtype specifically for sharing dma_alloc_coherent
> > memory with userspace, backed by dma_mmap_coherent.
> >
> > This is mainly for the bnx2/bnx2x/bnx2i "cnic" interface, although there
> > are a few other uio drivers which map dma_alloc_coherent memory and
> > could be converted to use dma_mmap_coherent as well.
>
> What other drivers could use this?  Patches doing the conversion would
> be welcome, otherwise, again, I am very loath to take this
> one-off-change for just a single driver that shouldn't be doing this in
> the first place :)

uio_pruss and uio_dmem_genirq both appear to mmap dma_alloc_coherent
memory as UIO_MEM_PHYS.  It might not be an issue on that platforms
where those are used, but I'd be happy to include untested patches to
convert them for better adherence to the DMA APIs.

(sorry for the double send on this Greg, missed the reply-all)

- Chris
Christoph Hellwig Feb. 1, 2024, 4:44 a.m. UTC | #3
On Wed, Jan 31, 2024 at 01:44:50PM -0800, Chris Leech wrote:
> On Wed, Jan 31, 2024 at 1:29 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Jan 31, 2024 at 11:17:31AM -0800, Chris Leech wrote:
> > > Add a UIO memtype specifically for sharing dma_alloc_coherent
> > > memory with userspace, backed by dma_mmap_coherent.
> > >
> > > This is mainly for the bnx2/bnx2x/bnx2i "cnic" interface, although there
> > > are a few other uio drivers which map dma_alloc_coherent memory and
> > > could be converted to use dma_mmap_coherent as well.
> >
> > What other drivers could use this?  Patches doing the conversion would
> > be welcome, otherwise, again, I am very loath to take this
> > one-off-change for just a single driver that shouldn't be doing this in
> > the first place :)
> 
> uio_pruss and uio_dmem_genirq both appear to mmap dma_alloc_coherent
> memory as UIO_MEM_PHYS.  It might not be an issue on that platforms
> where those are used, but I'd be happy to include untested patches to
> convert them for better adherence to the DMA APIs.

Yes, they do need fixing.
Christoph Hellwig Feb. 1, 2024, 4:46 a.m. UTC | #4
As the least horrible way out this looks ok:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Bt maybe you can add some commentary why this mem mode exists and
why no one should be using it in new code?
Greg KH Feb. 1, 2024, 3:03 p.m. UTC | #5
On Thu, Feb 01, 2024 at 05:46:37AM +0100, Christoph Hellwig wrote:
> As the least horrible way out this looks ok:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Bt maybe you can add some commentary why this mem mode exists and
> why no one should be using it in new code?
> 

Good idea, and perhaps a kernel log warning when this is used as well
just to prevent anyone new from ever considering it.

thanks,

greg k-h
Thomas Weißschuh March 12, 2024, 8:40 a.m. UTC | #6
Hi,

On Wed, Jan 31, 2024 at 11:17:31AM -0800, Chris Leech wrote:
> Add a UIO memtype specifically for sharing dma_alloc_coherent
> memory with userspace, backed by dma_mmap_coherent.
> 
> This is mainly for the bnx2/bnx2x/bnx2i "cnic" interface, although there
> are a few other uio drivers which map dma_alloc_coherent memory and
> could be converted to use dma_mmap_coherent as well.
> 
> Signed-off-by: Nilesh Javali <njavali@marvell.com>
> Signed-off-by: Chris Leech <cleech@redhat.com>
> ---
>  drivers/uio/uio.c          | 40 ++++++++++++++++++++++++++++++++++++++
>  include/linux/uio_driver.h |  3 +++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> index 2d572f6c8ec83..dde3f49855233 100644
> --- a/drivers/uio/uio.c
> +++ b/drivers/uio/uio.c
> @@ -24,6 +24,7 @@

[..]

> +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;

This cast introduces a build error when building with
sizeof(void *) != sizeof(phys_addr_t).

For example on i386 with PHYS_ADDR_T_64BIT.
(Enabled through allmodconfig)

drivers/uio/uio.c: In function 'uio_mmap_dma_coherent':
drivers/uio/uio.c:795:16: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
  795 |         addr = (void *)mem->addr;
      |                ^
drivers/uio/uio_pruss.c: In function 'pruss_probe':
drivers/uio/uio_pruss.c:194:34: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
  194 |                 p->mem[2].addr = (phys_addr_t) gdev->ddr_vaddr;
      |                                  ^
drivers/uio/uio_dmem_genirq.c: In function 'uio_dmem_genirq_open':
drivers/uio/uio_dmem_genirq.c:63:39: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
   63 |                 uiomem->addr = addr ? (phys_addr_t) addr : DMEM_MAP_ERROR;
      |                                       ^
drivers/uio/uio_dmem_genirq.c: In function 'uio_dmem_genirq_release':
drivers/uio/uio_dmem_genirq.c:92:43: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
   92 |                                           (void *) uiomem->addr,

As you can see some other files are also affected, which seem to be
triggered by other but related patches.

This is on next-20240312.

> +	ret = dma_mmap_coherent(mem->dma_device,
> +				vma,
> +				addr,
> +				mem->dma_addr,
> +				vma->vm_end - vma->vm_start);
> +	vma->vm_pgoff = mi;
> +
> +	return ret;
> +}
> +

[..]

Thomas
diff mbox series

Patch

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 2d572f6c8ec83..dde3f49855233 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(mem->dma_device,
+				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 47c5962b876b0..14d9dd2a07e85 100644
--- a/include/linux/uio_driver.h
+++ b/include/linux/uio_driver.h
@@ -37,10 +37,12 @@  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;
 	void __iomem		*internal_addr;
+	struct device		*dma_device;
 	struct uio_map		*map;
 };
 
@@ -158,6 +160,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