[PATCH v1 1/3] objtool/LoongArch: Fix fall through warning about efi_boot_kernel()

Tiezhu Yang posted 3 patches 1 month ago
There is a newer version of this series
[PATCH v1 1/3] objtool/LoongArch: Fix fall through warning about efi_boot_kernel()
Posted by Tiezhu Yang 1 month ago
When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
the following objtool warning:

  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 it is not a proper way.

After more analysis, one simple way is to ignore these EFISTUB functions
in validate_branch() of objtool since they are useless for stack unwinder.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/check.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index d14f20ef1db1..2dcebf75d95e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3567,6 +3567,10 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 			    !strncmp(func->name, "__pfx_", 6))
 				return 0;
 
+			/* Ignore EFISTUB functions which are useless for stack unwinder */
+			if (!strncmp(func->name, "__efistub_", 10))
+				return 0;
+
 			if (file->ignore_unreachables)
 				return 0;
 
-- 
2.42.0
Re: [PATCH v1 1/3] objtool/LoongArch: Fix fall through warning about efi_boot_kernel()
Posted by Peter Zijlstra 1 month ago
On Mon, Sep 01, 2025 at 03:21:54PM +0800, Tiezhu Yang wrote:
> When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
> the following objtool warning:
> 
>   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 it is not a proper way.
> 
> After more analysis, one simple way is to ignore these EFISTUB functions
> in validate_branch() of objtool since they are useless for stack unwinder.
> 

This is drivers/firmware/efi/libstub/loongarch.c:efi_boot_kernel(),
right?

Why not simply do something like:

diff --git a/drivers/firmware/efi/libstub/loongarch.c b/drivers/firmware/efi/libstub/loongarch.c
index 3782d0a187d1..082611a5f1f0 100644
--- a/drivers/firmware/efi/libstub/loongarch.c
+++ b/drivers/firmware/efi/libstub/loongarch.c
@@ -81,4 +81,5 @@ 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);
+	BUG();
 }
Re: [PATCH v1 1/3] objtool/LoongArch: Fix fall through warning about efi_boot_kernel()
Posted by Tiezhu Yang 1 month ago
On 2025/9/1 下午4:16, Peter Zijlstra wrote:
> On Mon, Sep 01, 2025 at 03:21:54PM +0800, Tiezhu Yang wrote:
>> When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
>> the following objtool warning:
>>
>>    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 it is not a proper way.
>>
>> After more analysis, one simple way is to ignore these EFISTUB functions
>> in validate_branch() of objtool since they are useless for stack unwinder.
>>
> 
> This is drivers/firmware/efi/libstub/loongarch.c:efi_boot_kernel(),
> right?
> 
> Why not simply do something like:
> 
> diff --git a/drivers/firmware/efi/libstub/loongarch.c b/drivers/firmware/efi/libstub/loongarch.c
> index 3782d0a187d1..082611a5f1f0 100644
> --- a/drivers/firmware/efi/libstub/loongarch.c
> +++ b/drivers/firmware/efi/libstub/loongarch.c
> @@ -81,4 +81,5 @@ 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);
> +	BUG();
>   }

At the beginning, I did the above change, but no effect.

The first thing is to remove the attribute __noreturn for
real_kernel_entry(), otherwise the compiler can not generate
instructions after that.

But there is an argument in the previous RFC [1]:

"From my point of view this is incorrect, this function is indeed a
noreturn function, and this modification makes LoongArch different to
other architectures."

Josh suggested to do something so that the EFI stub code isn't linked 
into vmlinux.o [2], it needs to modify the link process and seems too
complicated and expensive for this warning to some extent.

So I did this change for objtool.

[1] 
https://lore.kernel.org/loongarch/CAAhV-H5wW_04NHQ7z+SCPb6-T5Hc__n+x=ykg-u9vn4b4GXuww@mail.gmail.com/
[2] 
https://lore.kernel.org/loongarch/xyrcgkl7ud5pgh4h5yjyejz646bc22fnnwxahaoafqvnqintf3@mdhtfaybai67/

Thanks,
Tiezhu

