Re: [PATCH v1 1/2] arm: Add ability to relocate Xen in over 4GB space

[Iurii Konovalenko <iurii.konovalenko@globallogic.com>]

Hi, Julien!

On Wed, Apr 8, 2015 at 7:05 PM, Julien Grall <julien.grall@citrix.com> wrote:

> The virtualization extension requires LPAE. Any reason to make this
> optional?
I agree with you - there is no real reason to make it optional. I will
rework patch
to include functionality without any flags by default.

>> +#ifdef ARM32_RELOCATE_OVER_4GB
>> +        ldr   r1, =_start
>> +        sub r4, r1
>> +        add r4, #BOOT_RELOC_VIRT_START
>> +#endif
>
> This chunk need some comment in order to explain what it's doing.
>
> AFAIU the resulting value in r4, will be exactly the same as  "ldr r4,
> =init_ttbr".
Not exactly. Virtual address of _start is 0x00200000, but
#BOOT_RELOC_VIRT_START is 0x00800000.
This part of code is needed to get value, that was "stashed by CPU 0" in
relocated copy of Xen when secondary CPUs run in non-relocated.

>> +    xen_relocation_offset = __pa(init_secondary) - xen_relocation_offset;
>
> The Xen may be relocated below the boot copy. So the final result may be
> negative.
This offset is used to get physical address of non-relocated variables
and write there values.
If it is negative, then calculating non-relocated address gives bigger
number (substraction of  negative value).
In addition, it's hard to imagine when it can be negative if we take
highest region of highest memory bank.

> ioremap_nocache may fail and "info" doesn't have a real meaning.
Ok, I will add verifying block and rename variable.

> Regards,
>
> --
> Julien Grall


Best regards.

Iurii Konovalenko | Senior Software Engineer
GlobalLogic