Re: [Xen-devel] [PATCH for-4.5 V9] Add ARM EFI boot support

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

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, <roy.franz@xxxxxxxxxx> 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 <roy.franz@xxxxxxxxxx>
> > 
> > For the non-ARM parts:
> > Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> 
> 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 <konrad.wilk@xxxxxxxxxx>
> 
> > 
> > Two minor comments nevertheless:
> > 
> > > --- a/xen/common/efi/boot.c
> > > +++ b/xen/common/efi/boot.c
> > > @@ -2,6 +2,7 @@
> > >  #include <efi/efiprot.h>
> > >  #include <efi/efipciio.h>
> > >  #include <public/xen.h>
> > > +#include <xen/bitops.h>
> > >  #include <xen/compile.h>
> > >  #include <xen/ctype.h>
> > >  #include <xen/dmi.h>
> > > @@ -18,9 +19,16 @@
> > >  #include <xen/string.h>
> > >  #include <xen/stringify.h>
> > >  #include <xen/vga.h>
> > > +#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.
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel