On Thu, Jul 02, 2026 at 11:29:02AM +0100, Simon Glass wrote:
> 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

For the entry sequence and .reloc stub part, I'm not sure whether it
deserves much explanation, since they're common to the most of (I didn't
check all though) U-Boot's ports with UEFI support. I could add some
explanation though if you insist on this.

For handling of IMAGE_REL_BASED_LOONGARCH64_MARK_LA, I'll separate it
into another patch.

> format being decoded, and why it is needed.

...

> > 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

Though the SPDX identifier indeed doesn't match, the file originates
from the gnuefi project, not the RISC-V/ARM ones in U-Boot, so I guess
I'll follow the origin license instead, and mention the origin in the
commit message.

> 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.

This file isn't inherited from RISC-V, but the gnuefi upstream - whose
LoongArch port seems to unnecessarily keep the DT_PLTGOT handling when
taking the MIPS port as reference.

I'll remove this DT_PLTGOT branch, and send a patch to gnuefi upstream.

...

> Regards,
> Simon

Best regards,
Yao Zi

Reply via email to