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@xxxxxxxxxx>
> ---
> 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.
