Re: [Xen-devel] [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
>>> On 19.01.17 at 12:37, wrote: > On Thu, Jan 19, 2017 at 02:09:37AM -0700, Jan Beulich wrote: >> >>> On 18.01.17 at 20:51, wrote: >> > Are we strictly guaranteed to have the entire image, including multiboot >> > tags, loaded below 4GB virtual even in the 64bit case? Even on non-EFI >> > capable grub2's ? >> >> I can't speak for GrUB, but I've recently learned that EFI alone >> doesn't appear to make such guarantees, and also doesn't look >> to have ways to request such address restricted loading of an > > Why? Due to lack of memory below 4 GiB, it being reserved for > something, EFI implementation bugs or anything else? I don't know the precise reason in that specific case, but I did go through the spec again after I had read that report, and I wasn't able to find anything regarding mandated or optional address restrictions. My recollection of there being such a restriction must have been based on the EFI sample implementation and/or the EDK. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
On Thu, Jan 19, 2017 at 02:09:37AM -0700, Jan Beulich wrote: > >>> On 18.01.17 at 20:51, wrote: > > On 18/01/17 14:17, Doug Goldstein wrote: > >> +.Lefi_multiboot2_proto: > >> +/* Zero EFI SystemTable and EFI ImageHandle addresses. */ > >> +xor %esi,%esi > >> +xor %edi,%edi > >> + > >> +/* Skip Multiboot2 information fixed part. */ > >> +lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx > >> +and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx > > > > Are we strictly guaranteed to have the entire image, including multiboot > > tags, loaded below 4GB virtual even in the 64bit case? Even on non-EFI > > capable grub2's ? > > I can't speak for GrUB, but I've recently learned that EFI alone GRUB is able to enforce 4 GiB restriction. At least right now. However, I am afraid that sooner or later machines will appear in the wild which do not have memory below 4 GiB (well, they must have some below 1 MiB but everything above is not a must as I know) or everything will be reserved/used by EFI boot/runtime services, etc. Then GRUB will also fail and we will be forced to extend multiboot2 proto further. However, it looks that current solution should not change much then. > doesn't appear to make such guarantees, and also doesn't look > to have ways to request such address restricted loading of an Why? Due to lack of memory below 4 GiB, it being reserved for something, EFI implementation bugs or anything else? Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
>>> On 18.01.17 at 20:51, wrote: > On 18/01/17 14:17, Doug Goldstein wrote: >> +.Lefi_multiboot2_proto: >> +/* Zero EFI SystemTable and EFI ImageHandle addresses. */ >> +xor %esi,%esi >> +xor %edi,%edi >> + >> +/* Skip Multiboot2 information fixed part. */ >> +lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx >> +and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx > > Are we strictly guaranteed to have the entire image, including multiboot > tags, loaded below 4GB virtual even in the 64bit case? Even on non-EFI > capable grub2's ? I can't speak for GrUB, but I've recently learned that EFI alone doesn't appear to make such guarantees, and also doesn't look to have ways to request such address restricted loading of an image. Hence there'll be work needed to make at least xen.efi cope with being loaded above that boundary. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
On 1/18/17 2:51 PM, Andrew Cooper wrote: > On 18/01/17 14:17, Doug Goldstein wrote: >> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S >> index d423fd8..ac93df0 100644 >> --- a/xen/arch/x86/boot/head.S >> +++ b/xen/arch/x86/boot/head.S >> @@ -89,6 +89,13 @@ multiboot2_header_start: >> 0, /* Number of the lines - no preference. */ \ >> 0 /* Number of bits per pixel - no preference. */ >> >> +/* Inhibit bootloader from calling ExitBootServices(). */ > > /* Request that ExitBootServices() not be called. */ > > This tag doesn't make any guarantees. > Agreed. The multiboot2 spec is clear in that regard. I have tested the case when the bootloader ignores this and it does work. >> +.code64 >> + >> +__efi64_start: > > __mb2_efi64_start: > > This entry point is distinct from the PE efi64 entry point in > common/efi/boot.c I agree here as well. It does need one other update however. On line 97. /* EFI64 entry point. */ mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \ sym_phys(__efi64_start) I can reroll the series or if people are comfortable with committing this then the change can be done then. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
On 18/01/17 14:17, Doug Goldstein wrote: > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S > index d423fd8..ac93df0 100644 > --- a/xen/arch/x86/boot/head.S > +++ b/xen/arch/x86/boot/head.S > @@ -89,6 +89,13 @@ multiboot2_header_start: > 0, /* Number of the lines - no preference. */ \ > 0 /* Number of bits per pixel - no preference. */ > > +/* Inhibit bootloader from calling ExitBootServices(). */ /* Request that ExitBootServices() not be called. */ This tag doesn't make any guarantees. > +.code64 > + > +__efi64_start: __mb2_efi64_start: This entry point is distinct from the PE efi64 entry point in common/efi/boot.c > +cld > + > +/* VGA is not available on EFI platforms. */ > +movl $0,vga_text_buffer(%rip) > + > +/* Check for Multiboot2 bootloader. */ > +cmp $MULTIBOOT2_BOOTLOADER_MAGIC,%eax > +je .Lefi_multiboot2_proto > + > +/* Jump to not_multiboot after switching CPU to x86_32 mode. */ > +lea not_multiboot(%rip),%edi > +jmp x86_32_switch > + > +.Lefi_multiboot2_proto: > +/* Zero EFI SystemTable and EFI ImageHandle addresses. */ > +xor %esi,%esi > +xor %edi,%edi > + > +/* Skip Multiboot2 information fixed part. */ > +lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx > +and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx Are we strictly guaranteed to have the entire image, including multiboot tags, loaded below 4GB virtual even in the 64bit case? Even on non-EFI capable grub2's ? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
From: Daniel Kiper This way Xen can be loaded on EFI platforms using GRUB2 and other boot loaders which support multiboot2 protocol. Signed-off-by: Daniel Kiper Reviewed-by: Doug Goldstein Tested-by: Doug Goldstein --- Doug v2 - dropped all my changes and moved them into their own patch Doug v1 - fix incorrect assembly (identified by Andrew Cooper) - fix issue where the trampoline size was left as 0 and the way the memory is allocated for the trampolines we would go to the end of an available section and then subtract off the size to decide where to place it. The end result was that we would always copy the trampolines and the 32-bit stack into some form of reserved memory after the conventional region we wanted to put things into. On some systems this did not manifest as a crash while on others it did. Reworked the changes to always reserve 64kb for both the stack and the size of the trampolines. Added an ASSERT to make sure we never blow through this size. v10 - suggestions/fixes: - replace ljmpl with lretq (suggested by Andrew Cooper), - introduce efi_platform to increase code readability (suggested by Andrew Cooper). v9 - suggestions/fixes: - use .L labels instead of numeric ones in multiboot2 data scanning loops (suggested by Jan Beulich). v8 - suggestions/fixes: - use __bss_start(%rip)/__bss_end(%rip) instead of of .startof.(.bss)(%rip)/$.sizeof.(.bss) because latter is not tested extensively in different built environments yet (suggested by Andrew Cooper), - fix multiboot2 data scanning loop in x86_32 code (suggested by Jan Beulich), - add check for extra mem for mbi data if Xen is loaded via multiboot2 protocol on EFI platform (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich). v7 - suggestions/fixes: - do not allocate twice memory for trampoline if we were loaded via multiboot2 protocol on EFI platform, - wrap long line (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich). v6 - suggestions/fixes: - improve label names in assembly error printing code (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - various minor cleanups and fixes (suggested by Jan Beulich). v4 - suggestions/fixes: - remove redundant BSS alignment, - update BSS alignment check, - use __set_bit() instead of set_bit() if possible (suggested by Jan Beulich), - call efi_arch_cpu() from efi_multiboot2() even if the same work is done later in other place right now (suggested by Jan Beulich), - xen/arch/x86/efi/stub.c:efi_multiboot2() fail properly on EFI platforms, - do not read data beyond the end of multiboot2 information in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - use 32-bit registers in x86_64 code if possible (suggested by Jan Beulich), - multiboot2 information address is 64-bit in x86_64 code, so, treat it is as is (suggested by Jan Beulich), - use cmovcc if possible, - leave only one space between rep and stosq (suggested by Jan Beulich), - improve error handling, - improve early error messages, (suggested by Jan Beulich), - improve early error messages printing code, - improve label names (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - various minor cleanups. v3 - suggestions/fixes: - take into account alignment when skipping multiboot2 fixed part (suggested by Konrad Rzeszutek Wilk), - improve segment registers initialization (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich and Konrad Rzeszutek Wilk), - improve commit message (suggested by Jan Beulich). v2 - suggestions/fixes: - generate multiboot2 header using macros (suggested by Jan Beulich), - switch CPU to x86_32 mode before jumping to 32-bit code (suggested by Andrew Cooper), - reduce code changes to increase patch readability (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - ignore MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO tag on EFI platform and find on my own multiboot2.mem_lower value, - stop execution if EFI platform is detected in legacy BIOS path. --- --- xen/arch/x86/boot/head.S | 263 +-- xen/arch/x86/efi/efi-boot.h | 54 +- xen/arch/x86/efi/stub.c | 38 - xen/arch/x86/x86_64/asm-offsets.c | 2 +- xen/arch/x86/xen.lds.S| 4 +- xen/common/efi/boot.c | 11 +- 6 files changed, 349 insertions(+), 23 deletions(-) diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index d423fd8..ac93df0 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -8
