Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-31 Thread Oleksandr Andrushchenko 

On 05/31/2018 10:51 AM, Oleksandr Andrushchenko wrote:

On 05/30/2018 10:24 PM, Boris Ostrovsky wrote:

On 05/30/2018 01:46 PM, Oleksandr Andrushchenko wrote:

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


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, );


Sure, this also works.

Could you please take look at the patch attached if this is what we want?

Please ignore it, it is ugly ;)
I have implemented this as you suggested:

static inline void
xenmem_reservation_va_mapping_update(unsigned long count,
                 struct page **pages,
                 xen_pfn_t *frames)
{
#ifdef CONFIG_XEN_HAVE_PVMMU
    if (!xen_feature(XENFEAT_auto_translated_physmap))
        __xenmem_reservation_va_mapping_update(count, pages, frames);
#endif
}


-boris


Thank you,
Oleksandr


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

Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-31 Thread Oleksandr Andrushchenko 

On 05/30/2018 10:24 PM, Boris Ostrovsky wrote:

On 05/30/2018 01:46 PM, Oleksandr Andrushchenko wrote:

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


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, );


Sure, this also works.

Could you please take look at the patch attached if this is what we want?

-boris


Thank you,
Oleksandr
>From d41751068ac80ca5a375909d6c01cb25716a4975 Mon Sep 17 00:00:00 2001
From: Oleksandr Andrushchenko 
Date: Wed, 23 May 2018 16:52:45 +0300
Subject: [PATCH] xen/balloon: Share common memory reservation routines

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 file for the shared code and export corresponding
symbols for other kernel modules.

Signed-off-by: Oleksandr Andrushchenko 
---
 drivers/xen/Makefile  |   1 +
 drivers/xen/balloon.c |  71 ++---
 drivers/xen/mem-reservation.c |  78 +++
 include/xen/mem-reservation.h | 114 ++
 4 files changed, 199 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..1789be76e9c5 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -71,6 +71,7 @@
 #include 
 #include 
 #include 
+#include 
 
 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, );
+	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(page, frame_list[i]);
 
 		/* Relinquish the page back to the allocator. */
 		free_reserved_page(page);
@@ -535,11 +500,6 @@ static

Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-31 Thread Boris Ostrovsky 
On 05/30/2018 01:46 PM, Oleksandr Andrushchenko wrote:
> On 05/30/2018 06:54 PM, Boris Ostrovsky wrote:
>>
>>
>> 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, );


Sure, this also works.

-boris

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

Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-31 Thread Boris Ostrovsky 
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, );
 +    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, ,
 _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*, , _list[i]);
> from increase_reservation
> and
> xenmem_reservation_va_mapping_reset(*1*, );
> 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(>lru, );
>
> 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(>lru, );
>
> 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(, );
> But for xenmem_reservation_va_mapping_update it will look like:
> xenmem_reservation_va_mapping_update(, ,
> )
> 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

Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-30 Thread Oleksandr Andrushchenko 

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, );
+    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, ,
_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*, , _list[i]);
from increase_reservation
and
xenmem_reservation_va_mapping_reset(*1*, );
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(>lru, );

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(>lru, );

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(, );
But for xenmem_reservation_va_mapping_update it will look like:
xenmem_reservation_va_mapping_update(, ,
)
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

Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-30 Thread Oleksandr Andrushchenko 

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, );
+    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, , _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*, , _list[i]);
from increase_reservation
and
xenmem_reservation_va_mapping_reset(*1*, );
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(>lru, );

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(>lru, );

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(, );
But for xenmem_reservation_va_mapping_update it will look like:
xenmem_reservation_va_mapping_update(, , 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

Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-30 Thread Boris Ostrovsky 
On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:
> +
> +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);

One other thing I noticed --- both of these can be declared as NOPs in
the header file if !CONFIG_XEN_HAVE_PVMMU.

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

Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-30 Thread Boris Ostrovsky 
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, );
>> +    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, , _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?

-boris


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

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

Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-30 Thread Juergen Gross 
On 25/05/18 17:33, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko 
> 
> 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 
> ---
>  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 
>  #include 
>  #include 
> +#include 
>  
>  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, );
> + 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, , _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,

Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-30 Thread Boris Ostrovsky 
On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko 
>
> 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.


> for the shared code and export corresponding
> symbols for other kernel modules.
>
> Signed-off-by: Oleksandr Andrushchenko 
> ---
>  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 
>  #include 
>  #include 
> +#include 
>  
>  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, );
> + 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, , _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.




>  
>   /* 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);
>

Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-30 Thread Oleksandr Andrushchenko 

On 05/30/2018 07:32 AM, Juergen Gross wrote:

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

From: Oleksandr Andrushchenko 

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 
---
  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 ?


Will rename

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 
  #include 
  #include 
+#include 
  
  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, );
+   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, , _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);

Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-29 Thread Oleksandr Andrushchenko 



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

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

+
+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);

One other thing I noticed --- both of these can be declared as NOPs in
the header file if !CONFIG_XEN_HAVE_PVMMU.

Will rework it to be NOp for !CONFIG_XEN_HAVE_PVMMU

-boris


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

Re: [PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-29 Thread Oleksandr Andrushchenko 

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

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

From: Oleksandr Andrushchenko 

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 
---
  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 
  #include 
  #include 
+#include 
  
  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, );
+   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, , _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

[PATCH 2/8] xen/balloon: Move common memory reservation routines to a module

2018-05-25 Thread Oleksandr Andrushchenko 
From: Oleksandr Andrushchenko 

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 
---
 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 
 #include 
 #include 
+#include 
 
 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, );
+   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, , _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(>lru, );
}
 
@@ -575,25 +535,8 @@ static enum bp_state decrease_reservation(unsigned long