Re: [Linaro-mm-sig] [PATCH] dma-buf: add DMA_BUF_IOCTL_SYNC_PARTIAL support

Am 13.11.21 um 07:22 schrieb Jianqun Xu:

Add DMA_BUF_IOCTL_SYNC_PARTIAL support for user to sync dma-buf with offset and len.

You have not given an use case for this so it is a bit hard to review. And from the existing use cases I don't see why this should be necessary.

Even worse from the existing backend implementation I don't even see how drivers should be able to fulfill this semantics.

Please explain further, Christian.

Change-Id: I03d2d2e10e48d32aa83c31abade57e0931e1be49 Signed-off-by: Jianqun Xu jay.xu@rock-chips.com

---

drivers/dma-buf/dma-buf.c | 42 ++++++++++++++++++++++++++++++++++++ include/uapi/linux/dma-buf.h | 8 +++++++ 2 files changed, 50 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index d9948d58b3f4..78f37f7c3462 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -392,6 +392,7 @@ static long dma_buf_ioctl(struct file *file, { struct dma_buf *dmabuf; struct dma_buf_sync sync;

• struct dma_buf_sync_partial sync_p; enum dma_data_direction direction; int ret;

@@ -430,6 +431,47 @@ static long dma_buf_ioctl(struct file *file, case DMA_BUF_SET_NAME_B: return dma_buf_set_name(dmabuf, (const char __user *)arg);

• case DMA_BUF_IOCTL_SYNC_PARTIAL:

• if (copy_from_user(&sync_p, (void __user *) arg, sizeof(sync_p)))
  

• 	return -EFAULT;
  

• if (sync_p.len == 0)
  

• 	return 0;
  

• if ((sync_p.offset % cache_line_size()) || (sync_p.len % cache_line_size()))
  

• 	return -EINVAL;
  

• if (sync_p.len > dmabuf->size || sync_p.offset > dmabuf->size - sync_p.len)
  

• 	return -EINVAL;
  

• if (sync_p.flags & ~DMA_BUF_SYNC_VALID_FLAGS_MASK)
  

• 	return -EINVAL;
  

• switch (sync_p.flags & DMA_BUF_SYNC_RW) {
  

• case DMA_BUF_SYNC_READ:
  

• 	direction = DMA_FROM_DEVICE;
  

• 	break;
  

• case DMA_BUF_SYNC_WRITE:
  

• 	direction = DMA_TO_DEVICE;
  

• 	break;
  

• case DMA_BUF_SYNC_RW:
  

• 	direction = DMA_BIDIRECTIONAL;
  

• 	break;
  

• default:
  

• 	return -EINVAL;
  

• }
  

• if (sync_p.flags & DMA_BUF_SYNC_END)
  

• 	ret = dma_buf_end_cpu_access_partial(dmabuf, direction,
  

• 					     sync_p.offset,
  

• 					     sync_p.len);
  

• else
  

• 	ret = dma_buf_begin_cpu_access_partial(dmabuf, direction,
  

• 					       sync_p.offset,
  

• 					       sync_p.len);
  

• return ret;
  

• default: return -ENOTTY; }

diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h index 7f30393b92c3..6236c644624d 100644 --- a/include/uapi/linux/dma-buf.h +++ b/include/uapi/linux/dma-buf.h @@ -47,4 +47,12 @@ struct dma_buf_sync { #define DMA_BUF_SET_NAME_A _IOW(DMA_BUF_BASE, 1, u32) #define DMA_BUF_SET_NAME_B _IOW(DMA_BUF_BASE, 1, u64) +struct dma_buf_sync_partial {

• __u64 flags;
• __u32 offset;
• __u32 len;

+};

+#define DMA_BUF_IOCTL_SYNC_PARTIAL _IOW(DMA_BUF_BASE, 2, struct dma_buf_sync_partial)

• #endif