On Fri, Dec 27, 2019 at 12:51:56PM -0500, Arvind Sankar wrote:
> 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.
Do we even need to save/restore the fpu state at this point in boot? The
mixed-mode code path doesn't appear to be saving/restoring the XMM
registers during SetVirtualAddressMap.
