On 25/05/18 17:33, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

Make set/clear page private code shared and accessible to other kernel modules which can re-use these instead of open-coding.

Signed-off-by: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

drivers/xen/grant-table.c | 54 +++++++++++++++++++++++++-------------- include/xen/grant_table.h | 3 +++ 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c index 27be107d6480..d7488226e1f2 100644 --- a/drivers/xen/grant-table.c +++ b/drivers/xen/grant-table.c @@ -769,29 +769,18 @@ void gnttab_free_auto_xlat_frames(void) } EXPORT_SYMBOL_GPL(gnttab_free_auto_xlat_frames);

-/**

• • gnttab_alloc_pages - alloc pages suitable for grant mapping into
• • @nr_pages: number of pages to alloc
• • @pages: returns the pages
• */

-int gnttab_alloc_pages(int nr_pages, struct page **pages) +int gnttab_pages_set_private(int nr_pages, struct page **pages) { int i;

• int ret;

• ret = alloc_xenballooned_pages(nr_pages, pages);

• if (ret < 0)

• return ret;
  
  

  for (i = 0; i < nr_pages; i++) {

#if BITS_PER_LONG < 64 struct xen_page_foreign *foreign;

foreign = kzalloc(sizeof(*foreign), GFP_KERNEL);

• if (!foreign) {
  

• 	gnttab_free_pages(nr_pages, pages);
  

• if (!foreign)
  return -ENOMEM;
  

• }
  

• set_page_private(pages[i], (unsigned long)foreign);

#endif SetPagePrivate(pages[i]); @@ -799,14 +788,30 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)

return 0; } -EXPORT_SYMBOL(gnttab_alloc_pages); +EXPORT_SYMBOL(gnttab_pages_set_private);

EXPORT_SYMBOL_GPL()

/**

• • gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
• • @nr_pages; number of pages to free
• • @pages: the pages

• • gnttab_alloc_pages - alloc pages suitable for grant mapping into
• • @nr_pages: number of pages to alloc
• • @pages: returns the pages
  */

-void gnttab_free_pages(int nr_pages, struct page **pages) +int gnttab_alloc_pages(int nr_pages, struct page **pages) +{

• int ret;
• ret = alloc_xenballooned_pages(nr_pages, pages);
• if (ret < 0)

• return ret;
  

• ret = gnttab_pages_set_private(nr_pages, pages);
• if (ret < 0)

• gnttab_free_pages(nr_pages, pages);
  

• return ret;

+} +EXPORT_SYMBOL(gnttab_alloc_pages);

+void gnttab_pages_clear_private(int nr_pages, struct page **pages) { int i;

@@ -818,6 +823,17 @@ void gnttab_free_pages(int nr_pages, struct page **pages) ClearPagePrivate(pages[i]); } } +} +EXPORT_SYMBOL(gnttab_pages_clear_private);

EXPORT_SYMBOL_GPL()

Juergen

On Fri, May 25, 2018 at 06:33:24PM +0300, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

Make set/clear page private code shared and accessible to other kernel modules which can re-use these instead of open-coding.

Signed-off-by: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

drivers/xen/grant-table.c | 54 +++++++++++++++++++++++++-------------- include/xen/grant_table.h | 3 +++ 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c index 27be107d6480..d7488226e1f2 100644 --- a/drivers/xen/grant-table.c +++ b/drivers/xen/grant-table.c @@ -769,29 +769,18 @@ void gnttab_free_auto_xlat_frames(void) } EXPORT_SYMBOL_GPL(gnttab_free_auto_xlat_frames);

-/**

• • gnttab_alloc_pages - alloc pages suitable for grant mapping into
• • @nr_pages: number of pages to alloc
• • @pages: returns the pages
• */

-int gnttab_alloc_pages(int nr_pages, struct page **pages) +int gnttab_pages_set_private(int nr_pages, struct page **pages) { int i;

• int ret;

• ret = alloc_xenballooned_pages(nr_pages, pages);

• if (ret < 0)

• return ret;
  
  

  for (i = 0; i < nr_pages; i++) {

#if BITS_PER_LONG < 64 struct xen_page_foreign *foreign;

foreign = kzalloc(sizeof(*foreign), GFP_KERNEL);

• if (!foreign) {
  

• 	gnttab_free_pages(nr_pages, pages);
  

• if (!foreign)
  

Don't we have to free previously allocated "foreign"(s) if it fails in the middle (e.g. 0 < i && i < nr_pages - 1) before returning?

	return -ENOMEM;

• }
  

• set_page_private(pages[i], (unsigned long)foreign);

#endif SetPagePrivate(pages[i]); @@ -799,14 +788,30 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)

return 0; } -EXPORT_SYMBOL(gnttab_alloc_pages); +EXPORT_SYMBOL(gnttab_pages_set_private);

/**

• • gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
• • @nr_pages; number of pages to free
• • @pages: the pages

• • gnttab_alloc_pages - alloc pages suitable for grant mapping into
• • @nr_pages: number of pages to alloc
• • @pages: returns the pages
  */

-void gnttab_free_pages(int nr_pages, struct page **pages) +int gnttab_alloc_pages(int nr_pages, struct page **pages) +{

• int ret;
• ret = alloc_xenballooned_pages(nr_pages, pages);
• if (ret < 0)

• return ret;
  

• ret = gnttab_pages_set_private(nr_pages, pages);
• if (ret < 0)

• gnttab_free_pages(nr_pages, pages);
  

• return ret;

+} +EXPORT_SYMBOL(gnttab_alloc_pages);

+void gnttab_pages_clear_private(int nr_pages, struct page **pages) { int i;

@@ -818,6 +823,17 @@ void gnttab_free_pages(int nr_pages, struct page **pages) ClearPagePrivate(pages[i]); } } +} +EXPORT_SYMBOL(gnttab_pages_clear_private);

+/**

• • gnttab_free_pages - free pages allocated by gnttab_alloc_pages()
• • @nr_pages; number of pages to free
• • @pages: the pages
• */

+void gnttab_free_pages(int nr_pages, struct page **pages) +{

• gnttab_pages_clear_private(nr_pages, pages); free_xenballooned_pages(nr_pages, pages);

} EXPORT_SYMBOL(gnttab_free_pages); diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h index 2e37741f6b8d..de03f2542bb7 100644 --- a/include/xen/grant_table.h +++ b/include/xen/grant_table.h @@ -198,6 +198,9 @@ void gnttab_free_auto_xlat_frames(void); int gnttab_alloc_pages(int nr_pages, struct page **pages); void gnttab_free_pages(int nr_pages, struct page **pages);

+int gnttab_pages_set_private(int nr_pages, struct page **pages); +void gnttab_pages_clear_private(int nr_pages, struct page **pages);

int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops, struct gnttab_map_grant_ref *kmap_ops, struct page **pages, unsigned int count); -- 2.17.0

On 05/29/2018 09:04 PM, Boris Ostrovsky wrote:

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:

...

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

Memory {increase|decrease}_reservation and VA mappings update/reset code used in balloon driver can be made common, so other drivers can also re-use the same functionality without open-coding. Create a dedicated module

IIUIC this is not really a module, it's a common file.

Sure, will put "file" here

...

for the shared code and export corresponding symbols for other kernel modules.

Signed-off-by: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

drivers/xen/Makefile | 1 + drivers/xen/balloon.c | 71 ++---------------- drivers/xen/mem-reservation.c | 134 ++++++++++++++++++++++++++++++++++ include/xen/mem_reservation.h | 29 ++++++++ 4 files changed, 170 insertions(+), 65 deletions(-) create mode 100644 drivers/xen/mem-reservation.c create mode 100644 include/xen/mem_reservation.h

diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index 451e833f5931..3c87b0c3aca6 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o obj-$(CONFIG_X86) += fallback.o obj-y += grant-table.o features.o balloon.o manage.o preempt.o time.o +obj-y += mem-reservation.o obj-y += events/ obj-y += xenbus/

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c index 065f0b607373..57b482d67a3a 100644 --- a/drivers/xen/balloon.c +++ b/drivers/xen/balloon.c @@ -71,6 +71,7 @@ #include <xen/balloon.h> #include <xen/features.h> #include <xen/page.h> +#include <xen/mem_reservation.h>

static int xen_hotplug_unpopulated;

@@ -157,13 +158,6 @@ static DECLARE_DELAYED_WORK(balloon_worker, balloon_process); #define GFP_BALLOON \ (GFP_HIGHUSER | __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC)

-static void scrub_page(struct page *page) -{ -#ifdef CONFIG_XEN_SCRUB_PAGES

• clear_highpage(page);

-#endif -}

• /* balloon_append: add the given page to the balloon. */ static void __balloon_append(struct page *page) {

@@ -463,11 +457,6 @@ static enum bp_state increase_reservation(unsigned long nr_pages) int rc; unsigned long i; struct page *page;

• struct xen_memory_reservation reservation = {

• .address_bits = 0,
  

• .extent_order = EXTENT_ORDER,
  

• .domid        = DOMID_SELF
  

• };

  if (nr_pages > ARRAY_SIZE(frame_list)) nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages) page = balloon_next_page(page); }

• set_xen_guest_handle(reservation.extent_start, frame_list);
• reservation.nr_extents = nr_pages;
• rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);

• rc = xenmem_reservation_increase(nr_pages, frame_list); if (rc <= 0) return BP_EAGAIN;

@@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages) page = balloon_retrieve(false); BUG_ON(page == NULL);

-#ifdef CONFIG_XEN_HAVE_PVMMU

• /*
  

•  * We don't support PV MMU when Linux and Xen is using
  

•  * different page granularity.
  

•  */
  

• BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
  

• if (!xen_feature(XENFEAT_auto_translated_physmap)) {
  

• 	unsigned long pfn = page_to_pfn(page);
  

• 	set_phys_to_machine(pfn, frame_list[i]);
  

• 	/* Link back into the page tables if not highmem. */
  

• 	if (!PageHighMem(page)) {
  

• 		int ret;
  

• 		ret = HYPERVISOR_update_va_mapping(
  

• 				(unsigned long)__va(pfn << PAGE_SHIFT),
  

• 				mfn_pte(frame_list[i], PAGE_KERNEL),
  

• 				0);
  

• 		BUG_ON(ret);
  

• 	}
  

• }
  

-#endif

• xenmem_reservation_va_mapping_update(1, &page, &frame_list[i]);
  

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

This is actually how it is used later on for dma-buf, but I just didn't want to alter original balloon code too much, but this can be done, in order of simplicity:

1. Similar to frame_list, e.g. static array of struct page* of size ARRAY_SIZE(frame_list): more static memory is used, but no allocations

2. Allocated at run-time with kcalloc: allocation can fail

3. Make balloon_retrieve() return a set of pages: will require list/array allocation and handling, allocation may fail, balloon_retrieve prototype change

Could you please tell which of the above will fit better?

...

/* Relinquish the page back to the allocator. */
free_reserved_page(page);

@@ -535,11 +500,6 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) unsigned long i; struct page *page, *tmp; int ret;

• struct xen_memory_reservation reservation = {

• .address_bits = 0,
  

• .extent_order = EXTENT_ORDER,
  

• .domid        = DOMID_SELF
  

• }; LIST_HEAD(pages);

  if (nr_pages > ARRAY_SIZE(frame_list))

@@ -553,7 +513,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) break; } adjust_managed_page_count(page, -1);

• scrub_page(page);
  

• xenmem_reservation_scrub_page(page);
  

  list_add(&page->lru, &pages); }