Re: [PATCH v1 1/3] objtool/LoongArch: Fix fall through warning about efi_boot_kernel()
Posted by Josh Poimboeuf 4 weeks, 1 day ago
On Mon, Sep 01, 2025 at 04:31:36PM +0800, Tiezhu Yang wrote:
> On 2025/9/1 下午4:16, Peter Zijlstra wrote:
> > On Mon, Sep 01, 2025 at 03:21:54PM +0800, Tiezhu Yang wrote:
> > > When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
> > > the following objtool warning:
> > > 
> > >    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 it is not a proper way.
> > > 
> > > After more analysis, one simple way is to ignore these EFISTUB functions
> > > in validate_branch() of objtool since they are useless for stack unwinder.
> > > 
> > 
> > This is drivers/firmware/efi/libstub/loongarch.c:efi_boot_kernel(),
> > right?
> > 
> > Why not simply do something like:
> > 
> > diff --git a/drivers/firmware/efi/libstub/loongarch.c b/drivers/firmware/efi/libstub/loongarch.c
> > index 3782d0a187d1..082611a5f1f0 100644
> > --- a/drivers/firmware/efi/libstub/loongarch.c
> > +++ b/drivers/firmware/efi/libstub/loongarch.c
> > @@ -81,4 +81,5 @@ 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);
> > +	BUG();
> >   }
> 
> At the beginning, I did the above change, but no effect.
> 
> The first thing is to remove the attribute __noreturn for
> real_kernel_entry(), otherwise the compiler can not generate
> instructions after that.
> 
> But there is an argument in the previous RFC [1]:
> 
> "From my point of view this is incorrect, this function is indeed a
> noreturn function, and this modification makes LoongArch different to
> other architectures."
> 
> Josh suggested to do something so that the EFI stub code isn't linked into
> vmlinux.o [2], it needs to modify the link process and seems too
> complicated and expensive for this warning to some extent.
> 
> So I did this change for objtool.

I don't like adding these workarounds to objtool.  Is it really that
complicated to link efistub separately?  That seems like the proper
design.  vmlinux.o should only have real kernel code.

-- 
Josh
Re: [PATCH v1 1/3] objtool/LoongArch: Fix fall through warning about efi_boot_kernel()
Posted by Huacai Chen 4 weeks, 1 day ago
Hi, Josh,

On Thu, Sep 4, 2025 at 3:17 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Mon, Sep 01, 2025 at 04:31:36PM +0800, Tiezhu Yang wrote:
> > On 2025/9/1 下午4:16, Peter Zijlstra wrote:
> > > On Mon, Sep 01, 2025 at 03:21:54PM +0800, Tiezhu Yang wrote:
> > > > When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
> > > > the following objtool warning:
> > > >
> > > >    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 it is not a proper way.
> > > >
> > > > After more analysis, one simple way is to ignore these EFISTUB functions
> > > > in validate_branch() of objtool since they are useless for stack unwinder.
> > > >
> > >
> > > This is drivers/firmware/efi/libstub/loongarch.c:efi_boot_kernel(),
> > > right?
> > >
> > > Why not simply do something like:
> > >
> > > diff --git a/drivers/firmware/efi/libstub/loongarch.c b/drivers/firmware/efi/libstub/loongarch.c
> > > index 3782d0a187d1..082611a5f1f0 100644
> > > --- a/drivers/firmware/efi/libstub/loongarch.c
> > > +++ b/drivers/firmware/efi/libstub/loongarch.c
> > > @@ -81,4 +81,5 @@ 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);
> > > +   BUG();
> > >   }
> >
> > At the beginning, I did the above change, but no effect.
> >
> > The first thing is to remove the attribute __noreturn for
> > real_kernel_entry(), otherwise the compiler can not generate
> > instructions after that.
> >
> > But there is an argument in the previous RFC [1]:
> >
> > "From my point of view this is incorrect, this function is indeed a
> > noreturn function, and this modification makes LoongArch different to
> > other architectures."
> >
> > Josh suggested to do something so that the EFI stub code isn't linked into
> > vmlinux.o [2], it needs to modify the link process and seems too
> > complicated and expensive for this warning to some extent.
> >
> > So I did this change for objtool.
>
> I don't like adding these workarounds to objtool.  Is it really that
> complicated to link efistub separately?  That seems like the proper
> design.  vmlinux.o should only have real kernel code.
I don't think this is just a "workaround", ARM64, RISC-V and LoongArch
share the same logic in efistub which may be different from X86. When
ARM64 and RISC-V add objtool support, they will also need to ignore
the __efistub_ functions.

The other patch is similar.


Huacai

