On Thu, Sep 19, 2019 at 5:09 PM Chris Wilson chris@chris-wilson.co.uk wrote:
It is expected that processes will pass dma-buf fd between drivers, and only using the fd themselves for mmaping and synchronisation ioctls. However, it may be convenient for an application to read/write into the dma-buf, for instance a test case to check the internal dma_buf->ops->kmap interface. There may also be a small advantage to avoiding the mmap() for very simple/small operations.
Note in particular, synchronisation with the device is left to the caller with an explicit DMA_BUF_IOCTL_SYNC, rather than done implicitly inside the read/write, so that the user can avoid synchronisation if they so choose.
"It is easier to add synchronisation later, than it is to take it away."
Hm for mmap I think the explicit sync makes sense (we might even want to do it in userspace). Not so sure it's a great idea for read/write ... I guess we'd need to see what people have/had in mind for the userspace for this to decide.
Only other thought I have on this is that many dma-buf exporters don't bother with the kmap/kunmap interface (probably fewer than those who bother with kernel vmap and mmap), maybe we want at least a vmap fallback for this. Or maybe just use that as an excuse to get more people to wire up the kmap stuff :-) -Daniel
v2: Lots of little fixes, plus a real llseek() implements so that the first basic little test cases work!
Testcase: igt/prime_rw Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Laura Abbott labbott@redhat.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Sean Paul seanpaul@chromium.org Cc: Janusz Krzysztofik janusz.krzysztofik@linux.intel.com Tested-by: Laura Abbott labbott@redhat.com
Dusting this off again as it would be nice as a user to operate on dmabuf agnostic to the underyling driver. We have mmap, so naturally we would like to have read/write as well!
drivers/dma-buf/dma-buf.c | 108 ++++++++++++++++++++++++++++++++------ 1 file changed, 93 insertions(+), 15 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 433d91d710e4..a19fc881a99c 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -135,28 +135,104 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
static loff_t dma_buf_llseek(struct file *file, loff_t offset, int whence) {
struct dma_buf *dmabuf;loff_t base;
struct dma_buf *dmabuf = file->private_data; if (!is_dma_buf_file(file)) return -EBADF;
dmabuf = file->private_data;
return fixed_size_llseek(file, offset, whence, dmabuf->size);+}
/* only support discovering the end of the buffer,but also allow SEEK_SET to maintain the idiomaticSEEK_END(0), SEEK_CUR(0) pattern */if (whence == SEEK_END)base = dmabuf->size;else if (whence == SEEK_SET)base = 0;elsereturn -EINVAL;+static ssize_t dma_buf_read(struct file *file,
char __user *ubuf, size_t remain,loff_t *offset)+{
struct dma_buf *dmabuf = file->private_data;unsigned long idx;unsigned int start;size_t total;
if (offset != 0)return -EINVAL;
if (!is_dma_buf_file(file))return -EBADF;total = 0;idx = *offset >> PAGE_SHIFT;start = offset_in_page(*offset);while (remain) {unsigned int len = min_t(size_t, remain, PAGE_SIZE - start);unsigned int copied;void *vaddr;if (*offset >= dmabuf->size)return total;vaddr = dma_buf_kmap(dmabuf, idx);if (!vaddr)return total ?: -EIO;copied = copy_to_user(ubuf, vaddr + start, len);dma_buf_kunmap(dmabuf, idx, vaddr);total += copied ?: len;if (copied) {*offset += copied;return total ?: -EFAULT;}remain -= len;*offset += len;ubuf += len;start = 0;idx++;}return total;+}
+static ssize_t dma_buf_write(struct file *file,
const char __user *ubuf, size_t remain,loff_t *offset)+{
struct dma_buf *dmabuf = file->private_data;unsigned long idx;unsigned int start;size_t total;if (!is_dma_buf_file(file))return -EBADF;total = 0;idx = *offset >> PAGE_SHIFT;start = offset_in_page(*offset);while (remain) {unsigned int len = min_t(size_t, remain, PAGE_SIZE - start);unsigned int copied;void *vaddr;if (*offset >= dmabuf->size)return total;vaddr = dma_buf_kmap(dmabuf, idx);if (!vaddr)return total ?: -EIO;copied = copy_from_user(vaddr + start, ubuf, len);dma_buf_kunmap(dmabuf, idx, vaddr);total += copied ?: len;if (copied) {*offset += copied;return total ?: -EFAULT;}remain -= len;*offset += len;ubuf += len;start = 0;idx++;}
return base + offset;
return total;}
/** @@ -413,6 +489,8 @@ static const struct file_operations dma_buf_fops = { .release = dma_buf_release, .mmap = dma_buf_mmap_internal, .llseek = dma_buf_llseek,
.read = dma_buf_read,.write = dma_buf_write, .poll = dma_buf_poll, .unlocked_ioctl = dma_buf_ioctl,#ifdef CONFIG_COMPAT
2.23.0