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

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

[Tiezhu Yang]

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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

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
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Huacai Chen]

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

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()?

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

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?

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Huacai Chen]

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Tiezhu Yang]

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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Tiezhu Yang]

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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Huacai Chen]

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Tiezhu Yang]

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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Huacai Chen]

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
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Tiezhu Yang]

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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

On Sat, 11 Oct 2025 at 00:43, Huacai Chen <chenhuacai@kernel.org> 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.
>

You have made LoongArch a special case by adding objtool support,
which  arm64 and RISC-V do not have.

So NAK to changing arm64 and RISC-V as well.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Huacai Chen]

On Sat, Oct 11, 2025 at 10:48 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sat, 11 Oct 2025 at 00:43, Huacai Chen <chenhuacai@kernel.org> 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.
> >
>
> You have made LoongArch a special case by adding objtool support,
> which  arm64 and RISC-V do not have.
>
> So NAK to changing arm64 and RISC-V as well.
Hmmm, I want to know whether this problem is an objtool issue or an
efistub issue in essence. If it is an objtool issue, we should fix
objtool and don't touch efistub. If it is an efistub issue, then we
should modify efistub (but not specific to LoongArch, when RISC-V and
ARM64 add objtool they will meet the same issue).

Huacai

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

On Sat, 11 Oct 2025 at 08:01, Huacai Chen <chenhuacai@kernel.org> wrote:
>
> On Sat, Oct 11, 2025 at 10:48 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Sat, 11 Oct 2025 at 00:43, Huacai Chen <chenhuacai@kernel.org> 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.
> > >
> >
> > You have made LoongArch a special case by adding objtool support,
> > which  arm64 and RISC-V do not have.
> >
> > So NAK to changing arm64 and RISC-V as well.
> Hmmm, I want to know whether this problem is an objtool issue or an
> efistub issue in essence. If it is an objtool issue, we should fix
> objtool and don't touch efistub. If it is an efistub issue, then we
> should modify efistub (but not specific to LoongArch, when RISC-V and
> ARM64 add objtool they will meet the same issue).
>

It is an objtool issue in essence.

The generated code looks like this

9000000001743080: ff b7 fe 57   bl      -332 <__efistub_kernel_entry_address>
9000000001743084: 26 03 c0 28   ld.d    $a2, $s2, 0
9000000001743088: 87 00 15 00   move    $a3, $a0
900000000174308c: 04 04 80 03   ori     $a0, $zero, 1
9000000001743090: c5 02 15 00   move    $a1, $fp
9000000001743094: e1 00 00 4c   jirl    $ra, $a3, 0

9000000001743098 <__efistub_exit_boot_func>:
9000000001743098: 63 c0 ff 02   addi.d  $sp, $sp, -16

There is nothing wrong with this code, given that the indirect call is
to a __noreturn function, and so the fact that it falls through into
__efistub_exit_boot_func() is not a problem.

Even though the compiler does nothing wrong here, it would be nice if
it would emit some kind of UD or BRK instruction after such a call, if
only to make the backtrace more reliable. But the code is fine, and
objtool simply does not have the information it needs to determine
that the indirect call is of a variety that never returns.

So I don't mind fixing it in the code, but only for LoongArch, given
that the problem does not exist on arm64 or RISC-V.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Tiezhu Yang]

On 2025/10/11 下午11:58, Ard Biesheuvel wrote:
> On Sat, 11 Oct 2025 at 08:01, Huacai Chen <chenhuacai@kernel.org> wrote:...>> Hmmm, I want to know whether this problem is an objtool issue or an
>> efistub issue in essence. If it is an objtool issue, we should fix
>> objtool and don't touch efistub. If it is an efistub issue, then we
>> should modify efistub (but not specific to LoongArch, when RISC-V and
>> ARM64 add objtool they will meet the same issue).
>>
> 
> It is an objtool issue in essence.
> 
> The generated code looks like this
> 
> 9000000001743080: ff b7 fe 57   bl      -332 <__efistub_kernel_entry_address>
> 9000000001743084: 26 03 c0 28   ld.d    $a2, $s2, 0
> 9000000001743088: 87 00 15 00   move    $a3, $a0
> 900000000174308c: 04 04 80 03   ori     $a0, $zero, 1
> 9000000001743090: c5 02 15 00   move    $a1, $fp
> 9000000001743094: e1 00 00 4c   jirl    $ra, $a3, 0
> 
> 9000000001743098 <__efistub_exit_boot_func>:
> 9000000001743098: 63 c0 ff 02   addi.d  $sp, $sp, -16
> 
> There is nothing wrong with this code, given that the indirect call is
> to a __noreturn function, and so the fact that it falls through into
> __efistub_exit_boot_func() is not a problem.
> 
> Even though the compiler does nothing wrong here, it would be nice if
> it would emit some kind of UD or BRK instruction after such a call, if
> only to make the backtrace more reliable. But the code is fine, and
> objtool simply does not have the information it needs to determine
> that the indirect call is of a variety that never returns.
> 
> So I don't mind fixing it in the code, but only for LoongArch, given
> that the problem does not exist on arm64 or RISC-V.

I assume this is the final conclusion, if there is no objection,
I will send patch according to Ard's suggestion and update the
commit message in the next week, the code looks like this:

-----8<-----
diff --git a/drivers/firmware/efi/libstub/loongarch.c 
b/drivers/firmware/efi/libstub/loongarch.c
index 3782d0a187d1..e5991aa9f805 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);
+
+       /* We should never get here, only to fix the objtool warning */
+       return EFI_LOAD_ERROR;
  }
-----8<-----

Thanks,
Tiezhu


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Huacai Chen]

On Sat, Oct 11, 2025 at 11:59 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sat, 11 Oct 2025 at 08:01, Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > On Sat, Oct 11, 2025 at 10:48 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Sat, 11 Oct 2025 at 00:43, Huacai Chen <chenhuacai@kernel.org> 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.
> > > >
> > >
> > > You have made LoongArch a special case by adding objtool support,
> > > which  arm64 and RISC-V do not have.
> > >
> > > So NAK to changing arm64 and RISC-V as well.
> > Hmmm, I want to know whether this problem is an objtool issue or an
> > efistub issue in essence. If it is an objtool issue, we should fix
> > objtool and don't touch efistub. If it is an efistub issue, then we
> > should modify efistub (but not specific to LoongArch, when RISC-V and
> > ARM64 add objtool they will meet the same issue).
> >
>
> It is an objtool issue in essence.
>
> The generated code looks like this
>
> 9000000001743080: ff b7 fe 57   bl      -332 <__efistub_kernel_entry_address>
> 9000000001743084: 26 03 c0 28   ld.d    $a2, $s2, 0
> 9000000001743088: 87 00 15 00   move    $a3, $a0
> 900000000174308c: 04 04 80 03   ori     $a0, $zero, 1
> 9000000001743090: c5 02 15 00   move    $a1, $fp
> 9000000001743094: e1 00 00 4c   jirl    $ra, $a3, 0
>
> 9000000001743098 <__efistub_exit_boot_func>:
> 9000000001743098: 63 c0 ff 02   addi.d  $sp, $sp, -16
>
> There is nothing wrong with this code, given that the indirect call is
> to a __noreturn function, and so the fact that it falls through into
> __efistub_exit_boot_func() is not a problem.
>
> Even though the compiler does nothing wrong here, it would be nice if
> it would emit some kind of UD or BRK instruction after such a call, if
> only to make the backtrace more reliable. But the code is fine, and
> objtool simply does not have the information it needs to determine
> that the indirect call is of a variety that never returns.
So the best way is to fix the objtool?

