Re: [REGRESSION] "efi: efistub: Convert into static library" and preparation patches

From: Ard Biesheuvel
Date: Wed Sep 03 2014 - 15:58:09 EST


On 3 September 2014 19:59, Matt Fleming <matt@xxxxxxxxxxxxxxxxx> wrote:
> On Wed, 03 Sep, at 05:37:26PM, Ard Biesheuvel wrote:
>>
>> Will do, thanks.
>>
>> @Matt: so there is two ways to fix this, the patch above addressing
>> this single instance, and alternatively, adding a #pragma GCC
>> visiblilty push(hidden) to all .c files under libstub/, *before* the
>> #includes. The latter would catch future problems regarding newly
>> introduced global variables, but it may be a bit overkill in this
>> case, as libstub is not expected to be in flux in the foreseeable
>> future.
>>
>> Any preferences?
>
> Any reason we can't reuse the existing GOT fixup code in the early x86
> boot code? We're not executing it before the EFI boot stub atm, which is
> the reason Maarten is hitting these difficulties.
>

I guess that is likely to work, I just wasn't aware it existed :-)
I think adding another visibility(hidden) attribute or 2 would
complete eliminate the need for GOT fixups, but I guess that is more
sensitive to compiler versions being recent enough etc.
The attached (build tested only) patch eliminates all GOT relocations
under boot/compressed for a 64-bit EFI stub build.

> Maarten, does the following help?
>
> If not, Ard please go ahead with option #2 above. Overkill yes, but I've
> done the single __attribute__() hacks in other projects and someone
> (usually me) always eventually forgets to tag some instance.
>

It appears we just got lucky on arm64, since we don't have any global
variables, but the issue does exist there as well.

--
Ard.


> ---
>
> diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> index 2884e0c3e8a5..7618857fcc60 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -32,6 +32,21 @@
> #include <asm/processor-flags.h>
> #include <asm/asm-offsets.h>
>
> +/*
> + * Adjust our own GOT
> + */
> +.macro FIXUP_GOT
> + leaq _got(%rip), %rdx
> + leaq _egot(%rip), %rcx
> +1:
> + cmpq %rcx, %rdx
> + jae 2f
> + addq %rbx, (%rdx)
> + addq $8, %rdx
> + jmp 1b
> +2:
> +.endm
> +
> __HEAD
> .code32
> ENTRY(startup_32)
> @@ -256,6 +271,8 @@ ENTRY(efi_pe_entry)
> */
> addq %rbp, efi64_config+88(%rip)
>
> + FIXUP_GOT
> +
> movq %rax, %rdi
> call make_boot_params
> cmpq $0,%rax
> @@ -275,6 +292,7 @@ handover_entry:
> */
> movq efi_config(%rip), %rax
> addq %rbp, 88(%rax)
> + FIXUP_GOT
> 2:
> movq efi_config(%rip), %rdi
> call efi_main
> @@ -385,19 +403,8 @@ relocated:
> shrq $3, %rcx
> rep stosq
>
> -/*
> - * Adjust our own GOT
> - */
> - leaq _got(%rip), %rdx
> - leaq _egot(%rip), %rcx
> -1:
> - cmpq %rcx, %rdx
> - jae 2f
> - addq %rbx, (%rdx)
> - addq $8, %rdx
> - jmp 1b
> -2:
> -
> + FIXUP_GOT
> +
> /*
> * Do the decompression, and jump to the new kernel..
> */
>
> --
> Matt Fleming, Intel Open Source Technology Center
From ad1503c3c770180c3540e9c82a58f84e21bdb868 Mon Sep 17 00:00:00 2001
From: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
Date: Wed, 3 Sep 2014 21:48:41 +0200
Subject: [PATCH] x86: eliminate x86_64 GOT relocations in early boot code

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
---
arch/x86/boot/boot.h | 4 ++++
arch/x86/boot/compressed/misc.h | 5 +++++
arch/x86/include/asm/efi.h | 2 +-
3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index bd49ec61255c..18a0010efc20 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -297,10 +297,14 @@ static inline int cmdline_find_option_bool(const char *option)
int check_cpu(int *cpu_level_ptr, int *req_level_ptr, u32 **err_flags_ptr);
int validate_cpu(void);

+#pragma GCC visibility push(hidden)
+
/* early_serial_console.c */
extern int early_serial_base;
void console_init(void);

+#pragma GCC visibility pop
+
/* edd.c */
void query_edd(void);

diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 24e3e569a13c..a0fd51f90add 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -29,10 +29,15 @@
#define memptr unsigned
#endif

+#pragma GCC visibility push(hidden)
+
/* misc.c */
extern memptr free_mem_ptr;
extern memptr free_mem_end_ptr;
extern struct boot_params *real_mode; /* Pointer to real-mode data */
+
+#pragma GCC visibility pop
+
void __putstr(const char *s);
#define error_putstr(__x) __putstr(__x)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 044a2fd3c5fe..8725d85f1903 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -178,7 +178,7 @@ struct efi_config {
bool is64;
} __packed;

-extern struct efi_config *efi_early;
+extern __attribute__((visibility("hidden"))) struct efi_config *efi_early;

#define efi_call_early(f, ...) \
efi_early->call(efi_early->f, __VA_ARGS__);
--
1.8.3.2