Hi Yao, On 2026-07-01T11:17:53, Yao Zi <[email protected]> wrote: > efi: LoongArch: Implement everything > > Implement crt, reloc, linker scripts, wire things up in > Makefiles and Kconfig. > > Signed-off-by: Jiaxun Yang <[email protected]> > Signed-off-by: Yao Zi <[email protected]> > > 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 <[email protected]> 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 <[email protected]> ... > + 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
