Re: [PATCH 3/3] efi/x86: simplify mixed mode call wrapper

[Arvind Sankar <nivedita@alum.mit.edu>]

On Thu, Dec 26, 2019 at 04:14:07PM +0100, Ard Biesheuvel wrote:
> Calling 32-bit EFI runtime services from a 64-bit OS involves
> switching back to the flat mapping with a stack carved out of
> memory that is 32-bit addressable.
> 
> There is no need to actually execute the 64-bit part of this
> routine from the flat mapping as well, as long as the entry
> and return address fit in 32 bits. There is also no need to
> preserve part of the calling context in global variables: we
> can simply preserve the old stack pointer in %r11 across the
> call into 32-bit firmware, and use either stack to preserve
> other values.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/x86/platform/efi/efi_thunk_64.S | 106 ++++++--------------
>  1 file changed, 29 insertions(+), 77 deletions(-)
> 
> diff --git a/arch/x86/platform/efi/efi_thunk_64.S b/arch/x86/platform/efi/efi_thunk_64.S
> index 3189f1394701..7357808d3ae8 100644
> --- a/arch/x86/platform/efi/efi_thunk_64.S
> +++ b/arch/x86/platform/efi/efi_thunk_64.S
> +	/*
> +	 * Convert 32-bit status code into 64-bit.
> +	 */
> +1:	btrl	$31, %eax
> +	jb	3f
> +2:	mov	%r11, %rsp
>  	pop	%rbx
>  	movl	%ebx, %ss
>  	pop	%rbx
>  	movl	%ebx, %es
>  	pop	%rbx
>  	movl	%ebx, %ds
> -
> -	/*
> -	 * 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:

Is it worth optimizing the conversion? The entire high nibble is
significant according to the spec. It probably doesn't matter except in
one potential case: according to the spec, transitioning secure boot
status to setup mode by deleting the platform key is allowed to return
EFI_WARN_RESET_REQUIRED and AFAICT this can take place after
ExitBootServices?

Separately, it might be worth considering moving the status translation
into C instead of asm for the version in
arch/x86/boot/compressed/efi_thunk_64.S -- there are (at least) three
protocols that have methods that don't return efi_status_t:
DEVICE_PATH_UTILITIES, DEVICE_PATH_TO_TEXT and DEVICE_PATH_FROM_TEXT. If
we ever want to use them (eg for debugging), it might be worth having a
thunk that doesn't mangle the return value.