Re: [PATCH 1/3] efi/x86: simplify 64-bit EFI firmware call wrapper

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

On Thu, Dec 26, 2019 at 04:14:05PM +0100, Ard Biesheuvel wrote:
> The efi_call() wrapper used to invoke EFI runtime services serves
> a number of purposes:
> - realign the stack to 16 bytes
> - preserve FP register state
> - translate from SysV to MS calling convention.
> 
> Preserving the FP register state is redundant in most cases, since
> efi_call() is almost always used from within the scope of a pair of
> kernel_fpu_begin()/_end() calls, with the exception of the early
> call to SetVirtualAddressMap() and the SGI UV support code. So let's
> add a pair of kernel_fpu_begin()/_end() calls there as well, and
> remove the unnecessary code from the assembly implementation of
> efi_call(), and only keep the pieces that deal with the stack
> alignment and the ABI translation.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/x86/platform/efi/efi_64.c      |  4 +++
>  arch/x86/platform/efi/efi_stub_64.S | 36 ++------------------
>  arch/x86/platform/uv/bios_uv.c      |  7 ++--
>  3 files changed, 11 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index 03c2ed3c645c..3690df1d31c6 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -84,6 +84,7 @@ pgd_t * __init efi_call_phys_prolog(void)
>  
>  	if (!efi_enabled(EFI_OLD_MEMMAP)) {
>  		efi_switch_mm(&efi_mm);
> +		kernel_fpu_begin();
>  		return efi_mm.pgd;
>  	}
>  
> @@ -141,6 +142,7 @@ pgd_t * __init efi_call_phys_prolog(void)
>  	}
>  
>  	__flush_tlb_all();
> +	kernel_fpu_begin();
>  	return save_pgd;
>  out:
>  	efi_call_phys_epilog(save_pgd);
> @@ -158,6 +160,8 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
>  	p4d_t *p4d;
>  	pud_t *pud;
>  
> +	kernel_fpu_end();
> +
>  	if (!efi_enabled(EFI_OLD_MEMMAP)) {
>  		efi_switch_mm(efi_scratch.prev_mm);
>  		return;

Does kernel_fpu_begin/kernel_fpu_end need to be outside the efi_switch_mm?

If there's an error in efi_call_phys_prolog during the old memmap code,
it will call efi_call_phys_epilog without having called
kernel_fpu_begin, which will cause an unbalanced kernel_fpu_end. Looks
like the next step will be a panic anyway though.