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

Re: [Xen-devel] [PATCH v1] xen/arm: Do not allocate pte entries for MAP_SMALL_PAGES



Hello Vijay,

On 18/02/2015 12:56, vijay.kilari@xxxxxxxxx wrote:
From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>

On x86, for the pages mapped with PAGE_HYPERVISOR attribute
non-leaf page tables are allocated with valid pte entries.
and with MAP_SMALL_PAGES attribute only non-leaf page tables are
allocated with invalid (valid bit set to 0) pte entries.
However on arm this is not the case. On arm for the pages
mapped with PAGE_HYPERVISOR and MAP_SMALL_PAGES both
non-leaf and leaf level page table are allocated with valid bit
in pte entries.

This behaviour in arm makes common vmap code fail to
allocate memory beyond 128MB as described below.

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.

With this patch, MAP_SMALL_PAGES attribute will only allocate
non-leaf page tables only.

Here we use bit[16] in the attribute flag to know if leaf page
tables should be allocated or not.

This bit is set only for MAP_SMALL_PAGES attribute.

Signed-off-by: Vijaya Kumar K<Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
---
  xen/arch/arm/mm.c          |    9 ++++++---
  xen/include/asm-arm/page.h |    8 +++++++-
  2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 7d4ba0c..a12f3f5 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -865,9 +865,12 @@ static int create_xen_entries(enum xenmap_operation op,
                             addr, mfn);
                      return -EINVAL;
                  }
-                pte = mfn_to_xen_entry(mfn, ai);
-                pte.pt.table = 1;
-                write_pte(&third[third_table_offset(addr)], pte);
+                if ( !(ai & PTE_INVALID) )

you could do if ( ai & PTE_INVALID ) break; It would avoid a new level of indentation.

Also, I would rename ai to flags as the variable is not anymore an attribute index.

+                {
+                    pte = mfn_to_xen_entry(mfn, (ai & 0xffff));

Please introduce a new macro for the mask.

+                    pte.pt.table = 1;
+                    write_pte(&third[third_table_offset(addr)], pte);
+                }
                  break;
              case REMOVE:
                  if ( !third[third_table_offset(addr)].pt.valid )
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 3e7b0ae..80415b3 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -61,10 +61,16 @@
  #define DEV_WC        BUFFERABLE
  #define DEV_CACHED    WRITEBACK

+/* bit 16 in the Attribute index can be used to know if
+ * PTE entry should be added or not. This is useful
+ * when ONLY non-leaf page table entries need to allocated
+ */
+#define PTE_INVALID   (0x1 << 16)

It makes more sense to introduce a PTE_PRESENT flags compare to PTE_INVALID. The former has more meaning that the latter.

Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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