[PATCH 1/5] udmabuf: use cache_sgt_mapping option

[PATCH 1/5] udmabuf: use cache_sgt_mapping option

[Gurchetan Singh]

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)
-- 
2.24.0.432.g9d3f5f5b63-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/5] udmabuf: allow userspace to set map attributes

[Gurchetan Singh]

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;
-- 
2.24.0.432.g9d3f5f5b63-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/5] udmabuf: add a pointer to the miscdevice in dma-buf private data

[Gurchetan Singh]

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;
 }
-- 
2.24.0.432.g9d3f5f5b63-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/5] udmabuf: separate out creating/destroying scatter-table

[Gurchetan Singh]

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)
-- 
2.24.0.432.g9d3f5f5b63-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/5] udmabuf: implement begin_cpu_access/end_cpu_access hooks

[Gurchetan Singh]

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)
-- 
2.24.0.432.g9d3f5f5b63-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] udmabuf: allow userspace to set map attributes

[Gerd Hoffmann]

  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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] udmabuf: allow userspace to set map attributes

[Gurchetan Singh]

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-userspace-requirements

>
> Otherwise the series looks good to me.
>
> cheers,
>   Gerd
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel