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

2018-12-14 Thread Andrii Anisov

Hello Julien,

Let me speculate a bit about the topic.

On 14.12.18 13:44, Julien Grall wrote:

At the moment, Xen is relocated towards the end of the memory.

This statement is not really true. Some time ago, XEN was relocated toward the 
end of
the low memory (under 4 GB). Currently, on my board I see some kind of mess:

(XEN) RAM: 4800 - bfff
(XEN) RAM: 0005 - 00057fff
(XEN) RAM: 0006 - 00067fff
(XEN) RAM: 0007 - 00077fff
(XEN)
(XEN) MODULE[0]: 4800 - 48013000 Device Tree
(XEN) MODULE[1]: 7a00 - 7c00 Kernel
(XEN) MODULE[2]: 7c00 - 7c01 XSM
(XEN)  RESVD[0]: 4800 - 48013000
(XEN)
(XEN)
(XEN) Command line: dom0_mem=3G console=dtuart dtuart=serial0 
dom0_max_vcpus=2 bootscrub=0 loglvl=all cpufreq=none tbuf_size=8192 
loglvl=all/none guest_loglvl=all/none
(XEN) parameter "cpufreq" unknown!
(XEN) Placing Xen at 0x00077fe0-0x00078000
(XEN) Update BOOTMOD_XEN from 7808-78188d81 => 
00077fe0-00077ff08d81

As you can see XEN is moved towards the end of the first GB of the low memory 
instead of the end of under 4GB RAM.



While
this has the advantage to free space in low memory, the code is not
compliant with the break-before-make because it requires to switch
between two sets of page-table. This is not entirely trivial to fix as
it would require us to go through an identity mapping and disabling MMU.

I understand this motivation though.


Furthermore, it looks like that some platform (such as the Hikey960)
may not be able to bring-up secondary CPUs if the entry is too high.Just a 
reminder that long time ago we implemented a move of XEN toward the real end of 
the memory, over 4GB.

As long as CPUs were not able to start to the code placed over 4 GB, we set 
secondary CPUs to be brought up to a XEN instance under 4GB, then jump to a 
copy over 4GB, following CPU0.


I don't believe the low memory is an issue because Xen is quite tiny
(< 2MB).

It is really tiny, but the problem is that Dom0 low memory (lower than 4 GB) 
RAM banks
start and end are aligned by 128MB. So existing of a single 1MB XEN cuts out 
128MB from low memory for Dom0.
On my current setup I have two 128MB chunks stolen: one by relocated XEN, 
kernel module an XSM module, other another by device tree. So Dom0 gets 1664MB 
of low RAM, instead of physically available 1920MB.

(XEN) Loading Domd0 kernel from boot module @ 7a00
(XEN) Allocating 1:1 mappings totalling 3072MB for dom0:
(XEN) BANK[0] 0x005000-0x007800 (640MB)
(XEN) BANK[1] 0x008000-0x00c000 (1024MB)
(XEN) BANK[2] 0x054000-0x058000 (1024MB)
(XEN) BANK[3] 0x074800-0x076000 (384MB)
(XEN) Grant table range: 0x077fe0-0x077fe4

Such loss might be painful for those who are targeting low memory eager 
use-cases (i.e. multimedia) and lack of IOMMU on a SoC.


So the best solution is to stop relocating Xen.

And those who cares about XEN placement should configure their bootloader to 
put XEN (and other boot modules) to a proper place right away.


This has the advantage to simplify the code and should speed-up the boot as 
relocation
is not necessary anymore.

Boot time improvements always make me glad :)

Please also note, that all above are kind of generic ideas. They are not linked 
to our target setup, we do not care about Dom0 and its 1:1 mapped memory. And, 
all in all, we have an IOMMU.

--
Sincerely,
Andrii Anisov.

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

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

2018-12-14 Thread Andrii Anisov



On 14.12.18 17:52, Andrii Anisov wrote:

Hello Julien,

Let me speculate a bit about the topic.

On 14.12.18 13:44, Julien Grall wrote:

At the moment, Xen is relocated towards the end of the memory.

This statement is not really true. Some time ago, XEN was relocated toward the 
end of
the low memory (under 4 GB). Currently, on my board I see some kind of mess:

     (XEN) RAM: 4800 - bfff
     (XEN) RAM: 0005 - 00057fff
     (XEN) RAM: 0006 - 00067fff
     (XEN) RAM: 0007 - 00077fff
     (XEN)
     (XEN) MODULE[0]: 4800 - 48013000 Device Tree
     (XEN) MODULE[1]: 7a00 - 7c00 Kernel
     (XEN) MODULE[2]: 7c00 - 7c01 XSM
     (XEN)  RESVD[0]: 4800 - 48013000
     (XEN)
     (XEN)
     (XEN) Command line: dom0_mem=3G console=dtuart dtuart=serial0 
dom0_max_vcpus=2 bootscrub=0 loglvl=all cpufreq=none tbuf_size=8192 
loglvl=all/none guest_loglvl=all/none
     (XEN) parameter "cpufreq" unknown!
     (XEN) Placing Xen at 0x00077fe0-0x00078000
     (XEN) Update BOOTMOD_XEN from 7808-78188d81 => 
00077fe0-00077ff08d81

As you can see XEN is moved towards the end of the first GB of the low memory 
instead of the end of under 4GB RAM.


Sorry, I have to recall the piece above. I miscounted zeros in the relocation 
address. XEN is actually set by the end of the RAM above 4GB.


On my current setup I have two 128MB chunks stolen: one by relocated XEN, kernel module an XSM module, 

Not by relocated XEN, but still by kernel and XSM

--
Sincerely,
Andrii Anisov.

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

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

2018-12-14 Thread Andrii Anisov

On 14.12.18 13:44, Julien Grall wrote:

At the moment, Xen is relocated towards the end of the memory. While
this has the advantage to free space in low memory, the code is not
compliant with the break-before-make because it requires to switch
between two sets of page-table. This is not entirely trivial to fix as
it would require us to go through an identity mapping and disabling MMU.

Furthermore, it looks like that some platform (such as the Hikey960)
may not be able to bring-up secondary CPUs if the entry is too high.

I don't believe the low memory is an issue because Xen is quite tiny
(< 2MB). So the best solution is to stop relocating Xen. This has the
advantage to simplify the code and should speed-up the boot as relocation
is not necessary anymore.

Note that the break-before-make issue is not fixed by this patch.

Signed-off-by: Julien Grall 
Reported-by: Matthew Daley 
Tested-by: Matthew Daley 


Tested-by: Andrii Anisov 
Reviewed-by: Andrii Anisov 

--
Sincerely,
Andrii Anisov.

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

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

2018-12-14 Thread Julien Grall


On 12/14/18 3:52 PM, Andrii Anisov wrote:
> Hello Julien,

Hi,

>
> Let me speculate a bit about the topic.
>
> On 14.12.18 13:44, Julien Grall wrote:
>> At the moment, Xen is relocated towards the end of the memory.
> This statement is not really true. Some time ago, XEN was relocated
> toward the end of
> the low memory (under 4 GB).
Are you using arm64 or arm32? Arm64 does not have the 4GB limit while Arm32 has.
I could make the difference in the commit message but this would add more
confusion below.

> Currently, on my board I see some kind of
> mess:
>
>  (XEN) RAM: 4800 - bfff
>  (XEN) RAM: 0005 - 00057fff
>  (XEN) RAM: 0006 - 00067fff
>  (XEN) RAM: 0007 - 00077fff
>  (XEN)
>  (XEN) MODULE[0]: 4800 - 48013000 Device Tree
>  (XEN) MODULE[1]: 7a00 - 7c00 Kernel
>  (XEN) MODULE[2]: 7c00 - 7c01 XSM
>  (XEN)  RESVD[0]: 4800 - 48013000
>  (XEN)
>  (XEN)
>  (XEN) Command line: dom0_mem=3G console=dtuart dtuart=serial0
> dom0_max_vcpus=2 bootscrub=0 loglvl=all cpufreq=none tbuf_size=8192
> loglvl=all/none guest_loglvl=all/none
>  (XEN) parameter "cpufreq" unknown!
>  (XEN) Placing Xen at 0x00077fe0-0x00078000
>  (XEN) Update BOOTMOD_XEN from 7808-78188d81 =>
> 00077fe0-00077ff08d81
>
> As you can see XEN is moved towards the end of the first GB of the low
> memory instead of the end of under 4GB RAM.

I don't understand what you mean. Looking at your log, Xen is relocated at the
end of the last bank. This is the expected behavior in unstable.
>> While
>> this has the advantage to free space in low memory, the code is not
>> compliant with the break-before-make because it requires to switch
>> between two sets of page-table. This is not entirely trivial to fix as
>> it would require us to go through an identity mapping and disabling MMU.
> I understand this motivation though.
>
>> Furthermore, it looks like that some platform (such as the Hikey960)
>> may not be able to bring-up secondary CPUs if the entry is too
>> high.Just a reminder that long time ago we implemented a move of XEN
>> toward the real end of the memory, over 4GB.

This not the first time part of answer is mangled with my e-mail. This is making
really difficult to follow the conversion. Can you please configure your e-mail
client to do proper quote/reply?

> As long as CPUs were not able to start to the code placed over 4 GB, we
> set secondary CPUs to be brought up to a XEN instance under 4GB, then
> jump to a copy over 4GB, following CPU0.

How were you switching between the page-tables? A proper solution would require
to switch between page-tables using an identify mappings. This is far more
complicate than what is worth here.

>
>> I don't believe the low memory is an issue because Xen is quite tiny
>> (< 2MB).
> It is really tiny, but the problem is that Dom0 low memory (lower than 4
> GB) RAM banks
> start and end are aligned by 128MB. So existing of a single 1MB XEN cuts
> out 128MB from low memory for Dom0.

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?

> On my current setup I have two 128MB chunks stolen: one by relocated
> XEN, kernel module an XSM module, other another by device tree.

Xen is free to allocate anything below 4GB. This is nothing new. But you should
not have two 128MB chunks stolen because of modules here. If that's the case
then there is a bug in Xen that should be fixed.

Cheers,

--
Julien Grall
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

2018-12-14 Thread Andrii Anisov



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

--
Sincerely,
Andrii Anisov.

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

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

2018-12-14 Thread Andrii Anisov



On 14.12.18 18:26, Julien Grall wrote:

Are you using arm64 or arm32?


I'm using arm64.
But also speak about arm32.


I don't understand what you mean. Looking at your log, Xen is relocated at the
end of the last bank. This is the expected behavior in unstable.


Yes, I miscounted zeroes, and recalled that part before your answer  ;)


This not the first time part of answer is mangled with my e-mail. This is making
really difficult to follow the conversion. Can you please configure your e-mail
client to do proper quote/reply?


OK. Will check it.


How were you switching between the page-tables? A proper solution would require
to switch between page-tables using an identify mappings. This is far more
complicate than what is worth here.


Sorry for my ignorance, what "identify mappings" stands for? Any links for the 
problem description?
I don't remember such details of implementation but pretty sure we didn't mean 
that thing. It was about 3 years ago in a different company on a arm32 SoC.


Xen is free to allocate anything below 4GB. This is nothing new. But you should
not have two 128MB chunks stolen because of modules here. If that's the case
then there is a bug in Xen that should be fixed.


They are stolen from 1:1 memory allocation because of `allocate_memory_11()` 
design and implementation.

--
Sincerely,
Andrii Anisov.

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

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

2018-12-14 Thread Julien Grall

Hi,

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.


Cheers,

[1] Documentation/arm/Booting
[2] Documentation/arm64/booting.txt

--
Julien Grall

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

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

2018-12-14 Thread Julien Grall

Hi,

On 14/12/2018 17:46, Andrii Anisov wrote:

How were you switching between the page-tables? A proper solution would require
to switch between page-tables using an identify mappings. This is far more
complicate than what is worth here.


Sorry for my ignorance, what "identify mappings" stands for?


I meant identity mapping sorry.

Any links for the 
problem description?


Have a look at "Using break-before-make when updating translation table entries" 
D5-2516 in ARM DDI 0487D.a. The underlying issue is you can't change the mapping 
with going through an invalid state. So if you switch between 2 page-tables you 
need roughly to:

- Disable MMU
- Flush TLBs
- Switch page-tables
- Enable MMU

The new set of page-tables needs to contain an identity mapping corresponding to 
where Xen is loaded in memory. Such mapping is quite difficult to have on Xen 
because the memory layout is static.


Just in case, I know that Xen does not respect that today. We are quite lucky so 
far that this didn't result to weird behavior when running Xen. Although, I am 
starting to see more report of interesting crash with newer processor. Removing 
the relocation is a first step towards avoiding to switch between page-tables.



Xen is free to allocate anything below 4GB. This is nothing new. But you should
not have two 128MB chunks stolen because of modules here. If that's the case
then there is a bug in Xen that should be fixed.


They are stolen from 1:1 memory allocation because of `allocate_memory_11()` 
design and implementation.


Then the code needs to be fixed... It would be nice to get some helps here as I 
can't scale.


Cheers,

--
Julien Grall

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

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

2018-12-17 Thread Julien Grall

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]: f2afb000 - f2b02000 Device Tree
(XEN) MODULE[1]: f695b000 - f7f6fa00 Kernel
(XEN) MODULE[2]: f2c23000 - f6959200 Ramdisk

[...]

(XEN) Placing Xen at 0x00099be0-0x00099c00
(XEN) Update BOOTMOD_XEN from f2b02000-f2c22d81 => 00099be00

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.


Cheers,



Cheers,

[1] Documentation/arm/Booting
[2] Documentation/arm64/booting.txt



--
Julien Grall

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

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

2018-12-17 Thread Andrii Anisov



On 14.12.18 19:48, Julien Grall wrote:

Please try to reference the documentation (or code if lack of documentation) 
when making such statement.


Ah, yes, sure.


AFAICT, Linux 32-bit [1] imposes the kernel to be loaded in the first 128MB of 
RAM. Nothing about the 128MB aligned RAM.


Right, the documentation sets a restriction for a compressed image to be loaded 
in the first 128MB of RAM. But the comment in a decompressor code (and the code 
itself) explicitly refer alignment [11].
Calculating a RAM start address with this mask [22] give us a wrong value if 
the real physical address is 64MB aligned, not 128MB. Then crash on 
decompressing. You know, I stepped into that with J6 (arm32) while setting up 
thin Dom0 with only 64MB RAM.


Linux 64-bit [1] requires to be loaded at a 2MB aligned address.


Great, 64-bit Linux documentation directly refers the alignment :)


So technically allocating the RAM using a 2MB alignment should be enough.


For 64-bit and, maybe, raw 32-bit Linux kernel images.
For 32-bit compressed Linux kernel - still 128MB aligned first bank is 
required. It can be changed on kernel side by setting ZRELADDR, but we can't 
designate that from XEN runtime.


Yet we need to make sure the first bank is at least 128MB.


Well, I'm not sure the ARM64 documentation [33] or implementation mention the 
size of the first bank. Except it should be enough to hold the kernel image 
[44].
Also I would not treat [55] as a strict requirement to have 128MB in the first 
bank. But we can stick at that to make things easier.


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.


I'm totally agree that it is not related to the code changes done by the patch. 
But leads to the patch message change.


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. 


It might work for the current code.


I can add this recommendation in the commit message and potentially 
documentation.


I vote for this.

[11] 
https://elixir.bootlin.com/linux/v4.20-rc7/source/arch/arm/boot/compressed/head.S#L217
[22] 
https://elixir.bootlin.com/linux/v4.20-rc7/source/arch/arm/boot/compressed/head.S#L236
[33] 
https://elixir.bootlin.com/linux/v4.20-rc7/source/Documentation/arm64/booting.txt
[44] 
https://elixir.bootlin.com/linux/v4.20-rc7/source/Documentation/arm64/booting.txt#L129
[55] 
https://elixir.bootlin.com/linux/v4.20-rc7/source/Documentation/arm/Booting#L160

[1] Documentation/arm/Booting
[2] Documentation/arm64/booting.txt



--
Sincerely,
Andrii Anisov.

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

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

2018-12-17 Thread Julien Grall

Hi,

On 17/12/2018 13:14, Andrii Anisov wrote:



On 14.12.18 19:48, Julien Grall wrote:
Please try to reference the documentation (or code if lack of documentation) 
when making such statement.


Ah, yes, sure.

AFAICT, Linux 32-bit [1] imposes the kernel to be loaded in the first 128MB of 
RAM. Nothing about the 128MB aligned RAM.


Right, the documentation sets a restriction for a compressed image to be loaded 
in the first 128MB of RAM. But the comment in a decompressor code (and the code 
itself) explicitly refer alignment [11].
Calculating a RAM start address with this mask [22] give us a wrong value if the 
real physical address is 64MB aligned, not 128MB. Then crash on decompressing. 
You know, I stepped into that with J6 (arm32) while setting up thin Dom0 with 
only 64MB RAM.


Thank you for the pointer.




Linux 64-bit [1] requires to be loaded at a 2MB aligned address.


Great, 64-bit Linux documentation directly refers the alignment :)


So technically allocating the RAM using a 2MB alignment should be enough.


For 64-bit and, maybe, raw 32-bit Linux kernel images.
For 32-bit compressed Linux kernel - still 128MB aligned first bank is required.
It can be changed on kernel side by setting ZRELADDR, but we can't designate 
that from XEN runtime.


I also don't think this would be a good approach as we still want to keep the 
kernel as much as possible position independent.





Yet we need to make sure the first bank is at least 128MB.


Well, I'm not sure the ARM64 documentation [33] or implementation mention the 
size of the first bank. Except it should be enough to hold the kernel image [44].
Also I would not treat [55] as a strict requirement to have 128MB in the first 
bank. But we can stick at that to make things easier.


The size of the first bank is implicit on arm32. If you look at the 
Documentation/arm32/Booting.txt, the DTB/initramfs should be loaded just above 
128MB to avoid the decompressor to avoid overwrite them. So technically, we 
should allow more than 128MB for the first bank.


At the moment, the algorithm to load 64-bit and 32-bit Image are the same. Hence 
why I said we need at least 128MB in the first bank. I am open for using 
different algorithm if Dom0 should be smaller.


Cheers,

--
Julien Grall

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

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

2018-12-17 Thread Andrii Anisov

On 14.12.18 20:04, Julien Grall wrote:

Then the code needs to be fixed... It would be nice to get some helps here as I 
can't scale.


I can take this.
But I would like to align on the algorithm first.

--
Sincerely,
Andrii Anisov.

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

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

2018-12-17 Thread Andrii Anisov



On 17.12.18 15:38, Julien Grall wrote:

So technically allocating the RAM using a 2MB alignment should be enough.


For 64-bit and, maybe, raw 32-bit Linux kernel images.
For 32-bit compressed Linux kernel - still 128MB aligned first bank is required.
It can be changed on kernel side by setting ZRELADDR, but we can't designate 
that from XEN runtime.


I also don't think this would be a good approach as we still want to keep the 
kernel as much as possible position independent.


I suppose you get me wrong.
I'm saying that 128MB alignment requirement for RAM start might be relaxed by 
kernel itself, if it has set ZRELADDR. But we, from XEN side, can not rely on 
that, nor detect that.
So we must follow RAM start alignment for compressed Linux kernel images (32 
bit).


Yet we need to make sure the first bank is at least 128MB.


Well, I'm not sure the ARM64 documentation [33] or implementation mention the 
size of the first bank. Except it should be enough to hold the kernel image 
[44].
Also I would not treat [55] as a strict requirement to have 128MB in the first 
bank. But we can stick at that to make things easier.


The size of the first bank is implicit on arm32. If you look at the 
Documentation/arm32/Booting.txt, the DTB/initramfs should be loaded just above 
128MB to avoid the decompressor to avoid overwrite them.


It is rather a recommendation. A good way to choose the place for initramfs and 
dtb. Because decompressor is pretty dumb and limited by 128MB addresses range.
And on XEN side we follow this recommendation whenever it is possible.


So technically, we should allow more than 128MB for the first bank


Although, it is not said that DTB or initramfs should reside in the first RAM 
bank ;)


At the moment, the algorithm to load 64-bit and 32-bit Image are the same. 
Hence why I said we need at least 128MB in the first bank. I am open for using 
different algorithm if Dom0 should be smaller.


