[PATCH v3 1/3] objtool/LoongArch: Fix fall through warning about efistub

Tiezhu Yang posted 3 patches 2 weeks, 1 day ago
[PATCH v3 1/3] objtool/LoongArch: Fix fall through warning about efistub
Posted by Tiezhu Yang 2 weeks, 1 day ago
When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
the following objtool warning:

  vmlinux.o: warning: objtool: __efistub_efi_boot_kernel()
  falls through to next function __efistub_exit_boot_func()

This is because efi_boot_kernel() doesn't end with a return instruction
or an unconditional jump, then objtool has determined that the function
can fall through into the next function.

At the beginning, try to do something to make efi_boot_kernel() ends with
an unconditional jump instruction, but this modification seems not proper
and makes LoongArch different to other architectures.

Furthermore, it is not proper to link efistub separately for LoongArch,
ARM64 and RISC-V too due to there is technical reason why vmlinux.o needs
efistub linked in.

For example, there are two ways of populating the core kernel's struct
screen_info via the stub, the latter is preferred because it makes the
EFIFB earlycon available very early, but it only works if the EFI stub
is part of the core kernel image itself. For more information, please
see the comments in drivers/firmware/efi/libstub/screen_info.c.

After more analysis, one simple way is to ignore these efistub functions
in validate_branch() of objtool since they are useless for stack unwinder.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/check.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index d14f20ef1db1..2dcebf75d95e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3567,6 +3567,10 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 			    !strncmp(func->name, "__pfx_", 6))
 				return 0;
 
+			/* Ignore EFISTUB functions which are useless for stack unwinder */
+			if (!strncmp(func->name, "__efistub_", 10))
+				return 0;
+
 			if (file->ignore_unreachables)
 				return 0;
 
-- 
2.42.0
Re: [PATCH v3 1/3] objtool/LoongArch: Fix fall through warning about efistub
Posted by Josh Poimboeuf 2 weeks ago
On Wed, Sep 17, 2025 at 07:27:14PM +0800, Tiezhu Yang wrote:
> When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
> the following objtool warning:
> 
>   vmlinux.o: warning: objtool: __efistub_efi_boot_kernel()
>   falls through to next function __efistub_exit_boot_func()
> 
> This is because efi_boot_kernel() doesn't end with a return instruction
> or an unconditional jump, then objtool has determined that the function
> can fall through into the next function.
> 
> At the beginning, try to do something to make efi_boot_kernel() ends with
> an unconditional jump instruction, but this modification seems not proper
> and makes LoongArch different to other architectures.
> 
> Furthermore, it is not proper to link efistub separately for LoongArch,
> ARM64 and RISC-V too due to there is technical reason why vmlinux.o needs
> efistub linked in.
> 
> For example, there are two ways of populating the core kernel's struct
> screen_info via the stub, the latter is preferred because it makes the
> EFIFB earlycon available very early, but it only works if the EFI stub
> is part of the core kernel image itself. For more information, please
> see the comments in drivers/firmware/efi/libstub/screen_info.c.

Hm?  Of course libstub needs to be linked into *vmlinux*.  But that
doesn't mean it needs to be in vmlinux.o.

Why not build libstub.a separately, and then link it with vmlinux.o
during the final vmlinux link, as x86 does?

-- 
Josh
Re: [PATCH v3 1/3] objtool/LoongArch: Fix fall through warning about efistub
Posted by Tiezhu Yang 2 weeks ago
On 2025/9/18 上午6:32, Josh Poimboeuf wrote:
> On Wed, Sep 17, 2025 at 07:27:14PM +0800, Tiezhu Yang wrote:
>> When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
>> the following objtool warning:
>>
>>    vmlinux.o: warning: objtool: __efistub_efi_boot_kernel()
>>    falls through to next function __efistub_exit_boot_func()
>>
>> This is because efi_boot_kernel() doesn't end with a return instruction
>> or an unconditional jump, then objtool has determined that the function
>> can fall through into the next function.
>>
>> At the beginning, try to do something to make efi_boot_kernel() ends with
>> an unconditional jump instruction, but this modification seems not proper
>> and makes LoongArch different to other architectures.
>>
>> Furthermore, it is not proper to link efistub separately for LoongArch,
>> ARM64 and RISC-V too due to there is technical reason why vmlinux.o needs
>> efistub linked in.
>>
>> For example, there are two ways of populating the core kernel's struct
>> screen_info via the stub, the latter is preferred because it makes the
>> EFIFB earlycon available very early, but it only works if the EFI stub
>> is part of the core kernel image itself. For more information, please
>> see the comments in drivers/firmware/efi/libstub/screen_info.c.
> 
> Hm?  Of course libstub needs to be linked into *vmlinux*.  But that
> doesn't mean it needs to be in vmlinux.o.
> 
> Why not build libstub.a separately, and then link it with vmlinux.o
> during the final vmlinux link, as x86 does?

If this change is not OK, what do you think of the following changes?
Which one do you prefer?

(1) libstub doesn't link to vmlinux.o, only link libstub with vmlinux.o
during the final vmlinux link.

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

https://lore.kernel.org/loongarch/49ceb19c-6107-d026-3ae6-ae897d1fcae4@loongson.cn/

(2) make efi_boot_kernel() ends with an unconditional jump instruction.

----->8-----
diff --git a/drivers/firmware/efi/libstub/loongarch.c 
b/drivers/firmware/efi/libstub/loongarch.c
index 3782d0a187d1..e309fd78fca7 100644
--- a/drivers/firmware/efi/libstub/loongarch.c
+++ b/drivers/firmware/efi/libstub/loongarch.c
@@ -10,7 +10,7 @@
  #include "efistub.h"
  #include "loongarch-stub.h"

-typedef void __noreturn (*kernel_entry_t)(bool efi, unsigned long cmdline,
+typedef void (*kernel_entry_t)(bool efi, unsigned long cmdline,
  					  unsigned long systab);

  efi_status_t check_platform_features(void)
@@ -81,4 +81,7 @@ efi_status_t efi_boot_kernel(void *handle, 
efi_loaded_image_t *image,

  	real_kernel_entry(true, (unsigned long)cmdline_ptr,
  			  (unsigned long)efi_system_table);
+
+	/* We should never get here */
+	while (1);
  }

https://lore.kernel.org/loongarch/20250826064631.9617-2-yangtiezhu@loongson.cn/

Thanks,
Tiezhu

Re: [PATCH v3 1/3] objtool/LoongArch: Fix fall through warning about efistub
Posted by Josh Poimboeuf 1 week, 6 days ago
On Thu, Sep 18, 2025 at 09:44:24AM +0800, Tiezhu Yang wrote:
> (1) libstub doesn't link to vmlinux.o, only link libstub with vmlinux.o
> during the final vmlinux link.
> 
> ----->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/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

Right, though I think it would need to be something more generic so that
other arches can have "post-objtool libs" as well.

For example, arch/loongarch/Makefile could have 

  KBUILD_VMLINUX_LIBS_PRELINK += $(objtree)/drivers/firmware/efi/libstub/lib.a

which can be exported by the top-level Makefile:

  export KBUILD_VMLINUX_LIBS_PRELINK

and then used by scripts/link-vmlinux.sh (untested):

  ${ld} ${ldflags} -o ${output} \
	${wl}--whole-archive ${objs} ${wl}--no-whole-archive \
	${wl}--start-group ${libs} ${KBUILD_VMLINUX_LIBS_PRELINK} ${wl}--end-group \
	${kallsymso} ${btf_vmlinux_bin_o} ${arch_vmlinux_o} ${ldlibs}

-- 
Josh
Re: [PATCH v3 1/3] objtool/LoongArch: Fix fall through warning about efistub
Posted by Huacai Chen 1 week, 6 days ago
On Fri, Sep 19, 2025 at 8:20 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Thu, Sep 18, 2025 at 09:44:24AM +0800, Tiezhu Yang wrote:
> > (1) libstub doesn't link to vmlinux.o, only link libstub with vmlinux.o
> > during the final vmlinux link.
> >
> > ----->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/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
>
> Right, though I think it would need to be something more generic so that
> other arches can have "post-objtool libs" as well.
I don't like this solution, but if ARM64, RISC-V and LoongArch are
changed together, I can accept.
And please remember to ensure ZBOOT on EFISTUB still works.

Huacai

>
> For example, arch/loongarch/Makefile could have
>
>   KBUILD_VMLINUX_LIBS_PRELINK += $(objtree)/drivers/firmware/efi/libstub/lib.a
>
> which can be exported by the top-level Makefile:
>
>   export KBUILD_VMLINUX_LIBS_PRELINK
>
> and then used by scripts/link-vmlinux.sh (untested):
>
>   ${ld} ${ldflags} -o ${output} \
>         ${wl}--whole-archive ${objs} ${wl}--no-whole-archive \
>         ${wl}--start-group ${libs} ${KBUILD_VMLINUX_LIBS_PRELINK} ${wl}--end-group \
>         ${kallsymso} ${btf_vmlinux_bin_o} ${arch_vmlinux_o} ${ldlibs}
>
> --
> Josh