Re: [GIT PULL] x86 fixes

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Sep 23, 2014 at 02:18:05PM +0100, Matt Fleming wrote:
> On Tue, 23 Sep, at 10:18:07AM, Ard Biesheuvel wrote:
> > It will be difficult for me to keep up with this thread over the next
> > days, so I have added Leif and Roy on cc. If you need to make any
> > changes that affect arm64, they should be able to confirm whether it
> > causes any problems or not.
> 
> OK, below is what I've got on top of commit f3670394c29f in Linus' tree.
> Please don't anyone pick this up yet, I'm still testing it on my
> machines and it desperately needs some Tested-by tags.
> 
> Josh, your machines probably boot fine after the revert from Linus but
> it'd be good to verify that this patch doesn't make things any worse.
> 
> Maarten, please verify that your Macbook pro boots with this patch. I
> very much want to be able to add a Tested-by from you.
> 
> Ard, Leif, Roy, can you please validate this for arm64? The thing to
> look out for is whether the changes to drivers/firmware/efi/Makefile
> make sense, and in particular, whether it's ever possible to have
> CONFIG_EFI_STUB=y and CONFIG_EFI_ARM_STUB=n.

Currently, these are both selected automatically by CONFIG_EFI, and
are not selectable through the user interface. There is no relevant
case for them being individually selectable, so perhaps _EFI_ARM_STUB
should simply be set to the value of _EFI_STUB.
 
> If the answer is yes, then this patch might break your build and we
> probably want to do something like unconditionally select
> CONFIG_EFI_ARM_STUB when CONFIG_EFI_STUB=y.

