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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.09.15 at 22:58, <julien.grall@xxxxxxxxxx> wrote:
> On 03/09/2015 21:01, Julien Grall wrote:
>> 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@xxxxxxxx>
>> 
>> 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.

Agreed.

> 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).

No, this is wrong: xenheap_bits isn't meant to cover all RAM, it is
meant to indicate how much (as an exact power of 2) of RAM is
always accessible. I'm surprised anyway that ARM64 uses
xenheap_max_mfn() (and even unconditionally); I thought all RAM
is always accessible there. The invocation is off by one now in any
case, but rather than correcting it that way the proper fix likely
will involve more than just this simple an adjustment, as it looks
like its use was wrong from the beginning (commit 5263507b1b).

Jan


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