* [PATCH 1/2] x86/efi: Optimize away setup_gop32/64 if unused
@ 2016-08-22 10:01 Lukas Wunner
2016-08-22 10:01 ` [PATCH 2/2] x86/efi: Allow invocation of arbitrary boot services Lukas Wunner
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Lukas Wunner @ 2016-08-22 10:01 UTC (permalink / raw)
To: linux-efi, Matt Fleming; +Cc: Andreas Noever, Ard Biesheuvel, linux-kernel, x86
Eliminate the 64 bit code path when compiling for x86_32, and vice-versa
the 32 bit code path when compiling for x86_64 with mixed mode disabled.
Size of gop.o text section:
CONFIG_X86_32: 1758 before, 1299 after
CONFIG_X86_64 && !CONFIG_EFI_MIXED: 2201 before, 1406 after
CONFIG_X86_64 && CONFIG_EFI_MIXED: 2201 before and after
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/x86/include/asm/efi.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 6b06939..1bda245 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -207,14 +207,16 @@ struct efi_config {
__pure const struct efi_config *__efi_early(void);
+#define efi_is_64bit() \
+ (IS_ENABLED(CONFIG_X86_64) && (!IS_ENABLED(CONFIG_EFI_MIXED) \
+ || __efi_early()->is64))
+
#define efi_call_early(f, ...) \
__efi_early()->call(__efi_early()->f, __VA_ARGS__);
#define __efi_call_early(f, ...) \
__efi_early()->call((unsigned long)f, __VA_ARGS__);
-#define efi_is_64bit() __efi_early()->is64
-
extern bool efi_reboot_required(void);
#else
--
2.8.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] x86/efi: Allow invocation of arbitrary boot services 2016-08-22 10:01 [PATCH 1/2] x86/efi: Optimize away setup_gop32/64 if unused Lukas Wunner @ 2016-08-22 10:01 ` Lukas Wunner 2016-09-05 13:02 ` Matt Fleming 2016-09-05 12:37 ` [PATCH 1/2] x86/efi: Optimize away setup_gop32/64 if unused Matt Fleming 2016-09-06 6:05 ` [PATCH v2] " Lukas Wunner 2 siblings, 1 reply; 6+ messages in thread From: Lukas Wunner @ 2016-08-22 10:01 UTC (permalink / raw) To: linux-efi, Matt Fleming; +Cc: Andreas Noever, linux-kernel, x86 We currently allow invocation of 8 boot services with efi_call_early(). Not included are LocateHandleBuffer and LocateProtocol in particular. For graphics output or to retrieve PCI ROMs and Apple device properties, we're thus forced to use the LocateHandle + AllocatePool + LocateHandle combo, which is cumbersome and needs more code. The ARM folks allow invocation of the full set of boot services but are restricted to our 8 boot services in functions shared across arches. Thus, rather than adding just LocateHandleBuffer and LocateProtocol to struct efi_config, let's rework efi_call_early() to allow invocation of arbitrary boot services by selecting the 64 bit vs 32 bit code path in the macro itself. When compiling for 32 bit or for 64 bit without mixed mode, the unused code path is optimized away and the binary code is the same as before. But on 64 bit with mixed mode enabled, this commit adds one compare instruction to each invocation of a boot service and, depending on the code path selected, two jump instructions. (Most of the time gcc arranges the jumps in the 32 bit code path.) The result is a minuscule performance penalty and the binary code becomes slightly larger and more difficult to read when disassembled. This isn't a hot path, so these drawbacks are arguably outweighed by the attainable simplification of the C code. We have some overhead anyway for thunking or conversion between calling conventions. The 8 boot services can consequently be removed from struct efi_config. No functional change intended (for now). Example -- invocation of free_pool before (64 bit code path): 0x2d4 movq %ds:efi_early, %rdx ; efi_early 0x2db movq %ss:arg_0-0x20(%rsp), %rsi 0x2e0 xorl %eax, %eax 0x2e2 movq %ds:0x28(%rdx), %rdi ; efi_early->free_pool 0x2e6 callq *%ds:0x58(%rdx) ; efi_early->call() Example -- invocation of free_pool after (64 / 32 bit mixed code path): 0x0dc movq %ds:efi_early, %rax ; efi_early 0x0e3 cmpb $0, %ds:0x28(%rax) ; !efi_early->is64 ? 0x0e7 movq %ds:0x20(%rax), %rdx ; efi_early->call() 0x0eb movq %ds:0x10(%rax), %rax ; efi_early->boot_services 0x0ef je $0x150 0x0f1 movq %ds:0x48(%rax), %rdi ; free_pool (64 bit) 0x0f5 xorl %eax, %eax 0x0f7 callq *%rdx ... 0x150 movl %ds:0x30(%rax), %edi ; free_pool (32 bit) 0x153 jmp $0x0f5 Size of eboot.o text section: CONFIG_X86_32: 6464 before, 6318 after CONFIG_X86_64 && !CONFIG_EFI_MIXED: 7670 before, 7573 after CONFIG_X86_64 && CONFIG_EFI_MIXED: 7670 before, 8319 after Signed-off-by: Lukas Wunner <lukas@wunner.de> Cc: Matt Fleming <matt@codeblueprint.co.uk> --- arch/x86/boot/compressed/eboot.c | 13 +------------ arch/x86/boot/compressed/head_32.S | 6 +++--- arch/x86/boot/compressed/head_64.S | 8 ++++---- arch/x86/include/asm/efi.h | 15 ++++++--------- 4 files changed, 14 insertions(+), 28 deletions(-) diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index f7fc85b..447a6a2 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -29,22 +29,11 @@ __pure const struct efi_config *__efi_early(void) static void setup_boot_services##bits(struct efi_config *c) \ { \ efi_system_table_##bits##_t *table; \ - efi_boot_services_##bits##_t *bt; \ \ table = (typeof(table))sys_table; \ \ + c->boot_services = table->boottime; \ c->text_output = table->con_out; \ - \ - bt = (typeof(bt))(unsigned long)(table->boottime); \ - \ - c->allocate_pool = bt->allocate_pool; \ - c->allocate_pages = bt->allocate_pages; \ - c->get_memory_map = bt->get_memory_map; \ - c->free_pool = bt->free_pool; \ - c->free_pages = bt->free_pages; \ - c->locate_handle = bt->locate_handle; \ - c->handle_protocol = bt->handle_protocol; \ - c->exit_boot_services = bt->exit_boot_services; \ } BOOT_SERVICES(32); BOOT_SERVICES(64); diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S index 1038524..fd0b6a2 100644 --- a/arch/x86/boot/compressed/head_32.S +++ b/arch/x86/boot/compressed/head_32.S @@ -82,7 +82,7 @@ ENTRY(efi_pe_entry) /* Relocate efi_config->call() */ leal efi32_config(%esi), %eax - add %esi, 88(%eax) + add %esi, 32(%eax) pushl %eax call make_boot_params @@ -108,7 +108,7 @@ ENTRY(efi32_stub_entry) /* Relocate efi_config->call() */ leal efi32_config(%esi), %eax - add %esi, 88(%eax) + add %esi, 32(%eax) pushl %eax 2: call efi_main @@ -264,7 +264,7 @@ relocated: #ifdef CONFIG_EFI_STUB .data efi32_config: - .fill 11,8,0 + .fill 4,8,0 .long efi_call_phys .long 0 .byte 0 diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S index 0d80a7a..efdfba2 100644 --- a/arch/x86/boot/compressed/head_64.S +++ b/arch/x86/boot/compressed/head_64.S @@ -265,7 +265,7 @@ ENTRY(efi_pe_entry) /* * Relocate efi_config->call(). */ - addq %rbp, efi64_config+88(%rip) + addq %rbp, efi64_config+32(%rip) movq %rax, %rdi call make_boot_params @@ -285,7 +285,7 @@ handover_entry: * Relocate efi_config->call(). */ movq efi_config(%rip), %rax - addq %rbp, 88(%rax) + addq %rbp, 32(%rax) 2: movq efi_config(%rip), %rdi call efi_main @@ -457,14 +457,14 @@ efi_config: #ifdef CONFIG_EFI_MIXED .global efi32_config efi32_config: - .fill 11,8,0 + .fill 4,8,0 .quad efi64_thunk .byte 0 #endif .global efi64_config efi64_config: - .fill 11,8,0 + .fill 4,8,0 .quad efi_call .byte 1 #endif /* CONFIG_EFI_STUB */ diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h index 1bda245..65c714a 100644 --- a/arch/x86/include/asm/efi.h +++ b/arch/x86/include/asm/efi.h @@ -192,14 +192,7 @@ static inline efi_status_t efi_thunk_set_virtual_address_map( 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 boot_services; u64 text_output; efi_status_t (*call)(unsigned long, ...); bool is64; @@ -212,7 +205,11 @@ __pure const struct efi_config *__efi_early(void); || __efi_early()->is64)) #define efi_call_early(f, ...) \ - __efi_early()->call(__efi_early()->f, __VA_ARGS__); + __efi_early()->call(efi_is_64bit() ? \ + ((efi_boot_services_64_t *)(unsigned long) \ + __efi_early()->boot_services)->f : \ + ((efi_boot_services_32_t *)(unsigned long) \ + __efi_early()->boot_services)->f, __VA_ARGS__) #define __efi_call_early(f, ...) \ __efi_early()->call((unsigned long)f, __VA_ARGS__); -- 2.8.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] x86/efi: Allow invocation of arbitrary boot services 2016-08-22 10:01 ` [PATCH 2/2] x86/efi: Allow invocation of arbitrary boot services Lukas Wunner @ 2016-09-05 13:02 ` Matt Fleming 0 siblings, 0 replies; 6+ messages in thread From: Matt Fleming @ 2016-09-05 13:02 UTC (permalink / raw) To: Lukas Wunner; +Cc: linux-efi, Andreas Noever, linux-kernel, x86 On Mon, 22 Aug, at 12:01:21PM, Lukas Wunner wrote: > We currently allow invocation of 8 boot services with efi_call_early(). > Not included are LocateHandleBuffer and LocateProtocol in particular. > For graphics output or to retrieve PCI ROMs and Apple device properties, > we're thus forced to use the LocateHandle + AllocatePool + LocateHandle > combo, which is cumbersome and needs more code. > > The ARM folks allow invocation of the full set of boot services but are > restricted to our 8 boot services in functions shared across arches. > > Thus, rather than adding just LocateHandleBuffer and LocateProtocol to > struct efi_config, let's rework efi_call_early() to allow invocation of > arbitrary boot services by selecting the 64 bit vs 32 bit code path in > the macro itself. > > When compiling for 32 bit or for 64 bit without mixed mode, the unused > code path is optimized away and the binary code is the same as before. > But on 64 bit with mixed mode enabled, this commit adds one compare > instruction to each invocation of a boot service and, depending on the > code path selected, two jump instructions. (Most of the time gcc > arranges the jumps in the 32 bit code path.) The result is a minuscule > performance penalty and the binary code becomes slightly larger and more > difficult to read when disassembled. This isn't a hot path, so these > drawbacks are arguably outweighed by the attainable simplification of > the C code. We have some overhead anyway for thunking or conversion > between calling conventions. > > The 8 boot services can consequently be removed from struct efi_config. > > No functional change intended (for now). > > Example -- invocation of free_pool before (64 bit code path): > 0x2d4 movq %ds:efi_early, %rdx ; efi_early > 0x2db movq %ss:arg_0-0x20(%rsp), %rsi > 0x2e0 xorl %eax, %eax > 0x2e2 movq %ds:0x28(%rdx), %rdi ; efi_early->free_pool > 0x2e6 callq *%ds:0x58(%rdx) ; efi_early->call() > > Example -- invocation of free_pool after (64 / 32 bit mixed code path): > 0x0dc movq %ds:efi_early, %rax ; efi_early > 0x0e3 cmpb $0, %ds:0x28(%rax) ; !efi_early->is64 ? > 0x0e7 movq %ds:0x20(%rax), %rdx ; efi_early->call() > 0x0eb movq %ds:0x10(%rax), %rax ; efi_early->boot_services > 0x0ef je $0x150 > 0x0f1 movq %ds:0x48(%rax), %rdi ; free_pool (64 bit) > 0x0f5 xorl %eax, %eax > 0x0f7 callq *%rdx > ... > 0x150 movl %ds:0x30(%rax), %edi ; free_pool (32 bit) > 0x153 jmp $0x0f5 > > Size of eboot.o text section: > CONFIG_X86_32: 6464 before, 6318 after > CONFIG_X86_64 && !CONFIG_EFI_MIXED: 7670 before, 7573 after > CONFIG_X86_64 && CONFIG_EFI_MIXED: 7670 before, 8319 after > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > Cc: Matt Fleming <matt@codeblueprint.co.uk> > --- > arch/x86/boot/compressed/eboot.c | 13 +------------ > arch/x86/boot/compressed/head_32.S | 6 +++--- > arch/x86/boot/compressed/head_64.S | 8 ++++---- > arch/x86/include/asm/efi.h | 15 ++++++--------- > 4 files changed, 14 insertions(+), 28 deletions(-) Nice, I like this. Applied pending the update to the first patch. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] x86/efi: Optimize away setup_gop32/64 if unused 2016-08-22 10:01 [PATCH 1/2] x86/efi: Optimize away setup_gop32/64 if unused Lukas Wunner 2016-08-22 10:01 ` [PATCH 2/2] x86/efi: Allow invocation of arbitrary boot services Lukas Wunner @ 2016-09-05 12:37 ` Matt Fleming 2016-09-06 6:10 ` Lukas Wunner 2016-09-06 6:05 ` [PATCH v2] " Lukas Wunner 2 siblings, 1 reply; 6+ messages in thread From: Matt Fleming @ 2016-09-05 12:37 UTC (permalink / raw) To: Lukas Wunner; +Cc: linux-efi, Andreas Noever, Ard Biesheuvel, linux-kernel, x86 On Mon, 22 Aug, at 12:01:21PM, Lukas Wunner wrote: > Eliminate the 64 bit code path when compiling for x86_32, and vice-versa > the 32 bit code path when compiling for x86_64 with mixed mode disabled. > > Size of gop.o text section: > CONFIG_X86_32: 1758 before, 1299 after > CONFIG_X86_64 && !CONFIG_EFI_MIXED: 2201 before, 1406 after > CONFIG_X86_64 && CONFIG_EFI_MIXED: 2201 before and after > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > arch/x86/include/asm/efi.h | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h > index 6b06939..1bda245 100644 > --- a/arch/x86/include/asm/efi.h > +++ b/arch/x86/include/asm/efi.h > @@ -207,14 +207,16 @@ struct efi_config { > > __pure const struct efi_config *__efi_early(void); > > +#define efi_is_64bit() \ > + (IS_ENABLED(CONFIG_X86_64) && (!IS_ENABLED(CONFIG_EFI_MIXED) \ > + || __efi_early()->is64)) > + > #define efi_call_early(f, ...) \ > __efi_early()->call(__efi_early()->f, __VA_ARGS__); > > #define __efi_call_early(f, ...) \ > __efi_early()->call((unsigned long)f, __VA_ARGS__); > > -#define efi_is_64bit() __efi_early()->is64 > - > extern bool efi_reboot_required(void); > > #else Could you make this into a more readable static inline? e.g. something like this, static inline bool efi_is_64bit(void) { if (!IS_ENABLED(CONFIG_X86_64)) return false; if (!IS_ENABLED(CONFIG_EFI_MIXED) || __efi_early()->is64) return true; return false; } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] x86/efi: Optimize away setup_gop32/64 if unused 2016-09-05 12:37 ` [PATCH 1/2] x86/efi: Optimize away setup_gop32/64 if unused Matt Fleming @ 2016-09-06 6:10 ` Lukas Wunner 0 siblings, 0 replies; 6+ messages in thread From: Lukas Wunner @ 2016-09-06 6:10 UTC (permalink / raw) To: Matt Fleming; +Cc: linux-efi, Andreas Noever, Ard Biesheuvel, linux-kernel, x86 On Mon, Sep 05, 2016 at 01:37:25PM +0100, Matt Fleming wrote: > On Mon, 22 Aug, at 12:01:21PM, Lukas Wunner wrote: > > Eliminate the 64 bit code path when compiling for x86_32, and vice-versa > > the 32 bit code path when compiling for x86_64 with mixed mode disabled. > > > > Size of gop.o text section: > > CONFIG_X86_32: 1758 before, 1299 after > > CONFIG_X86_64 && !CONFIG_EFI_MIXED: 2201 before, 1406 after > > CONFIG_X86_64 && CONFIG_EFI_MIXED: 2201 before and after > > > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > arch/x86/include/asm/efi.h | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h > > index 6b06939..1bda245 100644 > > --- a/arch/x86/include/asm/efi.h > > +++ b/arch/x86/include/asm/efi.h > > @@ -207,14 +207,16 @@ struct efi_config { > > > > __pure const struct efi_config *__efi_early(void); > > > > +#define efi_is_64bit() \ > > + (IS_ENABLED(CONFIG_X86_64) && (!IS_ENABLED(CONFIG_EFI_MIXED) \ > > + || __efi_early()->is64)) > > + > > #define efi_call_early(f, ...) \ > > __efi_early()->call(__efi_early()->f, __VA_ARGS__); > > > > #define __efi_call_early(f, ...) \ > > __efi_early()->call((unsigned long)f, __VA_ARGS__); > > > > -#define efi_is_64bit() __efi_early()->is64 > > - > > extern bool efi_reboot_required(void); > > > > #else > > Could you make this into a more readable static inline? e.g. something > like this, > > static inline bool efi_is_64bit(void) > { > if (!IS_ENABLED(CONFIG_X86_64)) > return false; > > if (!IS_ENABLED(CONFIG_EFI_MIXED) || __efi_early()->is64) > return true; > > return false; > } Indeed this is much better. I've just sent out an updated version in-reply-to the original patch. Thanks a lot, Lukas ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] x86/efi: Optimize away setup_gop32/64 if unused 2016-08-22 10:01 [PATCH 1/2] x86/efi: Optimize away setup_gop32/64 if unused Lukas Wunner 2016-08-22 10:01 ` [PATCH 2/2] x86/efi: Allow invocation of arbitrary boot services Lukas Wunner 2016-09-05 12:37 ` [PATCH 1/2] x86/efi: Optimize away setup_gop32/64 if unused Matt Fleming @ 2016-09-06 6:05 ` Lukas Wunner 2 siblings, 0 replies; 6+ messages in thread From: Lukas Wunner @ 2016-09-06 6:05 UTC (permalink / raw) To: linux-efi, Matt Fleming; +Cc: Andreas Noever, linux-kernel, x86 Commit 2c23b73c2d02 ("x86/efi: Prepare GOP handling code for reuse as generic code") introduced an efi_is_64bit() macro to x86 which previously only existed for arm arches. The macro is used to choose between the 64 bit or 32 bit code path in gop.c at runtime. However the code path that's going to be taken is known at compile time when compiling for x86_32 or for x86_64 with mixed mode disabled. Amend the macro to eliminate the unused code path in those cases. Size of gop.o text section: CONFIG_X86_32: 1758 before, 1299 after CONFIG_X86_64 && !CONFIG_EFI_MIXED: 2201 before, 1406 after CONFIG_X86_64 && CONFIG_EFI_MIXED: 2201 before and after Signed-off-by: Lukas Wunner <lukas@wunner.de> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- arch/x86/include/asm/efi.h | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h index d0bb76d..520c4b7 100644 --- a/arch/x86/include/asm/efi.h +++ b/arch/x86/include/asm/efi.h @@ -207,14 +207,23 @@ struct efi_config { __pure const struct efi_config *__efi_early(void); +static inline bool efi_is_64bit(void) +{ + if (!IS_ENABLED(CONFIG_X86_64)) + return false; + + if (!IS_ENABLED(CONFIG_EFI_MIXED)) + return true; + + return __efi_early()->is64; +} + #define efi_call_early(f, ...) \ __efi_early()->call(__efi_early()->f, __VA_ARGS__); #define __efi_call_early(f, ...) \ __efi_early()->call((unsigned long)f, __VA_ARGS__); -#define efi_is_64bit() __efi_early()->is64 - extern bool efi_reboot_required(void); #else -- 2.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-06 6:15 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-22 10:01 [PATCH 1/2] x86/efi: Optimize away setup_gop32/64 if unused Lukas Wunner 2016-08-22 10:01 ` [PATCH 2/2] x86/efi: Allow invocation of arbitrary boot services Lukas Wunner 2016-09-05 13:02 ` Matt Fleming 2016-09-05 12:37 ` [PATCH 1/2] x86/efi: Optimize away setup_gop32/64 if unused Matt Fleming 2016-09-06 6:10 ` Lukas Wunner 2016-09-06 6:05 ` [PATCH v2] " Lukas Wunner
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).