* [PATCH v2] efistub: Only link libstub to final vmlinux @ 2025-09-28 8:55 Tiezhu Yang 2025-09-28 13:41 ` Ard Biesheuvel 0 siblings, 1 reply; 43+ messages in thread From: Tiezhu Yang @ 2025-09-28 8:55 UTC (permalink / raw) To: Ard Biesheuvel, Josh Poimboeuf, Huacai Chen Cc: loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-09-28 8:55 [PATCH v2] efistub: Only link libstub to final vmlinux Tiezhu Yang @ 2025-09-28 13:41 ` Ard Biesheuvel 2025-09-28 13:52 ` Huacai Chen 0 siblings, 1 reply; 43+ messages in thread From: Ard Biesheuvel @ 2025-09-28 13:41 UTC (permalink / raw) To: Tiezhu Yang Cc: Josh Poimboeuf, Huacai Chen, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-09-28 13:41 ` Ard Biesheuvel @ 2025-09-28 13:52 ` Huacai Chen 2025-09-28 14:39 ` Ard Biesheuvel 0 siblings, 1 reply; 43+ messages in thread From: Huacai Chen @ 2025-09-28 13:52 UTC (permalink / raw) To: Ard Biesheuvel Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-09-28 13:52 ` Huacai Chen @ 2025-09-28 14:39 ` Ard Biesheuvel 2025-09-28 14:41 ` Ard Biesheuvel 2025-09-30 2:52 ` Huacai Chen 0 siblings, 2 replies; 43+ messages in thread From: Ard Biesheuvel @ 2025-09-28 14:39 UTC (permalink / raw) To: Huacai Chen Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-09-28 14:39 ` Ard Biesheuvel @ 2025-09-28 14:41 ` Ard Biesheuvel 2025-10-09 7:27 ` Tiezhu Yang 2025-09-30 2:52 ` Huacai Chen 1 sibling, 1 reply; 43+ messages in thread From: Ard Biesheuvel @ 2025-09-28 14:41 UTC (permalink / raw) To: Huacai Chen Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-09-28 14:41 ` Ard Biesheuvel @ 2025-10-09 7:27 ` Tiezhu Yang 2025-10-10 16:25 ` Ard Biesheuvel 0 siblings, 1 reply; 43+ messages in thread From: Tiezhu Yang @ 2025-10-09 7:27 UTC (permalink / raw) To: Ard Biesheuvel, Huacai Chen Cc: Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-09 7:27 ` Tiezhu Yang @ 2025-10-10 16:25 ` Ard Biesheuvel 2025-10-11 1:13 ` Tiezhu Yang 0 siblings, 1 reply; 43+ messages in thread From: Ard Biesheuvel @ 2025-10-10 16:25 UTC (permalink / raw) To: Tiezhu Yang Cc: Huacai Chen, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-10 16:25 ` Ard Biesheuvel @ 2025-10-11 1:13 ` Tiezhu Yang 2025-10-11 2:54 ` Huacai Chen 0 siblings, 1 reply; 43+ messages in thread From: Tiezhu Yang @ 2025-10-11 1:13 UTC (permalink / raw) To: Ard Biesheuvel Cc: Huacai Chen, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-11 1:13 ` Tiezhu Yang @ 2025-10-11 2:54 ` Huacai Chen 2025-10-11 3:40 ` Ard Biesheuvel 0 siblings, 1 reply; 43+ messages in thread From: Huacai Chen @ 2025-10-11 2:54 UTC (permalink / raw) To: Tiezhu Yang Cc: Ard Biesheuvel, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-11 2:54 ` Huacai Chen @ 2025-10-11 3:40 ` Ard Biesheuvel 2025-10-11 7:29 ` Tiezhu Yang 0 siblings, 1 reply; 43+ messages in thread From: Ard Biesheuvel @ 2025-10-11 3:40 UTC (permalink / raw) To: Huacai Chen Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-11 3:40 ` Ard Biesheuvel @ 2025-10-11 7:29 ` Tiezhu Yang 2025-10-11 7:42 ` Huacai Chen 0 siblings, 1 reply; 43+ messages in thread From: Tiezhu Yang @ 2025-10-11 7:29 UTC (permalink / raw) To: Ard Biesheuvel, Huacai Chen Cc: Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-11 7:29 ` Tiezhu Yang @ 2025-10-11 7:42 ` Huacai Chen 2025-10-11 8:13 ` Tiezhu Yang 2025-10-11 14:48 ` Ard Biesheuvel 0 siblings, 2 replies; 43+ messages in thread From: Huacai Chen @ 2025-10-11 7:42 UTC (permalink / raw) To: Tiezhu Yang Cc: Ard Biesheuvel, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-11 7:42 ` Huacai Chen @ 2025-10-11 8:13 ` Tiezhu Yang 2025-10-11 14:48 ` Ard Biesheuvel 1 sibling, 0 replies; 43+ messages in thread From: Tiezhu Yang @ 2025-10-11 8:13 UTC (permalink / raw) To: Huacai Chen Cc: Ard Biesheuvel, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-11 7:42 ` Huacai Chen 2025-10-11 8:13 ` Tiezhu Yang @ 2025-10-11 14:48 ` Ard Biesheuvel 2025-10-11 15:01 ` Huacai Chen 1 sibling, 1 reply; 43+ messages in thread From: Ard Biesheuvel @ 2025-10-11 14:48 UTC (permalink / raw) To: Huacai Chen Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-11 14:48 ` Ard Biesheuvel @ 2025-10-11 15:01 ` Huacai Chen 2025-10-11 15:58 ` Ard Biesheuvel 0 siblings, 1 reply; 43+ messages in thread From: Huacai Chen @ 2025-10-11 15:01 UTC (permalink / raw) To: Ard Biesheuvel Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-11 15:01 ` Huacai Chen @ 2025-10-11 15:58 ` Ard Biesheuvel 2025-10-13 7:34 ` Tiezhu Yang 2025-10-13 14:09 ` Huacai Chen 0 siblings, 2 replies; 43+ messages in thread From: Ard Biesheuvel @ 2025-10-11 15:58 UTC (permalink / raw) To: Huacai Chen Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-11 15:58 ` Ard Biesheuvel @ 2025-10-13 7:34 ` Tiezhu Yang 2025-10-13 14:09 ` Huacai Chen 1 sibling, 0 replies; 43+ messages in thread From: Tiezhu Yang @ 2025-10-13 7:34 UTC (permalink / raw) To: Ard Biesheuvel, Huacai Chen Cc: Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-11 15:58 ` Ard Biesheuvel 2025-10-13 7:34 ` Tiezhu Yang @ 2025-10-13 14:09 ` Huacai Chen 2025-10-13 14:36 ` Ard Biesheuvel 1 sibling, 1 reply; 43+ messages in thread From: Huacai Chen @ 2025-10-13 14:09 UTC (permalink / raw) To: Ard Biesheuvel Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-13 14:09 ` Huacai Chen @ 2025-10-13 14:36 ` Ard Biesheuvel 2025-10-14 16:47 ` Josh Poimboeuf 0 siblings, 1 reply; 43+ messages in thread From: Ard Biesheuvel @ 2025-10-13 14:36 UTC (permalink / raw) To: Huacai Chen Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-13 14:36 ` Ard Biesheuvel @ 2025-10-14 16:47 ` Josh Poimboeuf 2025-10-16 14:52 ` Ard Biesheuvel 0 siblings, 1 reply; 43+ messages in thread From: Josh Poimboeuf @ 2025-10-14 16:47 UTC (permalink / raw) To: Ard Biesheuvel Cc: Huacai Chen, Tiezhu Yang, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-14 16:47 ` Josh Poimboeuf @ 2025-10-16 14:52 ` Ard Biesheuvel 2025-10-16 15:49 ` Josh Poimboeuf 0 siblings, 1 reply; 43+ messages in thread From: Ard Biesheuvel @ 2025-10-16 14:52 UTC (permalink / raw) To: Josh Poimboeuf Cc: Huacai Chen, Tiezhu Yang, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-16 14:52 ` Ard Biesheuvel @ 2025-10-16 15:49 ` Josh Poimboeuf 2025-10-17 11:00 ` Ard Biesheuvel 0 siblings, 1 reply; 43+ messages in thread From: Josh Poimboeuf @ 2025-10-16 15:49 UTC (permalink / raw) To: Ard Biesheuvel Cc: Huacai Chen, Tiezhu Yang, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-16 15:49 ` Josh Poimboeuf @ 2025-10-17 11:00 ` Ard Biesheuvel 2025-10-17 16:22 ` Josh Poimboeuf 0 siblings, 1 reply; 43+ messages in thread From: Ard Biesheuvel @ 2025-10-17 11:00 UTC (permalink / raw) To: Josh Poimboeuf Cc: Huacai Chen, Tiezhu Yang, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-17 11:00 ` Ard Biesheuvel @ 2025-10-17 16:22 ` Josh Poimboeuf 2025-10-17 16:26 ` Ard Biesheuvel 0 siblings, 1 reply; 43+ messages in thread From: Josh Poimboeuf @ 2025-10-17 16:22 UTC (permalink / raw) To: Ard Biesheuvel Cc: Huacai Chen, Tiezhu Yang, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-17 16:22 ` Josh Poimboeuf @ 2025-10-17 16:26 ` Ard Biesheuvel 2025-10-17 17:05 ` Josh Poimboeuf 0 siblings, 1 reply; 43+ messages in thread From: Ard Biesheuvel @ 2025-10-17 16:26 UTC (permalink / raw) To: Josh Poimboeuf Cc: Huacai Chen, Tiezhu Yang, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-17 16:26 ` Ard Biesheuvel @ 2025-10-17 17:05 ` Josh Poimboeuf 2025-10-20 1:24 ` Tiezhu Yang 0 siblings, 1 reply; 43+ messages in thread From: Josh Poimboeuf @ 2025-10-17 17:05 UTC (permalink / raw) To: Ard Biesheuvel Cc: Huacai Chen, Tiezhu Yang, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-17 17:05 ` Josh Poimboeuf @ 2025-10-20 1:24 ` Tiezhu Yang 2025-10-20 6:55 ` Huacai Chen 0 siblings, 1 reply; 43+ messages in thread From: Tiezhu Yang @ 2025-10-20 1:24 UTC (permalink / raw) To: Josh Poimboeuf, Ard Biesheuvel Cc: Huacai Chen, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-20 1:24 ` Tiezhu Yang @ 2025-10-20 6:55 ` Huacai Chen 2025-10-23 6:55 ` Tiezhu Yang 0 siblings, 1 reply; 43+ messages in thread From: Huacai Chen @ 2025-10-20 6:55 UTC (permalink / raw) To: Tiezhu Yang Cc: Josh Poimboeuf, Ard Biesheuvel, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-20 6:55 ` Huacai Chen @ 2025-10-23 6:55 ` Tiezhu Yang 2025-10-23 8:01 ` Huacai Chen 0 siblings, 1 reply; 43+ messages in thread From: Tiezhu Yang @ 2025-10-23 6:55 UTC (permalink / raw) To: Huacai Chen Cc: Josh Poimboeuf, Ard Biesheuvel, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-23 6:55 ` Tiezhu Yang @ 2025-10-23 8:01 ` Huacai Chen 2025-10-23 8:06 ` Ard Biesheuvel 0 siblings, 1 reply; 43+ messages in thread From: Huacai Chen @ 2025-10-23 8:01 UTC (permalink / raw) To: Tiezhu Yang Cc: Josh Poimboeuf, Ard Biesheuvel, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-23 8:01 ` Huacai Chen @ 2025-10-23 8:06 ` Ard Biesheuvel 2025-10-26 11:20 ` Huacai Chen 0 siblings, 1 reply; 43+ messages in thread From: Ard Biesheuvel @ 2025-10-23 8:06 UTC (permalink / raw) To: Huacai Chen Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-23 8:06 ` Ard Biesheuvel @ 2025-10-26 11:20 ` Huacai Chen 2025-10-28 13:47 ` Ard Biesheuvel 0 siblings, 1 reply; 43+ messages in thread From: Huacai Chen @ 2025-10-26 11:20 UTC (permalink / raw) To: Ard Biesheuvel Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-26 11:20 ` Huacai Chen @ 2025-10-28 13:47 ` Ard Biesheuvel 2025-11-10 1:18 ` Tiezhu Yang 0 siblings, 1 reply; 43+ messages in thread From: Ard Biesheuvel @ 2025-10-28 13:47 UTC (permalink / raw) To: Huacai Chen Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-10-28 13:47 ` Ard Biesheuvel @ 2025-11-10 1:18 ` Tiezhu Yang 2025-11-10 7:00 ` Huacai Chen 0 siblings, 1 reply; 43+ messages in thread From: Tiezhu Yang @ 2025-11-10 1:18 UTC (permalink / raw) To: Ard Biesheuvel, Huacai Chen Cc: Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-11-10 1:18 ` Tiezhu Yang @ 2025-11-10 7:00 ` Huacai Chen 2025-11-11 16:49 ` Ard Biesheuvel 2025-11-11 18:00 ` Josh Poimboeuf 0 siblings, 2 replies; 43+ messages in thread From: Huacai Chen @ 2025-11-10 7:00 UTC (permalink / raw) To: Tiezhu Yang Cc: Ard Biesheuvel, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-11-10 7:00 ` Huacai Chen @ 2025-11-11 16:49 ` Ard Biesheuvel 2025-11-11 18:00 ` Josh Poimboeuf 1 sibling, 0 replies; 43+ messages in thread From: Ard Biesheuvel @ 2025-11-11 16:49 UTC (permalink / raw) To: Huacai Chen Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-11-10 7:00 ` Huacai Chen 2025-11-11 16:49 ` Ard Biesheuvel @ 2025-11-11 18:00 ` Josh Poimboeuf 2025-11-15 3:16 ` Huacai Chen 1 sibling, 1 reply; 43+ messages in thread From: Josh Poimboeuf @ 2025-11-11 18:00 UTC (permalink / raw) To: Huacai Chen Cc: Tiezhu Yang, Ard Biesheuvel, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-11-11 18:00 ` Josh Poimboeuf @ 2025-11-15 3:16 ` Huacai Chen 2025-11-17 11:33 ` Will Deacon 0 siblings, 1 reply; 43+ messages in thread From: Huacai Chen @ 2025-11-15 3:16 UTC (permalink / raw) To: Josh Poimboeuf, Catalin Marinas, Will Deacon, Paul Walmsley, Palmer Dabbelt, Albert Ou, Madhavan T. Venkataraman Cc: Tiezhu Yang, Ard Biesheuvel, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-11-15 3:16 ` Huacai Chen @ 2025-11-17 11:33 ` Will Deacon 2025-11-21 2:09 ` Paul Walmsley 2025-11-21 14:36 ` Huacai Chen 0 siblings, 2 replies; 43+ messages in thread From: Will Deacon @ 2025-11-17 11:33 UTC (permalink / raw) To: Huacai Chen Cc: Josh Poimboeuf, Catalin Marinas, Paul Walmsley, Palmer Dabbelt, Albert Ou, Madhavan T. Venkataraman, Tiezhu Yang, Ard Biesheuvel, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-11-17 11:33 ` Will Deacon @ 2025-11-21 2:09 ` Paul Walmsley 2025-11-21 14:36 ` Huacai Chen 1 sibling, 0 replies; 43+ messages in thread From: Paul Walmsley @ 2025-11-21 2:09 UTC (permalink / raw) To: Huacai Chen Cc: Will Deacon, Josh Poimboeuf, Catalin Marinas, Paul Walmsley, Palmer Dabbelt, Albert Ou, Madhavan T. Venkataraman, Tiezhu Yang, Ard Biesheuvel, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel [-- 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-11-17 11:33 ` Will Deacon 2025-11-21 2:09 ` Paul Walmsley @ 2025-11-21 14:36 ` Huacai Chen 2025-11-22 11:04 ` Tiezhu Yang 1 sibling, 1 reply; 43+ messages in thread From: Huacai Chen @ 2025-11-21 14:36 UTC (permalink / raw) To: Will Deacon Cc: Josh Poimboeuf, Catalin Marinas, Paul Walmsley, Palmer Dabbelt, Albert Ou, Madhavan T. Venkataraman, Tiezhu Yang, Ard Biesheuvel, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-11-21 14:36 ` Huacai Chen @ 2025-11-22 11:04 ` Tiezhu Yang 0 siblings, 0 replies; 43+ messages in thread From: Tiezhu Yang @ 2025-11-22 11:04 UTC (permalink / raw) To: Huacai Chen, Will Deacon Cc: Josh Poimboeuf, Catalin Marinas, Paul Walmsley, Palmer Dabbelt, Albert Ou, Madhavan T. Venkataraman, Ard Biesheuvel, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel, Nathan Chancellor 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2] efistub: Only link libstub to final vmlinux 2025-09-28 14:39 ` Ard Biesheuvel 2025-09-28 14:41 ` Ard Biesheuvel @ 2025-09-30 2:52 ` Huacai Chen 1 sibling, 0 replies; 43+ messages in thread From: Huacai Chen @ 2025-09-30 2:52 UTC (permalink / raw) To: Ard Biesheuvel Cc: Tiezhu Yang, Josh Poimboeuf, loongarch, linux-arm-kernel, linux-riscv, linux-efi, linux-kbuild, linux-kernel 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
end of thread, other threads:[~2025-11-22 11:05 UTC | newest] Thread overview: 43+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-28 8:55 [PATCH v2] efistub: Only link libstub to final vmlinux Tiezhu Yang 2025-09-28 13:41 ` Ard Biesheuvel 2025-09-28 13:52 ` Huacai Chen 2025-09-28 14:39 ` Ard Biesheuvel 2025-09-28 14:41 ` Ard Biesheuvel 2025-10-09 7:27 ` Tiezhu Yang 2025-10-10 16:25 ` Ard Biesheuvel 2025-10-11 1:13 ` Tiezhu Yang 2025-10-11 2:54 ` Huacai Chen 2025-10-11 3:40 ` Ard Biesheuvel 2025-10-11 7:29 ` Tiezhu Yang 2025-10-11 7:42 ` Huacai Chen 2025-10-11 8:13 ` Tiezhu Yang 2025-10-11 14:48 ` Ard Biesheuvel 2025-10-11 15:01 ` Huacai Chen 2025-10-11 15:58 ` Ard Biesheuvel 2025-10-13 7:34 ` Tiezhu Yang 2025-10-13 14:09 ` Huacai Chen 2025-10-13 14:36 ` Ard Biesheuvel 2025-10-14 16:47 ` Josh Poimboeuf 2025-10-16 14:52 ` Ard Biesheuvel 2025-10-16 15:49 ` Josh Poimboeuf 2025-10-17 11:00 ` Ard Biesheuvel 2025-10-17 16:22 ` Josh Poimboeuf 2025-10-17 16:26 ` Ard Biesheuvel 2025-10-17 17:05 ` Josh Poimboeuf 2025-10-20 1:24 ` Tiezhu Yang 2025-10-20 6:55 ` Huacai Chen 2025-10-23 6:55 ` Tiezhu Yang 2025-10-23 8:01 ` Huacai Chen 2025-10-23 8:06 ` Ard Biesheuvel 2025-10-26 11:20 ` Huacai Chen 2025-10-28 13:47 ` Ard Biesheuvel 2025-11-10 1:18 ` Tiezhu Yang 2025-11-10 7:00 ` Huacai Chen 2025-11-11 16:49 ` Ard Biesheuvel 2025-11-11 18:00 ` Josh Poimboeuf 2025-11-15 3:16 ` Huacai Chen 2025-11-17 11:33 ` Will Deacon 2025-11-21 2:09 ` Paul Walmsley 2025-11-21 14:36 ` Huacai Chen 2025-11-22 11:04 ` Tiezhu Yang 2025-09-30 2:52 ` Huacai Chen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).