[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/2] xen/mm: Introduce PGC_state_uninitialised

Hi David,

On 20/03/2020 15:17, David Woodhouse wrote:
On Fri, 2020-03-20 at 13:33 +0000, Paul Durrant wrote:
-----Original Message-----
From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of David 
Sent: 19 March 2020 21:22
To: xen-devel@xxxxxxxxxxxxxxxxxxxx
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Julien Grall <julien@xxxxxxx>; Wei 
Liu <wl@xxxxxxx>;
Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Ian Jackson 
<ian.jackson@xxxxxxxxxxxxx>; George Dunlap
<george.dunlap@xxxxxxxxxx>; hongyxia@xxxxxxxxxx; Jan Beulich 
<jbeulich@xxxxxxxx>; Volodymyr Babchuk
<Volodymyr_Babchuk@xxxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>
Subject: [Xen-devel] [PATCH 2/2] xen/mm: Introduce PGC_state_uninitialised

From: David Woodhouse <dwmw@xxxxxxxxxxxx>

It is possible for pages to enter general circulation without ever
being process by init_heap_pages().

For example, pages of the multiboot module containing the initramfs may
be assigned via assign_pages() to dom0 as it is created. And some code
including map_pages_to_xen() has checks on 'system_state' to determine
whether to use the boot or the heap allocator, but it seems impossible
to prove that pages allocated by the boot allocator are not subsequently
freed with free_heap_pages().

This actually works fine in the majority of cases; there are only a few
esoteric corner cases which init_heap_pages() handles before handing the
page range off to free_heap_pages():
  • Excluding MFN #0 to avoid inappropriate cross-zone merging.
  • Ensuring that the node information structures exist, when the first
    page(s) of a given node are handled.
  • High order allocations crossing from one node to another.

To handle this case, shift PG_state_inuse from its current value of
zero, to another value. Use zero, which is the initial state of the
entire frame table, as PG_state_uninitialised.

Fix a couple of assertions which were assuming that PG_state_inuse is
zero, and make them cope with the PG_state_uninitialised case too where

Finally, make free_heap_pages() call through to init_heap_pages() when
given a page range which has not been initialised. This cannot keep
recursing because init_heap_pages() will set each page state to
PGC_state_inuse before passing it back to free_heap_pages() for the
second time.

Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
  xen/arch/x86/mm.c        |  3 ++-
  xen/common/page_alloc.c  | 44 +++++++++++++++++++++++++++++-----------
  xen/include/asm-arm/mm.h |  3 ++-
  xen/include/asm-x86/mm.h |  3 ++-
  4 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 62507ca651..5f0581c072 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -491,7 +491,8 @@ void share_xen_page_with_guest(struct page_info *page, 
struct domain *d,

      page_set_owner(page, d);
      smp_wmb(); /* install valid domain ptr before updating refcnt. */
-    ASSERT((page->count_info & ~PGC_xen_heap) == 0);
+    ASSERT((page->count_info & ~PGC_xen_heap) == PGC_state_inuse ||
+           (page->count_info & ~PGC_xen_heap) == PGC_state_uninitialised);

Could the page state perhaps be bumped to inuse in this case? It
seems odd to leave state uninitialized yet succeed in sharing with a

No, that doesn't really work.

You can't just *declare* that the page was properly initialised,
because it isn't true. There's a pathological case where the zone
hasn't been initialised at all, because *all* the pages in that zone
got handed out by the boot allocator or used for initrd etc.

The first pages 'freed' in that zone end up being used (in
init_heap_pages) to create the zone structures.

Likewise, it could include a page which init_heap_pages() doesn't
actually *put* into the buddy allocator, to work around the cross-zone
merge problem. It's fine to use that page and share it with a guest,
but it can't ever be freed into the buddy allocator.

For liveupdate, we will need a way to initialize a page but mark it as already inuse (i.e in the same state as they would be if allocated normally).

It feels to me, this is also what we want in this case. The page would be first initialize and then we can use it normally including freeing later on.

Would it make sense to introduce an helper for this purpose?


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.