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

[Daniel Kiper]

On Fri, Jan 30, 2015 at 06:54:22PM +0100, Daniel Kiper wrote:
> Signed-off-by: Daniel Kiper <address@hidden>
> ---
>  xen/arch/x86/boot/head.S          |  174 
> +++++++++++++++++++++++++++++++++++--
>  xen/arch/x86/efi/efi-boot.h       |   29 +++++++
>  xen/arch/x86/setup.c              |   23 ++---
>  xen/arch/x86/x86_64/asm-offsets.c |    2 +
>  xen/common/efi/boot.c             |   11 +++
>  5 files changed, 222 insertions(+), 17 deletions(-)

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; So, it looks
that there are not any legacy BIOS machines with e.g. 1 MiB - 2 MiB region
reserved in the wild or they are not very common; Am I missing something?).
Sadly, this region is used by BS, so, GRUB2 loads Xen higher (at least
above 64 MiB). Please look at memory map on this machine:

Type       Start            End              # Pages          Attributes
BS_Data    0000000000010000-000000000009FFFF 0000000000000090 000000000000000F
BS_Data    0000000000100000-0000000003FFFFFF 0000000000003F00 000000000000000F
Available  0000000004000000-000000000FFFEFFF 000000000000BFFF 000000000000000F
BS_Code    000000000FFFF000-000000001006CFFF 000000000000006E 000000000000000F
Available  000000001006D000-00000000B3E73FFF 00000000000A3E07 000000000000000F

[...]

Additionally, early Xen boot code maps only first 16 MiB of memory. Hence,
it means that jump into __high_start fails immediately.

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.

Daniel

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.