Re: [PATCH v2] grub-core: Build fixes for i386

[Daniel Kiper]

Mostly nits... Please take a look below...

On Tue, May 18, 2021 at 12:47:33PM +0200, Jan (janneke) Nieuwenhuizen wrote:
> This fixes cross-compiling to x86 (e.g., the Hurd) from x86-linux.
>
> To reproduce, update the Grub source description in your local Guix

s/Grub/GRUB/

> archive and run
>
>    ./pre-inst-env guix build --system=i686-linux --target=i586-pc-gnu grub
>
> or install an x86 cross-build environment on x86-linux (32bit!) and

s/32bit/32-bit/

> configure to cross build and make, e.g., do something like
>
>     ./configure \
>        CC_FOR_BUILD=gcc \
>        --build=i686-unknown-linux-gnu

Missing "\" at the end of the line...

>        --host=i586-pc-gnu
>     make
>
> * grub-core/lib/i386/relocator64.S: Avoid x86_64 instructions on i386.

Hmmm... What is this?

And you should add your SOB at the end of commit message:

Signed-off-by: Jan (janneke) Nieuwenhuizen <janneke@gnu.org>

> ---
>  grub-core/lib/i386/relocator64.S | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/lib/i386/relocator64.S 
> b/grub-core/lib/i386/relocator64.S
> index 148f38adb..b4675dd16 100644
> --- a/grub-core/lib/i386/relocator64.S
> +++ b/grub-core/lib/i386/relocator64.S
> @@ -63,8 +63,10 @@ VARIABLE(grub_relocator64_cr3)
>       movq    %rax, %cr3
>  #endif
>
>
> +#ifdef __x86_64__
>       .code64
> +#endif
>
>       /* mov imm64, %rax */
>       .byte   0x48
> @@ -71,7 +73,14 @@ VARIABLE(grub_relocator64_cr3)
>  VARIABLE(grub_relocator64_rsp)
>       .quad   0
>
> +#ifdef __x86_64__
>       movq    %rax, %rsp
> +#else
> +     /* movq %rax, %rsp */
> +     .byte   0x48
> +     .byte   0x89
> +     .byte   0xc4
> +#endif
>
>  #ifdef GRUB_MACHINE_EFI
>       jmp     LOCAL(skip_efi_stack_align)
> @@ -85,7 +94,14 @@ VARIABLE(grub_relocator64_rsp)
>        */
>  VARIABLE(grub_relocator64_efi_start)
>       /* Align the stack as UEFI spec requires. */
> +#ifdef __x86_64__
>       andq    $~15, %rsp
> +#else
> +     .byte 0x48
> +        .byte 0x83
> +        .byte 0xe4
> +        .byte 0xf0

Formatting is broken here...

> +#endif
>
>  LOCAL(skip_efi_stack_align):
>  #endif
> @@ -95,8 +111,15 @@ LOCAL(skip_efi_stack_align):
>  VARIABLE(grub_relocator64_rsi)
>       .quad   0
>
> +#ifdef       __x86_64__
>       movq    %rax, %rsi
> -

This and...

> +#else
> +     /* movq %rax, %rsi */
> +     .byte   0x48
> +     .byte   0x89
> +     .byte   0xc6
> +#endif
> +

... this change look strange. Could you fix it? Or explain in the
commit message you are doing a cleanup by the way...

Daniel