The algorithm is the same, but it is buggy right now for smaller Dom0 sizes. 
The lines

const unsigned int min_low_order =
get_order_from_bytes(min_t(paddr_t, dom0_mem, MB(128)));

in `allocate_memory_11()` prone to end up with a wrong RAM start alignment.


--
Sincerely,
Andrii Anisov.

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

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

2018-12-17 Thread Andrii Anisov



On 17.12.18 13:11, Julien Grall wrote:

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.


BTW, I hope you already noticed I've passed my RB and TB for the patch.
I would prefer you reflect last findings in the commit message, but if you 
don't like - let it be as is.

--
Sincerely,
Andrii Anisov.

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

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

2018-12-17 Thread Julien Grall

Hi Andrii,

On 17/12/2018 16:02, Andrii Anisov wrote:



On 17.12.18 13:11, Julien Grall wrote:
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.


BTW, I hope you already noticed I've passed my RB and TB for the patch.


I have seen them. Thank you for the review and tested-by.

I would prefer you reflect last findings in the commit message, but if you don't 
like - let it be as is.


I am planning to reflect this in the commit message. I am just waiting on the 
discussion to come to a conclusion before resending it.


Cheers,

--
Julien Grall

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

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

2018-12-17 Thread Julien Grall

Hi,

On 17/12/2018 15:55, Andrii Anisov wrote:



On 17.12.18 15:38, Julien Grall wrote:

So technically allocating the RAM using a 2MB alignment should be enough.


For 64-bit and, maybe, raw 32-bit Linux kernel images.
For 32-bit compressed Linux kernel - still 128MB aligned first bank is required.
It can be changed on kernel side by setting ZRELADDR, but we can't designate 
that from XEN runtime.


I also don't think this would be a good approach as we still want to keep the 
kernel as much as possible position independent.


I suppose you get me wrong.
I'm saying that 128MB alignment requirement for RAM start might be relaxed by 
kernel itself, if it has set ZRELADDR. But we, from XEN side, can not rely on 
that, nor detect that.

So we must follow RAM start alignment for compressed Linux kernel images (32 
bit).


I didn't get you wrong. We are saying the same things :).




Yet we need to make sure the first bank is at least 128MB.


Well, I'm not sure the ARM64 documentation [33] or implementation mention the 
size of the first bank. Except it should be enough to hold the kernel image 
[44].
Also I would not treat [55] as a strict requirement to have 128MB in the 
first bank. But we can stick at that to make things easier.


The size of the first bank is implicit on arm32. If you look at the 
Documentation/arm32/Booting.txt, the DTB/initramfs should be loaded just above 
128MB to avoid the decompressor to avoid overwrite them.


It is rather a recommendation. A good way to choose the place for initramfs and 
dtb. Because decompressor is pretty dumb and limited by 128MB addresses range.

And on XEN side we follow this recommendation whenever it is possible.


So technically, we should allow more than 128MB for the first bank


Although, it is not said that DTB or initramfs should reside in the first RAM 
bank ;)


The documentation suggest to load the DTB and initramfs just above 128MB. I am 
not entirely how the kernel would behave if you put it in a separate bank far after.


Similarly, some version on Linux (i.e prior to 4.2) requires the DTB to within 
512MB from the kernel.




At the moment, the algorithm to load 64-bit and 32-bit Image are the same. 
Hence why I said we need at least 128MB in the first bank. I am open for using 
different algorithm if Dom0 should be smaller.


The algorithm is the same, but it is buggy right now for smaller Dom0 sizes. The 
lines


     const unsigned int min_low_order =
     get_order_from_bytes(min_t(paddr_t, dom0_mem, MB(128)));

in `allocate_memory_11()` prone to end up with a wrong RAM start alignment.


Patches are welcomed.

Cheers,

--
Julien Grall

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

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

2018-12-17 Thread Julien Grall

Hi Andrii,

On 17/12/2018 15:54, Andrii Anisov wrote:

On 14.12.18 20:04, Julien Grall wrote:
Then the code needs to be fixed... It would be nice to get some helps here as 
I can't scale.


I can take this.


Thank you.


But I would like to align on the algorithm first.


It is probably worth to start a separate discussion with your thoughts.

Cheers,

--
Julien Grall

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

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

2018-12-17 Thread Andrii Anisov



On 17.12.18 19:02, Julien Grall wrote:

I didn't get you wrong. We are saying the same things :).


Great!


Similarly, some version on Linux (i.e prior to 4.2) requires the DTB to within 
512MB from the kernel.


I've seen that restriction in the Linux for ARM64 documentation.


in `allocate_memory_11()` prone to end up with a wrong RAM start alignment.


Patches are welcomed.

I see something like following as a quick WA (not even build tested):

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d2c63a8..bf72ba9 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -223,8 +223,9 @@ fail:
  * meet these requirements directly. So instead of proceed as follows:
  *
  * We first allocate the largest allocation we can as low as we
- * can. This then becomes the first bank. This bank must be at least
- * 128MB (or dom0_mem if that is smaller).
+ * can. This then becomes the first bank. This bank is at least 128MB even if
+ * dom0 is configured for less. It is the way to get that bank 128MB aligned,
+ * what is required for 32-bit zImage.
  *
  * Then we start allocating more memory, trying to allocate the
  * largest possible size and trying smaller sizes until we
@@ -253,7 +254,7 @@ static void __init allocate_memory_11(struct domain *d,
   struct kernel_info *kinfo)
 {
 const unsigned int min_low_order =
-get_order_from_bytes(min_t(paddr_t, dom0_mem, MB(128)));
+get_order_from_bytes(MB(128));
 const unsigned int min_order = get_order_from_bytes(MB(4));
 struct page_info *pg;
 unsigned int order = get_allocation_size(kinfo->unassigned_mem);
@@ -268,6 +269,10 @@ static void __init allocate_memory_11(struct domain *d,
  */
 BUG_ON(!is_domain_direct_mapped(d));
 
+if ( dom0_mem < MB(128))

+printk(XENLOG_WARNING "Allocating 128MB for Domain0 with 
%"PRIu64"MB\n",
+   dom0_mem/MB(1));
+
 printk("Allocating 1:1 mappings totalling %ldMB for dom0:\n",
/* Don't want format this as PRIpaddr (16 digit hex) */
(unsigned long)(kinfo->unassigned_mem >> 20));


But I'm not sure if it worth to be sent, because I'm going to rewrite it soon.

--
Sincerely,
Andrii Anisov.

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

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

2018-12-17 Thread Stefano Stabellini
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]: f2afb000 - f2b02000 Device Tree
> (XEN) MODULE[1]: f695b000 - f7f6fa00 Kernel
> (XEN) MODULE[2]: f2c23000 - f6959200 Ramdisk
> 
> [...]
> 
> (XEN) Placing Xen at 0x00099be0-0x00099c00
> (XEN) Update BOOTMOD_XEN from f2b02000-f2c22d81 =>
> 00099be00
> 
> 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.


> > Cheers,
> > 
> > [1] Documentation/arm/Booting
> > [2] Documentation/arm64/booting.txt


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

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

2018-12-17 Thread Julien Grall

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]: f2afb000 - f2b02000 Device Tree
(XEN) MODULE[1]: f695b000 - f7f6fa00 Kernel
(XEN) MODULE[2]: f2c23000 - f6959200 Ramdisk

[...]

(XEN) Placing Xen at 0x00099be0-0x00099c00
(XEN) Update BOOTMOD_XEN from f2b02000-f2c22d81 =>
00099be00

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

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

2018-12-18 Thread Andrii Anisov

Hello Julien,

On 17.12.18 19:34, Andrii Anisov wrote:

I see something like following as a quick WA (not even build tested):

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d2c63a8..bf72ba9 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -223,8 +223,9 @@ fail:
   * meet these requirements directly. So instead of proceed as follows:
   *
   * We first allocate the largest allocation we can as low as we
- * can. This then becomes the first bank. This bank must be at least
- * 128MB (or dom0_mem if that is smaller).
+ * can. This then becomes the first bank. This bank is at least 128MB even if
+ * dom0 is configured for less. It is the way to get that bank 128MB aligned,
+ * what is required for 32-bit zImage.
   *
   * Then we start allocating more memory, trying to allocate the
   * largest possible size and trying smaller sizes until we
