From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH for-4.5 V9] Add ARM EFI boot support Date: Tue, 30 Sep 2014 13:20:06 -0400 Message-ID: <20140930172006.GB8692@laptop.dumpdata.com> References: <1411770301-3263-1-git-send-email-roy.franz@linaro.org> <1411770301-3263-2-git-send-email-roy.franz@linaro.org> <542938BD020000780003A4A4@mail.emea.novell.com> <1412065529.3801.34.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1412065529.3801.34.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: keir@xen.org, tim@xen.org, xen-devel@lists.xen.org, Roy Franz , stefano.stabellini@citrix.com, Jan Beulich , fu.wei@linaro.org List-Id: xen-devel@lists.xenproject.org On Tue, Sep 30, 2014 at 09:25:29AM +0100, Ian Campbell wrote: > On Mon, 2014-09-29 at 09:47 +0100, Jan Beulich wrote: > > >>> On 27.09.14 at 00:25, wrote: > > > This patch adds EFI boot support for ARM based on the previous refactoring of > > > the x86 EFI boot code. All ARM specific code is in the ARM efi-boot.h header > > > file, with the main EFI entry point common/efi/boot.c. The PE/COFF header > > > is > > > open-coded in head.S, which allows us to have a single binary be both an EFI > > > executable and a normal arm64 IMAGE file. There is currently no PE/COFF > > > toolchain support for arm64, so it is not possible to create the PE/COFF > > > header > > > in the same manner as on x86. This also simplifies the build as compared to > > > x86, as we always build the same executable, whereas x86 builds 2. An ARM > > > version of efi-bind.h is added, which is based on the x86_64 version with the > > > x86 specific portions removed. The Makefile in common/efi is different for > > > x86 > > > and ARM, as for ARM we always build in EFI support. > > > NR_MEM_BANKS is increased, as memory regions are now added from the UEFI > > > memory map, > > > rather than memory banks from a DTB. The UEFI memory map may be fragmented > > > so a larger > > > number of regions will be used. > > > > > > Signed-off-by: Roy Franz > > > > For the non-ARM parts: > > Acked-by: Jan Beulich > > Acked-by: Ian Campbell > > Konrad, we would really like to get this into 4.5. It will allow Xen to > boot on EFI hardware which is already starting to appear in the field. > It also unblocks other work such as the upstreaming of the Xen on ARM > boot protocol work which Linaro are doing on grub. > > All of the precursor refactoring is already in so the impact of this > change on x86 is minimal. On the ARM side the changes to the > common/non-EFI boot path are using well-worn techniques from the Linux > equivalent so I think we can be reasonably confident of them, the > remaining code is self contained and can't regress anything else. Thank you for anticipating my questions and answering them :-) Release-Acked-by: Konrad Rzeszutek Wilk > > > > > Two minor comments nevertheless: > > > > > --- a/xen/common/efi/boot.c > > > +++ b/xen/common/efi/boot.c > > > @@ -2,6 +2,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -18,9 +19,16 @@ > > > #include > > > #include > > > #include > > > +#ifdef CONFIG_X86 > > > > The context above shows xen/vga.h - this surely isn't needed in the > > common code anymore? > > Seems likely it isn't. Shall I fixup on commit (I always build test on > commit, so if it breaks I will certainly catch it) or shall we fix in a > followup? > > > > --- /dev/null > > > +++ b/xen/include/asm-arm/arm64/efibind.h > > > @@ -0,0 +1,216 @@ > > > +/*++ > > > + > > > +Copyright (c) 1998 Intel Corporation > > > + > > > +Module Name: > > > + > > > + efefind.h > > > > Is this so badly misspelled in the original? > > Apparently! > > $ git grep efefind.h > xen/include/asm-x86/x86_64/efibind.h: efefind.h > > Shall I fix both cases on commit? > > Ian. >