>
> --
> Josh
Re: [PATCH v1 1/3] objtool/LoongArch: Fix fall through warning about efi_boot_kernel()
Posted by Josh Poimboeuf 4 weeks ago
On Thu, Sep 04, 2025 at 10:17:11AM +0800, Huacai Chen wrote:
> Hi, Josh,
> 
> On Thu, Sep 4, 2025 at 3:17 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Mon, Sep 01, 2025 at 04:31:36PM +0800, Tiezhu Yang wrote:
> > > On 2025/9/1 下午4:16, Peter Zijlstra wrote:
> > > > On Mon, Sep 01, 2025 at 03:21:54PM +0800, Tiezhu Yang wrote:
> > > > > When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
> > > > > the following objtool warning:
> > > > >
> > > > >    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 it is not a proper way.
> > > > >
> > > > > After more analysis, one simple way is to ignore these EFISTUB functions
> > > > > in validate_branch() of objtool since they are useless for stack unwinder.
> > > > >
> > > >
> > > > This is drivers/firmware/efi/libstub/loongarch.c:efi_boot_kernel(),
> > > > right?
> > > >
> > > > Why not simply do something like:
> > > >
> > > > diff --git a/drivers/firmware/efi/libstub/loongarch.c b/drivers/firmware/efi/libstub/loongarch.c
> > > > index 3782d0a187d1..082611a5f1f0 100644
> > > > --- a/drivers/firmware/efi/libstub/loongarch.c
> > > > +++ b/drivers/firmware/efi/libstub/loongarch.c
> > > > @@ -81,4 +81,5 @@ 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);
> > > > +   BUG();
> > > >   }
> > >
> > > At the beginning, I did the above change, but no effect.
> > >
> > > The first thing is to remove the attribute __noreturn for
> > > real_kernel_entry(), otherwise the compiler can not generate
> > > instructions after that.
> > >
> > > But there is an argument in the previous RFC [1]:
> > >
> > > "From my point of view this is incorrect, this function is indeed a
> > > noreturn function, and this modification makes LoongArch different to
> > > other architectures."
> > >
> > > Josh suggested to do something so that the EFI stub code isn't linked into
> > > vmlinux.o [2], it needs to modify the link process and seems too
> > > complicated and expensive for this warning to some extent.
> > >
> > > So I did this change for objtool.
> >
> > I don't like adding these workarounds to objtool.  Is it really that
> > complicated to link efistub separately?  That seems like the proper
> > design.  vmlinux.o should only have real kernel code.
> I don't think this is just a "workaround", ARM64, RISC-V and LoongArch
> share the same logic in efistub which may be different from X86. When
> ARM64 and RISC-V add objtool support, they will also need to ignore
> the __efistub_ functions.
> 
> The other patch is similar.

Objtool expects/enforces certain rules.  One of them is that vmlinux.o
is proper runtime kernel code.  efistub is not that.

Is there some technical reason why vmlinux.o needs efistub linked in?

-- 
Josh
Re: [PATCH v1 1/3] objtool/LoongArch: Fix fall through warning about efi_boot_kernel()
Posted by Huacai Chen 4 weeks ago
Hi, Josh,

On Fri, Sep 5, 2025 at 1:26 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Thu, Sep 04, 2025 at 10:17:11AM +0800, Huacai Chen wrote:
> > Hi, Josh,
> >
> > On Thu, Sep 4, 2025 at 3:17 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > On Mon, Sep 01, 2025 at 04:31:36PM +0800, Tiezhu Yang wrote:
> > > > On 2025/9/1 下午4:16, Peter Zijlstra wrote:
> > > > > On Mon, Sep 01, 2025 at 03:21:54PM +0800, Tiezhu Yang wrote:
> > > > > > When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
> > > > > > the following objtool warning:
> > > > > >
> > > > > >    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 it is not a proper way.
> > > > > >
> > > > > > After more analysis, one simple way is to ignore these EFISTUB functions
> > > > > > in validate_branch() of objtool since they are useless for stack unwinder.
> > > > > >
> > > > >
> > > > > This is drivers/firmware/efi/libstub/loongarch.c:efi_boot_kernel(),
> > > > > right?
> > > > >
> > > > > Why not simply do something like:
> > > > >
> > > > > diff --git a/drivers/firmware/efi/libstub/loongarch.c b/drivers/firmware/efi/libstub/loongarch.c
> > > > > index 3782d0a187d1..082611a5f1f0 100644
> > > > > --- a/drivers/firmware/efi/libstub/loongarch.c
> > > > > +++ b/drivers/firmware/efi/libstub/loongarch.c
> > > > > @@ -81,4 +81,5 @@ 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);
> > > > > +   BUG();
> > > > >   }
> > > >
> > > > At the beginning, I did the above change, but no effect.
> > > >
> > > > The first thing is to remove the attribute __noreturn for
> > > > real_kernel_entry(), otherwise the compiler can not generate
> > > > instructions after that.
> > > >
> > > > But there is an argument in the previous RFC [1]:
> > > >
> > > > "From my point of view this is incorrect, this function is indeed a
> > > > noreturn function, and this modification makes LoongArch different to
> > > > other architectures."
> > > >
> > > > Josh suggested to do something so that the EFI stub code isn't linked into
> > > > vmlinux.o [2], it needs to modify the link process and seems too
> > > > complicated and expensive for this warning to some extent.
> > > >
> > > > So I did this change for objtool.
> > >
> > > I don't like adding these workarounds to objtool.  Is it really that
> > > complicated to link efistub separately?  That seems like the proper
> > > design.  vmlinux.o should only have real kernel code.
> > I don't think this is just a "workaround", ARM64, RISC-V and LoongArch
> > share the same logic in efistub which may be different from X86. When
> > ARM64 and RISC-V add objtool support, they will also need to ignore
> > the __efistub_ functions.
> >
> > The other patch is similar.
>
> Objtool expects/enforces certain rules.  One of them is that vmlinux.o
> is proper runtime kernel code.  efistub is not that.
>
> Is there some technical reason why vmlinux.o needs efistub linked in?
I think so. For example, EFISTUB prefer to directly use screen_info
that defined in vmlinux, see the comments in
drivers/firmware/efi/libstub/screen_info.c:

