Re: [Xen-devel] [PATCH v2 10/23] efi: build xen.gz with EFI code

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

On Mon, Jul 20, 2015 at 04:29:05PM +0200, Daniel Kiper wrote:
> Build xen.gz with EFI code. We need this to support multiboot2
> protocol on EFI platforms.
> 
> If we wish to load not ELF file using multiboot (v1) or multiboot2 then
> it must contain "linear" (or "flat") representation of code and data.
> Currently, PE file contains many sections which are not "linear" (one
> after another without any holes) or even do not have representation
> in a file (e.g. BSS). In theory there is a chance that we could build
> proper PE file using current build system. However, it means that
> xen.efi further diverge from xen ELF file (in terms of contents and
> build method). ELF have all needed properties. So, it means that this
> is good starting point for further development. Additionally, I think
> that this is also good starting point for further xen.efi code and
> build optimizations. It looks that there is a chance that finally we
> can generate xen.efi directly from xen ELF using just simple objcopy.
> 
> Signed-off-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
> ---
> v2 - suggestions/fixes:
>    - build EFI code only if it is supported in a given build environment
>      (suggested by Jan Beulich).
> ---
>  xen/arch/x86/Makefile     |   13 +++++--------
>  xen/arch/x86/efi/Makefile |   16 +++++++++-------
>  xen/arch/x86/mm.c         |    3 ++-
>  xen/common/efi/runtime.c  |    6 ++++++
>  4 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index 5f24951..0335445 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -80,7 +80,7 @@ ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o 
> $(BASEDIR)/arch/x86/efi/built_in
>  
>  ifeq ($(lto),y)
>  # Gather all LTO objects together
> -prelink_lto.o: $(ALL_OBJS)
> +prelink_lto.o: $(ALL_OBJS) efi/relocs-dummy.o
>       $(LD_LTO) -r -o $@ $^
>  
>  prelink-efi_lto.o: $(ALL_OBJS) efi/runtime.o efi/compat.o
> @@ -90,14 +90,14 @@ prelink-efi_lto.o: $(ALL_OBJS) efi/runtime.o efi/compat.o
>  prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) 
> prelink_lto.o
>       $(LD) $(LDFLAGS) -r -o $@ $^
>  
> -prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) 
> prelink-efi_lto.o efi/boot.init.o
> +prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) 
> prelink-efi_lto.o
>       $(guard) $(LD) $(LDFLAGS) -r -o $@ $^
>  else
> -prelink.o: $(ALL_OBJS)
> +prelink.o: $(ALL_OBJS) efi/relocs-dummy.o
>       $(LD) $(LDFLAGS) -r -o $@ $^
>  
> -prelink-efi.o: $(ALL_OBJS) efi/boot.init.o efi/runtime.o efi/compat.o
> -     $(guard) $(LD) $(LDFLAGS) -r -o $@ $(filter-out %/efi/built_in.o,$^)
> +prelink-efi.o: $(ALL_OBJS)
> +     $(guard) $(LD) $(LDFLAGS) -r -o $@ $^
>  endif
>  
>  $(BASEDIR)/common/symbols-dummy.o:
> @@ -146,9 +146,6 @@ $(TARGET).efi: prelink-efi.o efi.lds efi/relocs-dummy.o 
> $(BASEDIR)/common/symbol
>       if $(guard) false; then rm -f $@; echo 'EFI support disabled'; fi
>       rm -f $(@D)/.$(@F).[0-9]*
>  
> -efi/boot.init.o efi/runtime.o efi/compat.o: 
> $(BASEDIR)/arch/x86/efi/built_in.o
> -efi/boot.init.o efi/runtime.o efi/compat.o: ;
> -
>  asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
>       $(CC) $(filter-out -flto,$(CFLAGS)) -S -o $@ $<
>  
> diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
> index 1daa7ac..b1e8883 100644
> --- a/xen/arch/x86/efi/Makefile
> +++ b/xen/arch/x86/efi/Makefile
> @@ -1,14 +1,16 @@
>  CFLAGS += -fshort-wchar
>  
> -obj-y += stub.o
> -
> -create = test -e $(1) || touch -t 199901010000 $(1)
> -
>  efi := $(filter y,$(x86_64)$(shell rm -f disabled))
>  efi := $(if $(efi),$(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c 
> check.c 2>disabled && echo y))
>  efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi 
> check.o 2>disabled && echo y))
> -efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o); 
> $(call create,runtime.o)))
> +efi := $(if $(efi),$(shell rm disabled)y)
>  
> -extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o
> +extra-y += relocs-dummy.o
>  
> -stub.o: $(extra-y)
> +ifeq ($(efi),y)
> +obj-y += boot.init.o
> +obj-y += compat.o
> +obj-y += runtime.o
> +else
> +obj-y += stub.o
> +endif

That makefile magic I skipped over, but the C code below looks good, so

Half-Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>


> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 342414f..cef2eb6 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -344,7 +344,8 @@ void __init arch_init_memory(void)
>  
>      subarch_init_memory();
>  
> -    efi_init_memory();
> +    if ( efi_enabled(EFI_PLATFORM) )
> +        efi_init_memory();
>  
>      mem_sharing_init();
>  
> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index aa064e7..3eb21c1 100644
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -167,6 +167,9 @@ int efi_get_info(uint32_t idx, union xenpf_efi_info *info)
>  {
>      unsigned int i, n;
>  
> +    if ( !efi_enabled(EFI_PLATFORM) )
> +        return -EOPNOTSUPP;
> +
>      switch ( idx )
>      {
>      case XEN_FW_EFI_VERSION:
> @@ -301,6 +304,9 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
>      EFI_STATUS status = EFI_NOT_STARTED;
>      int rc = 0;
>  
> +    if ( !efi_enabled(EFI_PLATFORM) )
> +        return -EOPNOTSUPP;
> +
>      switch ( op->function )
>      {
>      case XEN_EFI_get_time:
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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