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

From: Andrei Borzenkov <arvidjaar@gmail.com>
To: "Jan Beulich" <JBeulich@suse.com>
Cc: Juergen Gross <JGross@suse.com>,
	grub-devel@gnu.org, keir@xen.org, ian.campbell@citrix.com,
	stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com,
	Daniel Kiper <daniel.kiper@oracle.com>,
	roy.franz@linaro.org, ning.sun@intel.com,
	david.vrabel@citrix.com, phcoder@gmail.com,
	xen-devel@lists.xenproject.org, qiaowei.ren@intel.com,
	richard.l.maliszewski@intel.com, gang.wei@intel.com,
	fu.wei@linaro.org
Subject: Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms
Date: Sat, 14 Feb 2015 20:23:45 +0300	[thread overview]
Message-ID: <20150214202345.4c71bead@opensuse.site> (raw)
In-Reply-To: <54DB1EC4020000780005EDDB@mail.emea.novell.com>

В Wed, 11 Feb 2015 08:20:04 +0000
"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).
> 

grub2 relocates own modules when loading them. It does not do
relocation when loading Xen binary, but it does not sound impossible.

> > 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
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel