* [PATCH 0/2] efi/libstub/x86: two more tweaks for the EFI stub startup code
@ 2020-01-08 7:45 Ard Biesheuvel
2020-01-08 7:45 ` [PATCH 1/2] efi/libstub/x86: use const attribute for efi_is_64bit() Ard Biesheuvel
2020-01-08 7:45 ` [PATCH 2/2] efi/libstub/x86: use mandatory 16-byte stack alignment in mixed mode Ard Biesheuvel
0 siblings, 2 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2020-01-08 7:45 UTC (permalink / raw)
To: linux-efi; +Cc: luto, x86, nivedita, Ard Biesheuvel
Two final cleanups for the x86 startup code, one that helps the compiler
generate better code, by annotating a helper function with the 'const'
function attribute, and one that fixes the misalignment of the stack in
mixed mode.
There are no known issues regarding entering the 32-bit firmware from the
64-bit kernel with the stack misaligned, and the 32-bit kernel does so all
the time, but it is better to comply with the UEFI spec.
Ard Biesheuvel (2):
efi/libstub/x86: use const attribute for efi_is_64bit()
efi/libstub/x86: use mandatory 16-byte stack alignment in mixed mode
arch/x86/boot/compressed/eboot.c | 14 +++---
arch/x86/boot/compressed/efi_thunk_64.S | 46 ++++++--------------
arch/x86/boot/compressed/head_64.S | 7 ++-
arch/x86/include/asm/efi.h | 2 +-
4 files changed, 23 insertions(+), 46 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] efi/libstub/x86: use const attribute for efi_is_64bit()
2020-01-08 7:45 [PATCH 0/2] efi/libstub/x86: two more tweaks for the EFI stub startup code Ard Biesheuvel
@ 2020-01-08 7:45 ` Ard Biesheuvel
2020-01-08 15:23 ` Arvind Sankar
2020-01-08 7:45 ` [PATCH 2/2] efi/libstub/x86: use mandatory 16-byte stack alignment in mixed mode Ard Biesheuvel
1 sibling, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2020-01-08 7:45 UTC (permalink / raw)
To: linux-efi; +Cc: luto, x86, nivedita, Ard Biesheuvel
Reshuffle the x86 stub code a bit so that we can tag the efi_is_64bit()
function with the 'const' attribute, which permits the compiler to
optimize away any redundant calls. Since we have two different entry
points for 32 and 64 bit firmware in the startup code, this also
simplifies the C code since we'll enter it with the efi_is64 variable
already set.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/x86/boot/compressed/eboot.c | 14 ++++++--------
arch/x86/boot/compressed/head_64.S | 7 +++----
arch/x86/include/asm/efi.h | 2 +-
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 4afd29eb5b34..ab3a40283db7 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -21,16 +21,18 @@
#include "eboot.h"
static efi_system_table_t *sys_table;
-static bool efi_is64 = IS_ENABLED(CONFIG_X86_64);
+extern const bool efi_is64;
__pure efi_system_table_t *efi_system_table(void)
{
return sys_table;
}
-__pure bool efi_is_64bit(void)
+__attribute_const__ bool efi_is_64bit(void)
{
- return efi_is64;
+ if (IS_ENABLED(CONFIG_EFI_MIXED))
+ return efi_is64;
+ return IS_ENABLED(CONFIG_X64_64);
}
static efi_status_t
@@ -710,8 +712,7 @@ static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
*/
struct boot_params *efi_main(efi_handle_t handle,
efi_system_table_t *sys_table_arg,
- struct boot_params *boot_params,
- bool is64)
+ struct boot_params *boot_params)
{
struct desc_ptr *gdt = NULL;
struct setup_header *hdr = &boot_params->hdr;
@@ -721,9 +722,6 @@ struct boot_params *efi_main(efi_handle_t handle,
sys_table = sys_table_arg;
- if (IS_ENABLED(CONFIG_EFI_MIXED))
- efi_is64 = is64;
-
/* Check if we were booted by the EFI firmware */
if (sys_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
goto fail;
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 44a6bb6964b5..1f1f6c8139b3 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -211,10 +211,9 @@ SYM_FUNC_START(startup_32)
movl efi32_boot_args(%ebp), %edi
cmp $0, %edi
jz 1f
- leal handover_entry(%ebp), %eax
+ leal efi64_stub_entry(%ebp), %eax
movl %esi, %edx
movl efi32_boot_args+4(%ebp), %esi
- movl $0x0, %ecx
1:
#endif
pushl %eax
@@ -242,6 +241,7 @@ SYM_FUNC_START(efi32_stub_entry)
movl %ecx, efi32_boot_args(%ebp)
movl %edx, efi32_boot_args+4(%ebp)
sgdtl efi32_boot_gdt(%ebp)
+ movb $0, efi_is64(%ebp)
/* Disable paging */
movl %cr0, %eax
@@ -452,8 +452,6 @@ SYM_CODE_END(startup_64)
.org 0x390
SYM_FUNC_START(efi64_stub_entry)
SYM_FUNC_START_ALIAS(efi_stub_entry)
- movq $1, %rcx
-SYM_INNER_LABEL(handover_entry, SYM_L_LOCAL)
and $~0xf, %rsp /* realign the stack */
call efi_main
movq %rax,%rsi
@@ -632,6 +630,7 @@ SYM_DATA_END_LABEL(gdt, SYM_L_LOCAL, gdt_end)
#ifdef CONFIG_EFI_MIXED
SYM_DATA_LOCAL(efi32_boot_args, .long 0, 0)
+SYM_DATA(efi_is64, .byte 1)
#endif
/*
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 9ce697a621cc..86169a24b0d8 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -223,7 +223,7 @@ efi_status_t efi_set_virtual_address_map(unsigned long memory_map_size,
/* arch specific definitions used by the stub code */
-__pure bool efi_is_64bit(void);
+__attribute_const__ bool efi_is_64bit(void);
static inline bool efi_is_native(void)
{
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] efi/libstub/x86: use mandatory 16-byte stack alignment in mixed mode
2020-01-08 7:45 [PATCH 0/2] efi/libstub/x86: two more tweaks for the EFI stub startup code Ard Biesheuvel
2020-01-08 7:45 ` [PATCH 1/2] efi/libstub/x86: use const attribute for efi_is_64bit() Ard Biesheuvel
@ 2020-01-08 7:45 ` Ard Biesheuvel
1 sibling, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2020-01-08 7:45 UTC (permalink / raw)
To: linux-efi; +Cc: luto, x86, nivedita, Ard Biesheuvel
Reduce the stack frame of the EFI stub's mixed mode thunk routine by
8 bytes, by moving the GDT and return addresses to EBP and EBX, which
we need to preserve anyway, since their top halves will be cleared by
the call into 32-bit firmware code. Doing so results in the UEFI code
being entered with a 16 byte aligned stack, as mandated by the UEFI
spec, fixing the last occurrence in the 64-bit kernel where we violate
this requirement.
Also, move the saved GDT from a global variable to an unused part of the
stack frame, and touch up some other parts of the code.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/x86/boot/compressed/efi_thunk_64.S | 46 ++++++--------------
1 file changed, 13 insertions(+), 33 deletions(-)
diff --git a/arch/x86/boot/compressed/efi_thunk_64.S b/arch/x86/boot/compressed/efi_thunk_64.S
index d040ff5458e5..8fb7f6799c52 100644
--- a/arch/x86/boot/compressed/efi_thunk_64.S
+++ b/arch/x86/boot/compressed/efi_thunk_64.S
@@ -27,12 +27,9 @@ SYM_FUNC_START(__efi64_thunk)
push %rbp
push %rbx
- subq $8, %rsp
- leaq 1f(%rip), %rax
- movl %eax, 4(%rsp)
- leaq efi_gdt64(%rip), %rax
- movl %eax, (%rsp)
- movl %eax, 2(%rax) /* Fixup the gdt base address */
+ leaq 1f(%rip), %rbp
+ leaq efi_gdt64(%rip), %rbx
+ movl %ebx, 2(%rbx) /* Fixup the gdt base address */
movl %ds, %eax
push %rax
@@ -48,12 +45,10 @@ SYM_FUNC_START(__efi64_thunk)
movl %esi, 0x0(%rsp)
movl %edx, 0x4(%rsp)
movl %ecx, 0x8(%rsp)
- movq %r8, %rsi
- movl %esi, 0xc(%rsp)
- movq %r9, %rsi
- movl %esi, 0x10(%rsp)
+ movl %r8d, 0xc(%rsp)
+ movl %r9d, 0x10(%rsp)
- sgdt save_gdt(%rip)
+ sgdt 0x14(%rsp)
/*
* Switch to gdt with 32-bit segments. This is the firmware GDT
@@ -68,11 +63,10 @@ SYM_FUNC_START(__efi64_thunk)
pushq %rax
lretq
-1: addq $32, %rsp
+1: lgdt 0x14(%rsp)
+ addq $32, %rsp
movq %rdi, %rax
- lgdt save_gdt(%rip)
-
pop %rbx
movl %ebx, %ss
pop %rbx
@@ -83,15 +77,9 @@ SYM_FUNC_START(__efi64_thunk)
/*
* Convert 32-bit status code into 64-bit.
*/
- test %rax, %rax
- jz 1f
- movl %eax, %ecx
- andl $0x0fffffff, %ecx
- andl $0xf0000000, %eax
- shl $32, %rax
- or %rcx, %rax
-1:
- addq $8, %rsp
+ roll $1, %eax
+ rorq $1, %rax
+
pop %rbx
pop %rbp
ret
@@ -135,9 +123,7 @@ SYM_FUNC_START_LOCAL(efi_enter32)
*/
cli
- movl 56(%esp), %eax
- movl %eax, 2(%eax)
- lgdtl (%eax)
+ lgdtl (%ebx)
movl %cr4, %eax
btsl $(X86_CR4_PAE_BIT), %eax
@@ -154,9 +140,8 @@ SYM_FUNC_START_LOCAL(efi_enter32)
xorl %eax, %eax
lldt %ax
- movl 60(%esp), %eax
pushl $__KERNEL_CS
- pushl %eax
+ pushl %ebp
/* Enable paging */
movl %cr0, %eax
@@ -172,11 +157,6 @@ SYM_DATA_START(efi32_boot_gdt)
.quad 0
SYM_DATA_END(efi32_boot_gdt)
-SYM_DATA_START_LOCAL(save_gdt)
- .word 0
- .quad 0
-SYM_DATA_END(save_gdt)
-
SYM_DATA_START(efi_gdt64)
.word efi_gdt64_end - efi_gdt64
.long 0 /* Filled out by user */
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] efi/libstub/x86: use const attribute for efi_is_64bit()
2020-01-08 7:45 ` [PATCH 1/2] efi/libstub/x86: use const attribute for efi_is_64bit() Ard Biesheuvel
@ 2020-01-08 15:23 ` Arvind Sankar
2020-01-08 15:25 ` Ard Biesheuvel
0 siblings, 1 reply; 7+ messages in thread
From: Arvind Sankar @ 2020-01-08 15:23 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, luto, x86, nivedita
On Wed, Jan 08, 2020 at 08:45:01AM +0100, Ard Biesheuvel wrote:
> Reshuffle the x86 stub code a bit so that we can tag the efi_is_64bit()
> function with the 'const' attribute, which permits the compiler to
> optimize away any redundant calls. Since we have two different entry
> points for 32 and 64 bit firmware in the startup code, this also
> simplifies the C code since we'll enter it with the efi_is64 variable
> already set.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/x86/boot/compressed/eboot.c | 14 ++++++--------
> arch/x86/boot/compressed/head_64.S | 7 +++----
> arch/x86/include/asm/efi.h | 2 +-
> 3 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> index 4afd29eb5b34..ab3a40283db7 100644
> --- a/arch/x86/boot/compressed/eboot.c
> +++ b/arch/x86/boot/compressed/eboot.c
> @@ -21,16 +21,18 @@
> #include "eboot.h"
>
> static efi_system_table_t *sys_table;
> -static bool efi_is64 = IS_ENABLED(CONFIG_X86_64);
> +extern const bool efi_is64;
>
Didn't we need to declare this with hidden visibility? Or use the
#pragma GCC visibility push(hidden)?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] efi/libstub/x86: use const attribute for efi_is_64bit()
2020-01-08 15:23 ` Arvind Sankar
@ 2020-01-08 15:25 ` Ard Biesheuvel
2020-01-08 15:25 ` Ard Biesheuvel
0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2020-01-08 15:25 UTC (permalink / raw)
To: Arvind Sankar
Cc: Ard Biesheuvel, linux-efi, Andy Lutomirski, the arch/x86 maintainers
On Wed, 8 Jan 2020 at 16:23, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Wed, Jan 08, 2020 at 08:45:01AM +0100, Ard Biesheuvel wrote:
> > Reshuffle the x86 stub code a bit so that we can tag the efi_is_64bit()
> > function with the 'const' attribute, which permits the compiler to
> > optimize away any redundant calls. Since we have two different entry
> > points for 32 and 64 bit firmware in the startup code, this also
> > simplifies the C code since we'll enter it with the efi_is64 variable
> > already set.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > arch/x86/boot/compressed/eboot.c | 14 ++++++--------
> > arch/x86/boot/compressed/head_64.S | 7 +++----
> > arch/x86/include/asm/efi.h | 2 +-
> > 3 files changed, 10 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> > index 4afd29eb5b34..ab3a40283db7 100644
> > --- a/arch/x86/boot/compressed/eboot.c
> > +++ b/arch/x86/boot/compressed/eboot.c
> > @@ -21,16 +21,18 @@
> > #include "eboot.h"
> >
> > static efi_system_table_t *sys_table;
> > -static bool efi_is64 = IS_ENABLED(CONFIG_X86_64);
> > +extern const bool efi_is64;
> >
>
> Didn't we need to declare this with hidden visibility? Or use the
> #pragma GCC visibility push(hidden)?
Yes. So this patch depends on
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] efi/libstub/x86: use const attribute for efi_is_64bit()
2020-01-08 15:25 ` Ard Biesheuvel
@ 2020-01-08 15:25 ` Ard Biesheuvel
2020-01-08 15:27 ` Arvind Sankar
0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2020-01-08 15:25 UTC (permalink / raw)
To: Arvind Sankar
Cc: Ard Biesheuvel, linux-efi, Andy Lutomirski, the arch/x86 maintainers
On Wed, 8 Jan 2020 at 16:25, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Wed, 8 Jan 2020 at 16:23, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > On Wed, Jan 08, 2020 at 08:45:01AM +0100, Ard Biesheuvel wrote:
> > > Reshuffle the x86 stub code a bit so that we can tag the efi_is_64bit()
> > > function with the 'const' attribute, which permits the compiler to
> > > optimize away any redundant calls. Since we have two different entry
> > > points for 32 and 64 bit firmware in the startup code, this also
> > > simplifies the C code since we'll enter it with the efi_is64 variable
> > > already set.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > > arch/x86/boot/compressed/eboot.c | 14 ++++++--------
> > > arch/x86/boot/compressed/head_64.S | 7 +++----
> > > arch/x86/include/asm/efi.h | 2 +-
> > > 3 files changed, 10 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> > > index 4afd29eb5b34..ab3a40283db7 100644
> > > --- a/arch/x86/boot/compressed/eboot.c
> > > +++ b/arch/x86/boot/compressed/eboot.c
> > > @@ -21,16 +21,18 @@
> > > #include "eboot.h"
> > >
> > > static efi_system_table_t *sys_table;
> > > -static bool efi_is64 = IS_ENABLED(CONFIG_X86_64);
> > > +extern const bool efi_is64;
> > >
> >
> > Didn't we need to declare this with hidden visibility? Or use the
> > #pragma GCC visibility push(hidden)?
>
> Yes. So this patch depends on
https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/commit/?h=next&id=c71339946177f235aa1f750b2dc556ede3288c23
which is part of the open PR to the TIP maintainers.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] efi/libstub/x86: use const attribute for efi_is_64bit()
2020-01-08 15:25 ` Ard Biesheuvel
@ 2020-01-08 15:27 ` Arvind Sankar
0 siblings, 0 replies; 7+ messages in thread
From: Arvind Sankar @ 2020-01-08 15:27 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Arvind Sankar, Ard Biesheuvel, linux-efi, Andy Lutomirski,
the arch/x86 maintainers
On Wed, Jan 08, 2020 at 04:25:41PM +0100, Ard Biesheuvel wrote:
> On Wed, 8 Jan 2020 at 16:25, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > On Wed, 8 Jan 2020 at 16:23, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > >
> > > On Wed, Jan 08, 2020 at 08:45:01AM +0100, Ard Biesheuvel wrote:
> > > > Reshuffle the x86 stub code a bit so that we can tag the efi_is_64bit()
> > > > function with the 'const' attribute, which permits the compiler to
> > > > optimize away any redundant calls. Since we have two different entry
> > > > points for 32 and 64 bit firmware in the startup code, this also
> > > > simplifies the C code since we'll enter it with the efi_is64 variable
> > > > already set.
> > > >
> > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > ---
> > > > arch/x86/boot/compressed/eboot.c | 14 ++++++--------
> > > > arch/x86/boot/compressed/head_64.S | 7 +++----
> > > > arch/x86/include/asm/efi.h | 2 +-
> > > > 3 files changed, 10 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> > > > index 4afd29eb5b34..ab3a40283db7 100644
> > > > --- a/arch/x86/boot/compressed/eboot.c
> > > > +++ b/arch/x86/boot/compressed/eboot.c
> > > > @@ -21,16 +21,18 @@
> > > > #include "eboot.h"
> > > >
> > > > static efi_system_table_t *sys_table;
> > > > -static bool efi_is64 = IS_ENABLED(CONFIG_X86_64);
> > > > +extern const bool efi_is64;
> > > >
> > >
> > > Didn't we need to declare this with hidden visibility? Or use the
> > > #pragma GCC visibility push(hidden)?
> >
> > Yes. So this patch depends on
>
> https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/commit/?h=next&id=c71339946177f235aa1f750b2dc556ede3288c23
>
> which is part of the open PR to the TIP maintainers.
Ah sorry.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-01-08 15:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-08 7:45 [PATCH 0/2] efi/libstub/x86: two more tweaks for the EFI stub startup code Ard Biesheuvel
2020-01-08 7:45 ` [PATCH 1/2] efi/libstub/x86: use const attribute for efi_is_64bit() Ard Biesheuvel
2020-01-08 15:23 ` Arvind Sankar
2020-01-08 15:25 ` Ard Biesheuvel
2020-01-08 15:25 ` Ard Biesheuvel
2020-01-08 15:27 ` Arvind Sankar
2020-01-08 7:45 ` [PATCH 2/2] efi/libstub/x86: use mandatory 16-byte stack alignment in mixed mode Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).