@@ -253,7 +254,7 @@ static void __init allocate_memory_11(struct domain *d,
    struct kernel_info *kinfo)
  {
  const unsigned int min_low_order =
-    get_order_from_bytes(min_t(paddr_t, dom0_mem, MB(128)));
+    get_order_from_bytes(MB(128));
  const unsigned int min_order = get_order_from_bytes(MB(4));
  struct page_info *pg;
  unsigned int order = get_allocation_size(kinfo->unassigned_mem);
@@ -268,6 +269,10 @@ static void __init allocate_memory_11(struct domain *d,
   */
  BUG_ON(!is_domain_direct_mapped(d));

+    if ( dom0_mem < MB(128))
+    printk(XENLOG_WARNING "Allocating 128MB for Domain0 with 
%"PRIu64"MB\n",
+   dom0_mem/MB(1));
+
  printk("Allocating 1:1 mappings totalling %ldMB for dom0:\n",
     /* Don't want format this as PRIpaddr (16 digit hex) */
     (unsigned long)(kinfo->unassigned_mem >> 20));


But I'm not sure if it worth to be sent, because I'm going to rewrite it soon.


From the second thought, we have last posting date for 4.12 already in the 
past. So redesign will not go to 4.12.
Maybe it worth to have the thing above as a bugfix for 4.12?
What do you think?

--
Sincerely,
Andrii Anisov.

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

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

2018-12-18 Thread Julien Grall



On 12/18/18 12:09 PM, Andrii Anisov wrote:

Hello Julien,


Hi,


On 17.12.18 19:34, Andrii Anisov wrote:

I see something like following as a quick WA (not even build tested):

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d2c63a8..bf72ba9 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -223,8 +223,9 @@ fail:
   * meet these requirements directly. So instead of proceed as follows:
   *
   * We first allocate the largest allocation we can as low as we
- * can. This then becomes the first bank. This bank must be at least
- * 128MB (or dom0_mem if that is smaller).
+ * can. This then becomes the first bank. This bank is at least 128MB 
even if
+ * dom0 is configured for less. It is the way to get that bank 128MB 
aligned,

+ * what is required for 32-bit zImage.
   *
   * Then we start allocating more memory, trying to allocate the
   * largest possible size and trying smaller sizes until we
@@ -253,7 +254,7 @@ static void __init allocate_memory_11(struct 
domain *d,

    struct kernel_info *kinfo)
  {
  const unsigned int min_low_order =
-    get_order_from_bytes(min_t(paddr_t, dom0_mem, MB(128)));
+    get_order_from_bytes(MB(128));
  const unsigned int min_order = get_order_from_bytes(MB(4));
  struct page_info *pg;
  unsigned int order = get_allocation_size(kinfo->unassigned_mem);
@@ -268,6 +269,10 @@ static void __init allocate_memory_11(struct 
domain *d,

   */
  BUG_ON(!is_domain_direct_mapped(d));

+    if ( dom0_mem < MB(128))
+    printk(XENLOG_WARNING "Allocating 128MB for Domain0 with 
%"PRIu64"MB\n",

+   dom0_mem/MB(1));
+
  printk("Allocating 1:1 mappings totalling %ldMB for dom0:\n",
 /* Don't want format this as PRIpaddr (16 digit hex) */
 (unsigned long)(kinfo->unassigned_mem >> 20));


But I'm not sure if it worth to be sent, because I'm going to rewrite 
it soon.


 From the second thought, we have last posting date for 4.12 already in 
the past. So redesign will not go to 4.12.

Maybe it worth to have the thing above as a bugfix for 4.12?
What do you think?


AFAICT, it was possible to boot a Dom0 with only 64MB on Arm64. So I am 
not entirely why we would also limit the size there.


For Arm32, I think we should just return an error and fail the domain 
build. If you ask 64MB and we give you 128MB then something is already 
really wrong.


Anyway, I think this should be submitted properly to discuss for Xen 
4.12 inclusion.


Cheers,

--
Julien Grall

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