>
> So I don't mind fixing it in the code, but only for LoongArch, given
> that the problem does not exist on arm64 or RISC-V.
You believe this problem won't exist even if they add objtool support
(because their objtool will be sane)?


Huacai

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

On Mon, 13 Oct 2025 at 16:09, Huacai Chen <chenhuacai@kernel.org> wrote:
>
> On Sat, Oct 11, 2025 at 11:59 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Sat, 11 Oct 2025 at 08:01, Huacai Chen <chenhuacai@kernel.org> wrote:
> > >
> > > On Sat, Oct 11, 2025 at 10:48 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Sat, 11 Oct 2025 at 00:43, Huacai Chen <chenhuacai@kernel.org> 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.
> > > > >
> > > >
> > > > You have made LoongArch a special case by adding objtool support,
> > > > which  arm64 and RISC-V do not have.
> > > >
> > > > So NAK to changing arm64 and RISC-V as well.
> > > Hmmm, I want to know whether this problem is an objtool issue or an
> > > efistub issue in essence. If it is an objtool issue, we should fix
> > > objtool and don't touch efistub. If it is an efistub issue, then we
> > > should modify efistub (but not specific to LoongArch, when RISC-V and
> > > ARM64 add objtool they will meet the same issue).
> > >
> >
> > It is an objtool issue in essence.
> >
> > The generated code looks like this
> >
> > 9000000001743080: ff b7 fe 57   bl      -332 <__efistub_kernel_entry_address>
> > 9000000001743084: 26 03 c0 28   ld.d    $a2, $s2, 0
> > 9000000001743088: 87 00 15 00   move    $a3, $a0
> > 900000000174308c: 04 04 80 03   ori     $a0, $zero, 1
> > 9000000001743090: c5 02 15 00   move    $a1, $fp
> > 9000000001743094: e1 00 00 4c   jirl    $ra, $a3, 0
> >
> > 9000000001743098 <__efistub_exit_boot_func>:
> > 9000000001743098: 63 c0 ff 02   addi.d  $sp, $sp, -16
> >
> > There is nothing wrong with this code, given that the indirect call is
> > to a __noreturn function, and so the fact that it falls through into
> > __efistub_exit_boot_func() is not a problem.
> >
> > Even though the compiler does nothing wrong here, it would be nice if
> > it would emit some kind of UD or BRK instruction after such a call, if
> > only to make the backtrace more reliable. But the code is fine, and
> > objtool simply does not have the information it needs to determine
> > that the indirect call is of a variety that never returns.
> So the best way is to fix the objtool?
>

I think the best solution is to fix the compiler, and ensure that call
instructions are always followed by some undefined or debug/break
opcode. This works around this problem, but it also ensures that the
return address does not point to the wrong function, which may cause
confusion in backtraces.


> >
> > So I don't mind fixing it in the code, but only for LoongArch, given
> > that the problem does not exist on arm64 or RISC-V.
> You believe this problem won't exist even if they add objtool support
> (because their objtool will be sane)?
>

It depends on the compiler.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Josh Poimboeuf]

On Mon, Oct 13, 2025 at 04:36:49PM +0200, Ard Biesheuvel wrote:
> On Mon, 13 Oct 2025 at 16:09, Huacai Chen <chenhuacai@kernel.org> wrote:
> > On Sat, Oct 11, 2025 at 11:59 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > It is an objtool issue in essence.
> > >
> > > The generated code looks like this
> > >
> > > 9000000001743080: ff b7 fe 57   bl      -332 <__efistub_kernel_entry_address>
> > > 9000000001743084: 26 03 c0 28   ld.d    $a2, $s2, 0
> > > 9000000001743088: 87 00 15 00   move    $a3, $a0
> > > 900000000174308c: 04 04 80 03   ori     $a0, $zero, 1
> > > 9000000001743090: c5 02 15 00   move    $a1, $fp
> > > 9000000001743094: e1 00 00 4c   jirl    $ra, $a3, 0
> > >
> > > 9000000001743098 <__efistub_exit_boot_func>:
> > > 9000000001743098: 63 c0 ff 02   addi.d  $sp, $sp, -16
> > >
> > > There is nothing wrong with this code, given that the indirect call is
> > > to a __noreturn function, and so the fact that it falls through into
> > > __efistub_exit_boot_func() is not a problem.
> > >
> > > Even though the compiler does nothing wrong here, it would be nice if
> > > it would emit some kind of UD or BRK instruction after such a call, if
> > > only to make the backtrace more reliable. But the code is fine, and
> > > objtool simply does not have the information it needs to determine
> > > that the indirect call is of a variety that never returns.
> > So the best way is to fix the objtool?
> >
> 
> I think the best solution is to fix the compiler, and ensure that call
> instructions are always followed by some undefined or debug/break
> opcode. This works around this problem, but it also ensures that the
> return address does not point to the wrong function, which may cause
> confusion in backtraces.

I think the compiler folks will say that's working as designed.  The
whole point of __noreturn is to eliminate unecessary code after the
call.

Unwinders are already designed to handle that case anyway.

If you don't want to optimize out the code after the call then just
remove the __noreturn annotation from the function pointer.

> > > So I don't mind fixing it in the code, but only for LoongArch, given
> > > that the problem does not exist on arm64 or RISC-V.
> > You believe this problem won't exist even if they add objtool support
> > (because their objtool will be sane)?
> >
> 
> It depends on the compiler.

I don't think so, all compilers do this...

My suggestion (which prompted this v2 patch) was to move the libstub
code out of vmlinux.o (but still keep it in vmlinux), to make it
consistent with what x86 already does.

The idea is that libstub code doesn't belong in vmlinux.o because it's
not a part of the kernel proper, and doesn't need to be validated or
modified by objtool for any reason.

-- 
Josh

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

