Re: [PATCH 2/3] dma-buf: Add DMA_BUF_IOCTL_GET_DMA_ADDR

[Simona Vetter <simona.vetter@ffwll.ch>]

Catching up after spring break, hence the late reply ...

On Fri, Apr 11, 2025 at 02:34:37PM -0400, Nicolas Dufresne wrote:
> Le jeudi 10 avril 2025 à 16:53 +0200, Bastien Curutchet a écrit :
> > There is no way to transmit the DMA address of a buffer to userspace.
> > Some UIO users need this to handle DMA from userspace.
> 
> To me this API is against all safe practice we've been pushing forward
> and has no place in DMA_BUF API.
> 
> If this is fine for the UIO subsystem to pass around physicial
> addresses, then make this part of the UIO device ioctl.

Yeah, this has no business in dma-buf since the entire point of dma-buf
was to stop all the nasty "just pass raw dma addr in userspace" hacks that
preceeded it.

And over the years since dma-buf landed, we've removed a lot of these,
like dri1 drivers. Or where that's not possible like with fbdev, hid the
raw dma addr uapi behind a Kconfig.

I concur with the overall sentiment that this should be done in
vfio/iommufd interfaces, maybe with some support added to map dma-buf. I
think patches for that have been floating around for a while, but I lost a
bit the status of where exactly they are.

Cheers, Sima

> 
> regards,
> Nicolas
> 
> > 
> > Add a new dma_buf_ops operation that returns the DMA address.
> > Add a new ioctl to transmit this DMA address to userspace.
> > 
> > Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
> > ---
> >  drivers/dma-buf/dma-buf.c    | 21 +++++++++++++++++++++
> >  include/linux/dma-buf.h      |  1 +
> >  include/uapi/linux/dma-buf.h |  1 +
> >  3 files changed, 23 insertions(+)
> > 
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index
> > 398418bd9731ad7a3a1f12eaea6a155fa77a22fe..cbbb518981e54e50f479c3d1fcf
> > 6da6971f639c1 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -454,6 +454,24 @@ static long dma_buf_import_sync_file(struct
> > dma_buf *dmabuf,
> >  }
> >  #endif
> >  
> > +static int dma_buf_get_dma_addr(struct dma_buf *dmabuf, u64 __user
> > *arg)
> > +{
> > +	u64 addr;
> > +	int ret;
> > +
> > +	if (!dmabuf->ops->get_dma_addr)
> > +		return -EINVAL;
> > +
> > +	ret = dmabuf->ops->get_dma_addr(dmabuf, &addr);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (copy_to_user(arg, &addr, sizeof(u64)))
> > +		return -EFAULT;
> > +
> > +	return 0;
> > +}
> > +
> >  static long dma_buf_ioctl(struct file *file,
> >  			  unsigned int cmd, unsigned long arg)
> >  {
> > @@ -504,6 +522,9 @@ static long dma_buf_ioctl(struct file *file,
> >  		return dma_buf_import_sync_file(dmabuf, (const void
> > __user *)arg);
> >  #endif
> >  
> > +	case DMA_BUF_IOCTL_GET_DMA_ADDR:
> > +		return dma_buf_get_dma_addr(dmabuf, (u64 __user
> > *)arg);
> > +
> >  	default:
> >  		return -ENOTTY;
> >  	}
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index
> > 36216d28d8bdc01a9c9c47e27c392413f7f6c5fb..ed4bf15d3ce82e7a86323fff459
> > 699a9bc8baa3b 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -285,6 +285,7 @@ struct dma_buf_ops {
> >  
> >  	int (*vmap)(struct dma_buf *dmabuf, struct iosys_map *map);
> >  	void (*vunmap)(struct dma_buf *dmabuf, struct iosys_map
> > *map);
> > +	int (*get_dma_addr)(struct dma_buf *dmabuf, u64 *addr);
> >  };
> >  
> >  /**
> > diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-
> > buf.h
> > index
> > 5a6fda66d9adf01438619e7e67fa69f0fec2d88d..f3aba46942042de6a2e3a4cca3e
> > b3f87175e29c9 100644
> > --- a/include/uapi/linux/dma-buf.h
> > +++ b/include/uapi/linux/dma-buf.h
> > @@ -178,5 +178,6 @@ struct dma_buf_import_sync_file {
> >  #define DMA_BUF_SET_NAME_B	_IOW(DMA_BUF_BASE, 1, __u64)
> >  #define DMA_BUF_IOCTL_EXPORT_SYNC_FILE	_IOWR(DMA_BUF_BASE, 2,
> > struct dma_buf_export_sync_file)
> >  #define DMA_BUF_IOCTL_IMPORT_SYNC_FILE	_IOW(DMA_BUF_BASE, 3, struct
> > dma_buf_import_sync_file)
> > +#define DMA_BUF_IOCTL_GET_DMA_ADDR	_IOR(DMA_BUF_BASE, 4, __u64
> > *)
> >  
> >  #endif

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch