Re: [PATCH 3/3] efistub: fix missed the initialization of gp

[Ard Biesheuvel]

On Wed, 6 Mar 2024 at 09:56, Yunhui Cui <cuiyunhui@xxxxxxxxxxxxx> wrote:
>
> Compared with gcc version 12, gcc version 13 uses the gp
> register for compilation optimization, but the efistub module
> does not initialize gp.
>
> Signed-off-by: Yunhui Cui <cuiyunhui@xxxxxxxxxxxxx>
> Co-Developed-by: Zhipeng Xu <xuzhipeng.1973@xxxxxxxxxxxxx>

This needs a sign-off, and your signoff needs to come after.

> ---
>  arch/riscv/kernel/efi-header.S | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/efi-header.S b/arch/riscv/kernel/efi-header.S
> index 515b2dfbca75..fa17c08c092a 100644
> --- a/arch/riscv/kernel/efi-header.S
> +++ b/arch/riscv/kernel/efi-header.S
> @@ -40,7 +40,7 @@ optional_header:
>         .long   __pecoff_data_virt_end - __pecoff_text_end      // SizeOfInitializedData
>  #endif
>         .long   0                                       // SizeOfUninitializedData
> -       .long   __efistub_efi_pe_entry - _start         // AddressOfEntryPoint
> +       .long   _efistub_entry - _start         // AddressOfEntryPoint
>         .long   efi_header_end - _start                 // BaseOfCode
>  #ifdef CONFIG_32BIT
>         .long  __pecoff_text_end - _start               // BaseOfData
> @@ -121,4 +121,13 @@ section_table:
>
>         .balign 0x1000
>  efi_header_end:
> +
> +       .global _efistub_entry
> +_efistub_entry:

This should go into .text or .init.text, not the header.

> +       /* Reload the global pointer */
> +       load_global_pointer
> +

What is supposed to happen here if CONFIG_SHADOW_CALL_STACK=y? The EFI
stub Makefile removes the SCS CFLAGS, so the stub will be built
without shadow call stack support, which I guess means that it might
use GP as a global pointer as usual?

> +       call __efistub_efi_pe_entry
> +       ret
> +

You are returning to the firmware here, but after modifying the GP
register. Shouldn't you restore it to its old value?