On Sun, Dec 22, 2019 at 04:29:48PM +0100, Ard Biesheuvel wrote:
> On Sun, 22 Dec 2019 at 13:46, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
> >
> > Hmm. Most of the kernel is compiled with the stack alignment set to 8, and there a lot of asm that makes no effort to preserve alignment beyond 8 bytes. So if EFI calls need 16 byte alignment, you may need to do something special.
> >
> > On new enough gcc (the versions that actually support the flags to set the alignment to 8), maybe you can use function attributes, or maybe you can stick a 16-byte-aligned local variable in functions that call EFI functions? The latter would be rather fragile.
>
> This patch replaces open coded SysV to MS calling convention
> translation to GCC generated code (using __attribute__((ms_abi)) which
> we have been using for a long time in EDK2), because the former relies
> on a wrapper function
>
> efi_call(fn, ...)
>
> which is type unsafe and relies on a lot of nasty casting, especially
> combined with the mixed mode support. efi_call() is implemented as
> below, and as Hans reports, omitting this sequence causes a boot
> regression on one of the platforms he has tested this on.
>
> So the question is which of the pieces below this UEFI implementation
> is actually relying on, and the stack pointer alignment is my first
> guess, but it could be any of the other things as well. Once we
> identify what it is we are missing, I can simply stick it back in, but
> without reverting to using the efi_call() thunk.
>
> Note that the decompressor/stub are built with the default stack
> alignment of 16 afaict, but if GRUB enters the decompressor with a
> misaligned stack, we probably wouldn't notice until we hit something
> like a movaps, right?
>
> Thanks,
> Ard.
>
Won't the entry code misalign the stack when efi_main is called,
assuming the stack was properly aligned at efi_stub_entry? There needs
to be a sub $8, %rsp in there, no?
arch/x86/boot/compressed/head_64.S:
#ifdef CONFIG_EFI_STUB
.org 0x390
SYM_FUNC_START(efi64_stub_entry)
SYM_FUNC_START_ALIAS(efi_stub_entry)
movq $1, %rcx
handover_entry:
call efi_main <--- this will enter efi_main with a misaligned stack?
movq %rax,%rsi
movl BP_code32_start(%esi), %eax
leaq startup_64(%rax), %rax
jmp *%rax
SYM_FUNC_END(efi64_stub_entry)
SYM_FUNC_END_ALIAS(efi_stub_entry)
#endif
