All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matt Fleming <matt@codeblueprint.co.uk>
To: Lukas Wunner <lukas@wunner.de>
Cc: linux-efi@vger.kernel.org,
	Andreas Noever <andreas.noever@gmail.com>,
	linux-kernel@vger.kernel.org, x86@kernel.org
Subject: Re: [PATCH 2/2] x86/efi: Allow invocation of arbitrary boot services
Date: Mon, 5 Sep 2016 14:02:47 +0100	[thread overview]
Message-ID: <20160905130247.GG32579@codeblueprint.co.uk> (raw)
In-Reply-To: <19e0ac48c448dabbed8ed87bf872a39d1e7ea3b2.1471823100.git.lukas@wunner.de>

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.

  reply	other threads:[~2016-09-05 13:02 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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
     [not found] ` <188ea850c957034d482576dfdcf8c8a2536460cf.1471823100.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2016-08-22 10:01   ` [PATCH 2/2] x86/efi: Allow invocation of arbitrary boot services Lukas Wunner
2016-08-22 10:01     ` Lukas Wunner
2016-09-05 13:02     ` Matt Fleming [this message]
2016-09-05 12:37   ` [PATCH 1/2] x86/efi: Optimize away setup_gop32/64 if unused Matt Fleming
2016-09-05 12:37     ` Matt Fleming
     [not found]     ` <20160905123725.GF32579-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2016-09-06  6:10       ` Lukas Wunner
2016-09-06  6:10         ` Lukas Wunner
2016-09-06  6:05   ` [PATCH v2] " Lukas Wunner
2016-09-06  6:05     ` Lukas Wunner
     [not found]     ` <f80213780594fdb86b3c1f7256b459856ee953be.1473140971.git.lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
2016-09-06  8:03       ` Ard Biesheuvel
2016-09-06 11:08       ` Matt Fleming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160905130247.GG32579@codeblueprint.co.uk \
    --to=matt@codeblueprint.co.uk \
    --cc=andreas.noever@gmail.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.