Re: [Xen-devel] [RFC 3/3] xen/gntdev: Add support for Linux dma buffers
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 dir; }; static int unmap_grant_pages(struct grant_map *map, int offset, int page
Re: [Xen-devel] [RFC 3/3] xen/gntdev: Add support for Linux dma buffers
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; > + > /* -- */ > > static void g
[Xen-devel] [RFC 3/3] xen/gntdev: Add support for Linux dma buffers
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) { gnttab_free_pages(map->count, map->pages); + } + kfree(map->pages); kfre