[RFT PATCH] efi/x86: move x86 back to libstub

[RFT PATCH] efi/x86: move x86 back to libstub

[Ard Biesheuvel]

This reverts commit 84be880560fb, which itself reverted my original
attempt to move x86 from #include'ing .c files from across the tree
to using the EFI stub built as a static library.

The issue that affected the original approach was that splitting
the implementation into several .o files resulted in the variable
'efi_early' becoming a global with external linkage, which under
-fPIC implies that references to it must go through the GOT. However,
dealing with this additional GOT entry turned out to be troublesome
on some EFI implementations. (GCC's visibility=hidden attribute is
supposed to lift this requirement, but it turned out not to work on
the 32-bit build.)

Instead, use a pure getter function to get a reference to efi_early.
This approach results in no additional GOT entries being generated,
so there is no need for any changes in the early GOT handling.

Cc: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Cc: Josh Boyer <jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org>
Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---

Gents,

This is a request for testing: I would like to find out if this patch
fixes Maarten's issue without breaking anything like it did for Josh
and Linus the first time around.


 arch/x86/boot/compressed/Makefile |  3 ++-
 arch/x86/boot/compressed/eboot.c  |  8 ++++----
 arch/x86/boot/compressed/eboot.h  | 16 ----------------
 arch/x86/include/asm/efi.h        | 24 ++++++++++++++++++++++++
 drivers/firmware/efi/Makefile     |  2 +-
 5 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 0fcd9133790c..7a801a310e37 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -33,7 +33,8 @@ 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
+	VMLINUX_OBJS += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
+				$(objtree)/drivers/firmware/efi/libstub/lib.a
 endif
 
 $(obj)/vmlinux: $(VMLINUX_OBJS) FORCE
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index de8eebd6f67c..58ea406f5d60 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -21,8 +21,10 @@ static efi_system_table_t *sys_table;
 
 static struct efi_config *efi_early;
 
-#define efi_call_early(f, ...)						\
-	efi_early->call(efi_early->f, __VA_ARGS__);
+__pure const struct efi_config *__efi_early(void)
+{
+	return efi_early;
+}
 
 #define BOOT_SERVICES(bits)						\
 static void setup_boot_services##bits(struct efi_config *c)		\
@@ -285,8 +287,6 @@ 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 c88c31ecad12..d487e727f1ec 100644
--- a/arch/x86/boot/compressed/eboot.h
+++ b/arch/x86/boot/compressed/eboot.h
@@ -103,20 +103,4 @@ 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 0ec241ede5a2..b2c854a0f5ca 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -159,6 +159,30 @@ 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;
+
+__pure const struct efi_config *__efi_early(void);
+
+#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 aef6a95adef5..d8be608a9f3b 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_ARM_STUB)		+= libstub/
+obj-$(CONFIG_EFI_STUB)			+= libstub/
-- 
1.9.1

Re: [RFT PATCH] efi/x86: move x86 back to libstub

[Josh Boyer]

On Mon, Oct 6, 2014 at 7:06 AM, Ard Biesheuvel
<ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> This reverts commit 84be880560fb, which itself reverted my original
> attempt to move x86 from #include'ing .c files from across the tree
> to using the EFI stub built as a static library.
>
> The issue that affected the original approach was that splitting
> the implementation into several .o files resulted in the variable
> 'efi_early' becoming a global with external linkage, which under
> -fPIC implies that references to it must go through the GOT. However,
> dealing with this additional GOT entry turned out to be troublesome
> on some EFI implementations. (GCC's visibility=hidden attribute is
> supposed to lift this requirement, but it turned out not to work on
> the 32-bit build.)
>
> Instead, use a pure getter function to get a reference to efi_early.
> This approach results in no additional GOT entries being generated,
> so there is no need for any changes in the early GOT handling.
>
> Cc: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Cc: Josh Boyer <jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org>
> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>
> Gents,
>
> This is a request for testing: I would like to find out if this patch
> fixes Maarten's issue without breaking anything like it did for Josh
> and Linus the first time around.

