Re: [Xen-devel] [RFC 3/3] xen/gntdev: Add support for Linux dma buffers

[Oleksandr Andrushchenko]

On 05/22/2018 12:31 AM, Dongwon Kim wrote:

Still need more time to review the whole code changes

Take your time, I just wanted to make sure that all interested parties
are in the discussion, so we all finally have what we all want, not
a thing covering only my use-cases

  but I noticed one thing.

We've been using the term "hyper_dmabuf" for hypervisor-agnostic linux dmabuf
solution and we are planning to call any of our future solution for other
hypervisors the same name. So having same name for this xen-specific structure
or functions you implemented is confusing. Would you change it to something
else like... "xen_"?

Np, will rename


On Thu, May 17, 2018 at 11:26:04AM +0300, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko 

Signed-off-by: Oleksandr Andrushchenko 
---
  drivers/xen/gntdev.c  | 954 +-
  include/uapi/xen/gntdev.h | 101 
  include/xen/gntdev_exp.h  |  23 +
  3 files changed, 1066 insertions(+), 12 deletions(-)
  create mode 100644 include/xen/gntdev_exp.h

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 9510f228efe9..0ee88e193362 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -4,6 +4,8 @@
   * Device for accessing (in user-space) pages that have been granted by other
   * domains.
   *
+ * DMA buffer implementation is based on drivers/gpu/drm/drm_prime.c.
+ *
   * Copyright (c) 2006-2007, D G Murray.
   *   (c) 2009 Gerd Hoffmann 
   *
@@ -37,6 +39,9 @@
  #include 
  #include 
  
+#include 

+#include 
+
  #include 
  #include 
  #include 
@@ -61,16 +66,39 @@ static atomic_t pages_mapped = ATOMIC_INIT(0);
  static int use_ptemod;
  #define populate_freeable_maps use_ptemod
  
+#ifndef GRANT_INVALID_REF

+/*
+ * Note on usage of grant reference 0 as invalid grant reference:
+ * grant reference 0 is valid, but never exposed to a driver,
+ * because of the fact it is already in use/reserved by the PV console.
+ */
+#define GRANT_INVALID_REF  0
+#endif
+
  struct gntdev_priv {
/* maps with visible offsets in the file descriptor */
struct list_head maps;
/* maps that are not visible; will be freed on munmap.
 * Only populated if populate_freeable_maps == 1 */
struct list_head freeable_maps;
+   /* List of dma-bufs. */
+   struct list_head dma_bufs;
/* lock protects maps and freeable_maps */
struct mutex lock;
struct mm_struct *mm;
struct mmu_notifier mn;
+
+   /* Private data of the hyper DMA buffers. */
+
+   struct device *dev;
+   /* List of exported DMA buffers. */
+   struct list_head dmabuf_exp_list;
+   /* List of wait objects. */
+   struct list_head dmabuf_exp_wait_list;
+   /* List of imported DMA buffers. */
+   struct list_head dmabuf_imp_list;
+   /* This is the lock which protects dma_buf_xxx lists. */
+   struct mutex dmabuf_lock;
  };
  
  struct unmap_notify {

@@ -95,10 +123,65 @@ struct grant_map {
struct gnttab_unmap_grant_ref *kunmap_ops;
struct page **pages;
unsigned long pages_vm_start;
+
+   /*
+* All the fields starting with dmabuf_ are only valid if this
+* mapping is used for exporting a DMA buffer.
+* If dmabuf_vaddr is not NULL then this mapping is backed by DMA
+* capable memory.
+*/
+
+   /* Flags used to create this DMA buffer: GNTDEV_DMABUF_FLAG_XXX. */
+   bool dmabuf_flags;
+   /* Virtual/CPU address of the DMA buffer. */
+   void *dmabuf_vaddr;
+   /* Bus address of the DMA buffer. */
+   dma_addr_t dmabuf_bus_addr;
+};
+
+struct hyper_dmabuf {
+   struct gntdev_priv *priv;
+   struct dma_buf *dmabuf;
+   struct list_head next;
+   int fd;
+
+   union {
+   struct {
+   /* Exported buffers are reference counted. */
+   struct kref refcount;
+   struct grant_map *map;
+   } exp;
+   struct {
+   /* Granted references of the imported buffer. */
+   grant_ref_t *refs;
+   /* Scatter-gather table of the imported buffer. */
+   struct sg_table *sgt;
+   /* dma-buf attachment of the imported buffer. */
+   struct dma_buf_attachment *attach;
+   } imp;
+   } u;
+
+   /* Number of pages this buffer has. */
+   int nr_pages;
+   /* Pages of this buffer. */
+   struct page **pages;
+};
+
+struct hyper_dmabuf_wait_obj {
+   struct list_head next;
+   struct hyper_dmabuf *hyper_dmabuf;
+   struct completion completion;
+};
+
+struct hyper_dambuf_attachment {

minor typo: dam->dma (same thing in other places as well.)

sure, thanks



+   struct sg_table *sgt;
+   enum dma_data_direction 

Re: [Xen-devel] [RFC 3/3] xen/gntdev: Add support for Linux dma buffers

[Dongwon Kim]

Still need more time to review the whole code changes but I noticed one thing.

We've been using the term "hyper_dmabuf" for hypervisor-agnostic linux dmabuf
solution and we are planning to call any of our future solution for other
hypervisors the same name. So having same name for this xen-specific structure
or functions you implemented is confusing. Would you change it to something
else like... "xen_"? 

On Thu, May 17, 2018 at 11:26:04AM +0300, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko 
> 
> Signed-off-by: Oleksandr Andrushchenko 
> ---
>  drivers/xen/gntdev.c  | 954 +-
>  include/uapi/xen/gntdev.h | 101 
>  include/xen/gntdev_exp.h  |  23 +
>  3 files changed, 1066 insertions(+), 12 deletions(-)
>  create mode 100644 include/xen/gntdev_exp.h
> 
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 9510f228efe9..0ee88e193362 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -4,6 +4,8 @@
>   * Device for accessing (in user-space) pages that have been granted by other
>   * domains.
>   *
> + * DMA buffer implementation is based on drivers/gpu/drm/drm_prime.c.
> + *
>   * Copyright (c) 2006-2007, D G Murray.
>   *   (c) 2009 Gerd Hoffmann 
>   *
> @@ -37,6 +39,9 @@
>  #include 
>  #include 
>  
> +#include 
> +#include 
> +
>  #include 
>  #include 
>  #include 
> @@ -61,16 +66,39 @@ static atomic_t pages_mapped = ATOMIC_INIT(0);
>  static int use_ptemod;
>  #define populate_freeable_maps use_ptemod
>  
> +#ifndef GRANT_INVALID_REF
> +/*
> + * Note on usage of grant reference 0 as invalid grant reference:
> + * grant reference 0 is valid, but never exposed to a driver,
> + * because of the fact it is already in use/reserved by the PV console.
> + */
> +#define GRANT_INVALID_REF0
> +#endif
> +
>  struct gntdev_priv {
>   /* maps with visible offsets in the file descriptor */
>   struct list_head maps;
>   /* maps that are not visible; will be freed on munmap.
>* Only populated if populate_freeable_maps == 1 */
>   struct list_head freeable_maps;
> + /* List of dma-bufs. */
> + struct list_head dma_bufs;
>   /* lock protects maps and freeable_maps */
>   struct mutex lock;
>   struct mm_struct *mm;
>   struct mmu_notifier mn;
> +
> + /* Private data of the hyper DMA buffers. */
> +
> + struct device *dev;
> + /* List of exported DMA buffers. */
> + struct list_head dmabuf_exp_list;
> + /* List of wait objects. */
> + struct list_head dmabuf_exp_wait_list;
> + /* List of imported DMA buffers. */
> + struct list_head dmabuf_imp_list;
> + /* This is the lock which protects dma_buf_xxx lists. */
> + struct mutex dmabuf_lock;
>  };
>  
>  struct unmap_notify {
> @@ -95,10 +123,65 @@ struct grant_map {
>   struct gnttab_unmap_grant_ref *kunmap_ops;
>   struct page **pages;
>   unsigned long pages_vm_start;
> +
> + /*
> +  * All the fields starting with dmabuf_ are only valid if this
> +  * mapping is used for exporting a DMA buffer.
> +  * If dmabuf_vaddr is not NULL then this mapping is backed by DMA
> +  * capable memory.
> +  */
> +
> + /* Flags used to create this DMA buffer: GNTDEV_DMABUF_FLAG_XXX. */
> + bool dmabuf_flags;
> + /* Virtual/CPU address of the DMA buffer. */
> + void *dmabuf_vaddr;
> + /* Bus address of the DMA buffer. */
> + dma_addr_t dmabuf_bus_addr;
> +};
> +
> +struct hyper_dmabuf {
> + struct gntdev_priv *priv;
> + struct dma_buf *dmabuf;
> + struct list_head next;
> + int fd;
> +
> + union {
> + struct {
> + /* Exported buffers are reference counted. */
> + struct kref refcount;
> + struct grant_map *map;
> + } exp;
> + struct {
> + /* Granted references of the imported buffer. */
> + grant_ref_t *refs;
> + /* Scatter-gather table of the imported buffer. */
> + struct sg_table *sgt;
> + /* dma-buf attachment of the imported buffer. */
> + struct dma_buf_attachment *attach;
> + } imp;
> + } u;
> +
> + /* Number of pages this buffer has. */
> + int nr_pages;
> + /* Pages of this buffer. */
> + struct page **pages;
> +};
> +
> +struct hyper_dmabuf_wait_obj {
> + struct list_head next;
> + struct hyper_dmabuf *hyper_dmabuf;
> + struct completion completion;
> +};
> +
> +struct hyper_dambuf_attachment {
minor typo: dam->dma (same thing in other places as well.)

> + struct sg_table *sgt;
> + enum dma_data_direction dir;
>  };
>  
>  static int unmap_grant_pages(struct grant_map *map, int offset, int pages);
>  
> +static struct miscdevice gntdev_miscdev;
> +
>  /* 

[Xen-devel] [RFC 3/3] xen/gntdev: Add support for Linux dma buffers

[Oleksandr Andrushchenko]

From: Oleksandr Andrushchenko 

Signed-off-by: Oleksandr Andrushchenko 
---
 drivers/xen/gntdev.c  | 954 +-
 include/uapi/xen/gntdev.h | 101 
 include/xen/gntdev_exp.h  |  23 +
 3 files changed, 1066 insertions(+), 12 deletions(-)
 create mode 100644 include/xen/gntdev_exp.h

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 9510f228efe9..0ee88e193362 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -4,6 +4,8 @@
  * Device for accessing (in user-space) pages that have been granted by other
  * domains.
  *
+ * DMA buffer implementation is based on drivers/gpu/drm/drm_prime.c.
+ *
  * Copyright (c) 2006-2007, D G Murray.
  *   (c) 2009 Gerd Hoffmann 
  *
@@ -37,6 +39,9 @@
 #include 
 #include 
 
+#include 
+#include 
+
 #include 
 #include 
 #include 
@@ -61,16 +66,39 @@ static atomic_t pages_mapped = ATOMIC_INIT(0);
 static int use_ptemod;
 #define populate_freeable_maps use_ptemod
 
+#ifndef GRANT_INVALID_REF
+/*
+ * Note on usage of grant reference 0 as invalid grant reference:
+ * grant reference 0 is valid, but never exposed to a driver,
+ * because of the fact it is already in use/reserved by the PV console.
+ */
+#define GRANT_INVALID_REF  0
+#endif
+
 struct gntdev_priv {
/* maps with visible offsets in the file descriptor */
struct list_head maps;
/* maps that are not visible; will be freed on munmap.
 * Only populated if populate_freeable_maps == 1 */
struct list_head freeable_maps;
+   /* List of dma-bufs. */
+   struct list_head dma_bufs;
/* lock protects maps and freeable_maps */
struct mutex lock;
struct mm_struct *mm;
struct mmu_notifier mn;
+
+   /* Private data of the hyper DMA buffers. */
+
+   struct device *dev;
+   /* List of exported DMA buffers. */
+   struct list_head dmabuf_exp_list;
+   /* List of wait objects. */
+   struct list_head dmabuf_exp_wait_list;
+   /* List of imported DMA buffers. */
+   struct list_head dmabuf_imp_list;
+   /* This is the lock which protects dma_buf_xxx lists. */
+   struct mutex dmabuf_lock;
 };
 
 struct unmap_notify {
@@ -95,10 +123,65 @@ struct grant_map {
struct gnttab_unmap_grant_ref *kunmap_ops;
struct page **pages;
unsigned long pages_vm_start;
+
+   /*
+* All the fields starting with dmabuf_ are only valid if this
+* mapping is used for exporting a DMA buffer.
+* If dmabuf_vaddr is not NULL then this mapping is backed by DMA
+* capable memory.
+*/
+
+   /* Flags used to create this DMA buffer: GNTDEV_DMABUF_FLAG_XXX. */
+   bool dmabuf_flags;
+   /* Virtual/CPU address of the DMA buffer. */
+   void *dmabuf_vaddr;
+   /* Bus address of the DMA buffer. */
+   dma_addr_t dmabuf_bus_addr;
+};
+
+struct hyper_dmabuf {
+   struct gntdev_priv *priv;
+   struct dma_buf *dmabuf;
+   struct list_head next;
+   int fd;
+
+   union {
+   struct {
+   /* Exported buffers are reference counted. */
+   struct kref refcount;
+   struct grant_map *map;
+   } exp;
+   struct {
+   /* Granted references of the imported buffer. */
+   grant_ref_t *refs;
+   /* Scatter-gather table of the imported buffer. */
+   struct sg_table *sgt;
+   /* dma-buf attachment of the imported buffer. */
+   struct dma_buf_attachment *attach;
+   } imp;
+   } u;
+
+   /* Number of pages this buffer has. */
+   int nr_pages;
+   /* Pages of this buffer. */
+   struct page **pages;
+};
+
+struct hyper_dmabuf_wait_obj {
+   struct list_head next;
+   struct hyper_dmabuf *hyper_dmabuf;
+   struct completion completion;
+};
+
+struct hyper_dambuf_attachment {
+   struct sg_table *sgt;
+   enum dma_data_direction dir;
 };
 
 static int unmap_grant_pages(struct grant_map *map, int offset, int pages);
 
+static struct miscdevice gntdev_miscdev;
+
 /* -- */
 
 static void gntdev_print_maps(struct gntdev_priv *priv,
@@ -120,8 +203,17 @@ static void gntdev_free_map(struct grant_map *map)
if (map == NULL)
return;
 
-   if (map->pages)
+   if (map->dmabuf_vaddr) {
+   bool coherent = map->dmabuf_flags &
+   GNTDEV_DMABUF_FLAG_DMA_COHERENT;
+
+   gnttab_dma_free_pages(gntdev_miscdev.this_device,
+ coherent, map->count, map->pages,
+ map->dmabuf_vaddr, map->dmabuf_bus_addr);
+   } else if (map->pages) {