On Tue, 14 Oct 2025 at 18:47, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Mon, Oct 13, 2025 at 04:36:49PM +0200, Ard Biesheuvel wrote:
> > On Mon, 13 Oct 2025 at 16:09, Huacai Chen <chenhuacai@kernel.org> wrote:
> > > On Sat, Oct 11, 2025 at 11:59 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > It is an objtool issue in essence.
> > > >
> > > > The generated code looks like this
> > > >
> > > > 9000000001743080: ff b7 fe 57   bl      -332 <__efistub_kernel_entry_address>
> > > > 9000000001743084: 26 03 c0 28   ld.d    $a2, $s2, 0
> > > > 9000000001743088: 87 00 15 00   move    $a3, $a0
> > > > 900000000174308c: 04 04 80 03   ori     $a0, $zero, 1
> > > > 9000000001743090: c5 02 15 00   move    $a1, $fp
> > > > 9000000001743094: e1 00 00 4c   jirl    $ra, $a3, 0
> > > >
> > > > 9000000001743098 <__efistub_exit_boot_func>:
> > > > 9000000001743098: 63 c0 ff 02   addi.d  $sp, $sp, -16
> > > >
> > > > There is nothing wrong with this code, given that the indirect call is
> > > > to a __noreturn function, and so the fact that it falls through into
> > > > __efistub_exit_boot_func() is not a problem.
> > > >
> > > > Even though the compiler does nothing wrong here, it would be nice if
> > > > it would emit some kind of UD or BRK instruction after such a call, if
> > > > only to make the backtrace more reliable. But the code is fine, and
> > > > objtool simply does not have the information it needs to determine
> > > > that the indirect call is of a variety that never returns.
> > > So the best way is to fix the objtool?
> > >
> >
> > I think the best solution is to fix the compiler, and ensure that call
> > instructions are always followed by some undefined or debug/break
> > opcode. This works around this problem, but it also ensures that the
> > return address does not point to the wrong function, which may cause
> > confusion in backtraces.
>
> I think the compiler folks will say that's working as designed.  The
> whole point of __noreturn is to eliminate unecessary code after the
> call.
>
> Unwinders are already designed to handle that case anyway.
>
> If you don't want to optimize out the code after the call then just
> remove the __noreturn annotation from the function pointer.
>
> > > > So I don't mind fixing it in the code, but only for LoongArch, given
> > > > that the problem does not exist on arm64 or RISC-V.
> > > You believe this problem won't exist even if they add objtool support
> > > (because their objtool will be sane)?
> > >
> >
> > It depends on the compiler.
>
> I don't think so, all compilers do this...
>
> My suggestion (which prompted this v2 patch) was to move the libstub
> code out of vmlinux.o (but still keep it in vmlinux), to make it
> consistent with what x86 already does.
>

This is because x86 links the EFI stub into the decompressor, not into vmlinux.

> The idea is that libstub code doesn't belong in vmlinux.o because it's
> not a part of the kernel proper, and doesn't need to be validated or
> modified by objtool for any reason.
>

I don't see a reason to change this on architectures that a) do not
use objtool and b) link the EFI stub into vmlinux. If LoongArch wants
to change this, that is fine, but that still does not mean it needs to
change on other architectures too.

EFI related boot errors are a nightmare to debug, and I will be the
one getting the reports when this regresses arm64 on hardware that 2
people on the planet have access to.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Josh Poimboeuf]

On Thu, Oct 16, 2025 at 04:52:20PM +0200, Ard Biesheuvel wrote:
> On Tue, 14 Oct 2025 at 18:47, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > The idea is that libstub code doesn't belong in vmlinux.o because it's
> > not a part of the kernel proper, and doesn't need to be validated or
> > modified by objtool for any reason.
> >
> 
> I don't see a reason to change this on architectures that a) do not
> use objtool and b) link the EFI stub into vmlinux. If LoongArch wants
> to change this, that is fine, but that still does not mean it needs to
> change on other architectures too.
> 
> EFI related boot errors are a nightmare to debug, and I will be the
> one getting the reports when this regresses arm64 on hardware that 2
> people on the planet have access to.

The idea was to have more consistency, so vmlinux.o never has libstub,
regardless of arch, but that's your call.

I'd still propose we keep the KBUILD_VMLINUX_LIBS_PRELINK mechanism to
allow other arches to opt in as needed.

And that variable might even be useful for other cases (x86 startup
code?)

-- 
Josh

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

On Thu, 16 Oct 2025 at 17:49, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Thu, Oct 16, 2025 at 04:52:20PM +0200, Ard Biesheuvel wrote:
> > On Tue, 14 Oct 2025 at 18:47, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > The idea is that libstub code doesn't belong in vmlinux.o because it's
> > > not a part of the kernel proper, and doesn't need to be validated or
> > > modified by objtool for any reason.
> > >
> >
> > I don't see a reason to change this on architectures that a) do not
> > use objtool and b) link the EFI stub into vmlinux. If LoongArch wants
> > to change this, that is fine, but that still does not mean it needs to
> > change on other architectures too.
> >
> > EFI related boot errors are a nightmare to debug, and I will be the
> > one getting the reports when this regresses arm64 on hardware that 2
> > people on the planet have access to.
>
> The idea was to have more consistency, so vmlinux.o never has libstub,
> regardless of arch, but that's your call.
>

The code in libstub ends up in .init.text, which will be mapped
executable during boot on architectures that incorporate it into
vmlinux.

If objtool validation is never needed for such code, on the basis that
it is not actually called even though it is present, then I think that
is fine.

For the other architectures, I don't have any objections in principle,
I'm just being cautious due to the regression risk.

> I'd still propose we keep the KBUILD_VMLINUX_LIBS_PRELINK mechanism to
> allow other arches to opt in as needed.
>

Again, no objection in principle. To me, it just seems a lot of churn
just to avoid having to teach objtool about indirect calls to noreturn
functions.

> And that variable might even be useful for other cases (x86 startup
> code?)
>

Not all x86 startup code is in .init.text; some of it sticks around
and is still used at runtime. I reckon that implies that objtool
validation will remain needed for that, no?

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Josh Poimboeuf]

On Fri, Oct 17, 2025 at 01:00:17PM +0200, Ard Biesheuvel wrote:
> On Thu, 16 Oct 2025 at 17:49, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Thu, Oct 16, 2025 at 04:52:20PM +0200, Ard Biesheuvel wrote:
> > > On Tue, 14 Oct 2025 at 18:47, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > > The idea is that libstub code doesn't belong in vmlinux.o because it's
> > > > not a part of the kernel proper, and doesn't need to be validated or
> > > > modified by objtool for any reason.
> > > >
> > >
> > > I don't see a reason to change this on architectures that a) do not
> > > use objtool and b) link the EFI stub into vmlinux. If LoongArch wants
> > > to change this, that is fine, but that still does not mean it needs to
> > > change on other architectures too.
> > >
> > > EFI related boot errors are a nightmare to debug, and I will be the
> > > one getting the reports when this regresses arm64 on hardware that 2
> > > people on the planet have access to.
> >
> > The idea was to have more consistency, so vmlinux.o never has libstub,
> > regardless of arch, but that's your call.
> >
> 
> The code in libstub ends up in .init.text, which will be mapped
> executable during boot on architectures that incorporate it into
> vmlinux.
> 
> If objtool validation is never needed for such code, on the basis that
> it is not actually called even though it is present, then I think that
> is fine.
> 
> For the other architectures, I don't have any objections in principle,
> I'm just being cautious due to the regression risk.
> 
> > I'd still propose we keep the KBUILD_VMLINUX_LIBS_PRELINK mechanism to
> > allow other arches to opt in as needed.
> >
> 
> Again, no objection in principle. To me, it just seems a lot of churn
> just to avoid having to teach objtool about indirect calls to noreturn
> functions.

Well, one of these days we will need to do that with some kind of
compiler -fannotate-noreturn feature or plugin or whatever, but this was
more about "why are we validating libstub code anyway, it doesn't seem
necessary and x86 doesn't do it, so lets make them consistent".

> > And that variable might even be useful for other cases (x86 startup
> > code?)
> >
> 
> Not all x86 startup code is in .init.text; some of it sticks around
> and is still used at runtime. I reckon that implies that objtool
> validation will remain needed for that, no?

I wasn't aware of that, in that case I guess the x86 startup code
belongs in vmlinux.o, or at least the runtime portions of it.

-- 
Josh

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

