diff mbox

[Xen-devel,for-4.5,V9] Add ARM EFI boot support

Message ID CAFECyb_BOLP-JpDvF1_UuWn3YpJT8+NmO3dVZ=X-SduEBLML5A@mail.gmail.com
State New
Headers show

Commit Message

Roy Franz Sept. 30, 2014, 6:38 p.m. UTC 
On Tue, Sep 30, 2014 at 1:25 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2014-09-29 at 09:47 +0100, Jan Beulich wrote:
>> >>> On 27.09.14 at 00:25, <roy.franz@linaro.org> 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@linaro.org>
>>
>> For the non-ARM parts:
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> 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.
>
>>
>> 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?

It's not needed in boot.c, but is in x86/efi/efi-boot.h.  It should have been
moved along with the vga code.  The following moves it from
boot.c to x86/efi/efi-boot.h.

Ian - can you apply this fixup when you take the patch?




>
>> > --- /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.
>

Comments

Jan Beulich Oct. 1, 2014, 9:03 a.m. UTC | #1 
>>> On 30.09.14 at 20:38, <roy.franz@linaro.org> wrote:
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -7,6 +7,7 @@
>  #include <asm/edd.h>
>  #include <asm/msr.h>
>  #include <asm/processor.h>
> +#include <xen/vga.h>

Except that we commonly put xen/ includes ahead of asm/ ones.

Jan
Ian Campbell Oct. 1, 2014, 10:39 a.m. UTC | #2 
On Wed, 2014-10-01 at 10:03 +0100, Jan Beulich wrote:
> >>> On 30.09.14 at 20:38, <roy.franz@linaro.org> wrote:
> > --- a/xen/arch/x86/efi/efi-boot.h
> > +++ b/xen/arch/x86/efi/efi-boot.h
> > @@ -7,6 +7,7 @@
> >  #include <asm/edd.h>
> >  #include <asm/msr.h>
> >  #include <asm/processor.h>
> > +#include <xen/vga.h>
> 
> Except that we commonly put xen/ includes ahead of asm/ ones.

I made that change as I committed the patch and it passed my build tests
so I pushed. Hope that's ok.

Ian.
diff mbox

Patch

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index b44746f..82bb74a 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -7,6 +7,7 @@ 
 #include <asm/edd.h>
 #include <asm/msr.h>
 #include <asm/processor.h>
+#include <xen/vga.h>

 static struct file __initdata ucode;
 static multiboot_info_t __initdata mbi = {
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index e1b243c..f272171 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -18,7 +18,6 @@ 
 #endif
 #include <xen/string.h>
 #include <xen/stringify.h>
-#include <xen/vga.h>
 #ifdef CONFIG_X86
 /*
  * Keep this arch-specific modified include in the common file, as moving