Re: [PATCH 4/4] efi: On x86-64, align the stack to a 16-byte boundary as required by ABI

[Jordan Justen]

On Tue, Nov 12, 2013 at 6:27 PM, Josh Triplett <address@hidden> wrote:
> The x86-64 ABI specification requires a 16-byte-aligned stack.  In some
> cases, GCC emits code that assumes this alignment, which crashes if not
> aligned.  The EFI firmware is also entitled to assume that stack
> alignment without checking, and some firmware does make that assumption.
> ---
>
> ChangeLog entry:
>
> 2013-11-13  Josh Triplett  <address@hidden>
>
>         * grub-core/kern/x86_64/efi/startup.S (_start): Align the stack to a
>           16-byte boundary, as required by the x86-64 ABI, before calling
>           grub_main.  In some cases, GCC emits code that assumes this
>           alignment, which crashes if not aligned.  The EFI firmware is also
>           entitled to assume that stack alignment without checking.
>
>  grub-core/kern/x86_64/efi/startup.S | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/kern/x86_64/efi/startup.S 
> b/grub-core/kern/x86_64/efi/startup.S
> index f86f019..94bd6ae 100644
> --- a/grub-core/kern/x86_64/efi/startup.S
> +++ b/grub-core/kern/x86_64/efi/startup.S
> @@ -29,7 +29,11 @@ start:
>  _start:
>         movq    %rcx, EXT_C(grub_efi_image_handle)(%rip)
>         movq    %rdx, EXT_C(grub_efi_system_table)(%rip)
> -
> +       mov     %rsp, %rax
> +       subq    $8, %rsp
> +       and     $~0xf, %rsp
> +       mov     %rax, (%rsp)
>         call    EXT_C(grub_main)
> +       mov     (%rsp), %rsp

You can assume that the firmware followed the alignment convention, so
you just need to subtract 8 from the stack before calling, and add it
back after. Since rcx is not an output, how about:
push %rcx
call    EXT_C(grub_main)
pop %rcx

Or, use sub/add. Code might be larger, but would be more readable.

As far as Vladimir's comment about never returning, it seems like it
would be better to keep the path safe. But, either way, the comment
seems like a good idea.

-Jordan