[PATCH v2 16/16] efi: LoongArch: Implement everything

[Simon Glass]

Hi Yao,

On 2026-07-01T11:17:53, Yao Zi <me at ziyao.cc> wrote:
> efi: LoongArch: Implement everything
>
> Implement crt, reloc, linker scripts, wire things up in
> Makefiles and Kconfig.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang at flygoat.com>
> Signed-off-by: Yao Zi <me at ziyao.cc>
>
> arch/loongarch/config.mk                 |   4 +
>  arch/loongarch/lib/Makefile              |  12 ++
>  arch/loongarch/lib/crt0_loongarch_efi.S  | 182 +++++++++++++++++++++++++++++
>  arch/loongarch/lib/elf_loongarch_efi.lds |  76 +++++++++++++
>  arch/loongarch/lib/reloc_loongarch_efi.c | 107 ++++++++++++++++++
>  lib/efi_loader/Kconfig                   |   2 +-
>  lib/efi_loader/efi_helper.c              |   3 +
>  lib/efi_loader/efi_image_loader.c        |  26 +++++
>  8 files changed, 411 insertions(+), 1 deletion(-)

>     efi: LoongArch: Implement everything
>
>     Implement crt, reloc, linker scripts, wire things up in
>     Makefiles and Kconfig.

The subject and body do not really say what this patch does. It
also silently adds handling for a new PE relocation type
(IMAGE_REL_BASED_LOONGARCH64_MARK_LA) in the shared EFI loader,
which is not mentioned. Please expand to describe the LoongArch EFI
application entry sequence, the .reloc stub, the MARK_LA relocation
format being decoded, and why it is needed.

> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> @@ -230,6 +232,30 @@ static efi_status_t efi_loader_relocate(const IMAGE_BASE_RELOCATION *rel,
> +#elif defined(__loongarch__) && __loongarch_grlen == 64
> +                     case IMAGE_REL_BASED_LOONGARCH64_MARK_LA:

This is a non-trivial change to shared code and would be much
easier to review as its own patch, with a commit message explaining
the four-instruction pcalau12i/addi/lu32i/lu52i sequence being
patched and citing the PE-COFF spec. Please split it out.

> diff --git a/arch/loongarch/lib/crt0_loongarch_efi.S b/arch/loongarch/lib/crt0_loongarch_efi.S
> @@ -0,0 +1,182 @@
> + * Copright (C) 2024 Jiaxun Yang <jiaxun.yang at flygoat.com>

Typo - Copright -> Copyright (the RISC-V file this was cloned from
has the same typo, but please don't propagate it).

> diff --git a/arch/loongarch/lib/crt0_loongarch_efi.S b/arch/loongarch/lib/crt0_loongarch_efi.S
> @@ -0,0 +1,182 @@
> +     bl              _relocate

...
> +     jr                      ra
> +     .end      _start

The whitespace here (mixed tabs and multiple spaces after the
mnemonic) is inconsistent with the rest of the file - please tidy.

> diff --git a/arch/loongarch/lib/reloc_loongarch_efi.c b/arch/loongarch/lib/reloc_loongarch_efi.c
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* reloc_loongarch.c - position independent ELF shared object relocator
> +   Copyright (C) 2018 Alexander Graf <agraf at suse.de>

...
> +    THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND

The SPDX tag says GPL-2.0+ but the pasted licence block is
BSD-2-Clause from the RISC-V/ARM copies. Please either drop the BSD
text (relying on SPDX) or reconcile the two — as-is the file is
self-contradictory. Also the filename in the first line still reads
reloc_loongarch.c.

> diff --git a/arch/loongarch/lib/reloc_loongarch_efi.c b/arch/loongarch/lib/reloc_loongarch_efi.c
> @@ -0,0 +1,107 @@
> +             case DT_PLTGOT:
> +                     addr = (unsigned long *)
> +                             ((unsigned long)dyn[i].d_un.d_ptr
> +                                     + ldbase);
> +                     break;

The value written to addr here is never read before being
overwritten in the loop below. Dead code inherited from the RISC-V
copy - please drop the DT_PLTGOT case.

> diff --git a/arch/loongarch/lib/Makefile b/arch/loongarch/lib/Makefile
> @@ -12,3 +12,15 @@ ifeq ($(CONFIG_$(SPL_)SYSRESET),)
> +extra-$(CONFIG_CMD_BOOTEFI_HELLO_COMPILE) += $(EFI_CRT0) $(EFI_RELOC)
> +extra-$(CONFIG_CMD_BOOTEFI_SELFTEST) += $(EFI_CRT0) $(EFI_RELOC)
> +extra-$(CONFIG_EFI) += $(EFI_CRT0) $(EFI_RELOC)

Have you built and run the EFI selftest and the hello app on
qemu-loongarch64? The cover letter mentions efistub kernel and grub
but not these. If not, please drop the two lines that pull them in
until they have been exercised, or note in the commit message that
they build but are untested.

Regards,
Simon