I probably won't get to testing this today, but I will try tomorrow.

josh

Re: [RFT PATCH] efi/x86: move x86 back to libstub

[Ard Biesheuvel]

On 6 October 2014 13:06, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> This reverts commit 84be880560fb, which itself reverted my original
> attempt to move x86 from #include'ing .c files from across the tree
> to using the EFI stub built as a static library.
>
> The issue that affected the original approach was that splitting
> the implementation into several .o files resulted in the variable
> 'efi_early' becoming a global with external linkage, which under
> -fPIC implies that references to it must go through the GOT. However,
> dealing with this additional GOT entry turned out to be troublesome
> on some EFI implementations. (GCC's visibility=hidden attribute is
> supposed to lift this requirement, but it turned out not to work on
> the 32-bit build.)
>
> Instead, use a pure getter function to get a reference to efi_early.
> This approach results in no additional GOT entries being generated,
> so there is no need for any changes in the early GOT handling.
>
> Cc: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Cc: Josh Boyer <jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org>
> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>
> Gents,
>
> This is a request for testing: I would like to find out if this patch
> fixes Maarten's issue without breaking anything like it did for Josh
> and Linus the first time around.
>

Any takers?


>
>  arch/x86/boot/compressed/Makefile |  3 ++-
>  arch/x86/boot/compressed/eboot.c  |  8 ++++----
>  arch/x86/boot/compressed/eboot.h  | 16 ----------------
>  arch/x86/include/asm/efi.h        | 24 ++++++++++++++++++++++++
>  drivers/firmware/efi/Makefile     |  2 +-
>  5 files changed, 31 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 0fcd9133790c..7a801a310e37 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -33,7 +33,8 @@ 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
> +       VMLINUX_OBJS += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
> +                               $(objtree)/drivers/firmware/efi/libstub/lib.a
>  endif
>
>  $(obj)/vmlinux: $(VMLINUX_OBJS) FORCE
> diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> index de8eebd6f67c..58ea406f5d60 100644
> --- a/arch/x86/boot/compressed/eboot.c
> +++ b/arch/x86/boot/compressed/eboot.c
> @@ -21,8 +21,10 @@ static efi_system_table_t *sys_table;
>
>  static struct efi_config *efi_early;
>
> -#define efi_call_early(f, ...)                                         \
> -       efi_early->call(efi_early->f, __VA_ARGS__);
> +__pure const struct efi_config *__efi_early(void)
> +{
> +       return efi_early;
> +}
>
>  #define BOOT_SERVICES(bits)                                            \
>  static void setup_boot_services##bits(struct efi_config *c)            \
> @@ -285,8 +287,6 @@ 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 c88c31ecad12..d487e727f1ec 100644
> --- a/arch/x86/boot/compressed/eboot.h
> +++ b/arch/x86/boot/compressed/eboot.h
> @@ -103,20 +103,4 @@ 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 0ec241ede5a2..b2c854a0f5ca 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -159,6 +159,30 @@ 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;
> +
> +__pure const struct efi_config *__efi_early(void);
> +
> +#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 aef6a95adef5..d8be608a9f3b 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_ARM_STUB)             += libstub/
> +obj-$(CONFIG_EFI_STUB)                 += libstub/
> --
> 1.9.1
>

Re: [RFT PATCH] efi/x86: move x86 back to libstub

[Maarten Lankhorst]

Hey,