On Fri, 17 Oct 2025 at 18:22, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Fri, Oct 17, 2025 at 01:00:17PM +0200, Ard Biesheuvel wrote:
> > On Thu, 16 Oct 2025 at 17:49, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > On Thu, Oct 16, 2025 at 04:52:20PM +0200, Ard Biesheuvel wrote:
> > > > On Tue, 14 Oct 2025 at 18:47, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > > > The idea is that libstub code doesn't belong in vmlinux.o because it's
> > > > > not a part of the kernel proper, and doesn't need to be validated or
> > > > > modified by objtool for any reason.
> > > > >
> > > >
> > > > I don't see a reason to change this on architectures that a) do not
> > > > use objtool and b) link the EFI stub into vmlinux. If LoongArch wants
> > > > to change this, that is fine, but that still does not mean it needs to
> > > > change on other architectures too.
> > > >
> > > > EFI related boot errors are a nightmare to debug, and I will be the
> > > > one getting the reports when this regresses arm64 on hardware that 2
> > > > people on the planet have access to.
> > >
> > > The idea was to have more consistency, so vmlinux.o never has libstub,
> > > regardless of arch, but that's your call.
> > >
> >
> > The code in libstub ends up in .init.text, which will be mapped
> > executable during boot on architectures that incorporate it into
> > vmlinux.
> >
> > If objtool validation is never needed for such code, on the basis that
> > it is not actually called even though it is present, then I think that
> > is fine.
> >
> > For the other architectures, I don't have any objections in principle,
> > I'm just being cautious due to the regression risk.
> >
> > > I'd still propose we keep the KBUILD_VMLINUX_LIBS_PRELINK mechanism to
> > > allow other arches to opt in as needed.
> > >
> >
> > Again, no objection in principle. To me, it just seems a lot of churn
> > just to avoid having to teach objtool about indirect calls to noreturn
> > functions.
>
> Well, one of these days we will need to do that with some kind of
> compiler -fannotate-noreturn feature or plugin or whatever, but this was
> more about "why are we validating libstub code anyway, it doesn't seem
> necessary and x86 doesn't do it, so lets make them consistent".
>

Sure. So objtool validation is not needed even if the code is
incorporated into vmlinux, and therefore mapped executable at boot?
I.e., objtool does not check for gadgets or other things that may be
problematic even if they are never called directly from reachable
code?

> > > And that variable might even be useful for other cases (x86 startup
> > > code?)
> > >
> >
> > Not all x86 startup code is in .init.text; some of it sticks around
> > and is still used at runtime. I reckon that implies that objtool
> > validation will remain needed for that, no?
>
> I wasn't aware of that, in that case I guess the x86 startup code
> belongs in vmlinux.o, or at least the runtime portions of it.
>

OK thanks for confirming.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Josh Poimboeuf]

On Fri, Oct 17, 2025 at 06:26:47PM +0200, Ard Biesheuvel wrote:
> On Fri, 17 Oct 2025 at 18:22, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Fri, Oct 17, 2025 at 01:00:17PM +0200, Ard Biesheuvel wrote:
> > > On Thu, 16 Oct 2025 at 17:49, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > >
> > > > On Thu, Oct 16, 2025 at 04:52:20PM +0200, Ard Biesheuvel wrote:
> > > > > On Tue, 14 Oct 2025 at 18:47, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > > > > The idea is that libstub code doesn't belong in vmlinux.o because it's
> > > > > > not a part of the kernel proper, and doesn't need to be validated or
> > > > > > modified by objtool for any reason.
> > > > > >
> > > > >
> > > > > I don't see a reason to change this on architectures that a) do not
> > > > > use objtool and b) link the EFI stub into vmlinux. If LoongArch wants
> > > > > to change this, that is fine, but that still does not mean it needs to
> > > > > change on other architectures too.
> > > > >
> > > > > EFI related boot errors are a nightmare to debug, and I will be the
> > > > > one getting the reports when this regresses arm64 on hardware that 2
> > > > > people on the planet have access to.
> > > >
> > > > The idea was to have more consistency, so vmlinux.o never has libstub,
> > > > regardless of arch, but that's your call.
> > > >
> > >
> > > The code in libstub ends up in .init.text, which will be mapped
> > > executable during boot on architectures that incorporate it into
> > > vmlinux.
> > >
> > > If objtool validation is never needed for such code, on the basis that
> > > it is not actually called even though it is present, then I think that
> > > is fine.
> > >
> > > For the other architectures, I don't have any objections in principle,
> > > I'm just being cautious due to the regression risk.
> > >
> > > > I'd still propose we keep the KBUILD_VMLINUX_LIBS_PRELINK mechanism to
> > > > allow other arches to opt in as needed.
> > > >
> > >
> > > Again, no objection in principle. To me, it just seems a lot of churn
> > > just to avoid having to teach objtool about indirect calls to noreturn
> > > functions.
> >
> > Well, one of these days we will need to do that with some kind of
> > compiler -fannotate-noreturn feature or plugin or whatever, but this was
> > more about "why are we validating libstub code anyway, it doesn't seem
> > necessary and x86 doesn't do it, so lets make them consistent".
> >
> 
> Sure. So objtool validation is not needed even if the code is
> incorporated into vmlinux, and therefore mapped executable at boot?
> I.e., objtool does not check for gadgets or other things that may be
> problematic even if they are never called directly from reachable
> code?

Well, boot-time gadgets aren't really a thing.  And, as you said,
libstub is in .init.text, so it gets freed (and presumably mapped
non-executable) during boot, so we don't have to worry about any gadgets
hanging around in memory after the fact.

That said, objtool also generates ORC metadata, so in addition to
security-type things, it also cares about any code that might show up on
a stack trace during a boot-time warning/oops.  So it *does* care about
.init.text code in that sense.

But IIUC, the libstub code runs *very* early, and wouldn't show up in a
stack trace anyway, because there are no traces of it on the stack once
it branches to head.S code (which doesn't save the link register).

-- 
Josh

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Tiezhu Yang]

Hi Josh, Ard and Huacai,

On 2025/10/18 上午1:05, Josh Poimboeuf wrote:

...

> But IIUC, the libstub code runs *very* early, and wouldn't show up in a
> stack trace anyway, because there are no traces of it on the stack once
> it branches to head.S code (which doesn't save the link register).

Thanks for your discussions.

Are you OK with this current patch?
Or should I update this patch to remove the changes of arm64 and riscv?
Or any other suggestions?

Thanks,
Tiezhu


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Huacai Chen]

On Mon, Oct 20, 2025 at 9:24 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> Hi Josh, Ard and Huacai,
>
> On 2025/10/18 上午1:05, Josh Poimboeuf wrote:
>
> ...
>
> > But IIUC, the libstub code runs *very* early, and wouldn't show up in a
> > stack trace anyway, because there are no traces of it on the stack once
> > it branches to head.S code (which doesn't save the link register).
>
> Thanks for your discussions.
>
> Are you OK with this current patch?
For me the current patch is just OK.


Huacai

> Or should I update this patch to remove the changes of arm64 and riscv?
> Or any other suggestions?
>
> Thanks,
> Tiezhu
>
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Tiezhu Yang]

Hi Josh and Ard,

On 2025/10/20 下午2:55, Huacai Chen wrote:
> On Mon, Oct 20, 2025 at 9:24 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>
>> Hi Josh, Ard and Huacai,
>>
>> On 2025/10/18 上午1:05, Josh Poimboeuf wrote:
>>
>> ...
>>
>>> But IIUC, the libstub code runs *very* early, and wouldn't show up in a
>>> stack trace anyway, because there are no traces of it on the stack once
>>> it branches to head.S code (which doesn't save the link register).
>>
>> Thanks for your discussions.
>>
>> Are you OK with this current patch?
> For me the current patch is just OK.

We have discussed this a few times but there is almost no consensus
of what should happen next and nothing changes.

Could you please give me a clear reply? Then I can make progress for
the following series:

https://lore.kernel.org/loongarch/20250917112716.24415-1-yangtiezhu@loongson.cn/

Thanks,
Tiezhu


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Huacai Chen]

On Thu, Oct 23, 2025 at 2:55 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> Hi Josh and Ard,
>
> On 2025/10/20 下午2:55, Huacai Chen wrote:
> > On Mon, Oct 20, 2025 at 9:24 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >>
> >> Hi Josh, Ard and Huacai,
> >>
> >> On 2025/10/18 上午1:05, Josh Poimboeuf wrote:
> >>
> >> ...
> >>
> >>> But IIUC, the libstub code runs *very* early, and wouldn't show up in a
> >>> stack trace anyway, because there are no traces of it on the stack once
> >>> it branches to head.S code (which doesn't save the link register).
> >>
> >> Thanks for your discussions.
> >>
> >> Are you OK with this current patch?
> > For me the current patch is just OK.
>
> We have discussed this a few times but there is almost no consensus
> of what should happen next and nothing changes.
>
> Could you please give me a clear reply? Then I can make progress for
> the following series:
>
> https://lore.kernel.org/loongarch/20250917112716.24415-1-yangtiezhu@loongson.cn/
For me, this patch is OK, ignore __efistub_ prefix in objtool is also
OK [1]. But I cannot accept the way that modifying the efistub part
only for LoongArch.

Clear enough?

[1] https://lore.kernel.org/loongarch/20250917112716.24415-1-yangtiezhu@loongson.cn/T/#meef7411abd14f4c28c85e686614aa9211fccdca0

Huacai

>
> Thanks,
> Tiezhu
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

On Thu, 23 Oct 2025 at 10:01, Huacai Chen <chenhuacai@kernel.org> wrote:
>
> On Thu, Oct 23, 2025 at 2:55 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >
> > Hi Josh and Ard,
> >
> > On 2025/10/20 下午2:55, Huacai Chen wrote:
> > > On Mon, Oct 20, 2025 at 9:24 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > >>
> > >> Hi Josh, Ard and Huacai,
> > >>
> > >> On 2025/10/18 上午1:05, Josh Poimboeuf wrote:
> > >>
> > >> ...
> > >>
> > >>> But IIUC, the libstub code runs *very* early, and wouldn't show up in a
> > >>> stack trace anyway, because there are no traces of it on the stack once
> > >>> it branches to head.S code (which doesn't save the link register).
> > >>
> > >> Thanks for your discussions.
> > >>
> > >> Are you OK with this current patch?
> > > For me the current patch is just OK.
> >
> > We have discussed this a few times but there is almost no consensus
> > of what should happen next and nothing changes.
> >
> > Could you please give me a clear reply? Then I can make progress for
> > the following series:
> >
> > https://lore.kernel.org/loongarch/20250917112716.24415-1-yangtiezhu@loongson.cn/
> For me, this patch is OK, ignore __efistub_ prefix in objtool is also
> OK [1]. But I cannot accept the way that modifying the efistub part
> only for LoongArch.
>
> Clear enough?
>

LoongArch is the only architecture which has the problem, so I don't
see a reason to modify other architectures.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Huacai Chen]

On Thu, Oct 23, 2025 at 4:07 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 23 Oct 2025 at 10:01, Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > On Thu, Oct 23, 2025 at 2:55 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > >
> > > Hi Josh and Ard,
> > >
> > > On 2025/10/20 下午2:55, Huacai Chen wrote:
> > > > On Mon, Oct 20, 2025 at 9:24 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > > >>
> > > >> Hi Josh, Ard and Huacai,
> > > >>
> > > >> On 2025/10/18 上午1:05, Josh Poimboeuf wrote:
> > > >>
> > > >> ...
> > > >>
> > > >>> But IIUC, the libstub code runs *very* early, and wouldn't show up in a
> > > >>> stack trace anyway, because there are no traces of it on the stack once
> > > >>> it branches to head.S code (which doesn't save the link register).
> > > >>
> > > >> Thanks for your discussions.
> > > >>
> > > >> Are you OK with this current patch?
> > > > For me the current patch is just OK.
> > >
> > > We have discussed this a few times but there is almost no consensus
> > > of what should happen next and nothing changes.
> > >
> > > Could you please give me a clear reply? Then I can make progress for
> > > the following series:
> > >
> > > https://lore.kernel.org/loongarch/20250917112716.24415-1-yangtiezhu@loongson.cn/
> > For me, this patch is OK, ignore __efistub_ prefix in objtool is also
> > OK [1]. But I cannot accept the way that modifying the efistub part
> > only for LoongArch.
> >
> > Clear enough?
> >
>
> LoongArch is the only architecture which has the problem, so I don't
> see a reason to modify other architectures.
From your reply I think the efistub code is completely right, but
objtool cannot handle the "noreturn" function pointer. And this patch
is a workaround rather than a proper fix (so you don't want to touch
other architectures), right?


Huacai

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

On Sun, 26 Oct 2025 at 12:20, Huacai Chen <chenhuacai@kernel.org> wrote:
>
> On Thu, Oct 23, 2025 at 4:07 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Thu, 23 Oct 2025 at 10:01, Huacai Chen <chenhuacai@kernel.org> wrote:
> > >
> > > On Thu, Oct 23, 2025 at 2:55 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > > >
> > > > Hi Josh and Ard,
> > > >
> > > > On 2025/10/20 下午2:55, Huacai Chen wrote:
> > > > > On Mon, Oct 20, 2025 at 9:24 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > > > >>
> > > > >> Hi Josh, Ard and Huacai,
> > > > >>
> > > > >> On 2025/10/18 上午1:05, Josh Poimboeuf wrote:
> > > > >>
> > > > >> ...
> > > > >>
> > > > >>> But IIUC, the libstub code runs *very* early, and wouldn't show up in a
> > > > >>> stack trace anyway, because there are no traces of it on the stack once
> > > > >>> it branches to head.S code (which doesn't save the link register).
> > > > >>
> > > > >> Thanks for your discussions.
> > > > >>
> > > > >> Are you OK with this current patch?
> > > > > For me the current patch is just OK.
> > > >
> > > > We have discussed this a few times but there is almost no consensus
> > > > of what should happen next and nothing changes.
> > > >
> > > > Could you please give me a clear reply? Then I can make progress for
> > > > the following series:
> > > >
> > > > https://lore.kernel.org/loongarch/20250917112716.24415-1-yangtiezhu@loongson.cn/
> > > For me, this patch is OK, ignore __efistub_ prefix in objtool is also
> > > OK [1]. But I cannot accept the way that modifying the efistub part
> > > only for LoongArch.
> > >
> > > Clear enough?
> > >
> >
> > LoongArch is the only architecture which has the problem, so I don't
> > see a reason to modify other architectures.
> From your reply I think the efistub code is completely right, but
> objtool cannot handle the "noreturn" function pointer. And this patch
> is a workaround rather than a proper fix (so you don't want to touch
> other architectures), right?
>

That is my reasoning, yes. But Josh is right that it shouldn't make a
difference in practice, I am just reluctant to make changes to the
code running on the target to accommodate a flawed build time tool.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Tiezhu Yang]

On 2025/10/28 下午9:47, Ard Biesheuvel wrote:
> On Sun, 26 Oct 2025 at 12:20, Huacai Chen <chenhuacai@kernel.org> wrote:
>>
>> On Thu, Oct 23, 2025 at 4:07 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>
>>> On Thu, 23 Oct 2025 at 10:01, Huacai Chen <chenhuacai@kernel.org> wrote:
>>>>
>>>> On Thu, Oct 23, 2025 at 2:55 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>>>>
>>>>> Hi Josh and Ard,
>>>>>
>>>>> On 2025/10/20 下午2:55, Huacai Chen wrote:
>>>>>> On Mon, Oct 20, 2025 at 9:24 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>>>>>>
>>>>>>> Hi Josh, Ard and Huacai,
>>>>>>>
>>>>>>> On 2025/10/18 上午1:05, Josh Poimboeuf wrote:
>>>>>>>
>>>>>>> ...
>>>>>>>
>>>>>>>> But IIUC, the libstub code runs *very* early, and wouldn't show up in a
>>>>>>>> stack trace anyway, because there are no traces of it on the stack once
>>>>>>>> it branches to head.S code (which doesn't save the link register).
>>>>>>>
>>>>>>> Thanks for your discussions.
>>>>>>>
>>>>>>> Are you OK with this current patch?
>>>>>> For me the current patch is just OK.
>>>>>
>>>>> We have discussed this a few times but there is almost no consensus
>>>>> of what should happen next and nothing changes.
>>>>>
>>>>> Could you please give me a clear reply? Then I can make progress for
>>>>> the following series:
>>>>>
>>>>> https://lore.kernel.org/loongarch/20250917112716.24415-1-yangtiezhu@loongson.cn/
>>>> For me, this patch is OK, ignore __efistub_ prefix in objtool is also
>>>> OK [1]. But I cannot accept the way that modifying the efistub part
>>>> only for LoongArch.
>>>>
>>>> Clear enough?
>>>>
>>>
>>> LoongArch is the only architecture which has the problem, so I don't
>>> see a reason to modify other architectures.
>>  From your reply I think the efistub code is completely right, but
>> objtool cannot handle the "noreturn" function pointer. And this patch
>> is a workaround rather than a proper fix (so you don't want to touch
>> other architectures), right?
>>
> 
> That is my reasoning, yes. But Josh is right that it shouldn't make a
> difference in practice, I am just reluctant to make changes to the
> code running on the target to accommodate a flawed build time tool.

If I understand correctly, I should modify this patch to remove the
changes of arm and riscv for now, do the changes only when there is
a real problem or requirement some day, right? If no more comments,
I will send v3 later.

Thanks,
Tiezhu


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Huacai Chen]

On Mon, Nov 10, 2025 at 9:19 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> On 2025/10/28 下午9:47, Ard Biesheuvel wrote:
> > On Sun, 26 Oct 2025 at 12:20, Huacai Chen <chenhuacai@kernel.org> wrote:
> >>
> >> On Thu, Oct 23, 2025 at 4:07 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >>>
> >>> On Thu, 23 Oct 2025 at 10:01, Huacai Chen <chenhuacai@kernel.org> wrote:
> >>>>
> >>>> On Thu, Oct 23, 2025 at 2:55 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >>>>>
> >>>>> Hi Josh and Ard,
> >>>>>
> >>>>> On 2025/10/20 下午2:55, Huacai Chen wrote:
> >>>>>> On Mon, Oct 20, 2025 at 9:24 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >>>>>>>
> >>>>>>> Hi Josh, Ard and Huacai,
> >>>>>>>
> >>>>>>> On 2025/10/18 上午1:05, Josh Poimboeuf wrote:
> >>>>>>>
> >>>>>>> ...
> >>>>>>>
> >>>>>>>> But IIUC, the libstub code runs *very* early, and wouldn't show up in a
> >>>>>>>> stack trace anyway, because there are no traces of it on the stack once
> >>>>>>>> it branches to head.S code (which doesn't save the link register).
> >>>>>>>
> >>>>>>> Thanks for your discussions.
> >>>>>>>
> >>>>>>> Are you OK with this current patch?
> >>>>>> For me the current patch is just OK.
> >>>>>
> >>>>> We have discussed this a few times but there is almost no consensus
> >>>>> of what should happen next and nothing changes.
> >>>>>
> >>>>> Could you please give me a clear reply? Then I can make progress for
> >>>>> the following series:
> >>>>>
> >>>>> https://lore.kernel.org/loongarch/20250917112716.24415-1-yangtiezhu@loongson.cn/
> >>>> For me, this patch is OK, ignore __efistub_ prefix in objtool is also
> >>>> OK [1]. But I cannot accept the way that modifying the efistub part
> >>>> only for LoongArch.
> >>>>
> >>>> Clear enough?
> >>>>
> >>>
> >>> LoongArch is the only architecture which has the problem, so I don't
> >>> see a reason to modify other architectures.
> >>  From your reply I think the efistub code is completely right, but
> >> objtool cannot handle the "noreturn" function pointer. And this patch
> >> is a workaround rather than a proper fix (so you don't want to touch
> >> other architectures), right?
> >>
> >
> > That is my reasoning, yes. But Josh is right that it shouldn't make a
> > difference in practice, I am just reluctant to make changes to the
> > code running on the target to accommodate a flawed build time tool.
>
> If I understand correctly, I should modify this patch to remove the
> changes of arm and riscv for now, do the changes only when there is
> a real problem or requirement some day, right? If no more comments,
> I will send v3 later.
Now everyone involved agrees that the efistub code is correct, so the
proper solution is to fix the compiler. Changing efistub code and
changing objtool (ignore __efistub prefix) are both workarounds, but I
think changing objtool is a little more reasonable. Maybe Josh has
different ideas?

Huacai

>
> Thanks,
> Tiezhu
>
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Ard Biesheuvel]

On Mon, 10 Nov 2025 at 08:00, Huacai Chen <chenhuacai@kernel.org> wrote:
>
> On Mon, Nov 10, 2025 at 9:19 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >
> > On 2025/10/28 下午9:47, Ard Biesheuvel wrote:
> > > On Sun, 26 Oct 2025 at 12:20, Huacai Chen <chenhuacai@kernel.org> wrote:
> > >>
> > >> On Thu, Oct 23, 2025 at 4:07 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >>>
> > >>> On Thu, 23 Oct 2025 at 10:01, Huacai Chen <chenhuacai@kernel.org> wrote:
> > >>>>
> > >>>> On Thu, Oct 23, 2025 at 2:55 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > >>>>>
> > >>>>> Hi Josh and Ard,
> > >>>>>
> > >>>>> On 2025/10/20 下午2:55, Huacai Chen wrote:
> > >>>>>> On Mon, Oct 20, 2025 at 9:24 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > >>>>>>>
> > >>>>>>> Hi Josh, Ard and Huacai,
> > >>>>>>>
> > >>>>>>> On 2025/10/18 上午1:05, Josh Poimboeuf wrote:
> > >>>>>>>
> > >>>>>>> ...
> > >>>>>>>
> > >>>>>>>> But IIUC, the libstub code runs *very* early, and wouldn't show up in a
> > >>>>>>>> stack trace anyway, because there are no traces of it on the stack once
> > >>>>>>>> it branches to head.S code (which doesn't save the link register).
> > >>>>>>>
> > >>>>>>> Thanks for your discussions.
> > >>>>>>>
> > >>>>>>> Are you OK with this current patch?
> > >>>>>> For me the current patch is just OK.
> > >>>>>
> > >>>>> We have discussed this a few times but there is almost no consensus
> > >>>>> of what should happen next and nothing changes.
> > >>>>>
> > >>>>> Could you please give me a clear reply? Then I can make progress for
> > >>>>> the following series:
> > >>>>>
> > >>>>> https://lore.kernel.org/loongarch/20250917112716.24415-1-yangtiezhu@loongson.cn/
> > >>>> For me, this patch is OK, ignore __efistub_ prefix in objtool is also
> > >>>> OK [1]. But I cannot accept the way that modifying the efistub part
> > >>>> only for LoongArch.
> > >>>>
> > >>>> Clear enough?
> > >>>>
> > >>>
> > >>> LoongArch is the only architecture which has the problem, so I don't
> > >>> see a reason to modify other architectures.
> > >>  From your reply I think the efistub code is completely right, but
> > >> objtool cannot handle the "noreturn" function pointer. And this patch
> > >> is a workaround rather than a proper fix (so you don't want to touch
> > >> other architectures), right?
> > >>
> > >
> > > That is my reasoning, yes. But Josh is right that it shouldn't make a
> > > difference in practice, I am just reluctant to make changes to the
> > > code running on the target to accommodate a flawed build time tool.
> >
> > If I understand correctly, I should modify this patch to remove the
> > changes of arm and riscv for now, do the changes only when there is
> > a real problem or requirement some day, right? If no more comments,
> > I will send v3 later.
> Now everyone involved agrees that the efistub code is correct, so the
> proper solution is to fix the compiler. Changing efistub code and
> changing objtool (ignore __efistub prefix) are both workarounds, but I
> think changing objtool is a little more reasonable. Maybe Josh has
> different ideas?
>

Well, given that objtool cannot even infer this for direct calls (and
needs to built with a list of known noreturn functions in the entire
kernel), I think it is safe to assume that doing this for indirect
calls such as this one is intractable.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Josh Poimboeuf]

On Mon, Nov 10, 2025 at 03:00:00PM +0800, Huacai Chen wrote:
> On Mon, Nov 10, 2025 at 9:19 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > If I understand correctly, I should modify this patch to remove the
> > changes of arm and riscv for now, do the changes only when there is
> > a real problem or requirement some day, right? If no more comments,
> > I will send v3 later.
>
> Now everyone involved agrees that the efistub code is correct, so the
> proper solution is to fix the compiler.

Hm?  I don't see how it's a compiler bug.  It's really just an objtool
limitation.

> Changing efistub code and changing objtool (ignore __efistub prefix)
> are both workarounds, but I think changing objtool is a little more
> reasonable. Maybe Josh has different ideas?

I thought the conversation had converged on what Tiezhu mentioned above,
which is to skip objtool on libstub for loongarch, but leave the other
arches alone.  That way objtool behavior is consistent between loongarch
and x86, and objtool doesn't need to ignore any prefixes.

So basically, the v2 patch minus the arm64/riscv changes.

-- 
Josh

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Huacai Chen]

On Wed, Nov 12, 2025 at 2:00 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Mon, Nov 10, 2025 at 03:00:00PM +0800, Huacai Chen wrote:
> > On Mon, Nov 10, 2025 at 9:19 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > > If I understand correctly, I should modify this patch to remove the
> > > changes of arm and riscv for now, do the changes only when there is
> > > a real problem or requirement some day, right? If no more comments,
> > > I will send v3 later.
> >
> > Now everyone involved agrees that the efistub code is correct, so the
> > proper solution is to fix the compiler.
>
> Hm?  I don't see how it's a compiler bug.  It's really just an objtool
> limitation.
>
> > Changing efistub code and changing objtool (ignore __efistub prefix)
> > are both workarounds, but I think changing objtool is a little more
> > reasonable. Maybe Josh has different ideas?
>
> I thought the conversation had converged on what Tiezhu mentioned above,
> which is to skip objtool on libstub for loongarch, but leave the other
> arches alone.  That way objtool behavior is consistent between loongarch
> and x86, and objtool doesn't need to ignore any prefixes.
>
> So basically, the v2 patch minus the arm64/riscv changes.

Hi, ARM64 and RISC-V maintainers,

Would you mind that this patch modifies the three architectures
together (they are exactly the same style now)?

Madhavan is the author of ARM64's objtool, I think your opinion is
also very important.


Huacai

>
> --
> Josh

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Will Deacon]

