Re: [PATCH for-4.12 v2 2/2] xen/arm: Stop relocating Xen

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

Hi,

On 17/12/2018 17:57, Stefano Stabellini wrote:
> On Mon, 17 Dec 2018, Julien Grall wrote:
>> Hi,
>>
>> On 14/12/2018 17:48, Julien Grall wrote:
>>> On 14/12/2018 17:24, Andrii Anisov wrote:
>>>> On 14.12.18 18:26, Julien Grall wrote:
>>>>> I don't understand how you came up with the conclusion that 128MB will
>>>>> be
>>>>> removed from Dom0. We only have the requirement that the first bank is
>>>>> at least
>>>>> 128MB. So can you expand it?
>>>> IIRC Linux kernel requires that the machine RAM start must be 128MB
>>>> aligned.
>>>
>>> Please try to reference the documentation (or code if lack of documentation)
>>> when making such statement.
>>>
>>> AFAICT, Linux 32-bit [1] imposes the kernel to be loaded in the first 128MB
>>> of RAM. Nothing about the 128MB aligned RAM. Linux 64-bit [1] requires to be
>>> loaded at a 2MB aligned address.
>>>
>>> So technically allocating the RAM using a 2MB alignment should be enough.
>>> Yet we need to make sure the first bank is at least 128MB.
>>>
>>>> Look at `allocate_memory_11()`, `min_low_order` variable usage. It affects
>>>> all low memory 1:1 allocation and makes all low memory banks 128MB aligned
>>>> both start and end.
>>>> So that having a module in a low memory poisons the whole 128MB region.
>>>>
>>> That's definitely an unwanted behavior, but this is not related to the patch
>>> itself. As soon as you hand memory to the allocator, memory can be allocated
>>> at any place in the memory. I am still unsure whether the alignment is due
>>> to the algorithm in allocate_memory_11() or because of the order we pass to
>>> the allocator.
>>>
>>> Until we fix it, the best recommendation is to keep all the modules close
>>> together at the beginning of the RAM. So you only "waste" 128MB region. I
>>> can add this recommendation in the commit message and potentially
>>> documentation.
>>
>> Answering to myself. Stefano pointed out on IRC that grub/UEFI users are not
>> in control of the memory layout so this might be an issue for them.
>>
>> Looking at my GRUB setup, all the modules are loaded together:
>>
>> (XEN) MODULE[0]: 00000000f2afb000 - 00000000f2b02000 Device Tree
>> (XEN) MODULE[1]: 00000000f695b000 - 00000000f7f6fa00 Kernel
>> (XEN) MODULE[2]: 00000000f2c23000 - 00000000f6959200 Ramdisk
>>
>> [...]
>>
>> (XEN) Placing Xen at 0x000000099be00000-0x000000099c000000
>> (XEN) Update BOOTMOD_XEN from 00000000f2b02000-00000000f2c22d81 =>
>> 000000099be00
>>
>> So whether Xen is going to be relocated or not is not going to make much
>> difference.
>>
>> Now, let's image the bootloader decides to load the modules in different
>> places in the memory. Then you will have 4 slots (potential 5 slots) of 128MB
>> used. That's up to 640MB of low memory not available for Dom0. Relocating Xen
>> may or may not make available more low memory for Dom0. For instance, in my
>> use case above, this does not make any change.
>>
>> This is obviously the worst case scenario. I am pretty sure people would have
>> seen report if 640MB of low memory was not available for Dom0 and that was a
>> concern.
>>
>> So, to be honest, I think this is a non-issue. I am not saying this should not
>> be fixed. I am saying that the price is minimal compare to allow Xen booting
>> on platform such as the Hikey and bringing more compliance with the Arm Arm.
> 
> Make sense. Add some of these thoughts to the commit message so that
> this time we remember.

I will do.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel