ARM - why does setup_frametable_size() round frametable_size to 32MB ?

ARM - why does setup_frametable_size() round frametable_size to 32MB ?

[Chris (Christopher) Brand]

Hi,

I'm working on a platform with a mere 2GB of RAM, and trying to
trim the Xen footprint down as much as possible. I've found two
places where Xen uses more memory than it seems it needs to,
one of which is the frametable. On a 2GB system, frametable_size
is initially calculated as 16MB, but is then rounded up to 32MB.
can somebody tell me why this is done, and therefore whether
it can be avoided ? I assume it's because the code then calls
create_32mb_mappings(), in which case I guess my question
is what's special about 32MB ?

    unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
[...]
    /* Round up to 32M boundary */
    frametable_size = (frametable_size + 0x1ffffff) & ~0x1ffffff;

Thanks,

Chris

Re: ARM - why does setup_frametable_size() round frametable_size to 32MB ?

[Ian Campbell]

On Fri, 2015-07-17 at 21:19 +0000, Chris (Christopher) Brand wrote:
> Hi,
> 
> I'm working on a platform with a mere 2GB of RAM, and trying to
> trim the Xen footprint down as much as possible. I've found two
> places where Xen uses more memory than it seems it needs to,
> one of which is the frametable. On a 2GB system, frametable_size
> is initially calculated as 16MB, but is then rounded up to 32MB.
> can somebody tell me why this is done, and therefore whether
> it can be avoided ? I assume it's because the code then calls
> create_32mb_mappings(), in which case I guess my question
> is what's special about 32MB ?

It's 16 lots of 2MB, so we can use the contiguous hint in the PTE entry
(which saves on TLB space or whatever).

It would be fine to not use that for a sub-32MB sized mappings. I think
if the size is <32MB we should just round to a 2MB boundary (since we do
still want to use super pages) and map those without the contig bit.

I suppose for things which are >32MB we may as well keep rounding to
32MB, rather than creating N-1 32MB contiguous mappings and a bunch of
non-contiguous ones for the slop at the end.

Perhaps there is no harm in creating a 32MB mapping of a smaller region,
I suppose it depends on how the corner cases like e.g. that mapping
crossing into an MMIO region is handled. It's probably not worth the
hassle to find out...

Ian.


> 
>     unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
> [...]
>     /* Round up to 32M boundary */
>     frametable_size = (frametable_size + 0x1ffffff) & ~0x1ffffff;
> 
> Thanks,
> 
> Chris
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: ARM - why does setup_frametable_size() round frametable_size to 32MB ?

[Chris (Christopher) Brand]

Thanks, Ian. I tried that, and it does seem to work (everything boots, I can still bring up VMs, and I see an extra 16MB of free memory). The patch I came up with follows (it would be nice to share code between create_32mb_mappings() and create_2mb_mappings(), but the setting of the contig bit is right in the middle, and the functions are pretty short).

Chris

From: Chris Brand <chris.brand@broadcom.com>
Date: Mon, 20 Jul 2015 13:38:15 -0700
Subject: [PATCH] xen: arm: Support <32MB frametables

setup_frametable_mappings() rounds frametable_size up to a multiple
of 32MB. This is wasteful on systemes with less than 4GB of RAM,
although it does allow the "contig" bit to be set in the PTEs.

Where the frametable is less than 32MB in size, instead round up
to a multiple of 2MB, not setting the "contig" bit in the PTEs.

Signed-off-by: Chris Brand <chris.brand@broadcom.com>
---
 xen/arch/arm/mm.c | 39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index a91ea774f1f9..a7f4864f8d8f 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -656,6 +656,29 @@ static void __init create_32mb_mappings(lpae_t *second,
 }
 
 #ifdef CONFIG_ARM_32
+static void __init create_2mb_mappings(lpae_t *second,
+                                       unsigned long virt_offset,
+                                       unsigned long base_mfn,
+                                       unsigned long nr_mfns)
+{
+    unsigned long i, count;
+    lpae_t pte, *p;
+
+    ASSERT(!((virt_offset >> PAGE_SHIFT) % LPAE_ENTRIES));
+    ASSERT(!(base_mfn % LPAE_ENTRIES));
+    ASSERT(!(nr_mfns % LPAE_ENTRIES));
+
+    count = nr_mfns / LPAE_ENTRIES;
+    p = second + second_linear_offset(virt_offset);
+    pte = mfn_to_xen_entry(base_mfn, WRITEALLOC);
+    for ( i = 0; i < count; i++ )
+    {
+        write_pte(p + i, pte);
+        pte.pt.base += 1 << LPAE_SHIFT;
+    }
+    flush_xen_data_tlb_local();
+}
+
 /* Set up the xenheap: up to 1GB of contiguous, always-mapped memory. */
 void __init setup_xenheap_mappings(unsigned long base_mfn,
                                    unsigned long nr_mfns)
@@ -749,6 +772,7 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
     unsigned long nr_pdxs = pfn_to_pdx(nr_pages);
     unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
     unsigned long base_mfn;
+    unsigned long mask;
 #ifdef CONFIG_ARM_64
     lpae_t *second, pte;
     unsigned long nr_second, second_base;
@@ -757,8 +781,12 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
 
     frametable_base_pdx = pfn_to_pdx(ps >> PAGE_SHIFT);
 
-    /* Round up to 32M boundary */
-    frametable_size = (frametable_size + 0x1ffffff) & ~0x1ffffff;
+    /* Round up to 2M or 32M boundary, as appropriate */
+    if (frametable_size < MB(32))
+        mask = MB(2) - 1;
+    else
+        mask = MB(32) - 1;
+    frametable_size = (frametable_size + mask) & ~mask;
     base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 32<<(20-12));
 
 #ifdef CONFIG_ARM_64
@@ -773,7 +801,12 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe)
     }
     create_32mb_mappings(second, 0, base_mfn, frametable_size >> PAGE_SHIFT);
 #else
-    create_32mb_mappings(xen_second, FRAMETABLE_VIRT_START, base_mfn, frametable_size >> PAGE_SHIFT);
+    if (frametable_size < MB(32))
+        create_2mb_mappings(xen_second, FRAMETABLE_VIRT_START,
+                            base_mfn, frametable_size >> PAGE_SHIFT);
+    else
+        create_32mb_mappings(xen_second, FRAMETABLE_VIRT_START,
+                             base_mfn, frametable_size >> PAGE_SHIFT);
 #endif
 
     memset(&frame_table[0], 0, nr_pdxs * sizeof(struct page_info));
-- 
1.9.1