On 5 September 2014 22:27, Matt Fleming <matt@xxxxxxxxxxxxxxxxx> wrote:
> On Thu, 04 Sep, at 10:37:53PM, Matt Fleming wrote:
>>
>> Thanks Ard, I'll take a look in the morning.
>
> Maarten, could you try out this patch?
>
Any developments regarding this patch?
I tried to test it myself, but my Macbook happily boots straight from
EFI into the kernel without it.
--
Ard.
> ---
>
> From a058d81d9687671813560af72f34d63da3cc16bc Mon Sep 17 00:00:00 2001
> From: Matt Fleming <matt.fleming@xxxxxxxxx>
> Date: Fri, 5 Sep 2014 14:52:26 +0100
> Subject: [PATCH] x86/efi: Fixup GOT in all boot code paths
>
> Maarten reported that his Macbook pro 8.2 stopped booting after commit
> f23cf8bd5c1f49 ("efi/x86: efistub: Move shared dependencies to
> <asm/efi.h>"), the main feature of which is changing the visibility of
> symbol 'efi_early' from local to global.
>
> By making 'efi_early' global we end up requiring an entry in the Global
> Offset Table. Unfortunately, while we do include code to fixup GOT
> entries in the early boot code, it's only called after we've executed
> the EFI boot stub.
>
> What this amounts to is that references to 'efi_early' in the EFI boot
> stub don't point to the correct place.
>
> Since we've got multiple boot entry points we need to be prepared to
> fixup the GOT in multiple places, while ensuring that we never do it
> more than once, otherwise the GOT entries will still point to the wrong
> place.
>
> Reported-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxx>
> Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> Signed-off-by: Matt Fleming <matt.fleming@xxxxxxxxx>
> ---
> arch/x86/boot/compressed/head_32.S | 54 ++++++++++++++++++++++++++----------
> arch/x86/boot/compressed/head_64.S | 56 ++++++++++++++++++++++++++++----------
> 2 files changed, 81 insertions(+), 29 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
> index cbed1407a5cd..d6b8aa4c986c 100644
> --- a/arch/x86/boot/compressed/head_32.S
> +++ b/arch/x86/boot/compressed/head_32.S
> @@ -30,6 +30,33 @@
> #include <asm/boot.h>
> #include <asm/asm-offsets.h>
>
> +/*
> + * Adjust our own GOT
> + *
> + * The relocation base must be in %ebx
> + *
> + * It is safe to call this macro more than once, because in some of the
> + * code paths multiple invocations are inevitable, e.g. via the efi*
> + * entry points.
> + *
> + * Relocation is only performed the first time.
> + */
> +.macro FIXUP_GOT
> + cmpb $1, got_fixed(%ebx)
> + je 2f
> +
> + leal _got(%ebx), %edx
> + leal _egot(%ebx), %ecx
> +1:
> + cmpl %ecx, %edx
> + jae 2f
> + addl %ebx, (%edx)
> + addl $4, %edx
> + jmp 1b
> +2:
> + movb $1, got_fixed(%ebx)
> +.endm
> +
> __HEAD
> ENTRY(startup_32)
> #ifdef CONFIG_EFI_STUB
> @@ -56,6 +83,9 @@ ENTRY(efi_pe_entry)
> add %esi, 88(%eax)
> pushl %eax
>
> + movl %esi, %ebx
> + FIXUP_GOT
> +
> call make_boot_params
> cmpl $0, %eax
> je fail
> @@ -81,6 +111,10 @@ ENTRY(efi32_stub_entry)
> leal efi32_config(%esi), %eax
> add %esi, 88(%eax)
> pushl %eax
> +
> + movl %esi, %ebx
> + FIXUP_GOT
> +
> 2:
> call efi_main
> cmpl $0, %eax
> @@ -190,19 +224,7 @@ relocated:
> shrl $2, %ecx
> rep stosl
>
> -/*
> - * Adjust our own GOT
> - */
> - leal _got(%ebx), %edx
> - leal _egot(%ebx), %ecx
> -1:
> - cmpl %ecx, %edx
> - jae 2f
> - addl %ebx, (%edx)
> - addl $4, %edx
> - jmp 1b
> -2:
> -
> + FIXUP_GOT
> /*
> * Do the decompression, and jump to the new kernel..
> */
> @@ -225,8 +247,12 @@ relocated:
> xorl %ebx, %ebx
> jmp *%eax
>
> -#ifdef CONFIG_EFI_STUB
> .data
> +/* Have we relocated the GOT? */
> +got_fixed:
> + .byte 0
> +
> +#ifdef CONFIG_EFI_STUB
> efi32_config:
> .fill 11,8,0
> .long efi_call_phys
> diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> index 2884e0c3e8a5..50f69c7eaaf4 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -32,6 +32,33 @@
> #include <asm/processor-flags.h>
> #include <asm/asm-offsets.h>
>
> +/*
> + * Adjust our own GOT
> + *
> + * The relocation base must be in %rbx
> + *
> + * It is safe to call this macro more than once, because in some of the
> + * code paths multiple invocations are inevitable, e.g. via the efi*
> + * entry points.
> + *
> + * Relocation is only performed the first time.
> + */
> +.macro FIXUP_GOT
> + cmpb $1, got_fixed(%rip)
> + je 2f
> +
> + leaq _got(%rip), %rdx
> + leaq _egot(%rip), %rcx
> +1:
> + cmpq %rcx, %rdx
> + jae 2f
> + addq %rbx, (%rdx)
> + addq $8, %rdx
> + jmp 1b
> +2:
> + movb $1, got_fixed(%rip)
> +.endm
> +
> __HEAD
> .code32
> ENTRY(startup_32)
> @@ -252,10 +279,13 @@ ENTRY(efi_pe_entry)
> subq $1b, %rbp
>
> /*
> - * Relocate efi_config->call().
> + * Relocate efi_config->call() and the GOT entries.
> */
> addq %rbp, efi64_config+88(%rip)
>
> + movq %rbp, %rbx
> + FIXUP_GOT
> +
> movq %rax, %rdi
> call make_boot_params
> cmpq $0,%rax
> @@ -271,10 +301,13 @@ handover_entry:
> subq $1b, %rbp
>
> /*
> - * Relocate efi_config->call().
> + * Relocate efi_config->call() and the GOT entries.
> */
> movq efi_config(%rip), %rax
> addq %rbp, 88(%rax)
> +
> + movq %rbp, %rbx
> + FIXUP_GOT
> 2:
> movq efi_config(%rip), %rdi
> call efi_main
> @@ -385,19 +418,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..
> */
> @@ -437,6 +459,10 @@ gdt:
> .quad 0x0000000000000000 /* TS continued */
> gdt_end:
>
> +/* Have we relocated the GOT? */
> +got_fixed:
> + .byte 0
> +
> #ifdef CONFIG_EFI_STUB
> efi_config:
> .quad 0
> --
> 1.9.3
>
> --
> Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
