Re: [Xen-devel] [PATCH v3 11/16] efi: build xen.gz with EFI code

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.05.16 at 21:07, <daniel.kiper@xxxxxxxxxx> wrote:
> On Wed, May 25, 2016 at 01:53:31AM -0600, Jan Beulich wrote:
>> >>> On 15.04.16 at 14:33, <daniel.kiper@xxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/efi/Makefile
>> > +++ b/xen/arch/x86/efi/Makefile
>> > @@ -1,14 +1,9 @@
>> >  CFLAGS += -fshort-wchar
>> >
>> > -obj-y += stub.o
>> > -
>> > -create = test -e $(1) || touch -t 199901010000 $(1)
>> > -
>> >  efi := y$(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
>> > -
>> > -stub.o: $(extra-y)
>> > +obj-y := stub.o
>> > +obj-$(efi) := boot.init.o compat.o relocs-dummy.o runtime.o
>>
>> I assume/hope all these adjustments work for all intended cases, but
> 
> I have done some tests and it looks that everything works.
> 
>> they quite clearly leave stale bits in xen/arch/x86/Rules.mk: Its
> 
> I suppose that you were thinking about xen/arch/x86/Makefile.

Oh, yes, of course.

>> references to efi/*.o should all go away now afaict.
> 
> OK.
> 
>> > --- a/xen/common/efi/boot.c
>> > +++ b/xen/common/efi/boot.c
>> > @@ -1244,6 +1244,9 @@ void __init efi_init_memory(void)
>> >      } *extra, *extra_head = NULL;
>> >  #endif
>> >
>> > +    if ( !efi_enabled(EFI_PLATFORM) )
>> > +        return;
>>
>> Arguably such checks would then better be put at the call site,
>> allowing the respective stubs to just BUG().
> 
> Ugh... I am confused. Here 
> http://lists.xen.org/archives/html/xen-devel/2015-08/msg01790.html 
> you asked for what is done above. So, what is your final decision?

Well, in v2 you didn't alter stubs.c at all. It's that connection
which makes me think using that earlier approach might be better.
The more that, from a purely abstract pov, it could even allow to
remove some or all of stubs.c in a truly non-EFI build, provided we
never build with -O0.

But in the end, starting the sens with "arguably" I mean to express
that this isn't all that important.

>> Also - what's your rule for where to put such efi_enabled() checks?
>> I would have expected them to get added to everything that has
>> a counterpart in stubs.c, but things like efi_get_time() or
>> efi_{halt,reset}_system() don't get any added. If those are
>> unreachable, I'd at least expect respective ASSERT()s to get added
>> there.
> 
> I have added checks to functions which are called from common EFI/BIOS code.

And how are the ones I named not called from "common" code?

Jan


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