On 10-10-14 08:35, Ard Biesheuvel wrote:
> On 6 October 2014 13:06, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> This reverts commit 84be880560fb, which itself reverted my original
>> attempt to move x86 from #include'ing .c files from across the tree
>> to using the EFI stub built as a static library.
>>
>> The issue that affected the original approach was that splitting
>> the implementation into several .o files resulted in the variable
>> 'efi_early' becoming a global with external linkage, which under
>> -fPIC implies that references to it must go through the GOT. However,
>> dealing with this additional GOT entry turned out to be troublesome
>> on some EFI implementations. (GCC's visibility=hidden attribute is
>> supposed to lift this requirement, but it turned out not to work on
>> the 32-bit build.)
>>
>> Instead, use a pure getter function to get a reference to efi_early.
>> This approach results in no additional GOT entries being generated,
>> so there is no need for any changes in the early GOT handling.
>>
>> Cc: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>> Cc: Josh Boyer <jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org>
>> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>
>> Gents,
>>
>> This is a request for testing: I would like to find out if this patch
>> fixes Maarten's issue without breaking anything like it did for Josh
>> and Linus the first time around.
>>
> 
> Any takers?
Sorry it was on my todo list but I lost access to my laptop for a while.
Normal EFI boot through refind works at least, but I think netboot may use a slightly
different codepath which I can't test right now.

Tested-By: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Re: [RFT PATCH] efi/x86: move x86 back to libstub

[Ard Biesheuvel]

On 10 October 2014 10:30, Maarten Lankhorst
<maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
> Hey,
>
> On 10-10-14 08:35, Ard Biesheuvel wrote:
>> On 6 October 2014 13:06, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>> This reverts commit 84be880560fb, which itself reverted my original
>>> attempt to move x86 from #include'ing .c files from across the tree
>>> to using the EFI stub built as a static library.
>>>
>>> The issue that affected the original approach was that splitting
>>> the implementation into several .o files resulted in the variable
>>> 'efi_early' becoming a global with external linkage, which under
>>> -fPIC implies that references to it must go through the GOT. However,
>>> dealing with this additional GOT entry turned out to be troublesome
>>> on some EFI implementations. (GCC's visibility=hidden attribute is
>>> supposed to lift this requirement, but it turned out not to work on
>>> the 32-bit build.)
>>>
>>> Instead, use a pure getter function to get a reference to efi_early.
>>> This approach results in no additional GOT entries being generated,
>>> so there is no need for any changes in the early GOT handling.
>>>
>>> Cc: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>>> Cc: Josh Boyer <jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org>
>>> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> ---
>>>
>>> Gents,
>>>
>>> This is a request for testing: I would like to find out if this patch
>>> fixes Maarten's issue without breaking anything like it did for Josh
>>> and Linus the first time around.
>>>
>>
>> Any takers?
> Sorry it was on my todo list but I lost access to my laptop for a while.
> Normal EFI boot through refind works at least, but I think netboot may use a slightly
> different codepath which I can't test right now.
>
> Tested-By: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Thanks! So I suppose the code path you did test is the code path that
produced the failure last time?

Re: [RFT PATCH] efi/x86: move x86 back to libstub

[Maarten Lankhorst]

Op 10-10-14 om 10:37 schreef Ard Biesheuvel:
> On 10 October 2014 10:30, Maarten Lankhorst
> <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
>> Hey,
>>
>> On 10-10-14 08:35, Ard Biesheuvel wrote:
>>> On 6 October 2014 13:06, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>> This reverts commit 84be880560fb, which itself reverted my original
>>>> attempt to move x86 from #include'ing .c files from across the tree
>>>> to using the EFI stub built as a static library.
>>>>
>>>> The issue that affected the original approach was that splitting
>>>> the implementation into several .o files resulted in the variable
>>>> 'efi_early' becoming a global with external linkage, which under
>>>> -fPIC implies that references to it must go through the GOT. However,
>>>> dealing with this additional GOT entry turned out to be troublesome
>>>> on some EFI implementations. (GCC's visibility=hidden attribute is
>>>> supposed to lift this requirement, but it turned out not to work on
>>>> the 32-bit build.)
>>>>
>>>> Instead, use a pure getter function to get a reference to efi_early.
>>>> This approach results in no additional GOT entries being generated,
>>>> so there is no need for any changes in the early GOT handling.
>>>>
>>>> Cc: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>>>> Cc: Josh Boyer <jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org>
>>>> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>> ---
>>>>
>>>> Gents,
>>>>
>>>> This is a request for testing: I would like to find out if this patch
>>>> fixes Maarten's issue without breaking anything like it did for Josh
>>>> and Linus the first time around.
>>>>
>>> Any takers?
>> Sorry it was on my todo list but I lost access to my laptop for a while.
>> Normal EFI boot through refind works at least, but I think netboot may use a slightly
>> different codepath which I can't test right now.
>>
>> Tested-By: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Thanks! So I suppose the code path you did test is the code path that
> produced the failure last time?
>
I tested netboot now to be sure, seems to be working ok.