/*
 * There are two ways of populating the core kernel's struct
screen_info via the stub:
 * - using a configuration table, like below, which relies on the EFI init code
 *   to locate the table and copy the contents;
 * - by linking directly to the core kernel's copy of the global symbol.
 *
 * The latter is preferred because it makes the EFIFB earlycon available very
 * early, but it only works if the EFI stub is part of the core kernel image
 * itself. The zboot decompressor can only use the configuration table
 * approach.
 */

Huacai

>
> --
> Josh
Re: [PATCH v1 1/3] objtool/LoongArch: Fix fall through warning about efi_boot_kernel()
Posted by Tiezhu Yang 4 weeks ago
Hi Josh,

On 2025/9/5 下午12:33, Huacai Chen wrote:
> Hi, Josh,
> 
> On Fri, Sep 5, 2025 at 1:26 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>>
>> On Thu, Sep 04, 2025 at 10:17:11AM +0800, Huacai Chen wrote:

...

>> Is there some technical reason why vmlinux.o needs efistub linked in?
> I think so. For example, EFISTUB prefer to directly use screen_info
> that defined in vmlinux, see the comments in
> drivers/firmware/efi/libstub/screen_info.c:
> 
> /*
>   * There are two ways of populating the core kernel's struct
> screen_info via the stub:
>   * - using a configuration table, like below, which relies on the EFI init code
>   *   to locate the table and copy the contents;
>   * - by linking directly to the core kernel's copy of the global symbol.
>   *
>   * The latter is preferred because it makes the EFIFB earlycon available very
>   * early, but it only works if the EFI stub is part of the core kernel image
>   * itself. The zboot decompressor can only use the configuration table
>   * approach.
>   */

I wonder what is the final conclusion.

(1) For patch #1 and #2, keep the code as is and just update
the commit message.

(2) For patch #3, replace UNWIND_HINT_UNDEFINED with
UNWIND_HINT_END_OF_STACK and remove is_entry_func().

If there are any more comments, please let me know.

Thanks,
Tiezhu

Re: [PATCH v1 1/3] objtool/LoongArch: Fix fall through warning about efi_boot_kernel()
Posted by Tiezhu Yang 4 weeks, 1 day ago
On 2025/9/4 上午3:17, Josh Poimboeuf wrote:
> On Mon, Sep 01, 2025 at 04:31:36PM +0800, Tiezhu Yang wrote:
>> On 2025/9/1 下午4:16, Peter Zijlstra wrote:
>>> On Mon, Sep 01, 2025 at 03:21:54PM +0800, Tiezhu Yang wrote:
>>>> When compiling with LLVM and CONFIG_LTO_CLANG is set, there exists
>>>> the following objtool warning:

...

>> Josh suggested to do something so that the EFI stub code isn't linked into
>> vmlinux.o [2], it needs to modify the link process and seems too
>> complicated and expensive for this warning to some extent.
>>
>> So I did this change for objtool.
> 
> I don't like adding these workarounds to objtool.  Is it really that
> complicated to link efistub separately?  That seems like the proper
> design.  vmlinux.o should only have real kernel code.

OK, I see. If this is the only proper direction, I will do it
in the next version.

Thanks,
Tiezhu