From 4c78513e457f72d5554a0f6e2eabfad7b98e4f19 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 24 Apr 2012 14:38:52 +0530 Subject: dma-buf: mmap support Compared to Rob Clark's RFC I've ditched the prepare/finish hooks and corresponding ioctls on the dma_buf file. The major reason for that is that many people seem to be under the impression that this is also for synchronization with outstanding asynchronous processsing. I'm pretty massively opposed to this because: - It boils down reinventing a new rather general-purpose userspace synchronization interface. If we look at things like futexes, this is hard to get right. - Furthermore a lot of kernel code has to interact with this synchronization primitive. This smells a look like the dri1 hw_lock, a horror show I prefer not to reinvent. - Even more fun is that multiple different subsystems would interact here, so we have plenty of opportunities to create funny deadlock scenarios. I think synchronization is a wholesale different problem from data sharing and should be tackled as an orthogonal problem. Now we could demand that prepare/finish may only ensure cache coherency (as Rob intended), but that runs up into the next problem: We not only need mmap support to facilitate sw-only processing nodes in a pipeline (without jumping through hoops by importing the dma_buf into some sw-access only importer), which allows for a nicer ION->dma-buf upgrade path for existing Android userspace. We also need mmap support for existing importing subsystems to support existing userspace libraries. And a loot of these subsystems are expected to export coherent userspace mappings. So prepare/finish can only ever be optional and the exporter /needs/ to support coherent mappings. Given that mmap access is always somewhat fallback-y in nature I've decided to drop this optimization, instead of just making it optional. If we demonstrate a clear need for this, supported by benchmark results, we can always add it in again later as an optional extension. Other differences compared to Rob's RFC is the above mentioned support for mapping a dma-buf through facilities provided by the importer. Which results in mmap support no longer being optional. Note that this dma-buf mmap patch does _not_ support every possible insanity an existing subsystem could pull of with mmap: Because it does not allow to intercept pagefaults and shoot down ptes importing subsystems can't add some magic of their own at these points (e.g. to automatically synchronize with outstanding rendering or set up some special resources). I've done a cursory read through a few mmap implementions of various subsytems and I'm hopeful that we can avoid this (and the complexity it'd bring with it). Additonally I've extended the documentation a bit to explain the hows and whys of this mmap extension. In case we ever want to add support for explicitly cache maneged userspace mmap with a prepare/finish ioctl pair, we could specify that userspace needs to mmap a different part of the dma_buf, e.g. the range starting at dma_buf->size up to dma_buf->size*2. This works because the size of a dma_buf is invariant over it's lifetime. The exporter would obviously need to fall back to coherent mappings for both ranges if a legacy clients maps the coherent range and the architecture cannot suppor conflicting caching policies. Also, this would obviously be optional and userspace needs to be able to fall back to coherent mappings. v2: - Spelling fixes from Rob Clark. - Compile fix for !DMA_BUF from Rob Clark. - Extend commit message to explain how explicitly cache managed mmap support could be added later. - Extend the documentation with implementations notes for exporters that need to manually fake coherency. v3: - dma_buf pointer initialization goof-up noticed by Rebecca Schultz Zavin. Cc: Rob Clark Cc: Rebecca Schultz Zavin Acked-by: Rob Clark Signed-Off-by: Daniel Vetter Signed-off-by: Sumit Semwal --- drivers/base/dma-buf.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c index 07cbbc6fddb4..7cfb405b1ad5 100644 --- a/drivers/base/dma-buf.c +++ b/drivers/base/dma-buf.c @@ -44,8 +44,26 @@ static int dma_buf_release(struct inode *inode, struct file *file) return 0; } +static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma) +{ + struct dma_buf *dmabuf; + + if (!is_dma_buf_file(file)) + return -EINVAL; + + dmabuf = file->private_data; + + /* check for overflowing the buffer's size */ + if (vma->vm_pgoff + ((vma->vm_end - vma->vm_start) >> PAGE_SHIFT) > + dmabuf->size >> PAGE_SHIFT) + return -EINVAL; + + return dmabuf->ops->mmap(dmabuf, vma); +} + static const struct file_operations dma_buf_fops = { .release = dma_buf_release, + .mmap = dma_buf_mmap_internal, }; /* @@ -82,7 +100,8 @@ struct dma_buf *dma_buf_export(void *priv, const struct dma_buf_ops *ops, || !ops->unmap_dma_buf || !ops->release || !ops->kmap_atomic - || !ops->kmap)) { + || !ops->kmap + || !ops->mmap)) { return ERR_PTR(-EINVAL); } @@ -406,3 +425,46 @@ void dma_buf_kunmap(struct dma_buf *dmabuf, unsigned long page_num, dmabuf->ops->kunmap(dmabuf, page_num, vaddr); } EXPORT_SYMBOL_GPL(dma_buf_kunmap); + + +/** + * dma_buf_mmap - Setup up a userspace mmap with the given vma + * @dma_buf: [in] buffer that should back the vma + * @vma: [in] vma for the mmap + * @pgoff: [in] offset in pages where this mmap should start within the + * dma-buf buffer. + * + * This function adjusts the passed in vma so that it points at the file of the + * dma_buf operation. It alsog adjusts the starting pgoff and does bounds + * checking on the size of the vma. Then it calls the exporters mmap function to + * set up the mapping. + * + * Can return negative error values, returns 0 on success. + */ +int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, + unsigned long pgoff) +{ + if (WARN_ON(!dmabuf || !vma)) + return -EINVAL; + + /* check for offset overflow */ + if (pgoff + ((vma->vm_end - vma->vm_start) >> PAGE_SHIFT) < pgoff) + return -EOVERFLOW; + + /* check for overflowing the buffer's size */ + if (pgoff + ((vma->vm_end - vma->vm_start) >> PAGE_SHIFT) > + dmabuf->size >> PAGE_SHIFT) + return -EINVAL; + + /* readjust the vma */ + if (vma->vm_file) + fput(vma->vm_file); + + vma->vm_file = dmabuf->file; + get_file(vma->vm_file); + + vma->vm_pgoff = pgoff; + + return dmabuf->ops->mmap(dmabuf, vma); +} +EXPORT_SYMBOL_GPL(dma_buf_mmap); -- cgit v1.2.3-58-ga151