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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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
they quite clearly leave stale bits in xen/arch/x86/Rules.mk: Its
references to efi/*.o should all go away now afaict.

> --- 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().

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.

> --- 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;

Please do not introduce behavioral differences to the current stub
implementations: This and ...

> @@ -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;

... this return -ENOSYS there.

Jan


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