Thread (47 messages) 47 messages, 6 authors, 2025-11-28

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help