The GEM prime helpers do it, so should we. It's also possible to make it optional later.
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org --- drivers/dma-buf/udmabuf.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 9635897458a0..b345e91d831a 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -108,12 +108,13 @@ static void kunmap_udmabuf(struct dma_buf *buf, unsigned long page_num, }
static const struct dma_buf_ops udmabuf_ops = { - .map_dma_buf = map_udmabuf, - .unmap_dma_buf = unmap_udmabuf, - .release = release_udmabuf, - .map = kmap_udmabuf, - .unmap = kunmap_udmabuf, - .mmap = mmap_udmabuf, + .cache_sgt_mapping = true, + .map_dma_buf = map_udmabuf, + .unmap_dma_buf = unmap_udmabuf, + .release = release_udmabuf, + .map = kmap_udmabuf, + .unmap = kunmap_udmabuf, + .mmap = mmap_udmabuf, };
#define SEALS_WANTED (F_SEAL_SHRINK)
The main use for udmabuf is sending guest memory pages to the host.
It's generally a bad idea to have to separate mappings with different attributes. For example, a WC mapping the guest kernel and cached mapping on the host is problematic.
v2: Cache attribute flags instead of read/write flags (kraxel@) --- drivers/dma-buf/udmabuf.c | 12 ++++++++++++ include/uapi/linux/udmabuf.h | 2 ++ 2 files changed, 14 insertions(+)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index b345e91d831a..ce9caaaa9e4b 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -16,6 +16,7 @@ static const u32 list_limit = 1024; /* udmabuf_create_list->count limit */ static const size_t size_limit_mb = 64; /* total dmabuf size, in megabytes */
struct udmabuf { + u32 flags; pgoff_t pagecount; struct page **pages; }; @@ -37,6 +38,12 @@ static const struct vm_operations_struct udmabuf_vm_ops = { static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) { struct udmabuf *ubuf = buf->priv; + pgprot_t pgprot = vm_get_page_prot(vma->vm_flags); + + if (ubuf->flags & UDMABUF_FLAGS_WC) + vma->vm_page_prot = pgprot_writecombine(pgprot); + else if (ubuf->flags & UDMABUF_FLAGS_NONCACHED) + vma->vm_page_prot = pgprot_noncached(pgprot);
if ((vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) == 0) return -EINVAL; @@ -132,6 +139,10 @@ static long udmabuf_create(const struct udmabuf_create_list *head, int seals, ret = -EINVAL; u32 i, flags;
+ if ((head->flags & UDMABUF_FLAGS_NONCACHED) && + (head->flags & UDMABUF_FLAGS_WC)) + return -EINVAL; + ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL); if (!ubuf) return -ENOMEM; @@ -188,6 +199,7 @@ static long udmabuf_create(const struct udmabuf_create_list *head, exp_info.priv = ubuf; exp_info.flags = O_RDWR;
+ ubuf->flags = head->flags; buf = dma_buf_export(&exp_info); if (IS_ERR(buf)) { ret = PTR_ERR(buf); diff --git a/include/uapi/linux/udmabuf.h b/include/uapi/linux/udmabuf.h index 46b6532ed855..f90831f2bb0d 100644 --- a/include/uapi/linux/udmabuf.h +++ b/include/uapi/linux/udmabuf.h @@ -6,6 +6,8 @@ #include <linux/ioctl.h>
#define UDMABUF_FLAGS_CLOEXEC 0x01 +#define UDMABUF_FLAGS_WC 0x02 +#define UDMABUF_FLAGS_NONCACHED 0x04
struct udmabuf_create { __u32 memfd;
Hi,
diff --git a/include/uapi/linux/udmabuf.h b/include/uapi/linux/udmabuf.h index 46b6532ed855..f90831f2bb0d 100644 --- a/include/uapi/linux/udmabuf.h +++ b/include/uapi/linux/udmabuf.h @@ -6,6 +6,8 @@ #include <linux/ioctl.h>
#define UDMABUF_FLAGS_CLOEXEC 0x01 +#define UDMABUF_FLAGS_WC 0x02 +#define UDMABUF_FLAGS_NONCACHED 0x04
struct udmabuf_create { __u32 memfd;
This is a uapi change and should go to a separate patch, clearly flagging that in $subject.
(new policy by airlied for the drm tree).
Otherwise the series looks good to me.
cheers, Gerd
On Thu, Nov 28, 2019 at 3:48 AM Gerd Hoffmann kraxel@redhat.com wrote:
Hi,
diff --git a/include/uapi/linux/udmabuf.h b/include/uapi/linux/udmabuf.h index 46b6532ed855..f90831f2bb0d 100644 --- a/include/uapi/linux/udmabuf.h +++ b/include/uapi/linux/udmabuf.h @@ -6,6 +6,8 @@ #include <linux/ioctl.h>
#define UDMABUF_FLAGS_CLOEXEC 0x01 +#define UDMABUF_FLAGS_WC 0x02 +#define UDMABUF_FLAGS_NONCACHED 0x04
struct udmabuf_create { __u32 memfd;
This is a uapi change and should go to a separate patch, clearly flagging that in $subject.
(new policy by airlied for the drm tree).
The new policy requires a non-toy userspace[1], which may take a while. Removed the UAPI changes in the latest patch series.
[1] https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-uapi.html#open-source-user...
Otherwise the series looks good to me.
cheers, Gerd
Will be used later.
v2: rename 'udmabuf_misc' to 'device' (kraxel)
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org --- drivers/dma-buf/udmabuf.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index ce9caaaa9e4b..9e6fdd2bc979 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -19,6 +19,7 @@ struct udmabuf { u32 flags; pgoff_t pagecount; struct page **pages; + struct miscdevice *device; };
static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) @@ -127,8 +128,9 @@ static const struct dma_buf_ops udmabuf_ops = { #define SEALS_WANTED (F_SEAL_SHRINK) #define SEALS_DENIED (F_SEAL_WRITE)
-static long udmabuf_create(const struct udmabuf_create_list *head, - const struct udmabuf_create_item *list) +static long udmabuf_create(struct miscdevice *device, + struct udmabuf_create_list *head, + struct udmabuf_create_item *list) { DEFINE_DMA_BUF_EXPORT_INFO(exp_info); struct file *memfd = NULL; @@ -200,6 +202,8 @@ static long udmabuf_create(const struct udmabuf_create_list *head, exp_info.flags = O_RDWR;
ubuf->flags = head->flags; + ubuf->device = device; + buf = dma_buf_export(&exp_info); if (IS_ERR(buf)) { ret = PTR_ERR(buf); @@ -237,7 +241,7 @@ static long udmabuf_ioctl_create(struct file *filp, unsigned long arg) list.offset = create.offset; list.size = create.size;
- return udmabuf_create(&head, &list); + return udmabuf_create(filp->private_data, &head, &list); }
static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg) @@ -256,7 +260,7 @@ static long udmabuf_ioctl_create_list(struct file *filp, unsigned long arg) if (IS_ERR(list)) return PTR_ERR(list);
- ret = udmabuf_create(&head, list); + ret = udmabuf_create(filp->private_data, &head, list); kfree(list); return ret; }
These are nice functions and can be re-used.
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org --- drivers/dma-buf/udmabuf.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 9e6fdd2bc979..67e89bb034c5 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -54,10 +54,10 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) return 0; }
-static struct sg_table *map_udmabuf(struct dma_buf_attachment *at, - enum dma_data_direction direction) +static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, + enum dma_data_direction direction) { - struct udmabuf *ubuf = at->dmabuf->priv; + struct udmabuf *ubuf = buf->priv; struct sg_table *sg; int ret;
@@ -69,7 +69,7 @@ static struct sg_table *map_udmabuf(struct dma_buf_attachment *at, GFP_KERNEL); if (ret < 0) goto err; - if (!dma_map_sg(at->dev, sg->sgl, sg->nents, direction)) { + if (!dma_map_sg(dev, sg->sgl, sg->nents, direction)) { ret = -EINVAL; goto err; } @@ -81,13 +81,25 @@ static struct sg_table *map_udmabuf(struct dma_buf_attachment *at, return ERR_PTR(ret); }
+static void put_sg_table(struct device *dev, struct sg_table *sg, + enum dma_data_direction direction) +{ + dma_unmap_sg(dev, sg->sgl, sg->nents, direction); + sg_free_table(sg); + kfree(sg); +} + +static struct sg_table *map_udmabuf(struct dma_buf_attachment *at, + enum dma_data_direction direction) +{ + return get_sg_table(at->dev, at->dmabuf, direction); +} + static void unmap_udmabuf(struct dma_buf_attachment *at, struct sg_table *sg, enum dma_data_direction direction) { - dma_unmap_sg(at->dev, sg->sgl, sg->nents, direction); - sg_free_table(sg); - kfree(sg); + return put_sg_table(at->dev, sg, direction); }
static void release_udmabuf(struct dma_buf *buf)
With the misc device, we should end up using the result of get_arch_dma_ops(..) or dma-direct ops.
This can allow us to have WC mappings in the guest after synchronization.
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org --- drivers/dma-buf/udmabuf.c | 40 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 67e89bb034c5..142444922acd 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -19,6 +19,7 @@ struct udmabuf { u32 flags; pgoff_t pagecount; struct page **pages; + struct sg_table *sg; struct miscdevice *device; };
@@ -105,8 +106,12 @@ static void unmap_udmabuf(struct dma_buf_attachment *at, static void release_udmabuf(struct dma_buf *buf) { struct udmabuf *ubuf = buf->priv; + struct device *dev = ubuf->device->this_device; pgoff_t pg;
+ if (ubuf->sg) + put_sg_table(dev, ubuf->sg, DMA_BIDIRECTIONAL); + for (pg = 0; pg < ubuf->pagecount; pg++) put_page(ubuf->pages[pg]); kfree(ubuf->pages); @@ -127,6 +132,39 @@ static void kunmap_udmabuf(struct dma_buf *buf, unsigned long page_num, kunmap(vaddr); }
+static int begin_cpu_udmabuf(struct dma_buf *buf, + enum dma_data_direction direction) +{ + struct udmabuf *ubuf = buf->priv; + struct device *dev = ubuf->device->this_device; + + if (!ubuf->sg) { + ubuf->sg = get_sg_table(dev, buf, direction); + if (IS_ERR(ubuf->sg)) + return PTR_ERR(ubuf->sg); + } else { + dma_sync_sg_for_device(dev, ubuf->sg->sgl, + ubuf->sg->nents, + direction); + } + + return 0; +} + +static int end_cpu_udmabuf(struct dma_buf *buf, + enum dma_data_direction direction) +{ + struct udmabuf *ubuf = buf->priv; + struct device *dev = ubuf->device->this_device; + + if (!ubuf->sg) + return -EINVAL; + + dma_sync_sg_for_cpu(dev, ubuf->sg->sgl, ubuf->sg->nents, direction); + return 0; +} + + static const struct dma_buf_ops udmabuf_ops = { .cache_sgt_mapping = true, .map_dma_buf = map_udmabuf, @@ -135,6 +173,8 @@ static const struct dma_buf_ops udmabuf_ops = { .map = kmap_udmabuf, .unmap = kunmap_udmabuf, .mmap = mmap_udmabuf, + .begin_cpu_access = begin_cpu_udmabuf, + .end_cpu_access = end_cpu_udmabuf, };
#define SEALS_WANTED (F_SEAL_SHRINK)
dri-devel@lists.freedesktop.org
-
Gerd Hoffmann -
Gurchetan Singh