From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1YLJUk-0006PQ-Ix for mharc-grub-devel@gnu.org; Tue, 10 Feb 2015 17:41:26 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57303) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLJUi-0006L0-90 for grub-devel@gnu.org; Tue, 10 Feb 2015 17:41:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLJUe-0000x8-3e for grub-devel@gnu.org; Tue, 10 Feb 2015 17:41:24 -0500 Received: from ppsw-50.csi.cam.ac.uk ([131.111.8.150]:59738) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLJUd-0000w0-Lt for grub-devel@gnu.org; Tue, 10 Feb 2015 17:41:20 -0500 X-Cam-AntiVirus: no malware found X-Cam-ScannerInfo: http://www.cam.ac.uk/cs/email/scanner/ Received: from client-82-26-162-3.pete.adsl.virginm.net ([82.26.162.3]:59736 helo=[192.168.1.193]) by ppsw-50.csi.cam.ac.uk (smtp.hermes.cam.ac.uk [131.111.8.158]:587) with esmtpsa (PLAIN:amc96) (TLSv1.2:DHE-RSA-AES128-SHA:128) id 1YLJUU-0008VP-qQ (Exim 4.82_3-c0e5623) (return-path ); Tue, 10 Feb 2015 22:41:11 +0000 Message-ID: <54DA8901.7050603@citrix.com> Date: Tue, 10 Feb 2015 22:41:05 +0000 From: Andrew Cooper User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Daniel Kiper , xen-devel@lists.xenproject.org, grub-devel@gnu.org Subject: Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms References: <1422640462-28103-1-git-send-email-daniel.kiper@oracle.com> <1422640462-28103-19-git-send-email-daniel.kiper@oracle.com> <20150210212749.GG2227@olila.local.net-space.pl> In-Reply-To: <20150210212749.GG2227@olila.local.net-space.pl> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: Andrew Cooper X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 131.111.8.150 Cc: jgross@suse.com, keir@xen.org, ian.campbell@citrix.com, phcoder@gmail.com, stefano.stabellini@eu.citrix.com, roy.franz@linaro.org, ning.sun@intel.com, david.vrabel@citrix.com, jbeulich@suse.com, qiaowei.ren@intel.com, richard.l.maliszewski@intel.com, gang.wei@intel.com, fu.wei@linaro.org X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Feb 2015 22:41:25 -0000 On 10/02/2015 21:27, Daniel Kiper wrote: > On Fri, Jan 30, 2015 at 06:54:22PM +0100, Daniel Kiper wrote: >> Signed-off-by: Daniel Kiper >> --- >> 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. I have to admit to also being surprised at the fragility of the Xen, and how it cannot function if _start isn't loaded on the 1MB boundary. Given that there is provably one system in the wild which causes us to fall over this limitation, it clearly needs fixing. I would also agree that making the entry point relocatable is the correct way forwards. However, you cannot use something like bootsym_rel() as that requires infrastructure to patch the references (observe the loop over __trampoline_rel_{start,end} just before the call to cmdline_parse_early()) This can probably be achieved by having a small bit of hand-written PIC which puts an appropriate offset into %ds. ~Andrew