~Maarten

Re: [RFT PATCH] efi/x86: move x86 back to libstub

[Ard Biesheuvel]

On 13 October 2014 15:37, Maarten Lankhorst
<maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
> Op 10-10-14 om 10:37 schreef Ard Biesheuvel:
>> On 10 October 2014 10:30, Maarten Lankhorst
>> <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
>>> Hey,
>>>
>>> On 10-10-14 08:35, Ard Biesheuvel wrote:
>>>> On 6 October 2014 13:06, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>>> This reverts commit 84be880560fb, which itself reverted my original
>>>>> attempt to move x86 from #include'ing .c files from across the tree
>>>>> to using the EFI stub built as a static library.
>>>>>
>>>>> The issue that affected the original approach was that splitting
>>>>> the implementation into several .o files resulted in the variable
>>>>> 'efi_early' becoming a global with external linkage, which under
>>>>> -fPIC implies that references to it must go through the GOT. However,
>>>>> dealing with this additional GOT entry turned out to be troublesome
>>>>> on some EFI implementations. (GCC's visibility=hidden attribute is
>>>>> supposed to lift this requirement, but it turned out not to work on
>>>>> the 32-bit build.)
>>>>>
>>>>> Instead, use a pure getter function to get a reference to efi_early.
>>>>> This approach results in no additional GOT entries being generated,
>>>>> so there is no need for any changes in the early GOT handling.
>>>>>
>>>>> Cc: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>>>>> Cc: Josh Boyer <jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org>
>>>>> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>> ---
>>>>>
>>>>> Gents,
>>>>>
>>>>> This is a request for testing: I would like to find out if this patch
>>>>> fixes Maarten's issue without breaking anything like it did for Josh
>>>>> and Linus the first time around.
>>>>>
>>>> Any takers?
>>> Sorry it was on my todo list but I lost access to my laptop for a while.
>>> Normal EFI boot through refind works at least, but I think netboot may use a slightly
>>> different codepath which I can't test right now.
>>>
>>> Tested-By: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>> Thanks! So I suppose the code path you did test is the code path that
>> produced the failure last time?
>>
> I tested netboot now to be sure, seems to be working ok.
>

Thanks!

Re: [RFT PATCH] efi/x86: move x86 back to libstub

[Matt Fleming]

On Mon, 13 Oct, at 03:37:07PM, Maarten Lankhorst wrote:
> I tested netboot now to be sure, seems to be working ok.

Thanks guys, I'll take a look at this tomorrow.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [RFT PATCH] efi/x86: move x86 back to libstub

[Matt Fleming]

On Mon, 06 Oct, at 01:06:43PM, Ard Biesheuvel wrote:
> This reverts commit 84be880560fb, which itself reverted my original
> attempt to move x86 from #include'ing .c files from across the tree
> to using the EFI stub built as a static library.
> 
> The issue that affected the original approach was that splitting
> the implementation into several .o files resulted in the variable
> 'efi_early' becoming a global with external linkage, which under
> -fPIC implies that references to it must go through the GOT. However,
> dealing with this additional GOT entry turned out to be troublesome
> on some EFI implementations. (GCC's visibility=hidden attribute is
> supposed to lift this requirement, but it turned out not to work on
> the 32-bit build.)
> 
> Instead, use a pure getter function to get a reference to efi_early.
> This approach results in no additional GOT entries being generated,
> so there is no need for any changes in the early GOT handling.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Cc: Josh Boyer <jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org>
> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> 
> Gents,
> 
> This is a request for testing: I would like to find out if this patch
> fixes Maarten's issue without breaking anything like it did for Josh
> and Linus the first time around.
> 
> 
>  arch/x86/boot/compressed/Makefile |  3 ++-
>  arch/x86/boot/compressed/eboot.c  |  8 ++++----
>  arch/x86/boot/compressed/eboot.h  | 16 ----------------
>  arch/x86/include/asm/efi.h        | 24 ++++++++++++++++++++++++
>  drivers/firmware/efi/Makefile     |  2 +-
>  5 files changed, 31 insertions(+), 22 deletions(-)

Ard, this is a very neat trick.

Thanks for taking the time to reimplement this, it's much appreciated.

Things work fine on my end, and I do have a Fedora 20/grub box in my
testing farm now, so I'm confident this is a good patch.

But if other people want to test it out, I'd be grateful of some more
Tested-by tags (I've already picked up Maarten's).

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [RFT PATCH] efi/x86: move x86 back to libstub

[Josh Boyer]

On Thu, Oct 16, 2014 at 10:42 AM, Matt Fleming <matt-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org> wrote:
> On Mon, 06 Oct, at 01:06:43PM, Ard Biesheuvel wrote:
>> This reverts commit 84be880560fb, which itself reverted my original
>> attempt to move x86 from #include'ing .c files from across the tree
>> to using the EFI stub built as a static library.
>>
>> The issue that affected the original approach was that splitting
>> the implementation into several .o files resulted in the variable
>> 'efi_early' becoming a global with external linkage, which under
>> -fPIC implies that references to it must go through the GOT. However,
>> dealing with this additional GOT entry turned out to be troublesome
>> on some EFI implementations. (GCC's visibility=hidden attribute is
>> supposed to lift this requirement, but it turned out not to work on
>> the 32-bit build.)
>>
>> Instead, use a pure getter function to get a reference to efi_early.
>> This approach results in no additional GOT entries being generated,
>> so there is no need for any changes in the early GOT handling.
>>
>> Cc: Maarten Lankhorst <maarten.lankhorst-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>> Cc: Josh Boyer <jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org>
>> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>
>> Gents,
>>
>> This is a request for testing: I would like to find out if this patch
>> fixes Maarten's issue without breaking anything like it did for Josh
>> and Linus the first time around.
>>
>>
>>  arch/x86/boot/compressed/Makefile |  3 ++-
>>  arch/x86/boot/compressed/eboot.c  |  8 ++++----
>>  arch/x86/boot/compressed/eboot.h  | 16 ----------------
>>  arch/x86/include/asm/efi.h        | 24 ++++++++++++++++++++++++
>>  drivers/firmware/efi/Makefile     |  2 +-
>>  5 files changed, 31 insertions(+), 22 deletions(-)
>
> Ard, this is a very neat trick.
>
> Thanks for taking the time to reimplement this, it's much appreciated.
>
> Things work fine on my end, and I do have a Fedora 20/grub box in my
> testing farm now, so I'm confident this is a good patch.
>
> But if other people want to test it out, I'd be grateful of some more
> Tested-by tags (I've already picked up Maarten's).

I applied this (with minor massaging) on top if Linus' tree as of
commit 0429fbc0bdc297d64188483ba029a23773ae07b0 this morning.  It
built and booted on the macbook machine I originally reported the
problem with, so it does appear to be fixed in that case.  I'll try it
on some other machines likely tomorrow.

Sorry for the delay on this.  The merge window is particularly busy for me.

josh