@@ -575,25 +535,8 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) /* XENMEM_decrease_reservation requires a GFN */ frame_list[i++] = xen_page_to_gfn(page);

-#ifdef CONFIG_XEN_HAVE_PVMMU

• /*
  

•  * We don't support PV MMU when Linux and Xen is using
  

•  * different page granularity.
  

•  */
  

• BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
  

• if (!xen_feature(XENFEAT_auto_translated_physmap)) {
  

• 	unsigned long pfn = page_to_pfn(page);
  

• xenmem_reservation_va_mapping_reset(1, &page);
  

and here too.

see above

...

• 	if (!PageHighMem(page)) {
  

• 		ret = HYPERVISOR_update_va_mapping(
  

• 				(unsigned long)__va(pfn << PAGE_SHIFT),
  

• 				__pte_ma(0), 0);
  

• 		BUG_ON(ret);
  

• 	}
  

• 	__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
  

• }
  

-#endif list_del(&page->lru);

balloon_append(page);

@@ -601,9 +544,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)

flush_tlb_all();

• set_xen_guest_handle(reservation.extent_start, frame_list);
• reservation.nr_extents = nr_pages;
• ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation);

• ret = xenmem_reservation_decrease(nr_pages, frame_list); BUG_ON(ret != nr_pages);

  balloon_stats.current_pages -= nr_pages;

diff --git a/drivers/xen/mem-reservation.c b/drivers/xen/mem-reservation.c new file mode 100644 index 000000000000..29882e4324f5 --- /dev/null +++ b/drivers/xen/mem-reservation.c @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT

Why is this "OR MIT"? The original file was licensed GPLv2 only.

Will fix - I was not sure about the license to be used here, thanks

...

+/******************************************************************************

• • Xen memory reservation utilities.
• • Copyright (c) 2003, B Dragovic
• • Copyright (c) 2003-2004, M Williamson, K Fraser
• • Copyright (c) 2005 Dan M. Smith, IBM Corporation
• • Copyright (c) 2010 Daniel Kiper
• • Copyright (c) 2018, Oleksandr Andrushchenko, EPAM Systems Inc.
• */

+#include <linux/kernel.h> +#include <linux/slab.h>

+#include <asm/tlb.h> +#include <asm/xen/hypercall.h>

+#include <xen/interface/memory.h> +#include <xen/page.h>

+/*

• • Use one extent per PAGE_SIZE to avoid to break down the page into
• • multiple frame.
• */

+#define EXTENT_ORDER (fls(XEN_PFN_PER_PAGE) - 1)

+void xenmem_reservation_scrub_page(struct page *page) +{ +#ifdef CONFIG_XEN_SCRUB_PAGES

• clear_highpage(page);

+#endif +} +EXPORT_SYMBOL(xenmem_reservation_scrub_page);

+void xenmem_reservation_va_mapping_update(unsigned long count,

• 			  struct page **pages,
  

• 			  xen_pfn_t *frames)
  

+{ +#ifdef CONFIG_XEN_HAVE_PVMMU

• int i;
• for (i = 0; i < count; i++) {

• struct page *page;
  

• page = pages[i];
  

• BUG_ON(page == NULL);
  

• /*
  

•  * We don't support PV MMU when Linux and Xen is using
  

•  * different page granularity.
  

•  */
  

• BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
  

• if (!xen_feature(XENFEAT_auto_translated_physmap)) {
  

• 	unsigned long pfn = page_to_pfn(page);
  

• 	set_phys_to_machine(pfn, frames[i]);
  

• 	/* Link back into the page tables if not highmem. */
  

• 	if (!PageHighMem(page)) {
  

• 		int ret;
  

• 		ret = HYPERVISOR_update_va_mapping(
  

• 				(unsigned long)__va(pfn << PAGE_SHIFT),
  

• 				mfn_pte(frames[i], PAGE_KERNEL),
  

• 				0);
  

• 		BUG_ON(ret);
  

• 	}
  

• }
  

• }

+#endif +} +EXPORT_SYMBOL(xenmem_reservation_va_mapping_update);

+void xenmem_reservation_va_mapping_reset(unsigned long count,

• 			 struct page **pages)
  

+{ +#ifdef CONFIG_XEN_HAVE_PVMMU

• int i;
• for (i = 0; i < count; i++) {

• /*
  

•  * We don't support PV MMU when Linux and Xen is using
  

•  * different page granularity.
  

•  */
  

• BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
  

• if (!xen_feature(XENFEAT_auto_translated_physmap)) {
  

• 	struct page *page = pages[i];
  

• 	unsigned long pfn = page_to_pfn(page);
  

• 	if (!PageHighMem(page)) {
  

• 		int ret;
  

• 		ret = HYPERVISOR_update_va_mapping(
  

• 				(unsigned long)__va(pfn << PAGE_SHIFT),
  

• 				__pte_ma(0), 0);
  

• 		BUG_ON(ret);
  

• 	}
  

• 	__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
  

• }
  

• }

+#endif +} +EXPORT_SYMBOL(xenmem_reservation_va_mapping_reset);

+int xenmem_reservation_increase(int count, xen_pfn_t *frames) +{

• struct xen_memory_reservation reservation = {

• .address_bits = 0,
  

• .extent_order = EXTENT_ORDER,
  

• .domid        = DOMID_SELF
  

• };
• set_xen_guest_handle(reservation.extent_start, frames);
• reservation.nr_extents = count;
• return HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);

+} +EXPORT_SYMBOL(xenmem_reservation_increase);

+int xenmem_reservation_decrease(int count, xen_pfn_t *frames) +{

• struct xen_memory_reservation reservation = {

• .address_bits = 0,
  

• .extent_order = EXTENT_ORDER,
  

• .domid        = DOMID_SELF
  

• };
• set_xen_guest_handle(reservation.extent_start, frames);
• reservation.nr_extents = count;
• return HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation);

+} +EXPORT_SYMBOL(xenmem_reservation_decrease); diff --git a/include/xen/mem_reservation.h b/include/xen/mem_reservation.h new file mode 100644 index 000000000000..9306d9b8743c --- /dev/null +++ b/include/xen/mem_reservation.h @@ -0,0 +1,29 @@ +/* SPDX-License-Identifier: GPL-2.0 OR MIT */

and here too.

will fix

-boris

Thank you, Oleksandr

...

+/*

• • Xen memory reservation utilities.
• • Copyright (c) 2003, B Dragovic
• • Copyright (c) 2003-2004, M Williamson, K Fraser
• • Copyright (c) 2005 Dan M. Smith, IBM Corporation
• • Copyright (c) 2010 Daniel Kiper
• • Copyright (c) 2018, Oleksandr Andrushchenko, EPAM Systems Inc.
• */

+#ifndef _XENMEM_RESERVATION_H +#define _XENMEM_RESERVATION_H

+void xenmem_reservation_scrub_page(struct page *page);

+void xenmem_reservation_va_mapping_update(unsigned long count,

• 			  struct page **pages,
  

• 			  xen_pfn_t *frames);
  

+void xenmem_reservation_va_mapping_reset(unsigned long count,

• 			 struct page **pages);
  

+int xenmem_reservation_increase(int count, xen_pfn_t *frames);

+int xenmem_reservation_decrease(int count, xen_pfn_t *frames);

+#endif

On 05/29/2018 11:03 PM, Boris Ostrovsky wrote:

On 05/29/2018 02:22 PM, Oleksandr Andrushchenko wrote:

...

On 05/29/2018 09:04 PM, Boris Ostrovsky wrote:

...

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote: @@ -463,11 +457,6 @@ static enum bp_state increase_reservation(unsigned long nr_pages)       int rc;       unsigned long i;       struct page   *page; -    struct xen_memory_reservation reservation = { -        .address_bits = 0, -        .extent_order = EXTENT_ORDER, -        .domid        = DOMID_SELF -    };         if (nr_pages > ARRAY_SIZE(frame_list))           nr_pages = ARRAY_SIZE(frame_list); @@ -486,9 +475,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)           page = balloon_next_page(page);       }   -    set_xen_guest_handle(reservation.extent_start, frame_list); -    reservation.nr_extents = nr_pages; -    rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation); +    rc = xenmem_reservation_increase(nr_pages, frame_list);       if (rc <= 0)           return BP_EAGAIN;   @@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)           page = balloon_retrieve(false);           BUG_ON(page == NULL);   -#ifdef CONFIG_XEN_HAVE_PVMMU -        /* -         * We don't support PV MMU when Linux and Xen is using -         * different page granularity. -         */ -        BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

-        if (!xen_feature(XENFEAT_auto_translated_physmap)) { -            unsigned long pfn = page_to_pfn(page);

-            set_phys_to_machine(pfn, frame_list[i]);

-            /* Link back into the page tables if not highmem. */ -            if (!PageHighMem(page)) { -                int ret; -                ret = HYPERVISOR_update_va_mapping( -                        (unsigned long)__va(pfn << PAGE_SHIFT), -                        mfn_pte(frame_list[i], PAGE_KERNEL), -                        0); -                BUG_ON(ret); -            } -        } -#endif +        xenmem_reservation_va_mapping_update(1, &page, &frame_list[i]);

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

This is actually how it is used later on for dma-buf, but I just didn't want to alter original balloon code too much, but this can be done, in order of simplicity:

1. Similar to frame_list, e.g. static array of struct page* of size

ARRAY_SIZE(frame_list): more static memory is used, but no allocations

2. Allocated at run-time with kcalloc: allocation can fail

If this is called in freeing DMA buffer code path or in error path then we shouldn't do it.

...

3. Make balloon_retrieve() return a set of pages: will require

list/array allocation and handling, allocation may fail, balloon_retrieve prototype change

balloon pages are strung on the lru list. Can we keep have balloon_retrieve return a list of pages on that list?

First of all, before we go deep in details, I will highlight the goal of the requested change: for balloon driver we call xenmem_reservation_va_mapping_update(*1*, &page, &frame_list[i]); from increase_reservation and xenmem_reservation_va_mapping_reset(*1*, &page); from decrease_reservation and it seems to be not elegant because of that one page/frame passed while we might have multiple pages/frames passed at once.

In the balloon driver the producer of pages for increase_reservation is balloon_retrieve(false) and for decrease_reservation it is alloc_page(gfp). In case of decrease_reservation the page is added on the list: LIST_HEAD(pages); [...] list_add(&page->lru, &pages);

and in case of increase_reservation it is retrieved page by page and can be put on a list as well with the same code from decrease_reservation, e.g. LIST_HEAD(pages); [...] list_add(&page->lru, &pages);

Thus, both decrease_reservation and increase_reservation may hold their pages on a list before calling xenmem_reservation_va_mapping_{update|reset}.

For that we need a prototype change: xenmem_reservation_va_mapping_reset(<nr_pages>, <list of pages>); But for xenmem_reservation_va_mapping_update it will look like: xenmem_reservation_va_mapping_update(<nr_pages>, <list of pages>, <array of frames>) which seems to be inconsistent. Converting entries of the static frame_list array into corresponding list doesn't seem to be cute as well.

For dma-buf use-case arrays are more preferable as dma-buf constructs scatter-gather tables from array of pages etc. and if page list is passed then it needs to be converted into page array anyways.

So, we can: 1. Keep the prototypes as is, e.g. accept array of pages and use nr_pages == 1 in case of balloon driver (existing code) 2. Statically allocate struct page* array in the balloon driver and fill it with pages when those pages are retrieved: static struct page *page_list[ARRAY_SIZE(frame_list)]; which will take additional 8KiB of space on 64-bit platform, but simplify things a lot. 3. Allocate struct page *page_list[ARRAY_SIZE(frame_list)] dynamically