On Sat, Nov 15, 2025 at 11:16:42AM +0800, Huacai Chen wrote:
> On Wed, Nov 12, 2025 at 2:00 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Mon, Nov 10, 2025 at 03:00:00PM +0800, Huacai Chen wrote:
> > > On Mon, Nov 10, 2025 at 9:19 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > > > If I understand correctly, I should modify this patch to remove the
> > > > changes of arm and riscv for now, do the changes only when there is
> > > > a real problem or requirement some day, right? If no more comments,
> > > > I will send v3 later.
> > >
> > > Now everyone involved agrees that the efistub code is correct, so the
> > > proper solution is to fix the compiler.
> >
> > Hm?  I don't see how it's a compiler bug.  It's really just an objtool
> > limitation.
> >
> > > Changing efistub code and changing objtool (ignore __efistub prefix)
> > > are both workarounds, but I think changing objtool is a little more
> > > reasonable. Maybe Josh has different ideas?
> >
> > I thought the conversation had converged on what Tiezhu mentioned above,
> > which is to skip objtool on libstub for loongarch, but leave the other
> > arches alone.  That way objtool behavior is consistent between loongarch
> > and x86, and objtool doesn't need to ignore any prefixes.
> >
> > So basically, the v2 patch minus the arm64/riscv changes.
> 
> Hi, ARM64 and RISC-V maintainers,
> 
> Would you mind that this patch modifies the three architectures
> together (they are exactly the same style now)?
> 
> Madhavan is the author of ARM64's objtool, I think your opinion is
> also very important.

