Re: [PATCH] x86/NUMA: make init_node_heap() respect Xen heap limit

[Julien Grall <julien.grall@citrix.com>]

On 03/09/2015 21:01, Julien Grall wrote:
> Hi Jan,
> 
> On 27/08/2015 09:37, Jan Beulich wrote:
>> On NUMA systems, where we try to use node local memory for the basic
>> control structures of the buddy allocator, this special case needs to
>> take into consideration a possible address width limit placed on the
>> Xen heap. In turn this (but also other, more abstract considerations)
>> requires that xenheap_max_mfn() not be called more than once (at most
>> we might permit it to be called a second time with a larger value than
>> was passed the first time), and be called only before calling
>> end_boot_allocator().
>>
>> While inspecting all the involved code, a couple of off-by-one issues
>> were found (and are being corrected here at once):
>> - arch_init_memory() cleared one too many page table slots
>> - the highmem_start based invocation of xenheap_max_mfn() passed too
>>     big a value
>> - xenheap_max_mfn() calculated the wrong bit count in edge cases
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> This patch is breaking boot on aarch64 platform (particularly X-gene).
> 
> I think this should be considered as a block until I find a way to
> fix it.

And found why! The last xenheap_bits changed from 39 to 38.

On x-gene the last max mfn used for the xenheap is 0x4400000, which the
new computation, it will give 38 bits which doesn't cover the entire
xenheap range.

I have wrote a patch to fix the issue, but I'm not sure that it's
the right things to do (see below).

Regards,

commit d1f2e47326da87c55f3165156407f224d684bccd
Author: Julien Grall <julien.grall@citrix.com>
Date:   Thu Sep 3 21:49:31 2015 +0100

    xen: pagealloc: Correctly calculate the number of xenheap bits

    The commit 88e3ed61642bb393458acc7a9bd2f96edc337190 "x86/NUMA: make
    init_node_heap() respect Xen heap limit" breaks boot on the arm64 board
    X-Gene.

    This is because the last xen heaf MFN is 0x4400000. Which the new way to
    calculate the number bits, the result would 38 bits which doesn't cover
    all the xenheap. Fix it by dropping the -1 in the expression.

    Signed-off-by: Julien Grall <julien.grall@citrix.com>

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 74fc1de..24fb09c 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1558,7 +1558,7 @@ void __init xenheap_max_mfn(unsigned long mfn)
     ASSERT(!first_node_initialised);
     ASSERT(!xenheap_bits);
     BUILD_BUG_ON(PADDR_BITS >= BITS_PER_LONG);
-    xenheap_bits = min(flsl(mfn + 1) - 1 + PAGE_SHIFT, PADDR_BITS);
+    xenheap_bits = min(flsl(mfn + 1) + PAGE_SHIFT, PADDR_BITS);
     printk(XENLOG_INFO "Xen heap: %u bits\n", xenheap_bits);
 }

-- 
Julien Grall