For arm64:
Tested-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
> 
> From 8054c405f0acc8ecb74b7e29675a1e6894afedb0 Mon Sep 17 00:00:00 2001
> From: Matt Fleming <matt.fleming@intel.com>
> Date: Tue, 23 Sep 2014 10:37:43 +0100
> Subject: [PATCH] Revert "efi/x86: efistub: Move shared dependencies to
>  <asm/efi.h>"
> 
> This reverts commit f23cf8bd5c1f ("efi/x86: efistub: Move shared
> dependencies to <asm/efi.h>") as well as the x86 parts of commit
> f4f75ad5741f ("efi: efistub: Convert into static library").
> 
> The road leading to these two reverts is long and winding.
> 
> The above two commits were merged during the v3.17 merge window and
> turned the common EFI boot stub code into a static library. This
> necessitated making some symbols global in the x86 boot stub which
> introduced new entries into the early boot GOT.
> 
> The problem was that we weren't fixing up the newly created GOT entries
> before invoking the EFI boot stub, which sometimes resulted in hangs or
> resets. This failure was reported by Maarten on his Macbook pro.
> 
> The proposed fix was commit 9cb0e394234d ("x86/efi: Fixup GOT in all
> boot code paths"). However, that caused issues for Linus when booting
> his Sony Vaio Pro 11. It was subsequently reverted in commit
> f3670394c29f.
> 
> So that leaves us back with Maarten's Macbook pro not booting.
> 
> At this stage in the release cycle the least risky option is to revert
> the x86 EFI boot stub to the pre-merge window code structure where we
> explicitly #include efi-stub-helper.c instead of linking with the static
> library. The arm64 code remains unaffected.
> 
> We can take another swing at the x86 parts for v3.18.
> 
> Conflicts:
> 	arch/x86/include/asm/efi.h
> 
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
> Cc: Josh Boyer <jwboyer@fedoraproject.org>
> Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> ---
>  arch/x86/boot/compressed/Makefile |  3 +--
>  arch/x86/boot/compressed/eboot.c  |  7 ++++++-
>  arch/x86/boot/compressed/eboot.h  | 16 ++++++++++++++++
>  arch/x86/include/asm/efi.h        | 24 ------------------------
>  drivers/firmware/efi/Makefile     |  2 +-
>  5 files changed, 24 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 7a801a310e37..0fcd9133790c 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -33,8 +33,7 @@ VMLINUX_OBJS = $(obj)/vmlinux.lds $(obj)/head_$(BITS).o $(obj)/misc.o \
>  $(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
>  
>  ifeq ($(CONFIG_EFI_STUB), y)
> -	VMLINUX_OBJS += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
> -				$(objtree)/drivers/firmware/efi/libstub/lib.a
> +	VMLINUX_OBJS += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o
>  endif
>  
>  $(obj)/vmlinux: $(VMLINUX_OBJS) FORCE
> diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> index dca9842d8f91..b04f1e0e3fab 100644
> --- a/arch/x86/boot/compressed/eboot.c
> +++ b/arch/x86/boot/compressed/eboot.c
> @@ -19,7 +19,10 @@
>  
>  static efi_system_table_t *sys_table;
>  
> -struct efi_config *efi_early;
> +static struct efi_config *efi_early;
> +
> +#define efi_call_early(f, ...)						\
> +	efi_early->call(efi_early->f, __VA_ARGS__);
>  
>  #define BOOT_SERVICES(bits)						\
>  static void setup_boot_services##bits(struct efi_config *c)		\
> @@ -280,6 +283,8 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)
>  	}
>  }
>  
> +#include "../../../../drivers/firmware/efi/libstub/efi-stub-helper.c"
> +
>  static void find_bits(unsigned long mask, u8 *pos, u8 *size)
>  {
>  	u8 first, len;
> diff --git a/arch/x86/boot/compressed/eboot.h b/arch/x86/boot/compressed/eboot.h
> index d487e727f1ec..c88c31ecad12 100644
> --- a/arch/x86/boot/compressed/eboot.h
> +++ b/arch/x86/boot/compressed/eboot.h
> @@ -103,4 +103,20 @@ struct efi_uga_draw_protocol {
>  	void *blt;
>  };
>  
> +struct efi_config {
> +	u64 image_handle;
> +	u64 table;
> +	u64 allocate_pool;
> +	u64 allocate_pages;
> +	u64 get_memory_map;
> +	u64 free_pool;
> +	u64 free_pages;
> +	u64 locate_handle;
> +	u64 handle_protocol;
> +	u64 exit_boot_services;
> +	u64 text_output;
> +	efi_status_t (*call)(unsigned long, ...);
> +	bool is64;
> +} __packed;
> +
>  #endif /* BOOT_COMPRESSED_EBOOT_H */
> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> index 044a2fd3c5fe..0ec241ede5a2 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -159,30 +159,6 @@ static inline efi_status_t efi_thunk_set_virtual_address_map(
>  }
>  #endif /* CONFIG_EFI_MIXED */
>  
> -
> -/* arch specific definitions used by the stub code */
> -
> -struct efi_config {
> -	u64 image_handle;
> -	u64 table;
> -	u64 allocate_pool;
> -	u64 allocate_pages;
> -	u64 get_memory_map;
> -	u64 free_pool;
> -	u64 free_pages;
> -	u64 locate_handle;
> -	u64 handle_protocol;
> -	u64 exit_boot_services;
> -	u64 text_output;
> -	efi_status_t (*call)(unsigned long, ...);
> -	bool is64;
> -} __packed;
> -
> -extern struct efi_config *efi_early;
> -
> -#define efi_call_early(f, ...)						\
> -	efi_early->call(efi_early->f, __VA_ARGS__);
> -
>  extern bool efi_reboot_required(void);
>  
>  #else
> diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
> index d8be608a9f3b..aef6a95adef5 100644
> --- a/drivers/firmware/efi/Makefile
> +++ b/drivers/firmware/efi/Makefile
> @@ -7,4 +7,4 @@ obj-$(CONFIG_EFI_VARS_PSTORE)		+= efi-pstore.o
>  obj-$(CONFIG_UEFI_CPER)			+= cper.o
>  obj-$(CONFIG_EFI_RUNTIME_MAP)		+= runtime-map.o
>  obj-$(CONFIG_EFI_RUNTIME_WRAPPERS)	+= runtime-wrappers.o
> -obj-$(CONFIG_EFI_STUB)			+= libstub/
> +obj-$(CONFIG_EFI_ARM_STUB)		+= libstub/
> -- 
> 1.9.3
> 
> -- 
> Matt Fleming, Intel Open Source Technology Center