Re: [Xen-devel] [PATCH] x86/efi: move the logic to detect PE build support

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Jul 16, 2018 at 01:59:15AM -0600, Jan Beulich wrote:
> >>> On 13.07.18 at 18:02, <roger.pau@xxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -168,6 +168,16 @@ $(TARGET).efi: ALT_BASE = 0x$(shell $(NM) 
> > efi/relocs-dummy.o | sed -n 's, A ALT_
> >  # Don't use $(wildcard ...) here - at least make 3.80 expands this too 
> > early!
> >  $(TARGET).efi: guard = $(if $(shell echo efi/dis* | grep disabled),:)
> >  
> > +# Check if the build system supports PE.
> > +efi := y$(shell rm -f efi/disabled)
> > +efi := $(if $(efi),$(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) 
> > -c efi/check.c -o efi/check.o 2>efi/disabled && echo y))
> > +efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o efi/check.efi 
> > efi/check.o 2>efi/disabled && echo y))
> > +efi := $(if $(efi),$(shell rm efi/disabled)y)
> > +export BUILD_PE := $(efi)
> > +ifeq ($(efi),y)
> > +CFLAGS += -DBUILD_PE
> > +endif
> 
> For one I'm not really happy about this being moved here: I did place it
> in efi/Makefile for the simple reason of having as much as possible of
> the EFI specifics in that single file.
> 
> Additionally I think it would be better if setting propagated through the
> environment had XEN_ prefixes.
> 
> > --- a/xen/arch/x86/xen.lds.S
> > +++ b/xen/arch/x86/xen.lds.S
> > @@ -304,7 +304,9 @@ SECTIONS
> >    } :text
> >  #endif
> >  
> > -  efi = DEFINED(efi) ? efi : .;
> > +#ifndef BUILD_PE
> > +  efi = .;
> > +#endif
> 
> And then I don't really understand how this is different from the
> original #ifndef EFI that Daniel had problems with.

As I understand it EFI only signals whether a PE binary will be
created, but the ELF binary will also contain efi support if the
build toolchain supports PE. Hence whether the efi symbol is defined
or not solely depends on the detection done above (and whether
runtime.c or stub.c is used).

Maybe I'm missing something, I have to admit the code is quite
convoluted.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel