[PATCH v2] efistub: Only link libstub to final vmlinux

Tiezhu Yang posted 1 patch 4 months, 1 week ago
Makefile                | 1 +
arch/arm64/Makefile     | 5 ++++-
arch/loongarch/Makefile | 5 ++++-
arch/riscv/Makefile     | 5 ++++-
scripts/link-vmlinux.sh | 5 ++---
5 files changed, 15 insertions(+), 6 deletions(-)
[PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Tiezhu Yang 4 months, 1 week ago
When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
the following objtool warning on LoongArch:

  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.

Since the efistub functions are useless for stack unwinder, they can be
ignored by objtool. After many discussions, no need to link libstub to
the vmlinux.o, only link libstub to the final vmlinux.

Do the similar things for arm64 and riscv, otherwise there may be objtool
warnings when arm64 and riscv support objtool, this is to make consistent
with the archs that use libstub.

Link: https://lore.kernel.org/lkml/pq4h7jgndnt6p45lj4kgubxjd5gidfetugcuf5rcxzxxanzetd@6rrlpjnjsmuy/
Suggested-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 Makefile                | 1 +
 arch/arm64/Makefile     | 5 ++++-
 arch/loongarch/Makefile | 5 ++++-
 arch/riscv/Makefile     | 5 ++++-
 scripts/link-vmlinux.sh | 5 ++---
 5 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 10355ecf32cb..8ba2e28ef3d1 100644
--- a/Makefile
+++ b/Makefile
@@ -1201,6 +1201,7 @@ KBUILD_VMLINUX_OBJS := built-in.a $(patsubst %/, %/lib.a, $(filter %/, $(libs-y)
 KBUILD_VMLINUX_LIBS := $(filter-out %/, $(libs-y))
 
 export KBUILD_VMLINUX_LIBS
+export KBUILD_VMLINUX_LIBS_PRELINK
 export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux.lds
 
 ifdef CONFIG_TRIM_UNUSED_KSYMS
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 73a10f65ce8b..038f37ef2143 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -156,7 +156,10 @@ KBUILD_CPPFLAGS += -DKASAN_SHADOW_SCALE_SHIFT=$(KASAN_SHADOW_SCALE_SHIFT)
 KBUILD_AFLAGS += -DKASAN_SHADOW_SCALE_SHIFT=$(KASAN_SHADOW_SCALE_SHIFT)
 
 libs-y		:= arch/arm64/lib/ $(libs-y)
-libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
+
+ifdef CONFIG_EFI_STUB
+KBUILD_VMLINUX_LIBS_PRELINK += $(objtree)/drivers/firmware/efi/libstub/lib.a
+endif
 
 # Default target when executing plain make
 boot		:= arch/arm64/boot
diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index ae419e32f22e..4eb904c20718 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -169,7 +169,10 @@ 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
+
+ifdef CONFIG_EFI_STUB
+KBUILD_VMLINUX_LIBS_PRELINK += $(objtree)/drivers/firmware/efi/libstub/lib.a
+endif
 
 drivers-y		+= arch/loongarch/crypto/
 
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index df57654a615e..cfd82b2c1bbf 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -173,7 +173,10 @@ boot-image-$(CONFIG_XIP_KERNEL)		:= xipImage
 KBUILD_IMAGE				:= $(boot)/$(boot-image-y)
 
 libs-y += arch/riscv/lib/
-libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
+
+ifdef CONFIG_EFI_STUB
+KBUILD_VMLINUX_LIBS_PRELINK += $(objtree)/drivers/firmware/efi/libstub/lib.a
+endif
 
 ifeq ($(KBUILD_EXTMOD),)
 ifeq ($(CONFIG_MMU),y)
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 51367c2bfc21..b3cbff31d8a9 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -61,12 +61,11 @@ vmlinux_link()
 	shift
 
 	if is_enabled CONFIG_LTO_CLANG || is_enabled CONFIG_X86_KERNEL_IBT; then
-		# Use vmlinux.o instead of performing the slow LTO link again.
 		objs=vmlinux.o
-		libs=
+		libs="${KBUILD_VMLINUX_LIBS_PRELINK}"
 	else
 		objs=vmlinux.a
-		libs="${KBUILD_VMLINUX_LIBS}"
+		libs="${KBUILD_VMLINUX_LIBS} ${KBUILD_VMLINUX_LIBS_PRELINK}"
 	fi
 
 	if is_enabled CONFIG_GENERIC_BUILTIN_DTB; then
-- 
2.42.0
Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Ard Biesheuvel 4 months, 1 week ago
On Sun, 28 Sept 2025 at 10:55, Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
> the following objtool warning on LoongArch:
>
>   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.
>
> Since the efistub functions are useless for stack unwinder, they can be
> ignored by objtool. After many discussions, no need to link libstub to
> the vmlinux.o, only link libstub to the final vmlinux.
>

Please try keeping these changes confined to arch/loongarch. This
problem does not exist on other architectures, and changing the way
vmlinux is constructed might create other issues down the road.

> Do the similar things for arm64 and riscv, otherwise there may be objtool
> warnings when arm64 and riscv support objtool, this is to make consistent
> with the archs that use libstub.
>
> Link: https://lore.kernel.org/lkml/pq4h7jgndnt6p45lj4kgubxjd5gidfetugcuf5rcxzxxanzetd@6rrlpjnjsmuy/
> Suggested-by: Josh Poimboeuf <jpoimboe@kernel.org>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  Makefile                | 1 +
>  arch/arm64/Makefile     | 5 ++++-
>  arch/loongarch/Makefile | 5 ++++-
>  arch/riscv/Makefile     | 5 ++++-

>  scripts/link-vmlinux.sh | 5 ++---
>  5 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 10355ecf32cb..8ba2e28ef3d1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1201,6 +1201,7 @@ KBUILD_VMLINUX_OBJS := built-in.a $(patsubst %/, %/lib.a, $(filter %/, $(libs-y)
>  KBUILD_VMLINUX_LIBS := $(filter-out %/, $(libs-y))
>
>  export KBUILD_VMLINUX_LIBS
> +export KBUILD_VMLINUX_LIBS_PRELINK
>  export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux.lds
>
>  ifdef CONFIG_TRIM_UNUSED_KSYMS
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 73a10f65ce8b..038f37ef2143 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -156,7 +156,10 @@ KBUILD_CPPFLAGS += -DKASAN_SHADOW_SCALE_SHIFT=$(KASAN_SHADOW_SCALE_SHIFT)
>  KBUILD_AFLAGS += -DKASAN_SHADOW_SCALE_SHIFT=$(KASAN_SHADOW_SCALE_SHIFT)
>
>  libs-y         := arch/arm64/lib/ $(libs-y)
> -libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
> +
> +ifdef CONFIG_EFI_STUB
> +KBUILD_VMLINUX_LIBS_PRELINK += $(objtree)/drivers/firmware/efi/libstub/lib.a
> +endif
>
>  # Default target when executing plain make
>  boot           := arch/arm64/boot
> diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
> index ae419e32f22e..4eb904c20718 100644
> --- a/arch/loongarch/Makefile
> +++ b/arch/loongarch/Makefile
> @@ -169,7 +169,10 @@ 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
> +
> +ifdef CONFIG_EFI_STUB
> +KBUILD_VMLINUX_LIBS_PRELINK += $(objtree)/drivers/firmware/efi/libstub/lib.a
> +endif
>
>  drivers-y              += arch/loongarch/crypto/
>
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index df57654a615e..cfd82b2c1bbf 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -173,7 +173,10 @@ boot-image-$(CONFIG_XIP_KERNEL)            := xipImage
>  KBUILD_IMAGE                           := $(boot)/$(boot-image-y)
>
>  libs-y += arch/riscv/lib/
> -libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
> +
> +ifdef CONFIG_EFI_STUB
> +KBUILD_VMLINUX_LIBS_PRELINK += $(objtree)/drivers/firmware/efi/libstub/lib.a
> +endif
>
>  ifeq ($(KBUILD_EXTMOD),)
>  ifeq ($(CONFIG_MMU),y)
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 51367c2bfc21..b3cbff31d8a9 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -61,12 +61,11 @@ vmlinux_link()
>         shift
>
>         if is_enabled CONFIG_LTO_CLANG || is_enabled CONFIG_X86_KERNEL_IBT; then
> -               # Use vmlinux.o instead of performing the slow LTO link again.
>                 objs=vmlinux.o
> -               libs=
> +               libs="${KBUILD_VMLINUX_LIBS_PRELINK}"
>         else
>                 objs=vmlinux.a
> -               libs="${KBUILD_VMLINUX_LIBS}"
> +               libs="${KBUILD_VMLINUX_LIBS} ${KBUILD_VMLINUX_LIBS_PRELINK}"
>         fi
>
>         if is_enabled CONFIG_GENERIC_BUILTIN_DTB; then
> --
> 2.42.0
>
Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Huacai Chen 4 months, 1 week ago
Hi, Ard,

On Sun, Sep 28, 2025 at 9:42 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sun, 28 Sept 2025 at 10:55, Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >
> > When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
> > the following objtool warning on LoongArch:
> >
> >   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.
> >
> > Since the efistub functions are useless for stack unwinder, they can be
> > ignored by objtool. After many discussions, no need to link libstub to
> > the vmlinux.o, only link libstub to the final vmlinux.
> >
>
> Please try keeping these changes confined to arch/loongarch. This
> problem does not exist on other architectures, and changing the way
> vmlinux is constructed might create other issues down the road.
ARM, RISC-V and LoongArch do things exactly in the same way. Now
LoongArch is the first of the three to enable objtool, so we meet the
problem first.

But yes, I also don't want to change the way of constructing vmlinux.
So I prefer the earliest way to fix this problem.
https://lore.kernel.org/loongarch/CAAhV-H7fRHGFVKV8HitRgmuoDPt5ODt--iSuV0EmeeUb9d5FNw@mail.gmail.com/T/#meef7411abd14f4c28c85e686614aa9211fccdca0

Huacai

>
> > Do the similar things for arm64 and riscv, otherwise there may be objtool
> > warnings when arm64 and riscv support objtool, this is to make consistent
> > with the archs that use libstub.
> >
> > Link: https://lore.kernel.org/lkml/pq4h7jgndnt6p45lj4kgubxjd5gidfetugcuf5rcxzxxanzetd@6rrlpjnjsmuy/
> > Suggested-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> > ---
> >  Makefile                | 1 +
> >  arch/arm64/Makefile     | 5 ++++-
> >  arch/loongarch/Makefile | 5 ++++-
> >  arch/riscv/Makefile     | 5 ++++-
>
> >  scripts/link-vmlinux.sh | 5 ++---
> >  5 files changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 10355ecf32cb..8ba2e28ef3d1 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1201,6 +1201,7 @@ KBUILD_VMLINUX_OBJS := built-in.a $(patsubst %/, %/lib.a, $(filter %/, $(libs-y)
> >  KBUILD_VMLINUX_LIBS := $(filter-out %/, $(libs-y))
> >
> >  export KBUILD_VMLINUX_LIBS
> > +export KBUILD_VMLINUX_LIBS_PRELINK
> >  export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux.lds
> >
> >  ifdef CONFIG_TRIM_UNUSED_KSYMS
> > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> > index 73a10f65ce8b..038f37ef2143 100644
> > --- a/arch/arm64/Makefile
> > +++ b/arch/arm64/Makefile
> > @@ -156,7 +156,10 @@ KBUILD_CPPFLAGS += -DKASAN_SHADOW_SCALE_SHIFT=$(KASAN_SHADOW_SCALE_SHIFT)
> >  KBUILD_AFLAGS += -DKASAN_SHADOW_SCALE_SHIFT=$(KASAN_SHADOW_SCALE_SHIFT)
> >
> >  libs-y         := arch/arm64/lib/ $(libs-y)
> > -libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
> > +
> > +ifdef CONFIG_EFI_STUB
> > +KBUILD_VMLINUX_LIBS_PRELINK += $(objtree)/drivers/firmware/efi/libstub/lib.a
> > +endif
> >
> >  # Default target when executing plain make
> >  boot           := arch/arm64/boot
> > diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
> > index ae419e32f22e..4eb904c20718 100644
> > --- a/arch/loongarch/Makefile
> > +++ b/arch/loongarch/Makefile
> > @@ -169,7 +169,10 @@ 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
> > +
> > +ifdef CONFIG_EFI_STUB
> > +KBUILD_VMLINUX_LIBS_PRELINK += $(objtree)/drivers/firmware/efi/libstub/lib.a
> > +endif
> >
> >  drivers-y              += arch/loongarch/crypto/
> >
> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > index df57654a615e..cfd82b2c1bbf 100644
> > --- a/arch/riscv/Makefile
> > +++ b/arch/riscv/Makefile
> > @@ -173,7 +173,10 @@ boot-image-$(CONFIG_XIP_KERNEL)            := xipImage
> >  KBUILD_IMAGE                           := $(boot)/$(boot-image-y)
> >
> >  libs-y += arch/riscv/lib/
> > -libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
> > +
> > +ifdef CONFIG_EFI_STUB
> > +KBUILD_VMLINUX_LIBS_PRELINK += $(objtree)/drivers/firmware/efi/libstub/lib.a
> > +endif
> >
> >  ifeq ($(KBUILD_EXTMOD),)
> >  ifeq ($(CONFIG_MMU),y)
> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > index 51367c2bfc21..b3cbff31d8a9 100755
> > --- a/scripts/link-vmlinux.sh
> > +++ b/scripts/link-vmlinux.sh
> > @@ -61,12 +61,11 @@ vmlinux_link()
> >         shift
> >
> >         if is_enabled CONFIG_LTO_CLANG || is_enabled CONFIG_X86_KERNEL_IBT; then
> > -               # Use vmlinux.o instead of performing the slow LTO link again.
> >                 objs=vmlinux.o
> > -               libs=
> > +               libs="${KBUILD_VMLINUX_LIBS_PRELINK}"
> >         else
> >                 objs=vmlinux.a
> > -               libs="${KBUILD_VMLINUX_LIBS}"
> > +               libs="${KBUILD_VMLINUX_LIBS} ${KBUILD_VMLINUX_LIBS_PRELINK}"
> >         fi
> >
> >         if is_enabled CONFIG_GENERIC_BUILTIN_DTB; then
> > --
> > 2.42.0
> >
Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Ard Biesheuvel 4 months, 1 week ago
On Sun, 28 Sept 2025 at 15:52, Huacai Chen <chenhuacai@kernel.org> wrote:
>
> Hi, Ard,
>
> On Sun, Sep 28, 2025 at 9:42 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Sun, 28 Sept 2025 at 10:55, Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > >
> > > When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
> > > the following objtool warning on LoongArch:
> > >
> > >   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.
> > >
> > > Since the efistub functions are useless for stack unwinder, they can be
> > > ignored by objtool. After many discussions, no need to link libstub to
> > > the vmlinux.o, only link libstub to the final vmlinux.
> > >
> >
> > Please try keeping these changes confined to arch/loongarch. This
> > problem does not exist on other architectures, and changing the way
> > vmlinux is constructed might create other issues down the road.
> ARM, RISC-V and LoongArch do things exactly in the same way. Now
> LoongArch is the first of the three to enable objtool, so we meet the
> problem first.
>
> But yes, I also don't want to change the way of constructing vmlinux.
> So I prefer the earliest way to fix this problem.
> https://lore.kernel.org/loongarch/CAAhV-H7fRHGFVKV8HitRgmuoDPt5ODt--iSuV0EmeeUb9d5FNw@mail.gmail.com/T/#meef7411abd14f4c28c85e686614aa9211fccdca0
>

Can we just drop the __noreturn annotation from kernel_entry_t, and
return EFI_SUCCESS from efi_boot_kernel()?
Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Huacai Chen 4 months, 1 week ago
On Sun, Sep 28, 2025 at 10:40 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sun, 28 Sept 2025 at 15:52, Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > Hi, Ard,
> >
> > On Sun, Sep 28, 2025 at 9:42 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Sun, 28 Sept 2025 at 10:55, Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > > >
> > > > When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
> > > > the following objtool warning on LoongArch:
> > > >
> > > >   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.
> > > >
> > > > Since the efistub functions are useless for stack unwinder, they can be
> > > > ignored by objtool. After many discussions, no need to link libstub to
> > > > the vmlinux.o, only link libstub to the final vmlinux.
> > > >
> > >
> > > Please try keeping these changes confined to arch/loongarch. This
> > > problem does not exist on other architectures, and changing the way
> > > vmlinux is constructed might create other issues down the road.
> > ARM, RISC-V and LoongArch do things exactly in the same way. Now
> > LoongArch is the first of the three to enable objtool, so we meet the
> > problem first.
> >
> > But yes, I also don't want to change the way of constructing vmlinux.
> > So I prefer the earliest way to fix this problem.
> > https://lore.kernel.org/loongarch/CAAhV-H7fRHGFVKV8HitRgmuoDPt5ODt--iSuV0EmeeUb9d5FNw@mail.gmail.com/T/#meef7411abd14f4c28c85e686614aa9211fccdca0
> >
>
> Can we just drop the __noreturn annotation from kernel_entry_t, and
> return EFI_SUCCESS from efi_boot_kernel()?
Not good, because kernel_entry_t is really "noreturn", and at present
no architecture returns EFI_SUCCESS at the end of efi_boot_kernel().


Huacai
Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Ard Biesheuvel 4 months, 1 week ago
On Sun, 28 Sept 2025 at 16:39, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sun, 28 Sept 2025 at 15:52, Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > Hi, Ard,
> >
> > On Sun, Sep 28, 2025 at 9:42 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Sun, 28 Sept 2025 at 10:55, Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > > >
> > > > When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
> > > > the following objtool warning on LoongArch:
> > > >
> > > >   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.
> > > >
> > > > Since the efistub functions are useless for stack unwinder, they can be
> > > > ignored by objtool. After many discussions, no need to link libstub to
> > > > the vmlinux.o, only link libstub to the final vmlinux.
> > > >
> > >
> > > Please try keeping these changes confined to arch/loongarch. This
> > > problem does not exist on other architectures, and changing the way
> > > vmlinux is constructed might create other issues down the road.
> > ARM, RISC-V and LoongArch do things exactly in the same way. Now
> > LoongArch is the first of the three to enable objtool, so we meet the
> > problem first.
> >
> > But yes, I also don't want to change the way of constructing vmlinux.
> > So I prefer the earliest way to fix this problem.
> > https://lore.kernel.org/loongarch/CAAhV-H7fRHGFVKV8HitRgmuoDPt5ODt--iSuV0EmeeUb9d5FNw@mail.gmail.com/T/#meef7411abd14f4c28c85e686614aa9211fccdca0
> >
>
> Can we just drop the __noreturn annotation from kernel_entry_t, and
> return EFI_SUCCESS from efi_boot_kernel()?

... or add efi_boot_kernel() to ./tools/objtool/noreturns.h?
Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Tiezhu Yang 4 months ago
On 2025/9/28 下午10:41, Ard Biesheuvel wrote:
> On Sun, 28 Sept 2025 at 16:39, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Sun, 28 Sept 2025 at 15:52, Huacai Chen <chenhuacai@kernel.org> wrote:
>>>
>>> Hi, Ard,
>>>
>>> On Sun, Sep 28, 2025 at 9:42 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>
>>>> On Sun, 28 Sept 2025 at 10:55, Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>>>>
>>>>> When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
>>>>> the following objtool warning on LoongArch:
>>>>>
>>>>>    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.
>>>>>
>>>>> Since the efistub functions are useless for stack unwinder, they can be
>>>>> ignored by objtool. After many discussions, no need to link libstub to
>>>>> the vmlinux.o, only link libstub to the final vmlinux.
>>>>>
>>>>
>>>> Please try keeping these changes confined to arch/loongarch. This
>>>> problem does not exist on other architectures, and changing the way
>>>> vmlinux is constructed might create other issues down the road.

This was discussed and tested in the v3 patch, it only touches the code
of LoongArch and works well like this:

diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index dc5bd3f1b8d2..f34b416f5ca2 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -169,7 +169,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 433849ff7529..ed94871c3606 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/pq4h7jgndnt6p45lj4kgubxjd5gidfetugcuf5rcxzxxanzetd@6rrlpjnjsmuy/

>>> ARM, RISC-V and LoongArch do things exactly in the same way. Now
>>> LoongArch is the first of the three to enable objtool, so we meet the
>>> problem first.
>>>
>>> But yes, I also don't want to change the way of constructing vmlinux.
>>> So I prefer the earliest way to fix this problem.
>>> https://lore.kernel.org/loongarch/CAAhV-H7fRHGFVKV8HitRgmuoDPt5ODt--iSuV0EmeeUb9d5FNw@mail.gmail.com/T/#meef7411abd14f4c28c85e686614aa9211fccdca0
>>>
>>
>> Can we just drop the __noreturn annotation from kernel_entry_t, and
>> return EFI_SUCCESS from efi_boot_kernel()?

This was done in the early RFC patch, it works well like this:

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/

> ... or add efi_boot_kernel() to ./tools/objtool/noreturns.h?

It can not fix the objtool warning.

Are you OK with the following changes?

(1) libstub doesn't link to vmlinux.o, only link libstub with vmlinux.o 
during the final vmlinux link, keep the changes confined to LoongArch, 
no need to be something more generic.

diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index dc5bd3f1b8d2..f34b416f5ca2 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -169,7 +169,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 433849ff7529..ed94871c3606 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

(2) remove the attribute __noreturn for real_kernel_entry() and add
"while (1);" at the end of efi_boot_kernel().

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);
  }

Taking all of the considerations in balance, what should to do?

Thanks,
Tiezhu

Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Ard Biesheuvel 4 months ago
On Thu, 9 Oct 2025 at 00:27, Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> On 2025/9/28 下午10:41, Ard Biesheuvel wrote:
> > On Sun, 28 Sept 2025 at 16:39, Ard Biesheuvel <ardb@kernel.org> wrote:
> >>
> >> On Sun, 28 Sept 2025 at 15:52, Huacai Chen <chenhuacai@kernel.org> wrote:
> >>>
> >>> Hi, Ard,
> >>>
> >>> On Sun, Sep 28, 2025 at 9:42 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >>>>
> >>>> On Sun, 28 Sept 2025 at 10:55, Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >>>>>
> >>>>> When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
> >>>>> the following objtool warning on LoongArch:
> >>>>>
> >>>>>    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.
> >>>>>
> >>>>> Since the efistub functions are useless for stack unwinder, they can be
> >>>>> ignored by objtool. After many discussions, no need to link libstub to
> >>>>> the vmlinux.o, only link libstub to the final vmlinux.
> >>>>>
> >>>>
...
> Are you OK with the following changes?
>
> (1) libstub doesn't link to vmlinux.o, only link libstub with vmlinux.o
> during the final vmlinux link, keep the changes confined to LoongArch,
> no need to be something more generic.
>
> diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
> index dc5bd3f1b8d2..f34b416f5ca2 100644
> --- a/arch/loongarch/Makefile
> +++ b/arch/loongarch/Makefile
> @@ -169,7 +169,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 433849ff7529..ed94871c3606 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
>
> (2) remove the attribute __noreturn for real_kernel_entry() and add
> "while (1);" at the end of efi_boot_kernel().
>
> 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);
>   }
>

Why do we need both (1) and (2)?
Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Tiezhu Yang 4 months ago
On 2025/10/11 上午12:25, Ard Biesheuvel wrote:
...
> Why do we need both (1) and (2)?

Not both, either (1) or (2).
Which one do you prefer? Or any other suggestions?

Taking all of the considerations in balance, we should decide
what is the proper way.

Thanks,
Tiezhu

Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Huacai Chen 4 months ago
On Sat, Oct 11, 2025 at 9:13 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> On 2025/10/11 上午12:25, Ard Biesheuvel wrote:
> ...
> > Why do we need both (1) and (2)?
>
> Not both, either (1) or (2).
> Which one do you prefer? Or any other suggestions?
>
> Taking all of the considerations in balance, we should decide
> what is the proper way.
As a summary, there are three methods:
(1) Only link libstub with vmlinux.o during the final vmlinux link.
(2) Remove the attribute __noreturn for real_kernel_entry() and add while (1).
(3) Ignore "__efistub_" prefix in objtool.

Josh prefers method (1), I prefer method (3) but also accept method
(1) if it is not only specific to loongarch.

Huacai

>
> Thanks,
> Tiezhu
>
>
Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Ard Biesheuvel 4 months ago
On Fri, 10 Oct 2025 at 19:54, Huacai Chen <chenhuacai@kernel.org> wrote:
>
> On Sat, Oct 11, 2025 at 9:13 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >
> > On 2025/10/11 上午12:25, Ard Biesheuvel wrote:
> > ...
> > > Why do we need both (1) and (2)?
> >
> > Not both, either (1) or (2).
> > Which one do you prefer? Or any other suggestions?
> >
> > Taking all of the considerations in balance, we should decide
> > what is the proper way.
> As a summary, there are three methods:
> (1) Only link libstub with vmlinux.o during the final vmlinux link.
> (2) Remove the attribute __noreturn for real_kernel_entry() and add while (1).
> (3) Ignore "__efistub_" prefix in objtool.
>
> Josh prefers method (1), I prefer method (3) but also accept method
> (1) if it is not only specific to loongarch.
>

This is a false positive warning in objtool, which complains about a
function that falls through, even though that can never happen in
reality.

To me, it is not acceptable to modify how vmlinux.o is constructed
also for other architectures, in order to hide some of its constituent
parts from objtool, which do not use objtool to begin with.


If you are not willing to fix objtool, I suggest fixing the loongarch
code like this:

--- 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,6 @@

        real_kernel_entry(true, (unsigned long)cmdline_ptr,
                          (unsigned long)efi_system_table);
+
+       return EFI_LOAD_ERROR;
 }
Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Tiezhu Yang 4 months ago
On 2025/10/11 上午11:40, Ard Biesheuvel wrote:
> On Fri, 10 Oct 2025 at 19:54, Huacai Chen <chenhuacai@kernel.org> wrote:
>>
>> On Sat, Oct 11, 2025 at 9:13 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>>
>>> On 2025/10/11 上午12:25, Ard Biesheuvel wrote:
>>> ...
>>>> Why do we need both (1) and (2)?
>>>
>>> Not both, either (1) or (2).
>>> Which one do you prefer? Or any other suggestions?
>>>
>>> Taking all of the considerations in balance, we should decide
>>> what is the proper way.
>> As a summary, there are three methods:
>> (1) Only link libstub with vmlinux.o during the final vmlinux link.
>> (2) Remove the attribute __noreturn for real_kernel_entry() and add while (1).
>> (3) Ignore "__efistub_" prefix in objtool.
>>
>> Josh prefers method (1), I prefer method (3) but also accept method
>> (1) if it is not only specific to loongarch.
>>
> 
> This is a false positive warning in objtool, which complains about a
> function that falls through, even though that can never happen in
> reality.
> 
> To me, it is not acceptable to modify how vmlinux.o is constructed
> also for other architectures, in order to hide some of its constituent
> parts from objtool, which do not use objtool to begin with.
> 
> 
> If you are not willing to fix objtool, I suggest fixing the loongarch
> code like this:

Thank you.

> --- 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,6 @@
> 
>          real_kernel_entry(true, (unsigned long)cmdline_ptr,
>                            (unsigned long)efi_system_table);
> +
> +       return EFI_LOAD_ERROR;
>   }

I tested the above changes, the falls through objtool warning can
be fixed because efi_boot_kernel() ends with a return instruction,
I think this is reasonable.

efi_boot_kernel() has a return value, there are "return status" in
other parts of efi_boot_kernel(), it should also return at the end
of efi_boot_kernel() in theory, although we should never get here.

If there are more comments, please let me know.

Thanks,
Tiezhu

Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Huacai Chen 4 months ago
On Sat, Oct 11, 2025 at 3:29 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> On 2025/10/11 上午11:40, Ard Biesheuvel wrote:
> > On Fri, 10 Oct 2025 at 19:54, Huacai Chen <chenhuacai@kernel.org> wrote:
> >>
> >> On Sat, Oct 11, 2025 at 9:13 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >>>
> >>> On 2025/10/11 上午12:25, Ard Biesheuvel wrote:
> >>> ...
> >>>> Why do we need both (1) and (2)?
> >>>
> >>> Not both, either (1) or (2).
> >>> Which one do you prefer? Or any other suggestions?
> >>>
> >>> Taking all of the considerations in balance, we should decide
> >>> what is the proper way.
> >> As a summary, there are three methods:
> >> (1) Only link libstub with vmlinux.o during the final vmlinux link.
> >> (2) Remove the attribute __noreturn for real_kernel_entry() and add while (1).
> >> (3) Ignore "__efistub_" prefix in objtool.
> >>
> >> Josh prefers method (1), I prefer method (3) but also accept method
> >> (1) if it is not only specific to loongarch.
> >>
> >
> > This is a false positive warning in objtool, which complains about a
> > function that falls through, even though that can never happen in
> > reality.
> >
> > To me, it is not acceptable to modify how vmlinux.o is constructed
> > also for other architectures, in order to hide some of its constituent
> > parts from objtool, which do not use objtool to begin with.
> >
> >
> > If you are not willing to fix objtool, I suggest fixing the loongarch
> > code like this:
>
> Thank you.
>
> > --- 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,6 @@
> >
> >          real_kernel_entry(true, (unsigned long)cmdline_ptr,
> >                            (unsigned long)efi_system_table);
> > +
> > +       return EFI_LOAD_ERROR;
> >   }
>
> I tested the above changes, the falls through objtool warning can
> be fixed because efi_boot_kernel() ends with a return instruction,
> I think this is reasonable.
>
> efi_boot_kernel() has a return value, there are "return status" in
> other parts of efi_boot_kernel(), it should also return at the end
> of efi_boot_kernel() in theory, although we should never get here.
>
> If there are more comments, please let me know.
I still don't want LoongArch to be a special case, which means
efi_boot_kernel() in fdt.c, jump_kernel_func in riscv.c and
enter_kernel in arm64.c should also be modified.

Huacai

>
> Thanks,
> Tiezhu
>
Re: [PATCH v2] efistub: Only link libstub to final vmlinux
Posted by Tiezhu Yang 4 months ago
On 2025/10/11 下午3:42, Huacai Chen wrote:
> On Sat, Oct 11, 2025 at 3:29 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>
>> On 2025/10/11 上午11:40, Ard Biesheuvel wrote:
>>> On Fri, 10 Oct 2025 at 19:54, Huacai Chen <chenhuacai@kernel.org> wrote:
>>>>
>>>> On Sat, Oct 11, 2025 at 9:13 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>>>>
>>>>> On 2025/10/11 上午12:25, Ard Biesheuvel wrote:
>>>>> ...
>>>>>> Why do we need both (1) and (2)?
>>>>>
>>>>> Not both, either (1) or (2).
>>>>> Which one do you prefer? Or any other suggestions?
>>>>>
>>>>> Taking all of the considerations in balance, we should decide
>>>>> what is the proper way.
>>>> As a summary, there are three methods:
>>>> (1) Only link libstub with vmlinux.o during the final vmlinux link.
>>>> (2) Remove the attribute __noreturn for real_kernel_entry() and add while (1).
>>>> (3) Ignore "__efistub_" prefix in objtool.
>>>>
>>>> Josh prefers method (1), I prefer method (3) but also accept method
>>>> (1) if it is not only specific to loongarch.
>>>>
>>>
>>> This is a false positive warning in objtool, which complains about a
>>> function that falls through, even though that can never happen in
>>> reality.
>>>
>>> To me, it is not acceptable to modify how vmlinux.o is constructed
>>> also for other architectures, in order to hide some of its constituent
>>> parts from objtool, which do not use objtool to begin with.
>>>
>>>
>>> If you are not willing to fix objtool, I suggest fixing the loongarch
>>> code like this:
>>
>> Thank you.
>>
>>> --- 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,6 @@
>>>
>>>           real_kernel_entry(true, (unsigned long)cmdline_ptr,
>>>                             (unsigned long)efi_system_table);
>>> +
>>> +       return EFI_LOAD_ERROR;
>>>    }
>>
>> I tested the above changes, the falls through objtool warning can
>> be fixed because efi_boot_kernel() ends with a return instruction,
>> I think this is reasonable.
>>
>> efi_boot_kernel() has a return value, there are "return status" in
>> other parts of efi_boot_kernel(), it should also return at the end
>> of efi_boot_kernel() in theory, although we should never get here.
>>
>> If there are more comments, please let me know.
> I still don't want LoongArch to be a special case, which means
> efi_boot_kernel() in fdt.c, jump_kernel_func in riscv.c and
> enter_kernel in arm64.c should also be modified.

Here is the diff:

----- 8< -----
diff --git a/drivers/firmware/efi/libstub/arm64.c 
b/drivers/firmware/efi/libstub/arm64.c
index e57cd3de0a00..f3a1bb86bf8d 100644
--- a/drivers/firmware/efi/libstub/arm64.c
+++ b/drivers/firmware/efi/libstub/arm64.c
@@ -128,11 +128,11 @@ unsigned long __weak primary_entry_offset(void)
         return 0;
  }

-void __noreturn efi_enter_kernel(unsigned long entrypoint,
-                                unsigned long fdt_addr,
-                                unsigned long fdt_size)
+void efi_enter_kernel(unsigned long entrypoint,
+                     unsigned long fdt_addr,
+                     unsigned long fdt_size)
  {
-       void (* __noreturn enter_kernel)(u64, u64, u64, u64);
+       void (*enter_kernel)(u64, u64, u64, u64);

         enter_kernel = (void *)entrypoint + primary_entry_offset();
         enter_kernel(fdt_addr, 0, 0, 0);
diff --git a/drivers/firmware/efi/libstub/efistub.h 
b/drivers/firmware/efi/libstub/efistub.h
index f5ba032863a9..b4c716df4d47 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -1129,9 +1129,9 @@ efi_status_t efi_stub_common(efi_handle_t handle,

  efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char 
**cmdline_ptr);

-asmlinkage void __noreturn efi_enter_kernel(unsigned long entrypoint,
-                                           unsigned long fdt_addr,
-                                           unsigned long fdt_size);
+asmlinkage void efi_enter_kernel(unsigned long entrypoint,
+                                unsigned long fdt_addr,
+                                unsigned long fdt_size);

  void efi_handle_post_ebs_state(void);

diff --git a/drivers/firmware/efi/libstub/fdt.c 
b/drivers/firmware/efi/libstub/fdt.c
index 6a337f1f8787..7ece39ed70f6 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -358,7 +358,9 @@ efi_status_t efi_boot_kernel(void *handle, 
efi_loaded_image_t *image,
                 efi_handle_post_ebs_state();

         efi_enter_kernel(kernel_addr, fdt_addr, fdt_totalsize((void 
*)fdt_addr));
+
         /* not reached */
+       return EFI_LOAD_ERROR;
  }

  void *get_fdt(unsigned long *fdt_size)
diff --git a/drivers/firmware/efi/libstub/loongarch.c 
b/drivers/firmware/efi/libstub/loongarch.c
index 3782d0a187d1..88c500aeed86 100644
--- a/drivers/firmware/efi/libstub/loongarch.c
+++ b/drivers/firmware/efi/libstub/loongarch.c
@@ -10,8 +10,8 @@
  #include "efistub.h"
  #include "loongarch-stub.h"

-typedef void __noreturn (*kernel_entry_t)(bool efi, unsigned long cmdline,
-                                         unsigned long systab);
+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);
+
+       /* not reached */
+       return EFI_LOAD_ERROR;
  }
diff --git a/drivers/firmware/efi/libstub/riscv.c 
b/drivers/firmware/efi/libstub/riscv.c
index f66f33ceb99e..6589e8f5cf1a 100644
--- a/drivers/firmware/efi/libstub/riscv.c
+++ b/drivers/firmware/efi/libstub/riscv.c
@@ -11,7 +11,7 @@

  #include "efistub.h"

-typedef void __noreturn (*jump_kernel_func)(unsigned long, unsigned long);
+typedef void (*jump_kernel_func)(unsigned long, unsigned long);

  static unsigned long hartid;

@@ -81,7 +81,7 @@ unsigned long __weak stext_offset(void)
         return 0;
  }

-void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned 
long fdt,
+void efi_enter_kernel(unsigned long entrypoint, unsigned long fdt,
                                  unsigned long fdt_size)
  {
         unsigned long kernel_entry = entrypoint + stext_offset();

Do the above changes in a single patch?
Or in two patches?
One is to modify drivers/firmware/efi/libstub/loongarch.c,
the other one is to modify the arm64 and riscv.

Let us wait for more review comments.

Thanks,
Tiezhu