Re: [PATCH v2] efistub: Only link libstub to final vmlinux
From: Ard Biesheuvel <ardb@kernel.org>
Date: 2025-10-11 15:59:08
Also in:
linux-efi, linux-kbuild, linux-riscv, lkml, loongarch
On Sat, 11 Oct 2025 at 08:01, Huacai Chen [off-list ref] wrote:
On Sat, Oct 11, 2025 at 10:48 PM Ard Biesheuvel [off-list ref] wrote:quoted
On Sat, 11 Oct 2025 at 00:43, Huacai Chen [off-list ref] wrote:quoted
On Sat, Oct 11, 2025 at 3:29 PM Tiezhu Yang [off-list ref] wrote:quoted
On 2025/10/11 上午11:40, Ard Biesheuvel wrote:quoted
On Fri, 10 Oct 2025 at 19:54, Huacai Chen [off-list ref] wrote:quoted
On Sat, Oct 11, 2025 at 9:13 AM Tiezhu Yang [off-list ref] wrote:quoted
On 2025/10/11 上午12:25, Ard Biesheuvel wrote: ...quoted
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.quoted
--- 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.