As to Boris' suggestion "balloon pages are strung on the lru list. Can we keep have balloon_retrieve return a list of pages on that list?" Because of alloc_xenballooned_pages' retry logic for page retireval, e.g.     while (pgno < nr_pages) {         page = balloon_retrieve(true);         if (page) { [...]         } else {             ret = add_ballooned_pages(nr_pages - pgno); [...]     } I wouldn't change things that much.

IMO, we can keep 1 page based API with the only overhead for balloon driver of function calls to xenmem_reservation_va_mapping_{update|reset} for each page.

-boris

Thank you, Oleksandr

...

Could you please tell which of the above will fit better?

...

On 05/30/2018 06:54 PM, Boris Ostrovsky wrote:

On 05/30/2018 04:29 AM, Oleksandr Andrushchenko wrote:

...

On 05/29/2018 11:03 PM, Boris Ostrovsky wrote:

...

On 05/29/2018 02:22 PM, Oleksandr Andrushchenko wrote:

...

On 05/29/2018 09:04 PM, Boris Ostrovsky wrote:

...

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote: @@ -463,11 +457,6 @@ static enum bp_state increase_reservation(unsigned long nr_pages)        int rc;        unsigned long i;        struct page   *page; -    struct xen_memory_reservation reservation = { -        .address_bits = 0, -        .extent_order = EXTENT_ORDER, -        .domid        = DOMID_SELF -    };          if (nr_pages > ARRAY_SIZE(frame_list))            nr_pages = ARRAY_SIZE(frame_list); @@ -486,9 +475,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)            page = balloon_next_page(page);        }    -    set_xen_guest_handle(reservation.extent_start, frame_list); -    reservation.nr_extents = nr_pages; -    rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation); +    rc = xenmem_reservation_increase(nr_pages, frame_list);        if (rc <= 0)            return BP_EAGAIN;    @@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)            page = balloon_retrieve(false);            BUG_ON(page == NULL);    -#ifdef CONFIG_XEN_HAVE_PVMMU -        /* -         * We don't support PV MMU when Linux and Xen is using -         * different page granularity. -         */ -        BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

-        if (!xen_feature(XENFEAT_auto_translated_physmap)) { -            unsigned long pfn = page_to_pfn(page);

-            set_phys_to_machine(pfn, frame_list[i]);

-            /* Link back into the page tables if not highmem. */ -            if (!PageHighMem(page)) { -                int ret; -                ret = HYPERVISOR_update_va_mapping( -                        (unsigned long)__va(pfn << PAGE_SHIFT), -                        mfn_pte(frame_list[i], PAGE_KERNEL), -                        0); -                BUG_ON(ret); -            } -        } -#endif +        xenmem_reservation_va_mapping_update(1, &page, &frame_list[i]);

Can you make a single call to xenmem_reservation_va_mapping_update(rc, ...)? You need to keep track of pages but presumable they can be put into an array (or a list). In fact, perhaps we can have balloon_retrieve() return a set of pages.

This is actually how it is used later on for dma-buf, but I just didn't want to alter original balloon code too much, but this can be done, in order of simplicity:

1. Similar to frame_list, e.g. static array of struct page* of size

ARRAY_SIZE(frame_list): more static memory is used, but no allocations

2. Allocated at run-time with kcalloc: allocation can fail

If this is called in freeing DMA buffer code path or in error path then we shouldn't do it.

...

3. Make balloon_retrieve() return a set of pages: will require

list/array allocation and handling, allocation may fail, balloon_retrieve prototype change

balloon pages are strung on the lru list. Can we keep have balloon_retrieve return a list of pages on that list?

First of all, before we go deep in details, I will highlight the goal of the requested change: for balloon driver we call xenmem_reservation_va_mapping_update(*1*, &page, &frame_list[i]); from increase_reservation and xenmem_reservation_va_mapping_reset(*1*, &page); from decrease_reservation and it seems to be not elegant because of that one page/frame passed while we might have multiple pages/frames passed at once.

In the balloon driver the producer of pages for increase_reservation is balloon_retrieve(false) and for decrease_reservation it is alloc_page(gfp). In case of decrease_reservation the page is added on the list: LIST_HEAD(pages); [...] list_add(&page->lru, &pages);

and in case of increase_reservation it is retrieved page by page and can be put on a list as well with the same code from decrease_reservation, e.g. LIST_HEAD(pages); [...] list_add(&page->lru, &pages);

Thus, both decrease_reservation and increase_reservation may hold their pages on a list before calling xenmem_reservation_va_mapping_{update|reset}.

For that we need a prototype change: xenmem_reservation_va_mapping_reset(<nr_pages>, <list of pages>); But for xenmem_reservation_va_mapping_update it will look like: xenmem_reservation_va_mapping_update(<nr_pages>, <list of pages>, <array of frames>) which seems to be inconsistent. Converting entries of the static frame_list array into corresponding list doesn't seem to be cute as well.

For dma-buf use-case arrays are more preferable as dma-buf constructs scatter-gather tables from array of pages etc. and if page list is passed then it needs to be converted into page array anyways.

So, we can:

1. Keep the prototypes as is, e.g. accept array of pages and use

nr_pages == 1 in case of balloon driver (existing code) 2. Statically allocate struct page* array in the balloon driver and fill it with pages when those pages are retrieved: static struct page *page_list[ARRAY_SIZE(frame_list)]; which will take additional 8KiB of space on 64-bit platform, but simplify things a lot. 3. Allocate struct page *page_list[ARRAY_SIZE(frame_list)] dynamically

As to Boris' suggestion "balloon pages are strung on the lru list. Can we keep have balloon_retrieve return a list of pages on that list?" Because of alloc_xenballooned_pages' retry logic for page retireval, e.g.     while (pgno < nr_pages) {         page = balloon_retrieve(true);         if (page) { [...]         } else {             ret = add_ballooned_pages(nr_pages - pgno); [...]     } I wouldn't change things that much.

IMO, we can keep 1 page based API with the only overhead for balloon driver of function calls to xenmem_reservation_va_mapping_{update|reset} for each page.

I still think what I suggested is doable but we can come back to it later and keep your per-page implementation for now.

BTW, I also think you can further simplify xenmem_reservation_va_mapping_* routines by bailing out right away if xen_feature(XENFEAT_auto_translated_physmap). In fact, you might even make them inlines, along the lines of

inline void xenmem_reservation_va_mapping_reset(unsigned long count, struct page **pages) { #ifdef CONFIG_XEN_HAVE_PVMMU if (!xen_feature(XENFEAT_auto_translated_physmap)) __xenmem_reservation_va_mapping_reset(...) #endif }

How about:

#ifdef CONFIG_XEN_HAVE_PVMMU static inline __xenmem_reservation_va_mapping_reset(struct page *page) { [...] } #endif

and

void xenmem_reservation_va_mapping_reset(unsigned long count,                      struct page **pages) { #ifdef CONFIG_XEN_HAVE_PVMMU     if (!xen_feature(XENFEAT_auto_translated_physmap)) {         int i;

for (i = 0; i < count; i++)             __xenmem_reservation_va_mapping_reset(pages[i]);     } #endif }

This way I can use __xenmem_reservation_va_mapping_reset(page); instead of xenmem_reservation_va_mapping_reset(1, &page);

Or some such.

-boris

-boris

On 25/05/18 17:33, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

Memory {increase|decrease}_reservation and VA mappings update/reset code used in balloon driver can be made common, so other drivers can also re-use the same functionality without open-coding. Create a dedicated module for the shared code and export corresponding symbols for other kernel modules.

Signed-off-by: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

drivers/xen/Makefile | 1 + drivers/xen/balloon.c | 71 ++---------------- drivers/xen/mem-reservation.c | 134 ++++++++++++++++++++++++++++++++++ include/xen/mem_reservation.h | 29 ++++++++ 4 files changed, 170 insertions(+), 65 deletions(-) create mode 100644 drivers/xen/mem-reservation.c create mode 100644 include/xen/mem_reservation.h

Can you please name this include/xen/mem-reservation.h ?

diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index 451e833f5931..3c87b0c3aca6 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o obj-$(CONFIG_X86) += fallback.o obj-y += grant-table.o features.o balloon.o manage.o preempt.o time.o +obj-y += mem-reservation.o obj-y += events/ obj-y += xenbus/

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c index 065f0b607373..57b482d67a3a 100644 --- a/drivers/xen/balloon.c +++ b/drivers/xen/balloon.c @@ -71,6 +71,7 @@ #include <xen/balloon.h> #include <xen/features.h> #include <xen/page.h> +#include <xen/mem_reservation.h>

static int xen_hotplug_unpopulated;

@@ -157,13 +158,6 @@ static DECLARE_DELAYED_WORK(balloon_worker, balloon_process); #define GFP_BALLOON \ (GFP_HIGHUSER | __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC)

-static void scrub_page(struct page *page) -{ -#ifdef CONFIG_XEN_SCRUB_PAGES

• clear_highpage(page);

-#endif -}

/* balloon_append: add the given page to the balloon. */ static void __balloon_append(struct page *page) { @@ -463,11 +457,6 @@ static enum bp_state increase_reservation(unsigned long nr_pages) int rc; unsigned long i; struct page *page;

• struct xen_memory_reservation reservation = {

• .address_bits = 0,
  

• .extent_order = EXTENT_ORDER,
  

• .domid        = DOMID_SELF
  

• };

  if (nr_pages > ARRAY_SIZE(frame_list)) nr_pages = ARRAY_SIZE(frame_list);

@@ -486,9 +475,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages) page = balloon_next_page(page); }

• set_xen_guest_handle(reservation.extent_start, frame_list);
• reservation.nr_extents = nr_pages;
• rc = HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation);

• rc = xenmem_reservation_increase(nr_pages, frame_list); if (rc <= 0) return BP_EAGAIN;

@@ -496,29 +483,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages) page = balloon_retrieve(false); BUG_ON(page == NULL);

-#ifdef CONFIG_XEN_HAVE_PVMMU

• /*
  

•  * We don't support PV MMU when Linux and Xen is using
  

•  * different page granularity.
  

•  */
  

• BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
  

• if (!xen_feature(XENFEAT_auto_translated_physmap)) {
  

• 	unsigned long pfn = page_to_pfn(page);
  

• 	set_phys_to_machine(pfn, frame_list[i]);
  

• 	/* Link back into the page tables if not highmem. */
  

• 	if (!PageHighMem(page)) {
  

• 		int ret;
  

• 		ret = HYPERVISOR_update_va_mapping(
  

• 				(unsigned long)__va(pfn << PAGE_SHIFT),
  

• 				mfn_pte(frame_list[i], PAGE_KERNEL),
  

• 				0);
  

• 		BUG_ON(ret);
  

• 	}
  

• }
  

-#endif

• xenmem_reservation_va_mapping_update(1, &page, &frame_list[i]);
  
  

  /* Relinquish the page back to the allocator. */ free_reserved_page(page);

@@ -535,11 +500,6 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) unsigned long i; struct page *page, *tmp; int ret;

• struct xen_memory_reservation reservation = {

• .address_bits = 0,
  

• .extent_order = EXTENT_ORDER,
  

• .domid        = DOMID_SELF
  

• }; LIST_HEAD(pages);

  if (nr_pages > ARRAY_SIZE(frame_list))

@@ -553,7 +513,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) break; } adjust_managed_page_count(page, -1);

