Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms

["Jan Beulich" <JBeulich@suse.com>]

>>> On 10.02.15 at 22:27, <daniel.kiper@oracle.com> wrote:
> After some testing we have found at least one machine on which this thing
> does not work. It is Dell PowerEdge R820 with latest firmware. Machine
> crashes/stops because early 32-bit code is not relocatable and must live
> under 0x100000 address. (side note: I am surprised how it worked without
> any issue until now; Multiboot protocol, any version, does not guarantee
> that OS image will be loaded at specified/requested address;

How does it not? It's an ELF binary without relocations that's being
loaded - I can't see how such could be validly loaded anywhere but
at the virtual address(es) its program header states (and I don't
know whether grub [1 or 2] would correctly process relocations if
there were any, but I doubt it).

> Now I see two solutions for these issues:
> 
> 1) We can make early 32-bit code relocatable. We may use something similar
>    to xen/arch/x86/boot/trampoline.S:bootsym_rel(). Additionally, I think
>    that early code should not blindly map first 16 MiB of memory. It should
>    map first 1 MiB of memory and then 16 MiB of memory starting from
>    xen_phys_start. This way we also fix long standing bug in early code
>    which I described earlier.
> 
> 2) We can jump from EFI x86-64 mode directly into "Xen x86-64 mode" like
>    it is done in case of EFI loader. However, then we must duplicate 
> multiboot2
>    protocol implementation in x86-64 mode (if we wish that multiboot2 
> protocol
>    can be used on legacy BIOS and EFI platforms; I think that we should 
> support
>    this protocol on both for users convenience). Additionally, we must use
>    a workaround to relocate trampoline if boot services uses memory below 1 
> MiB
>    (please check commit c1f2dfe8f6a559bc28935f24e31bb33d17d9713d, x86/EFI: 
> make
>    trampoline allocation more flexible, for more details).
> 
> I prefer #1 because this way we do not duplicate multiboot2 protocol 
> implementation
> (one for legacy BIOS and EFI) and we avoid issues with trampoline relocation 
> when
> low memory is occupied by boot services and/or 1:1 EFI page tables.

Between the two, 1 is certainly the preferable option.

> PS I have just realized that commit c1f2dfe8f6a559bc28935f24e31bb33d17d9713d
>    will not work if trampoline code will overwrite some of EFI 1:1 page 
> tables.
>    Dell PowerEdge R820 store part of 1:1 page tables below 1 MiB. Xen loaded
>    by native EFI loader boots but it is only lucky coincidence that it does
>    not overwrite used entries. So, I tend to go and choose #1 even more.

How awful a firmware implementation! On PC-like systems, _nothing_
that _absolutely_ has to be below 1Mb should be placed there.

Jan