Re: [PATCH v1 2/3] objtool/LoongArch: Fix unreachable instruction warnings about EFISTUB

[Tiezhu Yang]

On 2025/9/6 上午12:04, Josh Poimboeuf wrote:
> On Fri, Sep 05, 2025 at 12:36:16PM +0800, Huacai Chen wrote:
> > Hi, Josh,
> >
> > On Fri, Sep 5, 2025 at 5:46 AM Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> > > On Thu, Sep 04, 2025 at 10:39:30AM -0700, Josh Poimboeuf wrote:
> > > > On Thu, Sep 04, 2025 at 11:59:30AM +0800, Huacai Chen wrote:
> > > > > This is from RISC-V code.
> > > > >
> > > > > __HEAD
> > > > > SYM_CODE_START(_start)
> > > > >          /*
> > > > >           * Image header expected by Linux boot-loaders. The image header data
> > > > >           * structure is described in asm/image.h.
> > > > >           * Do not modify it without modifying the structure and all bootloaders
> > > > >           * that expects this header format!!
> > > > >           */
> > > > > #ifdef CONFIG_EFI
> > > > >          /*
> > > > >           * This instruction decodes to "MZ" ASCII required by UEFI.
> > > > >           */
> > > > >          c.li s4,-13
> > > > >          j _start_kernel
> > > > > #else
> > > > >          /* jump to start kernel */
> > > > >          j _start_kernel
> > > > >          /* reserved */
> > > > >          .word 0
> > > > > #endif
> > > > >
> > > > > The HEAD section has instructions, if you change it into a data
> > > > > section then it loses the "x" attribute.
> > >
> > > Actually, the "x" attribute isn't needed for vmlinux.  The vmlinux
> > > linker script places it in the text region regardless.
> > >
> > > Moving the data to a data section should be really simple, something
> > > like the below.
> > >
> > > And yes, even the above RISC-V code can be in a data section.  Those
> > > instructions are part of the 'struct riscv_image_header' data structure.
> > This may work but also look strange (code in data section), it is more
> > like a "workaround". :)
>
> The "strange" part of the code is the intermixing of code and data.  If
> they can't be separated, then they are part of a data structure and
> belong in a data section.

I tried the following minimal changes, put the image header into
the section .head.data, do not link efistub lib.a into vmlinux.o,
just link efistub lib.a into vmlinux, no other changes, they have
same effect with patch #1 and #2, what do you think?

----->8-----
diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index a3a9759414f4..919c1970ce14 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -164,7 +164,6 @@ CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) 
$(KBUILD_CFLAGS) -dM -E -x c /dev
 endif

 libs-y += arch/loongarch/lib/
-libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a

 drivers-y              += arch/loongarch/crypto/

diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S
index e3865e92a917..c42500d9fad8 100644
--- a/arch/loongarch/kernel/head.S
+++ b/arch/loongarch/kernel/head.S
@@ -17,7 +17,7 @@

 #include "efi-header.S"

-       __HEAD
+       __HEADDATA

 _head:
        .word   IMAGE_DOS_SIGNATURE     /* "MZ", MS-DOS header */
diff --git a/arch/loongarch/kernel/vmlinux.lds.S 
b/arch/loongarch/kernel/vmlinux.lds.S
index 08ea921cdec1..fc35ef349aba 100644
--- a/arch/loongarch/kernel/vmlinux.lds.S
+++ b/arch/loongarch/kernel/vmlinux.lds.S
@@ -38,6 +38,7 @@ SECTIONS
        . = VMLINUX_LOAD_ADDRESS;

        _text = .;
+       HEAD_DATA_SECTION
        HEAD_TEXT_SECTION

        . = ALIGN(PECOFF_SEGMENT_ALIGN);
diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index ae2d2359b79e..0f95fb1649f3 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -645,6 +645,14 @@ defined(CONFIG_AUTOFDO_CLANG) || 
defined(CONFIG_PROPELLER_CLANG)
                *(.static_call.text)                                    \
                __static_call_text_end = .;

+/* Section used for early init (in .S files) */
+#define HEAD_DATA  KEEP(*(.head.data))
+
+#define HEAD_DATA_SECTION                                              \
+       .head.data : AT(ADDR(.head.data) - LOAD_OFFSET) {               \
+               HEAD_DATA                                               \
+       }
+
 /* Section used for early init (in .S files) */
 #define HEAD_TEXT  KEEP(*(.head.text))

diff --git a/include/linux/init.h b/include/linux/init.h
index a60d32d227ee..4e5be09c42cd 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -98,6 +98,7 @@

 /* For assembly routines */
 #define __HEAD         .section        ".head.text","ax"
+#define __HEADDATA     .section        ".head.data","aw"
 #define __INIT         .section        ".init.text","ax"
 #define __FINIT                .previous

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 51367c2bfc21..c664bfb9b15f 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -69,6 +69,12 @@ vmlinux_link()
                libs="${KBUILD_VMLINUX_LIBS}"
        fi

+       if [ "${SRCARCH}" = "loongarch" ]; then
+               if is_enabled CONFIG_EFI_STUB; then
+                       libs="${libs} drivers/firmware/efi/libstub/lib.a"
+               fi
+       fi
+
        if is_enabled CONFIG_GENERIC_BUILTIN_DTB; then
                objs="${objs} .builtin-dtbs.o"
        fi

Thanks,
Tiezhu