• scrub_page(page);
  

• xenmem_reservation_scrub_page(page);
  

  list_add(&page->lru, &pages); }

@@ -575,25 +535,8 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) /* XENMEM_decrease_reservation requires a GFN */ frame_list[i++] = xen_page_to_gfn(page);

-#ifdef CONFIG_XEN_HAVE_PVMMU

• /*
  

•  * We don't support PV MMU when Linux and Xen is using
  

•  * different page granularity.
  

•  */
  

• BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
  

• if (!xen_feature(XENFEAT_auto_translated_physmap)) {
  

• 	unsigned long pfn = page_to_pfn(page);
  

• xenmem_reservation_va_mapping_reset(1, &page);
  
  

• 	if (!PageHighMem(page)) {
  

• 		ret = HYPERVISOR_update_va_mapping(
  

• 				(unsigned long)__va(pfn << PAGE_SHIFT),
  

• 				__pte_ma(0), 0);
  

• 		BUG_ON(ret);
  

• 	}
  

• 	__set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
  

• }
  

-#endif list_del(&page->lru);

balloon_append(page);

@@ -601,9 +544,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)

flush_tlb_all();

• set_xen_guest_handle(reservation.extent_start, frame_list);
• reservation.nr_extents = nr_pages;
• ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation);

• ret = xenmem_reservation_decrease(nr_pages, frame_list); BUG_ON(ret != nr_pages);

  balloon_stats.current_pages -= nr_pages;

diff --git a/drivers/xen/mem-reservation.c b/drivers/xen/mem-reservation.c new file mode 100644 index 000000000000..29882e4324f5 --- /dev/null +++ b/drivers/xen/mem-reservation.c @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT

+/******************************************************************************

• • Xen memory reservation utilities.
• • Copyright (c) 2003, B Dragovic
• • Copyright (c) 2003-2004, M Williamson, K Fraser
• • Copyright (c) 2005 Dan M. Smith, IBM Corporation
• • Copyright (c) 2010 Daniel Kiper
• • Copyright (c) 2018, Oleksandr Andrushchenko, EPAM Systems Inc.
• */

+#include <linux/kernel.h> +#include <linux/slab.h>

+#include <asm/tlb.h> +#include <asm/xen/hypercall.h>

+#include <xen/interface/memory.h> +#include <xen/page.h>

+/*

• • Use one extent per PAGE_SIZE to avoid to break down the page into
• • multiple frame.
• */

+#define EXTENT_ORDER (fls(XEN_PFN_PER_PAGE) - 1)

+void xenmem_reservation_scrub_page(struct page *page) +{ +#ifdef CONFIG_XEN_SCRUB_PAGES

• clear_highpage(page);

+#endif +} +EXPORT_SYMBOL(xenmem_reservation_scrub_page);

EXPORT_SYMBOL_GPL()

Multiple times below, too.

As a general rule of thumb: new exports should all be EXPORT_SYMBOL_GPL() if you can't give a reason why they shouldn't be.

Juergen

On 05/29/2018 10:10 PM, Boris Ostrovsky wrote:

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:

...

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

Extend grant table module API to allow allocating buffers that can be used for DMA operations and mapping foreign grant references on top of those. The resulting buffer is similar to the one allocated by the balloon driver in terms that proper memory reservation is made ({increase|decrease}_reservation and VA mappings updated if needed). This is useful for sharing foreign buffers with HW drivers which cannot work with scattered buffers provided by the balloon driver, but require DMAable memory instead.

Signed-off-by: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

drivers/xen/Kconfig | 13 ++++ drivers/xen/grant-table.c | 124 ++++++++++++++++++++++++++++++++++++++ include/xen/grant_table.h | 25 ++++++++ 3 files changed, 162 insertions(+)

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index e5d0c28372ea..3431fe210624 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -161,6 +161,19 @@ config XEN_GRANT_DEV_ALLOC to other domains. This can be used to implement frontend drivers or as part of an inter-domain shared memory channel.

+config XEN_GRANT_DMA_ALLOC

• bool "Allow allocating DMA capable buffers with grant reference module"
• depends on XEN

Should it depend on anything from DMA? CONFIG_HAS_DMA for example?

Yes, it must be "depends on XEN && HAS_DMA", thank you

...

• help

•  Extends grant table module API to allow allocating DMA capable
  

•  buffers and mapping foreign grant references on top of it.
  

•  The resulting buffer is similar to one allocated by the balloon
  

•  driver in terms that proper memory reservation is made
  

•  ({increase|decrease}_reservation and VA mappings updated if needed).
  

•  This is useful for sharing foreign buffers with HW drivers which
  

•  cannot work with scattered buffers provided by the balloon driver,
  

•  but require DMAable memory instead.
  

• config SWIOTLB_XEN def_bool y select SWIOTLB

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c index d7488226e1f2..06fe6e7f639c 100644 --- a/drivers/xen/grant-table.c +++ b/drivers/xen/grant-table.c @@ -45,6 +45,9 @@ #include <linux/workqueue.h> #include <linux/ratelimit.h> #include <linux/moduleparam.h> +#ifdef CONFIG_XEN_GRANT_DMA_ALLOC +#include <linux/dma-mapping.h> +#endif

#include <xen/xen.h> #include <xen/interface/xen.h> @@ -57,6 +60,7 @@ #ifdef CONFIG_X86 #include <asm/xen/cpuid.h> #endif +#include <xen/mem_reservation.h> #include <asm/xen/hypercall.h> #include <asm/xen/interface.h>

@@ -811,6 +815,82 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages) } EXPORT_SYMBOL(gnttab_alloc_pages);

+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC +/**

• • gnttab_dma_alloc_pages - alloc DMAable pages suitable for grant mapping into
• • @args: arguments to the function
• */

+int gnttab_dma_alloc_pages(struct gnttab_dma_alloc_args *args) +{

• unsigned long pfn, start_pfn;
• xen_pfn_t *frames;
• size_t size;
• int i, ret;
• frames = kcalloc(args->nr_pages, sizeof(*frames), GFP_KERNEL);
• if (!frames)

• return -ENOMEM;
  

• size = args->nr_pages << PAGE_SHIFT;
• if (args->coherent)

• args->vaddr = dma_alloc_coherent(args->dev, size,
  

• 				 &args->dev_bus_addr,
  

• 				 GFP_KERNEL | __GFP_NOWARN);
  

• else

• args->vaddr = dma_alloc_wc(args->dev, size,
  

• 			   &args->dev_bus_addr,
  

• 			   GFP_KERNEL | __GFP_NOWARN);
  

• if (!args->vaddr) {

• pr_err("Failed to allocate DMA buffer of size %zu\n", size);
  

• ret = -ENOMEM;
  

• goto fail_free_frames;
  

• }
• start_pfn = __phys_to_pfn(args->dev_bus_addr);
• for (pfn = start_pfn, i = 0; pfn < start_pfn + args->nr_pages;

• 	pfn++, i++) {
  

• struct page *page = pfn_to_page(pfn);
  

• args->pages[i] = page;
  

• frames[i] = xen_page_to_gfn(page);
  

• xenmem_reservation_scrub_page(page);
  

• }
• xenmem_reservation_va_mapping_reset(args->nr_pages, args->pages);
• ret = xenmem_reservation_decrease(args->nr_pages, frames);
• if (ret != args->nr_pages) {

• pr_err("Failed to decrease reservation for DMA buffer\n");
  

• xenmem_reservation_increase(ret, frames);
  

• ret = -EFAULT;
  

• goto fail_free_dma;
  

• }
• ret = gnttab_pages_set_private(args->nr_pages, args->pages);
• if (ret < 0)

• goto fail_clear_private;
  

• kfree(frames);
• return 0;

+fail_clear_private:

• gnttab_pages_clear_private(args->nr_pages, args->pages);

+fail_free_dma:

Do you need to xenmem_reservation_increase()?

Yes, missed that on fail_clear_private error path, will fix

...

• xenmem_reservation_va_mapping_update(args->nr_pages, args->pages,

• 			     frames);
  

• if (args->coherent)

• dma_free_coherent(args->dev, size,
  

• 		  args->vaddr, args->dev_bus_addr);
  

• else

• dma_free_wc(args->dev, size,
  

• 	    args->vaddr, args->dev_bus_addr);
  

+fail_free_frames:

• kfree(frames);
• return ret;

+} +EXPORT_SYMBOL(gnttab_dma_alloc_pages); +#endif

• void gnttab_pages_clear_private(int nr_pages, struct page **pages) { int i;

@@ -838,6 +918,50 @@ void gnttab_free_pages(int nr_pages, struct page **pages) } EXPORT_SYMBOL(gnttab_free_pages);

+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC +/**

• • gnttab_dma_free_pages - free DMAable pages
• • @args: arguments to the function
• */