arm64 doesn't (yet) use objtool.

I defer to Ard on anything relating to the arm64 efistub. Reading the
start of this thread, it doesn't look like he's convinced and I'm not
surprised if it's purely an issue with objtool.

Will

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Paul Walmsley]

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

On Mon, 17 Nov 2025, Will Deacon wrote:

> On Sat, Nov 15, 2025 at 11:16:42AM +0800, Huacai Chen wrote:
> > On Wed, Nov 12, 2025 at 2:00 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > > On Mon, Nov 10, 2025 at 03:00:00PM +0800, Huacai Chen wrote:
> > >
> > > > Changing efistub code and changing objtool (ignore __efistub prefix)
> > > > are both workarounds, but I think changing objtool is a little more
> > > > reasonable. Maybe Josh has different ideas?
> > >
> > > I thought the conversation had converged on what Tiezhu mentioned above,
> > > which is to skip objtool on libstub for loongarch, but leave the other
> > > arches alone.  That way objtool behavior is consistent between loongarch
> > > and x86, and objtool doesn't need to ignore any prefixes.
> > >
> > > So basically, the v2 patch minus the arm64/riscv changes.
> > 
> > Hi, ARM64 and RISC-V maintainers,
> > 
> > Would you mind that this patch modifies the three architectures
> > together (they are exactly the same style now)?
> > 
> > Madhavan is the author of ARM64's objtool, I think your opinion is
> > also very important.
> 
> arm64 doesn't (yet) use objtool.
> 
> I defer to Ard on anything relating to the arm64 efistub. Reading the
> start of this thread, it doesn't look like he's convinced and I'm not
> surprised if it's purely an issue with objtool.

