Re: [PATCH v3 9/9] xen/gntdev: Implement dma-buf import functionality

2018-06-14 Thread Boris Ostrovsky
On 06/13/2018 05:04 AM, Oleksandr Andrushchenko wrote:
> On 06/13/2018 06:14 AM, Boris Ostrovsky wrote:
>>
>>
>> On 06/12/2018 09:42 AM, Oleksandr Andrushchenko wrote:
>>
>>>   int gntdev_dmabuf_imp_release(struct gntdev_dmabuf_priv *priv, u32
>>> fd)
>>>   {
>>> -    return -EINVAL;
>>> +    struct gntdev_dmabuf *gntdev_dmabuf;
>>> +    struct dma_buf_attachment *attach;
>>> +    struct dma_buf *dma_buf;
>>> +
>>> +    gntdev_dmabuf = dmabuf_imp_find_unlink(priv, fd);
>>> +    if (IS_ERR(gntdev_dmabuf))
>>> +    return PTR_ERR(gntdev_dmabuf);
>>> +
>>> +    pr_debug("Releasing DMA buffer with fd %d\n", fd);
>>> +
>>> +    attach = gntdev_dmabuf->u.imp.attach;
>>> +
>>> +    if (gntdev_dmabuf->u.imp.sgt)
>>> +    dma_buf_unmap_attachment(attach, gntdev_dmabuf->u.imp.sgt,
>>> + DMA_BIDIRECTIONAL);
>>> +    dma_buf = attach->dmabuf;
>>> +    dma_buf_detach(attach->dmabuf, attach);
>>> +    dma_buf_put(dma_buf);
>>> +
>>> +    dmabuf_imp_end_foreign_access(gntdev_dmabuf->u.imp.refs,
>>> +  gntdev_dmabuf->nr_pages);
>>
>>
>>
>> Should you first end foreign access, before doing anything?
>>
> I am rolling back in reverse order here, so I think we first need
> to finish local activities with the buffer and then end foreign
> access.

Looking at gntdev_dmabuf_imp_to_refs(), the order is
    dmabuf_imp_alloc_storage()
    dma_buf_attach()
    dma_buf_map_attachment()
    dmabuf_imp_grant_foreign_access()

Or was I looking at wrong place?

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


Re: [PATCH v3 9/9] xen/gntdev: Implement dma-buf import functionality

2018-06-14 Thread Oleksandr Andrushchenko

On 06/14/2018 01:03 AM, Boris Ostrovsky wrote:

On 06/13/2018 05:04 AM, Oleksandr Andrushchenko wrote:

On 06/13/2018 06:14 AM, Boris Ostrovsky wrote:


On 06/12/2018 09:42 AM, Oleksandr Andrushchenko wrote:


   int gntdev_dmabuf_imp_release(struct gntdev_dmabuf_priv *priv, u32
fd)
   {
-    return -EINVAL;
+    struct gntdev_dmabuf *gntdev_dmabuf;
+    struct dma_buf_attachment *attach;
+    struct dma_buf *dma_buf;
+
+    gntdev_dmabuf = dmabuf_imp_find_unlink(priv, fd);
+    if (IS_ERR(gntdev_dmabuf))
+    return PTR_ERR(gntdev_dmabuf);
+
+    pr_debug("Releasing DMA buffer with fd %d\n", fd);
+
+    attach = gntdev_dmabuf->u.imp.attach;
+
+    if (gntdev_dmabuf->u.imp.sgt)
+    dma_buf_unmap_attachment(attach, gntdev_dmabuf->u.imp.sgt,
+ DMA_BIDIRECTIONAL);
+    dma_buf = attach->dmabuf;
+    dma_buf_detach(attach->dmabuf, attach);
+    dma_buf_put(dma_buf);
+
+    dmabuf_imp_end_foreign_access(gntdev_dmabuf->u.imp.refs,
+  gntdev_dmabuf->nr_pages);



Should you first end foreign access, before doing anything?


I am rolling back in reverse order here, so I think we first need
to finish local activities with the buffer and then end foreign
access.

Looking at gntdev_dmabuf_imp_to_refs(), the order is
     dmabuf_imp_alloc_storage()
     dma_buf_attach()
     dma_buf_map_attachment()
     dmabuf_imp_grant_foreign_access()

Or was I looking at wrong place?

Agree, will move as you suggest

-boris


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


Re: [PATCH v3 9/9] xen/gntdev: Implement dma-buf import functionality

2018-06-14 Thread Oleksandr Andrushchenko

On 06/13/2018 06:14 AM, Boris Ostrovsky wrote:



On 06/12/2018 09:42 AM, Oleksandr Andrushchenko wrote:


  int gntdev_dmabuf_imp_release(struct gntdev_dmabuf_priv *priv, u32 fd)
  {
-    return -EINVAL;
+    struct gntdev_dmabuf *gntdev_dmabuf;
+    struct dma_buf_attachment *attach;
+    struct dma_buf *dma_buf;
+
+    gntdev_dmabuf = dmabuf_imp_find_unlink(priv, fd);
+    if (IS_ERR(gntdev_dmabuf))
+    return PTR_ERR(gntdev_dmabuf);
+
+    pr_debug("Releasing DMA buffer with fd %d\n", fd);
+
+    attach = gntdev_dmabuf->u.imp.attach;
+
+    if (gntdev_dmabuf->u.imp.sgt)
+    dma_buf_unmap_attachment(attach, gntdev_dmabuf->u.imp.sgt,
+ DMA_BIDIRECTIONAL);
+    dma_buf = attach->dmabuf;
+    dma_buf_detach(attach->dmabuf, attach);
+    dma_buf_put(dma_buf);
+
+    dmabuf_imp_end_foreign_access(gntdev_dmabuf->u.imp.refs,
+  gntdev_dmabuf->nr_pages);




Should you first end foreign access, before doing anything?


I am rolling back in reverse order here, so I think we first need
to finish local activities with the buffer and then end foreign
access.

-boris



+ dmabuf_imp_free_storage(gntdev_dmabuf);
+    return 0;
  }


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


Re: [PATCH v3 9/9] xen/gntdev: Implement dma-buf import functionality

2018-06-13 Thread Boris Ostrovsky



On 06/12/2018 09:42 AM, Oleksandr Andrushchenko wrote:


  int gntdev_dmabuf_imp_release(struct gntdev_dmabuf_priv *priv, u32 fd)
  {
-   return -EINVAL;
+   struct gntdev_dmabuf *gntdev_dmabuf;
+   struct dma_buf_attachment *attach;
+   struct dma_buf *dma_buf;
+
+   gntdev_dmabuf = dmabuf_imp_find_unlink(priv, fd);
+   if (IS_ERR(gntdev_dmabuf))
+   return PTR_ERR(gntdev_dmabuf);
+
+   pr_debug("Releasing DMA buffer with fd %d\n", fd);
+
+   attach = gntdev_dmabuf->u.imp.attach;
+
+   if (gntdev_dmabuf->u.imp.sgt)
+   dma_buf_unmap_attachment(attach, gntdev_dmabuf->u.imp.sgt,
+DMA_BIDIRECTIONAL);
+   dma_buf = attach->dmabuf;
+   dma_buf_detach(attach->dmabuf, attach);
+   dma_buf_put(dma_buf);
+
+   dmabuf_imp_end_foreign_access(gntdev_dmabuf->u.imp.refs,
+ gntdev_dmabuf->nr_pages);




Should you first end foreign access, before doing anything?

-boris



+   dmabuf_imp_free_storage(gntdev_dmabuf);
+   return 0;
  }
  

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


[PATCH v3 9/9] xen/gntdev: Implement dma-buf import functionality

2018-06-12 Thread Oleksandr Andrushchenko
From: Oleksandr Andrushchenko 

1. Import a dma-buf with the file descriptor provided and export
   granted references to the pages of that dma-buf into the array
   of grant references.

2. Add API to close all references to an imported buffer, so it can be
   released by the owner. This is only valid for buffers created with
   IOCTL_GNTDEV_DMABUF_IMP_TO_REFS.

Signed-off-by: Oleksandr Andrushchenko 
---
 drivers/xen/gntdev-dmabuf.c | 240 +++-
 1 file changed, 238 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c
index 84cba67c6ad7..4d250eb8babc 100644
--- a/drivers/xen/gntdev-dmabuf.c
+++ b/drivers/xen/gntdev-dmabuf.c
@@ -17,6 +17,15 @@
 #include "gntdev-common.h"
 #include "gntdev-dmabuf.h"
 
+#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_dmabuf {
struct gntdev_dmabuf_priv *priv;
struct dma_buf *dmabuf;
@@ -31,6 +40,14 @@ struct gntdev_dmabuf {
struct gntdev_priv *priv;
struct gntdev_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. */
@@ -55,6 +72,8 @@ struct gntdev_dmabuf_priv {
struct list_head exp_list;
/* List of wait objects. */
struct list_head exp_wait_list;
+   /* List of imported DMA buffers. */
+   struct list_head imp_list;
/* This is the lock which protects dma_buf_xxx lists. */
struct mutex lock;
 };
@@ -500,21 +519,237 @@ int gntdev_dmabuf_exp_from_refs(struct gntdev_priv 
*priv, int flags,
 
 /* DMA buffer import support. */
 
+static int
+dmabuf_imp_grant_foreign_access(struct page **pages, u32 *refs,
+   int count, int domid)
+{
+   grant_ref_t priv_gref_head;
+   int i, ret;
+
+   ret = gnttab_alloc_grant_references(count, _gref_head);
+   if (ret < 0) {
+   pr_debug("Cannot allocate grant references, ret %d\n", ret);
+   return ret;
+   }
+
+   for (i = 0; i < count; i++) {
+   int cur_ref;
+
+   cur_ref = gnttab_claim_grant_reference(_gref_head);
+   if (cur_ref < 0) {
+   ret = cur_ref;
+   pr_debug("Cannot claim grant reference, ret %d\n", ret);
+   goto out;
+   }
+
+   gnttab_grant_foreign_access_ref(cur_ref, domid,
+   xen_page_to_gfn(pages[i]), 0);
+   refs[i] = cur_ref;
+   }
+
+   return 0;
+
+out:
+   gnttab_free_grant_references(priv_gref_head);
+   return ret;
+}
+
+static void dmabuf_imp_end_foreign_access(u32 *refs, int count)
+{
+   int i;
+
+   for (i = 0; i < count; i++)
+   if (refs[i] != GRANT_INVALID_REF)
+   gnttab_end_foreign_access(refs[i], 0, 0UL);
+}
+
+static void dmabuf_imp_free_storage(struct gntdev_dmabuf *gntdev_dmabuf)
+{
+   kfree(gntdev_dmabuf->pages);
+   kfree(gntdev_dmabuf->u.imp.refs);
+   kfree(gntdev_dmabuf);
+}
+
+static struct gntdev_dmabuf *dmabuf_imp_alloc_storage(int count)
+{
+   struct gntdev_dmabuf *gntdev_dmabuf;
+   int i;
+
+   gntdev_dmabuf = kzalloc(sizeof(*gntdev_dmabuf), GFP_KERNEL);
+   if (!gntdev_dmabuf)
+   goto fail;
+
+   gntdev_dmabuf->u.imp.refs = kcalloc(count,
+   
sizeof(gntdev_dmabuf->u.imp.refs[0]),
+   GFP_KERNEL);
+   if (!gntdev_dmabuf->u.imp.refs)
+   goto fail;
+
+   gntdev_dmabuf->pages = kcalloc(count,
+  sizeof(gntdev_dmabuf->pages[0]),
+  GFP_KERNEL);
+   if (!gntdev_dmabuf->pages)
+   goto fail;
+
+   gntdev_dmabuf->nr_pages = count;
+
+   for (i = 0; i < count; i++)
+   gntdev_dmabuf->u.imp.refs[i] = GRANT_INVALID_REF;
+
+   return gntdev_dmabuf;
+
+fail:
+   dmabuf_imp_free_storage(gntdev_dmabuf);
+   return ERR_PTR(-ENOMEM);
+}
+
 struct gntdev_dmabuf *
 gntdev_dmabuf_imp_to_refs(struct gntdev_dmabuf_priv *priv, struct device *dev,
  int fd, int count, int domid)
 {
-   return ERR_PTR(-ENOMEM);
+   struct