+int gnttab_dma_free_pages(struct gnttab_dma_alloc_args *args) +{

• xen_pfn_t *frames;
• size_t size;
• int i, ret;
• gnttab_pages_clear_private(args->nr_pages, args->pages);
• frames = kcalloc(args->nr_pages, sizeof(*frames), GFP_KERNEL);

Any way you can do it without allocating memory? One possibility is to keep allocated frames from gnttab_dma_alloc_pages(). (Not sure I like that either but it's the only thing I can think of).

Yes, I was also thinking about storing the allocated frames array from gnttab_dma_alloc_pages(), but that seemed not to be clear enough as the caller of the gnttab_dma_alloc_pages will need to store those frames in some context, so we can pass them on free. But the caller doesn't really need the frames which might confuse, so I decided to make those allocations on the fly. But I can still rework that to store the frames if you insist: please let me know.

...

• if (!frames)

• return -ENOMEM;
  

• for (i = 0; i < args->nr_pages; i++)

• frames[i] = page_to_xen_pfn(args->pages[i]);
  

Not xen_page_to_gfn()?

Well, according to [1] it should be :     /* XENMEM_populate_physmap requires a PFN based on Xen      * granularity.      */     frame_list[i] = page_to_xen_pfn(page);

...

• ret = xenmem_reservation_increase(args->nr_pages, frames);
• if (ret != args->nr_pages) {

• pr_err("Failed to decrease reservation for DMA buffer\n");
  

• ret = -EFAULT;
  

• } else {

• ret = 0;
  

• }
• xenmem_reservation_va_mapping_update(args->nr_pages, args->pages,

• 			     frames);
  

• size = args->nr_pages << PAGE_SHIFT;
• if (args->coherent)

• dma_free_coherent(args->dev, size,
  

• 		  args->vaddr, args->dev_bus_addr);
  

• else

• dma_free_wc(args->dev, size,
  

• 	    args->vaddr, args->dev_bus_addr);
  

• kfree(frames);
• return ret;

+} +EXPORT_SYMBOL(gnttab_dma_free_pages); +#endif

• /* Handling of paged out grant targets (GNTST_eagain) */ #define MAX_DELAY 256 static inline void

diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h index de03f2542bb7..982e34242b9c 100644 --- a/include/xen/grant_table.h +++ b/include/xen/grant_table.h @@ -198,6 +198,31 @@ void gnttab_free_auto_xlat_frames(void); int gnttab_alloc_pages(int nr_pages, struct page **pages); void gnttab_free_pages(int nr_pages, struct page **pages);

+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC +struct gnttab_dma_alloc_args {

• /* Device for which DMA memory will be/was allocated. */
• struct device *dev;
• /*

• * If set then DMA buffer is coherent and write-combine otherwise.
  

• */
  

Single-line comment

will fix

...

• bool coherent;
• /*

• * Number of entries in the @pages array, defines the size
  

• * of the DMA buffer.
  

• */
  

This can be made into single line comment as well. However, I am not sure this comment as well as those below are necessary. Fields names are self-describing IMO.

will remove

-boris

Thank you, Oleksandr

...

• int nr_pages;
• /* Array of pages @pages filled with pages of the DMA buffer. */
• struct page **pages;
• /* Virtual/CPU address of the DMA buffer. */
• void *vaddr;
• /* Bus address of the DMA buffer. */
• dma_addr_t dev_bus_addr;

+};

+int gnttab_dma_alloc_pages(struct gnttab_dma_alloc_args *args); +int gnttab_dma_free_pages(struct gnttab_dma_alloc_args *args); +#endif

• int gnttab_pages_set_private(int nr_pages, struct page **pages); void gnttab_pages_clear_private(int nr_pages, struct page **pages);

[1] https://elixir.bootlin.com/linux/v4.17-rc7/source/drivers/xen/balloon.c#L485

On 05/30/2018 12:52 AM, Boris Ostrovsky wrote:

On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:

...

struct unmap_notify { @@ -96,10 +104,28 @@ struct grant_map { struct gnttab_unmap_grant_ref *kunmap_ops; struct page **pages; unsigned long pages_vm_start;

+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC

• /*

• * If dmabuf_vaddr is not NULL then this mapping is backed by DMA
  

• * capable memory.
  

• */
  

• /* Device for which DMA memory is allocated. */
• struct device *dma_dev;
• /* Flags used to create this DMA buffer: GNTDEV_DMABUF_FLAG_XXX. */
• bool dma_flags;

Again, I think most of the comments here can be dropped. Except possibly for the flags.

will drop most of those

...

• /* Virtual/CPU address of the DMA buffer. */
• void *dma_vaddr;
• /* Bus address of the DMA buffer. */
• dma_addr_t dma_bus_addr;

+#endif };

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,

@@ -121,8 +147,26 @@ static void gntdev_free_map(struct grant_map *map) if (map == NULL) return;

+#ifdef CONFIG_XEN_GRANT_DMA_ALLOC

• if (map->dma_vaddr) {

• struct gnttab_dma_alloc_args args;
  

• args.dev = map->dma_dev;
  

• args.coherent = map->dma_flags & GNTDEV_DMA_FLAG_COHERENT;
  

• args.nr_pages = map->count;
  

• args.pages = map->pages;
  

• args.vaddr = map->dma_vaddr;
  

• args.dev_bus_addr = map->dma_bus_addr;
  

• gnttab_dma_free_pages(&args);
  

• } else if (map->pages) {

• gnttab_free_pages(map->count, map->pages);
  

• }

+#else if (map->pages) gnttab_free_pages(map->count, map->pages); +#endif

} else #endif     if (map->pages)         gnttab_free_pages(map->count, map->pages);

(and elsewhere)

ok, just wasn't sure if it is ok to violate kernel coding style here ;)

...

kfree(map->pages); kfree(map->grants); kfree(map->map_ops);

...

diff --git a/include/uapi/xen/gntdev.h b/include/uapi/xen/gntdev.h index 6d1163456c03..2d5a4672f07c 100644 --- a/include/uapi/xen/gntdev.h +++ b/include/uapi/xen/gntdev.h @@ -200,4 +200,19 @@ struct ioctl_gntdev_grant_copy { /* Send an interrupt on the indicated event channel */ #define UNMAP_NOTIFY_SEND_EVENT 0x2

+/*

• • Flags to be used while requesting memory mapping's backing storage
• • to be allocated with DMA API.
• */

+/*

• • The buffer is backed with memory allocated with dma_alloc_wc.
• */

+#define GNTDEV_DMA_FLAG_WC (1 << 1)

Is there a reason you are not using bit 0?

No reason for that, will start from 0

-boris

Thank you, Oleksandr

...

+/*

• • The buffer is backed with memory allocated with dma_alloc_coherent.
• */

+#define GNTDEV_DMA_FLAG_COHERENT (1 << 2)

• #endif /* __LINUX_PUBLIC_GNTDEV_H__ */

On 05/25/2018 06:33 PM, Oleksandr Andrushchenko wrote:

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index 3431fe210624..eaf63a2c7ae6 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -152,6 +152,16 @@ config XEN_GNTDEV help Allows userspace processes to use grants.

+config XEN_GNTDEV_DMABUF

• bool "Add support for dma-buf grant access device driver extension"
• depends on XEN_GNTDEV && XEN_GRANT_DMA_ALLOC

This must be "depends on XEN_GNTDEV && XEN_GRANT_DMA_ALLOC && *DMA_SHARED_BUFFER*"

• help

•  Allows userspace processes and kernel modules to use Xen backed
  

•  dma-buf implementation. With this extension grant references to
  

•  the pages of an imported dma-buf can be exported for other domain
  

•  use and grant references coming from a foreign domain can be
  

•  converted into a local dma-buf for local export.
  

On Fri, May 25, 2018 at 06:33:29PM +0300, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

1. Create a dma-buf from grant references provided by the foreign domain. By default dma-buf is backed by system memory pages, but by providing GNTDEV_DMA_FLAG_XXX flags it can also be created as a DMA write-combine/coherent buffer, e.g. allocated with corresponding dma_alloc_xxx API. Export the resulting buffer as a new dma-buf.

2. Implement waiting for the dma-buf to be released: block until the dma-buf with the file descriptor provided is released. If within the time-out provided the buffer is not released then -ETIMEDOUT error is returned. If the buffer with the file descriptor does not exist or has already been released, then -ENOENT is returned. For valid file descriptors this must not be treated as error.

Signed-off-by: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

drivers/xen/gntdev.c | 478 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 476 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 9e450622af1a..52abc6cd5846 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 <kraxel@redhat.com>
    

  •       (c) 2018 Oleksandr Andrushchenko, EPAM Systems Inc.
    

@@ -41,6 +43,9 @@ #ifdef CONFIG_XEN_GRANT_DMA_ALLOC #include <linux/of_device.h> #endif +#ifdef CONFIG_XEN_GNTDEV_DMABUF +#include <linux/dma-buf.h> +#endif

#include <xen/xen.h> #include <xen/grant_table.h> @@ -81,6 +86,17 @@ struct gntdev_priv { /* Device for which DMA memory is allocated. */ struct device *dma_dev; #endif

+#ifdef CONFIG_XEN_GNTDEV_DMABUF

• /* Private data of the hyper DMA buffers. */
• /* List of exported DMA buffers. */
• struct list_head dmabuf_exp_list;
• /* List of wait objects. */
• struct list_head dmabuf_exp_wait_list;
• /* This is the lock which protects dma_buf_xxx lists. */
• struct mutex dmabuf_lock;

+#endif };

struct unmap_notify { @@ -125,12 +141,38 @@ struct grant_map {

#ifdef CONFIG_XEN_GNTDEV_DMABUF struct xen_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; } imp; } u;
• /* Number of pages this buffer has. */
• int nr_pages;
• /* Pages of this buffer. */
• struct page **pages;

+};

+struct xen_dmabuf_wait_obj {

• struct list_head next;
• struct xen_dmabuf *xen_dmabuf;
• struct completion completion;

+};

+struct xen_dmabuf_attachment {

• struct sg_table *sgt;
• enum dma_data_direction dir;

}; #endif

@@ -320,6 +362,16 @@ static void gntdev_put_map(struct gntdev_priv *priv, struct grant_map *map) gntdev_free_map(map); }

+#ifdef CONFIG_XEN_GNTDEV_DMABUF +static void gntdev_remove_map(struct gntdev_priv *priv, struct grant_map *map) +{

• mutex_lock(&priv->lock);
• list_del(&map->next);
• gntdev_put_map(NULL /* already removed */, map);
• mutex_unlock(&priv->lock);

+} +#endif

/* ------------------------------------------------------------------ */

static int find_grant_ptes(pte_t *pte, pgtable_t token, @@ -628,6 +680,12 @@ static int gntdev_open(struct inode *inode, struct file *flip) INIT_LIST_HEAD(&priv->freeable_maps); mutex_init(&priv->lock);

+#ifdef CONFIG_XEN_GNTDEV_DMABUF

• mutex_init(&priv->dmabuf_lock);
• INIT_LIST_HEAD(&priv->dmabuf_exp_list);
• INIT_LIST_HEAD(&priv->dmabuf_exp_wait_list);

+#endif

• if (use_ptemod) { priv->mm = get_task_mm(current); if (!priv->mm) {

@@ -1053,17 +1111,433 @@ static long gntdev_ioctl_grant_copy(struct gntdev_priv *priv, void __user *u) /* DMA buffer export support. */ /* ------------------------------------------------------------------ */

+/* ------------------------------------------------------------------ */ +/* Implementation of wait for exported DMA buffer to be released. */ +/* ------------------------------------------------------------------ */

+static void dmabuf_exp_release(struct kref *kref);

+static struct xen_dmabuf_wait_obj * +dmabuf_exp_wait_obj_new(struct gntdev_priv *priv,

• 	struct xen_dmabuf *xen_dmabuf)
  

+{

• struct xen_dmabuf_wait_obj *obj;
• obj = kzalloc(sizeof(*obj), GFP_KERNEL);
• if (!obj)

• return ERR_PTR(-ENOMEM);
  

• init_completion(&obj->completion);
• obj->xen_dmabuf = xen_dmabuf;
• mutex_lock(&priv->dmabuf_lock);
• list_add(&obj->next, &priv->dmabuf_exp_wait_list);
• /* Put our reference and wait for xen_dmabuf's release to fire. */
• kref_put(&xen_dmabuf->u.exp.refcount, dmabuf_exp_release);
• mutex_unlock(&priv->dmabuf_lock);
• return obj;

+}

+static void dmabuf_exp_wait_obj_free(struct gntdev_priv *priv,

• 		     struct xen_dmabuf_wait_obj *obj)
  

+{

• struct xen_dmabuf_wait_obj *cur_obj, *q;
• mutex_lock(&priv->dmabuf_lock);
• list_for_each_entry_safe(cur_obj, q, &priv->dmabuf_exp_wait_list, next)

• if (cur_obj == obj) {
  

• 	list_del(&obj->next);
  

• 	kfree(obj);
  

• 	break;
  

• }
  

• mutex_unlock(&priv->dmabuf_lock);

+}

+static int dmabuf_exp_wait_obj_wait(struct xen_dmabuf_wait_obj *obj,

• 		    u32 wait_to_ms)
  

+{

• if (wait_for_completion_timeout(&obj->completion,

• 	msecs_to_jiffies(wait_to_ms)) <= 0)
  

• return -ETIMEDOUT;
  

• return 0;

+}

+static void dmabuf_exp_wait_obj_signal(struct gntdev_priv *priv,

• 		       struct xen_dmabuf *xen_dmabuf)
  

+{

• struct xen_dmabuf_wait_obj *obj, *q;
• list_for_each_entry_safe(obj, q, &priv->dmabuf_exp_wait_list, next)

• if (obj->xen_dmabuf == xen_dmabuf) {
  

• 	pr_debug("Found xen_dmabuf in the wait list, wake\n");
  

• 	complete_all(&obj->completion);
  

• }
  

+}

+static struct xen_dmabuf * +dmabuf_exp_wait_obj_get_by_fd(struct gntdev_priv *priv, int fd) +{

• struct xen_dmabuf *q, *xen_dmabuf, *ret = ERR_PTR(-ENOENT);
• mutex_lock(&priv->dmabuf_lock);
• list_for_each_entry_safe(xen_dmabuf, q, &priv->dmabuf_exp_list, next)

• if (xen_dmabuf->fd == fd) {
  

• 	pr_debug("Found xen_dmabuf in the wait list\n");
  

• 	kref_get(&xen_dmabuf->u.exp.refcount);
  

• 	ret = xen_dmabuf;
  

• 	break;
  

• }
  

• mutex_unlock(&priv->dmabuf_lock);
• return ret;

+}

static int dmabuf_exp_wait_released(struct gntdev_priv *priv, int fd, int wait_to_ms) {

• return -ETIMEDOUT;

• struct xen_dmabuf *xen_dmabuf;
• struct xen_dmabuf_wait_obj *obj;
• int ret;
• pr_debug("Will wait for dma-buf with fd %d\n", fd);
• /*

• * Try to find the DMA buffer: if not found means that
  

• * either the buffer has already been released or file descriptor
  

• * provided is wrong.
  

• */
  

• xen_dmabuf = dmabuf_exp_wait_obj_get_by_fd(priv, fd);
• if (IS_ERR(xen_dmabuf))

• return PTR_ERR(xen_dmabuf);
  

• /*

• * xen_dmabuf still exists and is reference count locked by us now,
  

• * so prepare to wait: allocate wait object and add it to the wait list,
  

• * so we can find it on release.
  

• */
  

• obj = dmabuf_exp_wait_obj_new(priv, xen_dmabuf);
• if (IS_ERR(obj)) {

• pr_err("Failed to setup wait object, ret %ld\n", PTR_ERR(obj));
  

• return PTR_ERR(obj);
  

• }
• ret = dmabuf_exp_wait_obj_wait(obj, wait_to_ms);
• dmabuf_exp_wait_obj_free(priv, obj);
• return ret;

+}

+/* ------------------------------------------------------------------ */ +/* DMA buffer export support. */ +/* ------------------------------------------------------------------ */

+static struct sg_table * +dmabuf_pages_to_sgt(struct page **pages, unsigned int nr_pages) +{

• struct sg_table *sgt;
• int ret;
• sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
• if (!sgt) {

• ret = -ENOMEM;
  

• goto out;
  

• }
• ret = sg_alloc_table_from_pages(sgt, pages, nr_pages, 0,

• 			nr_pages << PAGE_SHIFT,
  

• 			GFP_KERNEL);
  

• if (ret)

• goto out;
  

• return sgt;

+out:

• kfree(sgt);
• return ERR_PTR(ret);

+}

+static int dmabuf_exp_ops_attach(struct dma_buf *dma_buf,

• 		 struct device *target_dev,
  

• 		 struct dma_buf_attachment *attach)
  

+{

• struct xen_dmabuf_attachment *xen_dmabuf_attach;
• xen_dmabuf_attach = kzalloc(sizeof(*xen_dmabuf_attach), GFP_KERNEL);
• if (!xen_dmabuf_attach)

• return -ENOMEM;
  

• xen_dmabuf_attach->dir = DMA_NONE;
• attach->priv = xen_dmabuf_attach;
• /* Might need to pin the pages of the buffer now. */
• return 0;

+}

+static void dmabuf_exp_ops_detach(struct dma_buf *dma_buf,

• 		  struct dma_buf_attachment *attach)
  

+{

• struct xen_dmabuf_attachment *xen_dmabuf_attach = attach->priv;
• if (xen_dmabuf_attach) {

• struct sg_table *sgt = xen_dmabuf_attach->sgt;
  

• if (sgt) {
  

• 	if (xen_dmabuf_attach->dir != DMA_NONE)
  

• 		dma_unmap_sg_attrs(attach->dev, sgt->sgl,
  

• 				   sgt->nents,
  

• 				   xen_dmabuf_attach->dir,
  

• 				   DMA_ATTR_SKIP_CPU_SYNC);
  

• 	sg_free_table(sgt);
  

• }
  

• kfree(sgt);
  

• kfree(xen_dmabuf_attach);
  

• attach->priv = NULL;
  

• }
• /* Might need to unpin the pages of the buffer now. */

+}

+static struct sg_table * +dmabuf_exp_ops_map_dma_buf(struct dma_buf_attachment *attach,

• 	   enum dma_data_direction dir)
  

+{

• struct xen_dmabuf_attachment *xen_dmabuf_attach = attach->priv;
• struct xen_dmabuf *xen_dmabuf = attach->dmabuf->priv;
• struct sg_table *sgt;
• pr_debug("Mapping %d pages for dev %p\n", xen_dmabuf->nr_pages,

•  attach->dev);
  

• if (WARN_ON(dir == DMA_NONE || !xen_dmabuf_attach))

• return ERR_PTR(-EINVAL);
  

• /* Return the cached mapping when possible. */
• if (xen_dmabuf_attach->dir == dir)

• return xen_dmabuf_attach->sgt;
  

may need to check xen_dmabuf_attach->sgt == NULL (i.e. first time mapping)? Also, I am not sure if this mechanism of reusing previously generated sgt for other mappings is universally ok for any use-cases... I don't know if it is acceptable as per the specification.

• /*

• * Two mappings with different directions for the same attachment are
  

• * not allowed.
  

• */
  

• if (WARN_ON(xen_dmabuf_attach->dir != DMA_NONE))

• return ERR_PTR(-EBUSY);
  

• sgt = dmabuf_pages_to_sgt(xen_dmabuf->pages, xen_dmabuf->nr_pages);
• if (!IS_ERR(sgt)) {

• if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
  

• 		      DMA_ATTR_SKIP_CPU_SYNC)) {
  

• 	sg_free_table(sgt);
  

• 	kfree(sgt);
  

• 	sgt = ERR_PTR(-ENOMEM);
  

• } else {
  

• 	xen_dmabuf_attach->sgt = sgt;
  

• 	xen_dmabuf_attach->dir = dir;
  

• }
  

• }
• if (IS_ERR(sgt))

• pr_err("Failed to map sg table for dev %p\n", attach->dev);
  

• return sgt;

+}

+static void dmabuf_exp_ops_unmap_dma_buf(struct dma_buf_attachment *attach,

• 			 struct sg_table *sgt,
  

• 			 enum dma_data_direction dir)
  

+{

• /* Not implemented. The unmap is done at dmabuf_exp_ops_detach(). */

Not sure if it's ok to do nothing here because the spec says this function is mandatory and it should unmap and "release" &sg_table associated with it.

/** * @unmap_dma_buf: * * This is called by dma_buf_unmap_attachment() and should unmap and * release the &sg_table allocated in @map_dma_buf, and it is mandatory. * It should also unpin the backing storage if this is the last mapping * of the DMA buffer, it the exporter supports backing storage * migration. */

+}

+static void dmabuf_exp_release(struct kref *kref) +{

• struct xen_dmabuf *xen_dmabuf =

• container_of(kref, struct xen_dmabuf, u.exp.refcount);
  

• dmabuf_exp_wait_obj_signal(xen_dmabuf->priv, xen_dmabuf);
• list_del(&xen_dmabuf->next);
• kfree(xen_dmabuf);

+}

+static void dmabuf_exp_ops_release(struct dma_buf *dma_buf) +{

• struct xen_dmabuf *xen_dmabuf = dma_buf->priv;
• struct gntdev_priv *priv = xen_dmabuf->priv;
• gntdev_remove_map(priv, xen_dmabuf->u.exp.map);
• mutex_lock(&priv->dmabuf_lock);
• kref_put(&xen_dmabuf->u.exp.refcount, dmabuf_exp_release);
• mutex_unlock(&priv->dmabuf_lock);

+}

+static void *dmabuf_exp_ops_kmap_atomic(struct dma_buf *dma_buf,

• 			unsigned long page_num)
  

+{

• /* Not implemented. */
• return NULL;

+}

+static void dmabuf_exp_ops_kunmap_atomic(struct dma_buf *dma_buf,

• 			 unsigned long page_num, void *addr)
  

+{

• /* Not implemented. */

+}

+static void *dmabuf_exp_ops_kmap(struct dma_buf *dma_buf,

• 		 unsigned long page_num)
  

+{

• /* Not implemented. */
• return NULL;

+}

+static void dmabuf_exp_ops_kunmap(struct dma_buf *dma_buf,

• 		  unsigned long page_num, void *addr)
  

+{

• /* Not implemented. */

+}

+static int dmabuf_exp_ops_mmap(struct dma_buf *dma_buf,

• 	       struct vm_area_struct *vma)
  

+{

• /* Not implemented. */
• return 0;

+}

+static const struct dma_buf_ops dmabuf_exp_ops = {

• .attach = dmabuf_exp_ops_attach,
• .detach = dmabuf_exp_ops_detach,
• .map_dma_buf = dmabuf_exp_ops_map_dma_buf,
• .unmap_dma_buf = dmabuf_exp_ops_unmap_dma_buf,
• .release = dmabuf_exp_ops_release,
• .map = dmabuf_exp_ops_kmap,
• .map_atomic = dmabuf_exp_ops_kmap_atomic,
• .unmap = dmabuf_exp_ops_kunmap,
• .unmap_atomic = dmabuf_exp_ops_kunmap_atomic,
• .mmap = dmabuf_exp_ops_mmap,

+};

+static int dmabuf_export(struct gntdev_priv *priv, struct grant_map *map,

• 	 int *fd)
  

+{

• DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
• struct xen_dmabuf *xen_dmabuf;
• int ret = 0;
• xen_dmabuf = kzalloc(sizeof(*xen_dmabuf), GFP_KERNEL);
• if (!xen_dmabuf)

• return -ENOMEM;
  

• kref_init(&xen_dmabuf->u.exp.refcount);
• xen_dmabuf->priv = priv;
• xen_dmabuf->nr_pages = map->count;
• xen_dmabuf->pages = map->pages;
• xen_dmabuf->u.exp.map = map;
• exp_info.exp_name = KBUILD_MODNAME;
• if (map->dma_dev->driver && map->dma_dev->driver->owner)

• exp_info.owner = map->dma_dev->driver->owner;
  

• else

• exp_info.owner = THIS_MODULE;
  

• exp_info.ops = &dmabuf_exp_ops;
• exp_info.size = map->count << PAGE_SHIFT;
• exp_info.flags = O_RDWR;
• exp_info.priv = xen_dmabuf;
• xen_dmabuf->dmabuf = dma_buf_export(&exp_info);
• if (IS_ERR(xen_dmabuf->dmabuf)) {

• ret = PTR_ERR(xen_dmabuf->dmabuf);
  

• xen_dmabuf->dmabuf = NULL;
  

• goto fail;
  

• }
• ret = dma_buf_fd(xen_dmabuf->dmabuf, O_CLOEXEC);
• if (ret < 0)

• goto fail;
  

• xen_dmabuf->fd = ret;
• *fd = ret;
• pr_debug("Exporting DMA buffer with fd %d\n", ret);
• mutex_lock(&priv->dmabuf_lock);
• list_add(&xen_dmabuf->next, &priv->dmabuf_exp_list);
• mutex_unlock(&priv->dmabuf_lock);
• return 0;

+fail:

• if (xen_dmabuf->dmabuf)

• dma_buf_put(xen_dmabuf->dmabuf);
  

• kfree(xen_dmabuf);
• return ret;

+}

+static struct grant_map * +dmabuf_exp_alloc_backing_storage(struct gntdev_priv *priv, int dmabuf_flags,

• 		 int count)
  

+{

• struct grant_map *map;
• if (unlikely(count <= 0))

• return ERR_PTR(-EINVAL);
  

• if ((dmabuf_flags & GNTDEV_DMA_FLAG_WC) &&

•    (dmabuf_flags & GNTDEV_DMA_FLAG_COHERENT)) {
  

• pr_err("Wrong dma-buf flags: either WC or coherent, not both\n");
  

• return ERR_PTR(-EINVAL);
  

• }
• map = gntdev_alloc_map(priv, count, dmabuf_flags);
• if (!map)

• return ERR_PTR(-ENOMEM);
  

• if (unlikely(atomic_add_return(count, &pages_mapped) > limit)) {

• pr_err("can't map: over limit\n");
  

• gntdev_put_map(NULL, map);
  

• return ERR_PTR(-ENOMEM);
  

• }
• return map;

}

When and how would this allocation be freed? I don't see any ioctl for freeing up shared pages.

static int dmabuf_exp_from_refs(struct gntdev_priv *priv, int flags, int count, u32 domid, u32 *refs, u32 *fd) {

• struct grant_map *map;
• int i, ret;
• *fd = -1;

• return -EINVAL;

• if (use_ptemod) {

• pr_err("Cannot provide dma-buf: use_ptemode %d\n",
  

•        use_ptemod);
  

• return -EINVAL;
  

• }
• map = dmabuf_exp_alloc_backing_storage(priv, flags, count);
• if (IS_ERR(map))

• return PTR_ERR(map);
  

• for (i = 0; i < count; i++) {

• map->grants[i].domid = domid;
  

• map->grants[i].ref = refs[i];
  

• }
• mutex_lock(&priv->lock);
• gntdev_add_map(priv, map);
• mutex_unlock(&priv->lock);
• map->flags |= GNTMAP_host_map;

+#if defined(CONFIG_X86)

• map->flags |= GNTMAP_device_map;

+#endif

• ret = map_grant_pages(map);
• if (ret < 0)

• goto out;
  

• ret = dmabuf_export(priv, map, fd);
• if (ret < 0)

• goto out;
  

• return 0;

+out:

• gntdev_remove_map(priv, map);
• return ret;

}

/* ------------------------------------------------------------------ */

2.17.0

On 05/31/2018 08:55 AM, Oleksandr Andrushchenko wrote:

On 05/31/2018 02:10 AM, Dongwon Kim wrote:

...

On Fri, May 25, 2018 at 06:33:29PM +0300, Oleksandr Andrushchenko wrote:

...

From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com

1. Create a dma-buf from grant references provided by the foreign

domain. By default dma-buf is backed by system memory pages, but     by providing GNTDEV_DMA_FLAG_XXX flags it can also be created     as a DMA write-combine/coherent buffer, e.g. allocated with     corresponding dma_alloc_xxx API.     Export the resulting buffer as a new dma-buf.

2. Implement waiting for the dma-buf to be released: block until the

dma-buf with the file descriptor provided is released.     If within the time-out provided the buffer is not released then     -ETIMEDOUT error is returned. If the buffer with the file descriptor     does not exist or has already been released, then -ENOENT is returned.     For valid file descriptors this must not be treated as error.

Signed-off-by: Oleksandr Andrushchenko

oleksandr_andrushchenko@epam.com

drivers/xen/gntdev.c | 478 ++++++++++++++++++++++++++++++++++++++++++-   1 file changed, 476 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c index 9e450622af1a..52abc6cd5846 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 kraxel@redhat.com    *           (c) 2018 Oleksandr Andrushchenko, EPAM Systems Inc. @@ -41,6 +43,9 @@   #ifdef CONFIG_XEN_GRANT_DMA_ALLOC   #include <linux/of_device.h>   #endif +#ifdef CONFIG_XEN_GNTDEV_DMABUF +#include <linux/dma-buf.h> +#endif     #include <xen/xen.h>   #include <xen/grant_table.h> @@ -81,6 +86,17 @@ struct gntdev_priv {       /* Device for which DMA memory is allocated. */       struct device *dma_dev;   #endif

+#ifdef CONFIG_XEN_GNTDEV_DMABUF +    /* Private data of the hyper DMA buffers. */

+    /* List of exported DMA buffers. */ +    struct list_head dmabuf_exp_list; +    /* List of wait objects. */ +    struct list_head dmabuf_exp_wait_list; +    /* This is the lock which protects dma_buf_xxx lists. */ +    struct mutex dmabuf_lock; +#endif   };     struct unmap_notify { @@ -125,12 +141,38 @@ struct grant_map {     #ifdef CONFIG_XEN_GNTDEV_DMABUF   struct xen_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;           } imp;       } u;

+    /* Number of pages this buffer has. */ +    int nr_pages; +    /* Pages of this buffer. */ +    struct page **pages; +};

+struct xen_dmabuf_wait_obj { +    struct list_head next; +    struct xen_dmabuf *xen_dmabuf; +    struct completion completion; +};

+struct xen_dmabuf_attachment { +    struct sg_table *sgt; +    enum dma_data_direction dir;   };   #endif   @@ -320,6 +362,16 @@ static void gntdev_put_map(struct gntdev_priv *priv, struct grant_map *map)       gntdev_free_map(map);   }   +#ifdef CONFIG_XEN_GNTDEV_DMABUF +static void gntdev_remove_map(struct gntdev_priv *priv, struct grant_map *map) +{ +    mutex_lock(&priv->lock); +    list_del(&map->next); +    gntdev_put_map(NULL /* already removed */, map); +    mutex_unlock(&priv->lock); +} +#endif

/* ------------------------------------------------------------------ */     static int find_grant_ptes(pte_t *pte, pgtable_t token, @@ -628,6 +680,12 @@ static int gntdev_open(struct inode *inode, struct file *flip)       INIT_LIST_HEAD(&priv->freeable_maps);       mutex_init(&priv->lock);   +#ifdef CONFIG_XEN_GNTDEV_DMABUF +    mutex_init(&priv->dmabuf_lock); +    INIT_LIST_HEAD(&priv->dmabuf_exp_list); +    INIT_LIST_HEAD(&priv->dmabuf_exp_wait_list); +#endif

if (use_ptemod) {           priv->mm = get_task_mm(current);           if (!priv->mm) { @@ -1053,17 +1111,433 @@ static long gntdev_ioctl_grant_copy(struct gntdev_priv *priv, void __user *u)   /* DMA buffer export support.                                         */   /* ------------------------------------------------------------------ */   +/* ------------------------------------------------------------------ */ +/* Implementation of wait for exported DMA buffer to be released.     */ +/* ------------------------------------------------------------------ */

+static void dmabuf_exp_release(struct kref *kref);

+static struct xen_dmabuf_wait_obj * +dmabuf_exp_wait_obj_new(struct gntdev_priv *priv, +            struct xen_dmabuf *xen_dmabuf) +{ +    struct xen_dmabuf_wait_obj *obj;

+    obj = kzalloc(sizeof(*obj), GFP_KERNEL); +    if (!obj) +        return ERR_PTR(-ENOMEM);

+    init_completion(&obj->completion); +    obj->xen_dmabuf = xen_dmabuf;

+    mutex_lock(&priv->dmabuf_lock); +    list_add(&obj->next, &priv->dmabuf_exp_wait_list); +    /* Put our reference and wait for xen_dmabuf's release to fire. */ +    kref_put(&xen_dmabuf->u.exp.refcount, dmabuf_exp_release); +    mutex_unlock(&priv->dmabuf_lock); +    return obj; +}

+static void dmabuf_exp_wait_obj_free(struct gntdev_priv *priv, +                     struct xen_dmabuf_wait_obj *obj) +{ +    struct xen_dmabuf_wait_obj *cur_obj, *q;

+    mutex_lock(&priv->dmabuf_lock); +    list_for_each_entry_safe(cur_obj, q, &priv->dmabuf_exp_wait_list, next) +        if (cur_obj == obj) { +            list_del(&obj->next); +            kfree(obj); +            break; +        } +    mutex_unlock(&priv->dmabuf_lock); +}

+static int dmabuf_exp_wait_obj_wait(struct xen_dmabuf_wait_obj *obj, +                    u32 wait_to_ms) +{ +    if (wait_for_completion_timeout(&obj->completion, +            msecs_to_jiffies(wait_to_ms)) <= 0) +        return -ETIMEDOUT;

+    return 0; +}

+static void dmabuf_exp_wait_obj_signal(struct gntdev_priv *priv, +                       struct xen_dmabuf *xen_dmabuf) +{ +    struct xen_dmabuf_wait_obj *obj, *q;

+    list_for_each_entry_safe(obj, q, &priv->dmabuf_exp_wait_list, next) +        if (obj->xen_dmabuf == xen_dmabuf) { +            pr_debug("Found xen_dmabuf in the wait list, wake\n"); +            complete_all(&obj->completion); +        } +}

+static struct xen_dmabuf * +dmabuf_exp_wait_obj_get_by_fd(struct gntdev_priv *priv, int fd) +{ +    struct xen_dmabuf *q, *xen_dmabuf, *ret = ERR_PTR(-ENOENT);

+    mutex_lock(&priv->dmabuf_lock); +    list_for_each_entry_safe(xen_dmabuf, q, &priv->dmabuf_exp_list, next) +        if (xen_dmabuf->fd == fd) { +            pr_debug("Found xen_dmabuf in the wait list\n"); +            kref_get(&xen_dmabuf->u.exp.refcount); +            ret = xen_dmabuf; +            break; +        } +    mutex_unlock(&priv->dmabuf_lock); +    return ret; +}

static int dmabuf_exp_wait_released(struct gntdev_priv *priv, int fd,                       int wait_to_ms)   { -    return -ETIMEDOUT; +    struct xen_dmabuf *xen_dmabuf; +    struct xen_dmabuf_wait_obj *obj; +    int ret;

+    pr_debug("Will wait for dma-buf with fd %d\n", fd); +    /* +     * Try to find the DMA buffer: if not found means that +     * either the buffer has already been released or file descriptor +     * provided is wrong. +     */ +    xen_dmabuf = dmabuf_exp_wait_obj_get_by_fd(priv, fd); +    if (IS_ERR(xen_dmabuf)) +        return PTR_ERR(xen_dmabuf);

+    /* +     * xen_dmabuf still exists and is reference count locked by us now, +     * so prepare to wait: allocate wait object and add it to the wait list, +     * so we can find it on release. +     */ +    obj = dmabuf_exp_wait_obj_new(priv, xen_dmabuf); +    if (IS_ERR(obj)) { +        pr_err("Failed to setup wait object, ret %ld\n", PTR_ERR(obj)); +        return PTR_ERR(obj); +    }

+    ret = dmabuf_exp_wait_obj_wait(obj, wait_to_ms); +    dmabuf_exp_wait_obj_free(priv, obj); +    return ret; +}

+/* ------------------------------------------------------------------ */ +/* DMA buffer export support.                                         */ +/* ------------------------------------------------------------------ */

+static struct sg_table * +dmabuf_pages_to_sgt(struct page **pages, unsigned int nr_pages) +{ +    struct sg_table *sgt; +    int ret;

+    sgt = kmalloc(sizeof(*sgt), GFP_KERNEL); +    if (!sgt) { +        ret = -ENOMEM; +        goto out; +    }

+    ret = sg_alloc_table_from_pages(sgt, pages, nr_pages, 0, +                    nr_pages << PAGE_SHIFT, +                    GFP_KERNEL); +    if (ret) +        goto out;

+    return sgt;

+out: +    kfree(sgt); +    return ERR_PTR(ret); +}

+static int dmabuf_exp_ops_attach(struct dma_buf *dma_buf, +                 struct device *target_dev, +                 struct dma_buf_attachment *attach) +{ +    struct xen_dmabuf_attachment *xen_dmabuf_attach;

+    xen_dmabuf_attach = kzalloc(sizeof(*xen_dmabuf_attach), GFP_KERNEL); +    if (!xen_dmabuf_attach) +        return -ENOMEM;

+    xen_dmabuf_attach->dir = DMA_NONE; +    attach->priv = xen_dmabuf_attach; +    /* Might need to pin the pages of the buffer now. */ +    return 0; +}

+static void dmabuf_exp_ops_detach(struct dma_buf *dma_buf, +                  struct dma_buf_attachment *attach) +{ +    struct xen_dmabuf_attachment *xen_dmabuf_attach = attach->priv;

+    if (xen_dmabuf_attach) { +        struct sg_table *sgt = xen_dmabuf_attach->sgt;

+        if (sgt) { +            if (xen_dmabuf_attach->dir != DMA_NONE) +                dma_unmap_sg_attrs(attach->dev, sgt->sgl, +                           sgt->nents, +                           xen_dmabuf_attach->dir, +                           DMA_ATTR_SKIP_CPU_SYNC); +            sg_free_table(sgt); +        }

+        kfree(sgt); +        kfree(xen_dmabuf_attach); +        attach->priv = NULL; +    } +    /* Might need to unpin the pages of the buffer now. */ +}

+static struct sg_table * +dmabuf_exp_ops_map_dma_buf(struct dma_buf_attachment *attach, +               enum dma_data_direction dir) +{ +    struct xen_dmabuf_attachment *xen_dmabuf_attach = attach->priv; +    struct xen_dmabuf *xen_dmabuf = attach->dmabuf->priv; +    struct sg_table *sgt;

+    pr_debug("Mapping %d pages for dev %p\n", xen_dmabuf->nr_pages, +         attach->dev);

+    if (WARN_ON(dir == DMA_NONE || !xen_dmabuf_attach)) +        return ERR_PTR(-EINVAL);

+    /* Return the cached mapping when possible. */ +    if (xen_dmabuf_attach->dir == dir) +        return xen_dmabuf_attach->sgt;

may need to check xen_dmabuf_attach->sgt == NULL (i.e. first time mapping)? Also, I am not sure if this mechanism of reusing previously generated sgt for other mappings is universally ok for any use-cases... I don't know if it is acceptable as per the specification.

Well, I was not sure about this piece of code as well, so I'll probably allocate a new sgt each time and do not reuse it as now

The sgt returned for the same attachment, so it is ok to return this cached one as we also check that the direction has not changed So, I'll leave it as is

...

...

+    /* +     * Two mappings with different directions for the same attachment are +     * not allowed. +     */ +    if (WARN_ON(xen_dmabuf_attach->dir != DMA_NONE)) +        return ERR_PTR(-EBUSY);

+    sgt = dmabuf_pages_to_sgt(xen_dmabuf->pages, xen_dmabuf->nr_pages); +    if (!IS_ERR(sgt)) { +        if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, +                      DMA_ATTR_SKIP_CPU_SYNC)) { +            sg_free_table(sgt); +            kfree(sgt); +            sgt = ERR_PTR(-ENOMEM); +        } else { +            xen_dmabuf_attach->sgt = sgt; +            xen_dmabuf_attach->dir = dir; +        } +    } +    if (IS_ERR(sgt)) +        pr_err("Failed to map sg table for dev %p\n", attach->dev); +    return sgt; +}

+static void dmabuf_exp_ops_unmap_dma_buf(struct dma_buf_attachment *attach, +                     struct sg_table *sgt, +                     enum dma_data_direction dir) +{ +    /* Not implemented. The unmap is done at dmabuf_exp_ops_detach(). */

Not sure if it's ok to do nothing here because the spec says this function is mandatory and it should unmap and "release" &sg_table associated with it.

/**      * @unmap_dma_buf:      *      * This is called by dma_buf_unmap_attachment() and should unmap and      * release the &sg_table allocated in @map_dma_buf, and it is mandatory.      * It should also unpin the backing storage if this is the last mapping      * of the DMA buffer, it the exporter supports backing storage      * migration.      */

Yes, as I say at the top of the file dma-buf handling is DRM PRIME based, so I have the workflow just like in there. Do you think we have to be more strict and rework this?

Daniel, what do you think?

I see other drivers in the kernel do the same. I think that *should* in the dma-buf documentation does allow that. So, I'll leave it as is for now

...

...

+}

+static void dmabuf_exp_release(struct kref *kref) +{ +    struct xen_dmabuf *xen_dmabuf = +        container_of(kref, struct xen_dmabuf, u.exp.refcount);

+    dmabuf_exp_wait_obj_signal(xen_dmabuf->priv, xen_dmabuf); +    list_del(&xen_dmabuf->next); +    kfree(xen_dmabuf); +}

+static void dmabuf_exp_ops_release(struct dma_buf *dma_buf) +{ +    struct xen_dmabuf *xen_dmabuf = dma_buf->priv; +    struct gntdev_priv *priv = xen_dmabuf->priv;

+    gntdev_remove_map(priv, xen_dmabuf->u.exp.map); +    mutex_lock(&priv->dmabuf_lock); +    kref_put(&xen_dmabuf->u.exp.refcount, dmabuf_exp_release); +    mutex_unlock(&priv->dmabuf_lock); +}

+static void *dmabuf_exp_ops_kmap_atomic(struct dma_buf *dma_buf, +                    unsigned long page_num) +{ +    /* Not implemented. */ +    return NULL; +}

+static void dmabuf_exp_ops_kunmap_atomic(struct dma_buf *dma_buf, +                     unsigned long page_num, void *addr) +{ +    /* Not implemented. */ +}

+static void *dmabuf_exp_ops_kmap(struct dma_buf *dma_buf, +                 unsigned long page_num) +{ +    /* Not implemented. */ +    return NULL; +}

+static void dmabuf_exp_ops_kunmap(struct dma_buf *dma_buf, +                  unsigned long page_num, void *addr) +{ +    /* Not implemented. */ +}

+static int dmabuf_exp_ops_mmap(struct dma_buf *dma_buf, +                   struct vm_area_struct *vma) +{ +    /* Not implemented. */ +    return 0; +}

+static const struct dma_buf_ops dmabuf_exp_ops =  { +    .attach = dmabuf_exp_ops_attach, +    .detach = dmabuf_exp_ops_detach, +    .map_dma_buf = dmabuf_exp_ops_map_dma_buf, +    .unmap_dma_buf = dmabuf_exp_ops_unmap_dma_buf, +    .release = dmabuf_exp_ops_release, +    .map = dmabuf_exp_ops_kmap, +    .map_atomic = dmabuf_exp_ops_kmap_atomic, +    .unmap = dmabuf_exp_ops_kunmap, +    .unmap_atomic = dmabuf_exp_ops_kunmap_atomic, +    .mmap = dmabuf_exp_ops_mmap, +};

+static int dmabuf_export(struct gntdev_priv *priv, struct grant_map *map, +             int *fd) +{ +    DEFINE_DMA_BUF_EXPORT_INFO(exp_info); +    struct xen_dmabuf *xen_dmabuf; +    int ret = 0;

+    xen_dmabuf = kzalloc(sizeof(*xen_dmabuf), GFP_KERNEL); +    if (!xen_dmabuf) +        return -ENOMEM;

+    kref_init(&xen_dmabuf->u.exp.refcount);

+    xen_dmabuf->priv = priv; +    xen_dmabuf->nr_pages = map->count; +    xen_dmabuf->pages = map->pages; +    xen_dmabuf->u.exp.map = map;

+    exp_info.exp_name = KBUILD_MODNAME; +    if (map->dma_dev->driver && map->dma_dev->driver->owner) +        exp_info.owner = map->dma_dev->driver->owner; +    else +        exp_info.owner = THIS_MODULE; +    exp_info.ops = &dmabuf_exp_ops; +    exp_info.size = map->count << PAGE_SHIFT; +    exp_info.flags = O_RDWR; +    exp_info.priv = xen_dmabuf;

+    xen_dmabuf->dmabuf = dma_buf_export(&exp_info); +    if (IS_ERR(xen_dmabuf->dmabuf)) { +        ret = PTR_ERR(xen_dmabuf->dmabuf); +        xen_dmabuf->dmabuf = NULL; +        goto fail; +    }

+    ret = dma_buf_fd(xen_dmabuf->dmabuf, O_CLOEXEC); +    if (ret < 0) +        goto fail;

+    xen_dmabuf->fd = ret; +    *fd = ret;

+    pr_debug("Exporting DMA buffer with fd %d\n", ret);

+    mutex_lock(&priv->dmabuf_lock); +    list_add(&xen_dmabuf->next, &priv->dmabuf_exp_list); +    mutex_unlock(&priv->dmabuf_lock); +    return 0;

+fail: +    if (xen_dmabuf->dmabuf) +        dma_buf_put(xen_dmabuf->dmabuf); +    kfree(xen_dmabuf); +    return ret; +}

+static struct grant_map * +dmabuf_exp_alloc_backing_storage(struct gntdev_priv *priv, int dmabuf_flags, +                 int count) +{ +    struct grant_map *map;

+    if (unlikely(count <= 0)) +        return ERR_PTR(-EINVAL);

+    if ((dmabuf_flags & GNTDEV_DMA_FLAG_WC) && +        (dmabuf_flags & GNTDEV_DMA_FLAG_COHERENT)) { +        pr_err("Wrong dma-buf flags: either WC or coherent, not both\n"); +        return ERR_PTR(-EINVAL); +    }

+    map = gntdev_alloc_map(priv, count, dmabuf_flags); +    if (!map) +        return ERR_PTR(-ENOMEM);

+    if (unlikely(atomic_add_return(count, &pages_mapped) > limit)) { +        pr_err("can't map: over limit\n"); +        gntdev_put_map(NULL, map); +        return ERR_PTR(-ENOMEM); +    } +    return map;   }

When and how would this allocation be freed? I don't see any ioctl for freeing up shared pages.

on xen_dmabuf.release callback which is refcounted

...

...

static int dmabuf_exp_from_refs(struct gntdev_priv *priv, int flags,                   int count, u32 domid, u32 *refs, u32 *fd)   { +    struct grant_map *map; +    int i, ret;

*fd = -1; -    return -EINVAL;

+    if (use_ptemod) { +        pr_err("Cannot provide dma-buf: use_ptemode %d\n", +               use_ptemod); +        return -EINVAL; +    }

+    map = dmabuf_exp_alloc_backing_storage(priv, flags, count); +    if (IS_ERR(map)) +        return PTR_ERR(map);

+    for (i = 0; i < count; i++) { +        map->grants[i].domid = domid; +        map->grants[i].ref = refs[i]; +    }

+    mutex_lock(&priv->lock); +    gntdev_add_map(priv, map); +    mutex_unlock(&priv->lock);

+    map->flags |= GNTMAP_host_map; +#if defined(CONFIG_X86) +    map->flags |= GNTMAP_device_map; +#endif

+    ret = map_grant_pages(map); +    if (ret < 0) +        goto out;

+    ret = dmabuf_export(priv, map, fd); +    if (ret < 0) +        goto out;

+    return 0;

+out: +    gntdev_remove_map(priv, map); +    return ret;   }     /*

------------------------------------------------------------------ */

2.17.0