Same for RISC-V.


- Paul

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Huacai Chen]

On Mon, Nov 17, 2025 at 7:33 PM Will Deacon <will@kernel.org> wrote:
>
> On Sat, Nov 15, 2025 at 11:16:42AM +0800, Huacai Chen wrote:
> > On Wed, Nov 12, 2025 at 2:00 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > On Mon, Nov 10, 2025 at 03:00:00PM +0800, Huacai Chen wrote:
> > > > On Mon, Nov 10, 2025 at 9:19 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > > > > If I understand correctly, I should modify this patch to remove the
> > > > > changes of arm and riscv for now, do the changes only when there is
> > > > > a real problem or requirement some day, right? If no more comments,
> > > > > I will send v3 later.
> > > >
> > > > Now everyone involved agrees that the efistub code is correct, so the
> > > > proper solution is to fix the compiler.
> > >
> > > Hm?  I don't see how it's a compiler bug.  It's really just an objtool
> > > limitation.
> > >
> > > > Changing efistub code and changing objtool (ignore __efistub prefix)
> > > > are both workarounds, but I think changing objtool is a little more
> > > > reasonable. Maybe Josh has different ideas?
> > >
> > > I thought the conversation had converged on what Tiezhu mentioned above,
> > > which is to skip objtool on libstub for loongarch, but leave the other
> > > arches alone.  That way objtool behavior is consistent between loongarch
> > > and x86, and objtool doesn't need to ignore any prefixes.
> > >
> > > So basically, the v2 patch minus the arm64/riscv changes.
> >
> > Hi, ARM64 and RISC-V maintainers,
> >
> > Would you mind that this patch modifies the three architectures
> > together (they are exactly the same style now)?
> >
> > Madhavan is the author of ARM64's objtool, I think your opinion is
> > also very important.
>
> arm64 doesn't (yet) use objtool.
>
> I defer to Ard on anything relating to the arm64 efistub. Reading the
> start of this thread, it doesn't look like he's convinced and I'm not
> surprised if it's purely an issue with objtool.
OK, I know, but I have a concern.

Ard said that he is reluctant to make changes to accommodate a flawed
build time tool and there may be regression risk.

So, I'm also reluctant and don't want LoongArch to meet regression
risk. If one day LoongArch has a regression, we probably need another
workaround to "fix" this workaround. But if these three architectures
change in the same way, we may have a chance to solve some fundamental
problems...

Huacai

>
> Will

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Tiezhu Yang]

Cc: Nathan Chancellor <nathan@kernel.org>

Hi all,

On 11/21/25 22:36, Huacai Chen wrote:
> On Mon, Nov 17, 2025 at 7:33 PM Will Deacon <will@kernel.org> wrote:
>>
>> On Sat, Nov 15, 2025 at 11:16:42AM +0800, Huacai Chen wrote:
>>> On Wed, Nov 12, 2025 at 2:00 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>>>>
>>>> On Mon, Nov 10, 2025 at 03:00:00PM +0800, Huacai Chen wrote:
>>>>> On Mon, Nov 10, 2025 at 9:19 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>>>>> If I understand correctly, I should modify this patch to remove the
>>>>>> changes of arm and riscv for now, do the changes only when there is
>>>>>> a real problem or requirement some day, right? If no more comments,
>>>>>> I will send v3 later.
>>>>>
>>>>> Now everyone involved agrees that the efistub code is correct, so the
>>>>> proper solution is to fix the compiler.
>>>>
>>>> Hm?  I don't see how it's a compiler bug.  It's really just an objtool
>>>> limitation.
>>>>
>>>>> Changing efistub code and changing objtool (ignore __efistub prefix)
>>>>> are both workarounds, but I think changing objtool is a little more
>>>>> reasonable. Maybe Josh has different ideas?
>>>>
>>>> I thought the conversation had converged on what Tiezhu mentioned above,
>>>> which is to skip objtool on libstub for loongarch, but leave the other
>>>> arches alone.  That way objtool behavior is consistent between loongarch
>>>> and x86, and objtool doesn't need to ignore any prefixes.
>>>>
>>>> So basically, the v2 patch minus the arm64/riscv changes.
>>>
>>> Hi, ARM64 and RISC-V maintainers,
>>>
>>> Would you mind that this patch modifies the three architectures
>>> together (they are exactly the same style now)?
>>>
>>> Madhavan is the author of ARM64's objtool, I think your opinion is
>>> also very important.
>>
>> arm64 doesn't (yet) use objtool.
>>
>> I defer to Ard on anything relating to the arm64 efistub. Reading the
>> start of this thread, it doesn't look like he's convinced and I'm not
>> surprised if it's purely an issue with objtool.
> OK, I know, but I have a concern.
> 
> Ard said that he is reluctant to make changes to accommodate a flawed
> build time tool and there may be regression risk.
> 
> So, I'm also reluctant and don't want LoongArch to meet regression
> risk. If one day LoongArch has a regression, we probably need another
> workaround to "fix" this workaround. But if these three architectures
> change in the same way, we may have a chance to solve some fundamental
> problems...

IIUC, Josh do not like to ignore these EFISTUB functions in
validate_branch() of objtool, Huacai do not like to only link
libstub to vmlinux only for LoongArch.

It seems that there is only one way to fix or workaround this
problem, remove the attribute __noreturn for real_kernel_entry()
and then make efi_boot_kernel() ends with a return instruction [1]
or an unconditional jump instruction [2] that only touches
drivers/firmware/efi/libstub/loongarch.c.

Since this is a issue only for LoongArch by now, what do you
think of this minimal change only for LoongArch libstub code?
Using "return EFI_LOAD_ERROR" [1] or "while (1)" [2]?

Maybe this is the simple and direct way for this special issue
only on LoongArch so far. If not, any other suggestions?

On the other hand, is it possible to add KBUILD_VMLINUX_LIBS_FINAL
and then use it only for LoongArch first [3]? Any potential risks?

What is the next step?

[1] 
https://lore.kernel.org/loongarch/CAMj1kXH-rK0bRyHXdJ-crAyMyvJHApH0WR7_8Qd8vrSPBLK+yg@mail.gmail.com/
[2] 
https://lore.kernel.org/loongarch/20250826064631.9617-2-yangtiezhu@loongson.cn/
[3] https://lore.kernel.org/loongarch/20251122000101.GA1996391@ax162/

Thanks,
Tiezhu


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv