Re: [PATCH V2 3/4] xen/unpopulated-alloc: Add mechanism to use Xen resource

From: Oleksandr
Date: Wed Nov 24 2021 - 04:33:36 EST



On 23.11.21 23:25, Stefano Stabellini wrote:

Hi Stefano

On Tue, 23 Nov 2021, Oleksandr wrote:
Actually after reading your replies and explanation I changed opinion: I
think we do need the fallback because Linux cannot really assume that
it is running on "new Xen" so it definitely needs to keep working if
CONFIG_XEN_UNPOPULATED_ALLOC is enabled and the extended regions are not
advertised.

I think we'll have to roll back some of the changes introduced by
121f2faca2c0a. That's because even if CONFIG_XEN_UNPOPULATED_ALLOC is
enabled we cannot know if we can use unpopulated-alloc or whether we
have to use alloc_xenballooned_pages until we parse the /hypervisor node
in device tree at runtime.
Exactly!


In short, we cannot switch between unpopulated-alloc and
alloc_xenballooned_pages at build time, we have to do it at runtime
(boot time).
+1


I created a patch to partially revert 121f2faca2c0a "xen/balloon: rename
alloc/free_xenballooned_pages".

If there is no objections I will add it to V3 (which is almost ready, except
the fallback bits). Could you please tell me what do you think?
It makes sense to me. You can add my Reviewed-by.

Great, thank you!



From dc79bcd425358596d95e715a8bd8b81deaaeb703 Mon Sep 17 00:00:00 2001
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
Date: Tue, 23 Nov 2021 18:14:41 +0200
Subject: [PATCH] xen/balloon: Bring alloc(free)_xenballooned_pages helpers
 back

This patch rolls back some of the changes introduced by commit
121f2faca2c0a "xen/balloon: rename alloc/free_xenballooned_pages"
in order to make possible to still allocate xenballooned pages
if CONFIG_XEN_UNPOPULATED_ALLOC is enabled.

On Arm the unpopulated pages will be allocated on top of extended
regions provided by Xen via device-tree (the subsequent patches
will add required bits to support unpopulated-alloc feature on Arm).
The problem is that extended regions feature has been introduced
into Xen quite recently (during 4.16 release cycle). So this
effectively means that Linux must only use unpopulated-alloc on Arm
if it is running on "new Xen" which advertises these regions.
But, it will only be known after parsing the "hypervisor" node
at boot time, so before doing that we cannot assume anything.

In order to keep working if CONFIG_XEN_UNPOPULATED_ALLOC is enabled
and the extended regions are not advertised (Linux is running on
"old Xen", etc) we need the fallback to alloc_xenballooned_pages().

This way we wouldn't reduce the amount of memory usable (wasting
RAM pages) for any of the external mappings anymore (and eliminate
XSA-300) with "new Xen", but would be still functional ballooning
out RAM pages with "old Xen".

Also rename alloc(free)_xenballooned_pages to xen_alloc(free)_ballooned_pages.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
---
 drivers/xen/balloon.c | 20 +++++++++-----------
 include/xen/balloon.h |  3 +++
 include/xen/xen.h     |  6 ++++++
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index ba2ea11..a2c4fc49 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -581,7 +581,6 @@ void balloon_set_new_target(unsigned long target)
 }
 EXPORT_SYMBOL_GPL(balloon_set_new_target);

-#ifndef CONFIG_XEN_UNPOPULATED_ALLOC
 static int add_ballooned_pages(unsigned int nr_pages)
 {
     enum bp_state st;
@@ -610,12 +609,12 @@ static int add_ballooned_pages(unsigned int nr_pages)
 }

 /**
- * xen_alloc_unpopulated_pages - get pages that have been ballooned out
+ * xen_alloc_ballooned_pages - get pages that have been ballooned out
  * @nr_pages: Number of pages to get
  * @pages: pages returned
  * @return 0 on success, error otherwise
  */
-int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages)
+int xen_alloc_ballooned_pages(unsigned int nr_pages, struct page **pages)
 {
     unsigned int pgno = 0;
     struct page *page;
@@ -652,23 +651,23 @@ int xen_alloc_unpopulated_pages(unsigned int nr_pages,
struct page **pages)
     return 0;
  out_undo:
     mutex_unlock(&balloon_mutex);
-    xen_free_unpopulated_pages(pgno, pages);
+    xen_free_ballooned_pages(pgno, pages);
     /*
-     * NB: free_xenballooned_pages will only subtract pgno pages, but since
+     * NB: xen_free_ballooned_pages will only subtract pgno pages, but since
      * target_unpopulated is incremented with nr_pages at the start we need
      * to remove the remaining ones also, or accounting will be screwed.
      */
     balloon_stats.target_unpopulated -= nr_pages - pgno;
     return ret;
 }
-EXPORT_SYMBOL(xen_alloc_unpopulated_pages);
+EXPORT_SYMBOL(xen_alloc_ballooned_pages);

 /**
- * xen_free_unpopulated_pages - return pages retrieved with
get_ballooned_pages
+ * xen_free_ballooned_pages - return pages retrieved with get_ballooned_pages
  * @nr_pages: Number of pages
  * @pages: pages to return
  */
-void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages)
+void xen_free_ballooned_pages(unsigned int nr_pages, struct page **pages)
 {
     unsigned int i;

@@ -687,9 +686,9 @@ void xen_free_unpopulated_pages(unsigned int nr_pages,
struct page **pages)

     mutex_unlock(&balloon_mutex);
 }
-EXPORT_SYMBOL(xen_free_unpopulated_pages);
+EXPORT_SYMBOL(xen_free_ballooned_pages);

-#if defined(CONFIG_XEN_PV)
+#if defined(CONFIG_XEN_PV) && !defined(CONFIG_XEN_UNPOPULATED_ALLOC)
 static void __init balloon_add_region(unsigned long start_pfn,
                       unsigned long pages)
 {
@@ -712,7 +711,6 @@ static void __init balloon_add_region(unsigned long
start_pfn,
     balloon_stats.total_pages += extra_pfn_end - start_pfn;
 }
 #endif
-#endif

 static int __init balloon_init(void)
 {
diff --git a/include/xen/balloon.h b/include/xen/balloon.h
index e93d4f0..f78a6cc 100644
--- a/include/xen/balloon.h
+++ b/include/xen/balloon.h
@@ -26,6 +26,9 @@ extern struct balloon_stats balloon_stats;

 void balloon_set_new_target(unsigned long target);

+int xen_alloc_ballooned_pages(unsigned int nr_pages, struct page **pages);
+void xen_free_ballooned_pages(unsigned int nr_pages, struct page **pages);
+
 #ifdef CONFIG_XEN_BALLOON
 void xen_balloon_init(void);
 #else
diff --git a/include/xen/xen.h b/include/xen/xen.h
index 9f031b5..410e3e4 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -52,7 +52,13 @@ bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
 extern u64 xen_saved_max_mem_size;
 #endif

+#ifdef CONFIG_XEN_UNPOPULATED_ALLOC
 int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages);
 void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages);
+#else
+#define xen_alloc_unpopulated_pages xen_alloc_ballooned_pages
+#define xen_free_unpopulated_pages xen_free_ballooned_pages
+#include <xen/balloon.h>
+#endif

 #endif    /* _XEN_XEN_H */

--
Regards,

Oleksandr Tyshchenko