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

Re: [Xen-devel] [RFC PATCH] xen/arm: Vmap allocator fails to allocate beyond 128M

On 23/02/2015 17:07, Ian Campbell wrote:
On Tue, 2015-02-17 at 14:50 +0000, Julien Grall wrote:
Hello Vijay,

The title looks more a title for a bug report rather than a patch.

On 17/02/15 06:07, Vijay Kilari wrote:
In vmap_init, map_pages_to_xen() is called for mapping
vm_bitmap. Initially one page of vm_bitmap is allocated
and mapped using PAGE_HYPERVISOR attribute.
For the rest of vm_bitmap pages, MAP_SMALL_PAGES attribute
is used to map.

In ARM for both PAGE_HYPERVISOR and MAP_SMALL_PAGES, valid bit
is set to 1 in pte entry for these mapping.

In vma_alloc(), map_pages_to_xen() is failing for >128MB because
for this next vm_bitmap page the mapping is already set in vm_init()
with valid bit set in pte entry. So map_pages_to_xen() in
ARM returns error.

On x86, for the pages mapped with MAP_SMALL_PAGES attribute
valid bit is not set. So the common vmap code assumes the
same behaviour from arm. However this is not the case.

This patch will set valid bit to 0 in pte entry when
pages are mapped with MAP_SMALL_PAGES.

Setting the valid bit to 0 or 1, as long as the 128M limit, are just
side-effects of the real problem.

You should explain what does MAP_SMALL_PAGES and PAGE_HYPERVISOR means.
For now, the reader needs to understand by himself that MAP_SMALL_PAGES
is used to pre-allocate non-leaf page table.


diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 53d4b63..e704ac8 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -58,6 +58,7 @@
  #define WRITEBACK     0x3
  #define DEV_SHARED    0x4
  #define WRITEALLOC    0x7
+#define WRITEALLOC_INVALID    0xf

Why WRITEALLOC_INVALID? The mapping is not at all invalid and doesn't
exist... We just asked to pre-allocate the non-leaf page.

  #define DEV_WC        BUFFERABLE
@@ -65,7 +66,7 @@
  #define PAGE_HYPERVISOR_WC      (DEV_WC)

The parameter "flags" of map_pages_to_xen is an unsigned int. It would
be better to use the top bits for adding new flags (such as a present flag).

Then you could use this flag in create_xen_entries to know if we need to
set the present bit or not. Or, even better, skip the creation of the PTE.

The underlying problem here seems to be that PAGE_FOO on x86 is not just
the attributes but is actually the present bit etc too:

changing ARM similarly (i.e. shifting WRITEALLOC etc to the appropriate
place and adding the present bit) would seem like a good way to both
address this issue and make the arch interfaces more consistent.

MAP_SMALL_PAGES could then be an avail bit or something like on x86.

Actually the v2 is going on this sense, though I had some comment on it.


Julien Grall

Xen-devel mailing list



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