linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] LoongArch: Define the symbol fault as a local label in fpu.S
@ 2023-08-30  2:47 Tiezhu Yang
  0 siblings, 0 replies; 7+ messages in thread
From: Tiezhu Yang @ 2023-08-30  2:47 UTC (permalink / raw)
  To: Huacai Chen; +Cc: loongarch, linux-kernel, loongson-kernel

The initial aim is to silence the following objtool warnings:

  arch/loongarch/kernel/fpu.o: warning: objtool: _save_fp_context() falls through to next function fault()
  arch/loongarch/kernel/fpu.o: warning: objtool: _restore_fp_context() falls through to next function fault()
  arch/loongarch/kernel/fpu.o: warning: objtool: _save_lsx_context() falls through to next function fault()
  arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lsx_context() falls through to next function fault()
  arch/loongarch/kernel/fpu.o: warning: objtool: _save_lasx_context() falls through to next function fault()
  arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lasx_context() falls through to next function fault()

Currently, SYM_FUNC_END() defines the symbol fault as SYM_T_FUNC which
is STT_FUNC, the objtool warnings are generated through the following
code:

tools/objtool/include/objtool/check.h
static inline struct symbol *insn_func(struct instruction *insn)
{
	struct symbol *sym = insn->sym;

	if (sym && sym->type != STT_FUNC)
		sym = NULL;

	return sym;
}

tools/objtool/check.c
static int validate_branch(struct objtool_file *file, struct symbol *func,
			   struct instruction *insn, struct insn_state state)
{
...
		if (func && insn_func(insn) && func != insn_func(insn)->pfunc) {
...
			WARN("%s() falls through to next function %s()",
			     func->name, insn_func(insn)->name);
			return 1;
		}
...
}

We can see that the fixup can be a local label in the following code:

arch/loongarch/include/asm/asm-extable.h
	.pushsection	__ex_table, "a";		\
	.balign		4;				\
	.long		((insn) - .);			\
	.long		((fixup) - .);			\
	.short		(type);				\
	.short		(data);				\
	.popsection;

	.macro		_asm_extable, insn, fixup
	__ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0)
	.endm

Like arch/loongarch/lib/*.S, just define the symbol fault as a local
label in fpu.S.

Before:

$ readelf -s arch/loongarch/kernel/fpu.o | awk -F: /fault/'{print $2}'
 000000000000053c     8 FUNC    GLOBAL DEFAULT    1 fault

After:

$ readelf -s arch/loongarch/kernel/fpu.o | awk -F: /fault/'{print $2}'
 000000000000053c     0 NOTYPE  LOCAL  DEFAULT    1 .L_fixup_fpu_fault

Co-developed-by: Youling Tang <tangyouling@loongson.cn>
Signed-off-by: Youling Tang <tangyouling@loongson.cn>
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---

There is a same issue in lbt.S, Huacai suggested to squash the changes
in the original patch:
"LoongArch: Add Loongson Binary Translation (LBT) extension support"

 arch/loongarch/kernel/fpu.S | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
index b4032de..3177725 100644
--- a/arch/loongarch/kernel/fpu.S
+++ b/arch/loongarch/kernel/fpu.S
@@ -22,7 +22,7 @@
 
 	.macro	EX insn, reg, src, offs
 .ex\@:	\insn	\reg, \src, \offs
-	_asm_extable .ex\@, fault
+	_asm_extable .ex\@, .L_fixup_fpu_fault
 	.endm
 
 	.macro sc_save_fp base
@@ -521,7 +521,6 @@ SYM_FUNC_START(_restore_lasx_context)
 	jr	ra
 SYM_FUNC_END(_restore_lasx_context)
 
-SYM_FUNC_START(fault)
+.L_fixup_fpu_fault:
 	li.w	a0, -EFAULT				# failure
 	jr	ra
-SYM_FUNC_END(fault)
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] LoongArch: Define the symbol fault as a local label in fpu.S
  2023-08-29 12:40   ` Tiezhu Yang
@ 2023-08-29 12:47     ` Huacai Chen
  0 siblings, 0 replies; 7+ messages in thread
From: Huacai Chen @ 2023-08-29 12:47 UTC (permalink / raw)
  To: Tiezhu Yang; +Cc: Xi Ruoyao, loongarch, linux-kernel, loongson-kernel

On Tue, Aug 29, 2023 at 8:40 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
>
>
> On 08/29/2023 02:45 PM, Xi Ruoyao wrote:
> > On Tue, 2023-08-29 at 14:28 +0800, Tiezhu Yang wrote:
> >> The initial aim is to silence the following objtool warnings:
> >>
> >>   arch/loongarch/kernel/fpu.o: warning: objtool: _save_fp_context() falls through to next function fault()
> >>   arch/loongarch/kernel/fpu.o: warning: objtool: _restore_fp_context() falls through to next function fault()
> >>   arch/loongarch/kernel/fpu.o: warning: objtool: _save_lsx_context() falls through to next function fault()
> >>   arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lsx_context() falls through to next function fault()
> >>   arch/loongarch/kernel/fpu.o: warning: objtool: _save_lasx_context() falls through to next function fault()
> >>   arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lasx_context() falls through to next function fault()
> >>
> >> Obviously, the symbol fault is not a function, it is just a local label,
> >
> > Hmm why?  To me this seems a function.  We don't branch to it but store
> > its address (a "function pointer") in the extable.
> >
> > And these warnings do not make any sense to me:
> >
> > /* snip */
> >
> >> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
> >> index b4032de..7defe50 100644
> >> --- a/arch/loongarch/kernel/fpu.S
> >> +++ b/arch/loongarch/kernel/fpu.S
> >> @@ -521,7 +521,7 @@ SYM_FUNC_START(_restore_lasx_context)
> >>         jr      ra
> >
> > _restore_lasx_context returns with this instruction.  How can it fall
> > through into fault?
> >
> >>  SYM_FUNC_END(_restore_lasx_context)
> >
> >> -SYM_FUNC_START(fault)
> >> +SYM_CODE_START_LOCAL(fault)
> >>         li.w    a0, -EFAULT                             # failure
> >>         jr      ra
> >> -SYM_FUNC_END(fault)
> >> +SYM_CODE_END(fault)
> >
>
> In the current code, SYM_FUNC_END() defines the symbol as SYM_T_FUNC
> which is STT_FUNC, the objtool warnings are generated through the
> following code.
>
> arch/loongarch/include/asm/linkage.h
> #define SYM_FUNC_END(name)                              \
>         .cfi_endproc;                                   \
>         SYM_END(name, SYM_T_FUNC)
>
> include/linux/linkage.h
> /* SYM_T_FUNC -- type used by assembler to mark functions */
> #ifndef SYM_T_FUNC
> #define SYM_T_FUNC                              STT_FUNC
> #endif
>
> tools/objtool/include/objtool/check.h
> static inline struct symbol *insn_func(struct instruction *insn)
> {
>         struct symbol *sym = insn->sym;
>
>         if (sym && sym->type != STT_FUNC)
>                 sym = NULL;
>
>         return sym;
> }
>
> tools/objtool/check.c
> static int validate_branch(struct objtool_file *file, struct symbol *func,
>                            struct instruction *insn, struct insn_state state)
> {
> ...
>         while (1) {
>                 next_insn = next_insn_to_validate(file, insn);
>
>                 if (func && insn_func(insn) && func != insn_func(insn)->pfunc) {
>                         /* Ignore KCFI type preambles, which always fall through */
>                         if (!strncmp(func->name, "__cfi_", 6) ||
>                             !strncmp(func->name, "__pfx_", 6))
>                                 return 0;
>
>                         WARN("%s() falls through to next function %s()",
>                              func->name, insn_func(insn)->name);
>                         return 1;
>                 }
> ...
> }
>
> We can see that the fixup can be a local label in the following code.
>
> arch/loongarch/include/asm/asm-extable.h
> #define __ASM_EXTABLE_RAW(insn, fixup, type, data)      \
>         .pushsection    __ex_table, "a";                \
>         .balign         4;                              \
>         .long           ((insn) - .);                   \
>         .long           ((fixup) - .);                  \
>         .short          (type);                         \
>         .short          (data);                         \
>         .popsection;
>
>         .macro          _asm_extable, insn, fixup
>         __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0)
>         .endm
>
> Like arch/loongarch/lib/*.S does, I prefer the following changes,
> if you are ok, I will send v2 later.
>
> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
> index b4032deb8e3b..3177725ea832 100644
> --- a/arch/loongarch/kernel/fpu.S
> +++ b/arch/loongarch/kernel/fpu.S
> @@ -22,7 +22,7 @@
>
>          .macro  EX insn, reg, src, offs
>   .ex\@: \insn   \reg, \src, \offs
> -       _asm_extable .ex\@, fault
> +       _asm_extable .ex\@, .L_fixup_fpu_fault
>          .endm
>
>          .macro sc_save_fp base
> @@ -521,7 +521,6 @@ SYM_FUNC_START(_restore_lasx_context)
>          jr      ra
>   SYM_FUNC_END(_restore_lasx_context)
>
> -SYM_FUNC_START(fault)
> +.L_fixup_fpu_fault:
>          li.w    a0, -EFAULT                             # failure
>          jr      ra
> -SYM_FUNC_END(fault)
You can only fix the fpu.S, lbt.S part can be squashed to its own patch.

Huacai

> diff --git a/arch/loongarch/kernel/lbt.S b/arch/loongarch/kernel/lbt.S
> index ecf08bbff650..402eb8ec4024 100644
> --- a/arch/loongarch/kernel/lbt.S
> +++ b/arch/loongarch/kernel/lbt.S
> @@ -16,7 +16,7 @@
>
>          .macro  EX insn, reg, src, offs
>   .ex\@: \insn   \reg, \src, \offs
> -       _asm_extable .ex\@, lbt_fault
> +       _asm_extable .ex\@, .L_fixup_lbt_fault
>          .endm
>
>   /*
> @@ -150,7 +150,6 @@ SYM_FUNC_START(_restore_ftop_context)
>          jr              ra
>   SYM_FUNC_END(_restore_ftop_context)
>
> -SYM_FUNC_START(lbt_fault)
> +.L_fixup_lbt_fault:
>          li.w            a0, -EFAULT             # failure
>          jr              ra
> -SYM_FUNC_END(lbt_fault)
>
> Thanks,
> Tiezhu
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] LoongArch: Define the symbol fault as a local label in fpu.S
  2023-08-29  6:45 ` Xi Ruoyao
  2023-08-29  8:46   ` Jinyang He
@ 2023-08-29 12:40   ` Tiezhu Yang
  2023-08-29 12:47     ` Huacai Chen
  1 sibling, 1 reply; 7+ messages in thread
From: Tiezhu Yang @ 2023-08-29 12:40 UTC (permalink / raw)
  To: Xi Ruoyao, Huacai Chen; +Cc: loongarch, linux-kernel, loongson-kernel



On 08/29/2023 02:45 PM, Xi Ruoyao wrote:
> On Tue, 2023-08-29 at 14:28 +0800, Tiezhu Yang wrote:
>> The initial aim is to silence the following objtool warnings:
>>
>>   arch/loongarch/kernel/fpu.o: warning: objtool: _save_fp_context() falls through to next function fault()
>>   arch/loongarch/kernel/fpu.o: warning: objtool: _restore_fp_context() falls through to next function fault()
>>   arch/loongarch/kernel/fpu.o: warning: objtool: _save_lsx_context() falls through to next function fault()
>>   arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lsx_context() falls through to next function fault()
>>   arch/loongarch/kernel/fpu.o: warning: objtool: _save_lasx_context() falls through to next function fault()
>>   arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lasx_context() falls through to next function fault()
>>
>> Obviously, the symbol fault is not a function, it is just a local label,
>
> Hmm why?  To me this seems a function.  We don't branch to it but store
> its address (a "function pointer") in the extable.
>
> And these warnings do not make any sense to me:
>
> /* snip */
>
>> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
>> index b4032de..7defe50 100644
>> --- a/arch/loongarch/kernel/fpu.S
>> +++ b/arch/loongarch/kernel/fpu.S
>> @@ -521,7 +521,7 @@ SYM_FUNC_START(_restore_lasx_context)
>>         jr      ra
>
> _restore_lasx_context returns with this instruction.  How can it fall
> through into fault?
>
>>  SYM_FUNC_END(_restore_lasx_context)
>
>> -SYM_FUNC_START(fault)
>> +SYM_CODE_START_LOCAL(fault)
>>         li.w    a0, -EFAULT                             # failure
>>         jr      ra
>> -SYM_FUNC_END(fault)
>> +SYM_CODE_END(fault)
>

In the current code, SYM_FUNC_END() defines the symbol as SYM_T_FUNC
which is STT_FUNC, the objtool warnings are generated through the
following code.

arch/loongarch/include/asm/linkage.h
#define SYM_FUNC_END(name)				\
	.cfi_endproc;					\
	SYM_END(name, SYM_T_FUNC)

include/linux/linkage.h
/* SYM_T_FUNC -- type used by assembler to mark functions */
#ifndef SYM_T_FUNC
#define SYM_T_FUNC				STT_FUNC
#endif

tools/objtool/include/objtool/check.h
static inline struct symbol *insn_func(struct instruction *insn)
{
	struct symbol *sym = insn->sym;

	if (sym && sym->type != STT_FUNC)
		sym = NULL;

	return sym;
}

tools/objtool/check.c
static int validate_branch(struct objtool_file *file, struct symbol *func,
			   struct instruction *insn, struct insn_state state)
{
...
	while (1) {
		next_insn = next_insn_to_validate(file, insn);

		if (func && insn_func(insn) && func != insn_func(insn)->pfunc) {
			/* Ignore KCFI type preambles, which always fall through */
			if (!strncmp(func->name, "__cfi_", 6) ||
			    !strncmp(func->name, "__pfx_", 6))
				return 0;

			WARN("%s() falls through to next function %s()",
			     func->name, insn_func(insn)->name);
			return 1;
		}
...
}

We can see that the fixup can be a local label in the following code.

arch/loongarch/include/asm/asm-extable.h
#define __ASM_EXTABLE_RAW(insn, fixup, type, data)	\
	.pushsection	__ex_table, "a";		\
	.balign		4;				\
	.long		((insn) - .);			\
	.long		((fixup) - .);			\
	.short		(type);				\
	.short		(data);				\
	.popsection;

	.macro		_asm_extable, insn, fixup
	__ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0)
	.endm

Like arch/loongarch/lib/*.S does, I prefer the following changes,
if you are ok, I will send v2 later.

diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
index b4032deb8e3b..3177725ea832 100644
--- a/arch/loongarch/kernel/fpu.S
+++ b/arch/loongarch/kernel/fpu.S
@@ -22,7 +22,7 @@

         .macro  EX insn, reg, src, offs
  .ex\@: \insn   \reg, \src, \offs
-       _asm_extable .ex\@, fault
+       _asm_extable .ex\@, .L_fixup_fpu_fault
         .endm

         .macro sc_save_fp base
@@ -521,7 +521,6 @@ SYM_FUNC_START(_restore_lasx_context)
         jr      ra
  SYM_FUNC_END(_restore_lasx_context)

-SYM_FUNC_START(fault)
+.L_fixup_fpu_fault:
         li.w    a0, -EFAULT                             # failure
         jr      ra
-SYM_FUNC_END(fault)
diff --git a/arch/loongarch/kernel/lbt.S b/arch/loongarch/kernel/lbt.S
index ecf08bbff650..402eb8ec4024 100644
--- a/arch/loongarch/kernel/lbt.S
+++ b/arch/loongarch/kernel/lbt.S
@@ -16,7 +16,7 @@

         .macro  EX insn, reg, src, offs
  .ex\@: \insn   \reg, \src, \offs
-       _asm_extable .ex\@, lbt_fault
+       _asm_extable .ex\@, .L_fixup_lbt_fault
         .endm

  /*
@@ -150,7 +150,6 @@ SYM_FUNC_START(_restore_ftop_context)
         jr              ra
  SYM_FUNC_END(_restore_ftop_context)

-SYM_FUNC_START(lbt_fault)
+.L_fixup_lbt_fault:
         li.w            a0, -EFAULT             # failure
         jr              ra
-SYM_FUNC_END(lbt_fault)

Thanks,
Tiezhu


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] LoongArch: Define the symbol fault as a local label in fpu.S
  2023-08-29  6:45 ` Xi Ruoyao
@ 2023-08-29  8:46   ` Jinyang He
  2023-08-29 12:40   ` Tiezhu Yang
  1 sibling, 0 replies; 7+ messages in thread
From: Jinyang He @ 2023-08-29  8:46 UTC (permalink / raw)
  To: Xi Ruoyao, Tiezhu Yang, Huacai Chen
  Cc: loongarch, linux-kernel, loongson-kernel

On 2023-08-29 14:45, Xi Ruoyao wrote:

> On Tue, 2023-08-29 at 14:28 +0800, Tiezhu Yang wrote:
>> The initial aim is to silence the following objtool warnings:
>>
>>    arch/loongarch/kernel/fpu.o: warning: objtool: _save_fp_context() falls through to next function fault()
>>    arch/loongarch/kernel/fpu.o: warning: objtool: _restore_fp_context() falls through to next function fault()
>>    arch/loongarch/kernel/fpu.o: warning: objtool: _save_lsx_context() falls through to next function fault()
>>    arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lsx_context() falls through to next function fault()
>>    arch/loongarch/kernel/fpu.o: warning: objtool: _save_lasx_context() falls through to next function fault()
>>    arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lasx_context() falls through to next function fault()
>>
>> Obviously, the symbol fault is not a function, it is just a local label,
> Hmm why?  To me this seems a function.  We don't branch to it but store
> its address (a "function pointer") in the extable.
>
> And these warnings do not make any sense to me:
>
> /* snip */
>
>> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
>> index b4032de..7defe50 100644
>> --- a/arch/loongarch/kernel/fpu.S
>> +++ b/arch/loongarch/kernel/fpu.S
>> @@ -521,7 +521,7 @@ SYM_FUNC_START(_restore_lasx_context)
>>          jr      ra
> _restore_lasx_context returns with this instruction.  How can it fall
> through into fault?

I think it is because objtool will check ex_table. Something special 
here. (in special.c)

So this function control flow reaches another global function, caused 
warning. (Just some impressions)


Thanks,

Jinyang

>
>>   SYM_FUNC_END(_restore_lasx_context)
>> -SYM_FUNC_START(fault)
>> +SYM_CODE_START_LOCAL(fault)
>>          li.w    a0, -EFAULT                             # failure
>>          jr      ra
>> -SYM_FUNC_END(fault)
>> +SYM_CODE_END(fault)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] LoongArch: Define the symbol fault as a local label in fpu.S
  2023-08-29  6:28 Tiezhu Yang
  2023-08-29  6:45 ` Xi Ruoyao
@ 2023-08-29  8:33 ` Jinyang He
  1 sibling, 0 replies; 7+ messages in thread
From: Jinyang He @ 2023-08-29  8:33 UTC (permalink / raw)
  To: Tiezhu Yang, Huacai Chen; +Cc: loongarch, linux-kernel, loongson-kernel

On 2023-08-29 14:28, Tiezhu Yang wrote:

> The initial aim is to silence the following objtool warnings:
>
>    arch/loongarch/kernel/fpu.o: warning: objtool: _save_fp_context() falls through to next function fault()
>    arch/loongarch/kernel/fpu.o: warning: objtool: _restore_fp_context() falls through to next function fault()
>    arch/loongarch/kernel/fpu.o: warning: objtool: _save_lsx_context() falls through to next function fault()
>    arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lsx_context() falls through to next function fault()
>    arch/loongarch/kernel/fpu.o: warning: objtool: _save_lasx_context() falls through to next function fault()
>    arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lasx_context() falls through to next function fault()
>
> Obviously, the symbol fault is not a function, it is just a local label,
> so use SYM_CODE_START_LOCAL and SYM_CODE_END to define the symbol fault.
>
> Before:
>
> $ readelf -s arch/loongarch/kernel/fpu.o | awk -F: /fault/'{print $2}'
>   000000000000053c     8 FUNC    GLOBAL DEFAULT    1 fault
>
> After:
>
> $ readelf -s arch/loongarch/kernel/fpu.o | awk -F: /fault/'{print $2}'
>   000000000000053c     8 NOTYPE  LOCAL  DEFAULT    1 fault
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>   arch/loongarch/kernel/fpu.S | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
> index b4032de..7defe50 100644
> --- a/arch/loongarch/kernel/fpu.S
> +++ b/arch/loongarch/kernel/fpu.S
> @@ -521,7 +521,7 @@ SYM_FUNC_START(_restore_lasx_context)
>   	jr	ra
>   SYM_FUNC_END(_restore_lasx_context)
>   
> -SYM_FUNC_START(fault)
> +SYM_CODE_START_LOCAL(fault)


Hi, Tiezhu,

The include/linux/linkage.h noted,
  * FUNC -- C-like functions (proper stack frame etc.)
  * CODE -- non-C code (e.g. irq handlers with different, special stack 
etc.)
I think here should be SYM_FUNC_START_LOCAL. Thus, it may create debug
info like sp_offset == 0 && ra == CFI_UNDEFINED, (just some impressions).

BTW, I've noticed some funcs should be marked as SYM_CODE.., e.g.
handle_syscall, handle_tlb_xxx. If you are sure that changes are
necessary, fix them.

Thanks!

Jinyang


>   	li.w	a0, -EFAULT				# failure
>   	jr	ra
> -SYM_FUNC_END(fault)
> +SYM_CODE_END(fault)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] LoongArch: Define the symbol fault as a local label in fpu.S
  2023-08-29  6:28 Tiezhu Yang
@ 2023-08-29  6:45 ` Xi Ruoyao
  2023-08-29  8:46   ` Jinyang He
  2023-08-29 12:40   ` Tiezhu Yang
  2023-08-29  8:33 ` Jinyang He
  1 sibling, 2 replies; 7+ messages in thread
From: Xi Ruoyao @ 2023-08-29  6:45 UTC (permalink / raw)
  To: Tiezhu Yang, Huacai Chen; +Cc: loongarch, linux-kernel, loongson-kernel

On Tue, 2023-08-29 at 14:28 +0800, Tiezhu Yang wrote:
> The initial aim is to silence the following objtool warnings:
> 
>   arch/loongarch/kernel/fpu.o: warning: objtool: _save_fp_context() falls through to next function fault()
>   arch/loongarch/kernel/fpu.o: warning: objtool: _restore_fp_context() falls through to next function fault()
>   arch/loongarch/kernel/fpu.o: warning: objtool: _save_lsx_context() falls through to next function fault()
>   arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lsx_context() falls through to next function fault()
>   arch/loongarch/kernel/fpu.o: warning: objtool: _save_lasx_context() falls through to next function fault()
>   arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lasx_context() falls through to next function fault()
> 
> Obviously, the symbol fault is not a function, it is just a local label,

Hmm why?  To me this seems a function.  We don't branch to it but store
its address (a "function pointer") in the extable.

And these warnings do not make any sense to me:

/* snip */

> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
> index b4032de..7defe50 100644
> --- a/arch/loongarch/kernel/fpu.S
> +++ b/arch/loongarch/kernel/fpu.S
> @@ -521,7 +521,7 @@ SYM_FUNC_START(_restore_lasx_context)
>         jr      ra

_restore_lasx_context returns with this instruction.  How can it fall
through into fault?

>  SYM_FUNC_END(_restore_lasx_context)

> -SYM_FUNC_START(fault)
> +SYM_CODE_START_LOCAL(fault)
>         li.w    a0, -EFAULT                             # failure
>         jr      ra
> -SYM_FUNC_END(fault)
> +SYM_CODE_END(fault)

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] LoongArch: Define the symbol fault as a local label in fpu.S
@ 2023-08-29  6:28 Tiezhu Yang
  2023-08-29  6:45 ` Xi Ruoyao
  2023-08-29  8:33 ` Jinyang He
  0 siblings, 2 replies; 7+ messages in thread
From: Tiezhu Yang @ 2023-08-29  6:28 UTC (permalink / raw)
  To: Huacai Chen; +Cc: loongarch, linux-kernel, loongson-kernel

The initial aim is to silence the following objtool warnings:

  arch/loongarch/kernel/fpu.o: warning: objtool: _save_fp_context() falls through to next function fault()
  arch/loongarch/kernel/fpu.o: warning: objtool: _restore_fp_context() falls through to next function fault()
  arch/loongarch/kernel/fpu.o: warning: objtool: _save_lsx_context() falls through to next function fault()
  arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lsx_context() falls through to next function fault()
  arch/loongarch/kernel/fpu.o: warning: objtool: _save_lasx_context() falls through to next function fault()
  arch/loongarch/kernel/fpu.o: warning: objtool: _restore_lasx_context() falls through to next function fault()

Obviously, the symbol fault is not a function, it is just a local label,
so use SYM_CODE_START_LOCAL and SYM_CODE_END to define the symbol fault.

Before:

$ readelf -s arch/loongarch/kernel/fpu.o | awk -F: /fault/'{print $2}'
 000000000000053c     8 FUNC    GLOBAL DEFAULT    1 fault

After:

$ readelf -s arch/loongarch/kernel/fpu.o | awk -F: /fault/'{print $2}'
 000000000000053c     8 NOTYPE  LOCAL  DEFAULT    1 fault

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/kernel/fpu.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
index b4032de..7defe50 100644
--- a/arch/loongarch/kernel/fpu.S
+++ b/arch/loongarch/kernel/fpu.S
@@ -521,7 +521,7 @@ SYM_FUNC_START(_restore_lasx_context)
 	jr	ra
 SYM_FUNC_END(_restore_lasx_context)
 
-SYM_FUNC_START(fault)
+SYM_CODE_START_LOCAL(fault)
 	li.w	a0, -EFAULT				# failure
 	jr	ra
-SYM_FUNC_END(fault)
+SYM_CODE_END(fault)
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-08-30  2:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-30  2:47 [PATCH] LoongArch: Define the symbol fault as a local label in fpu.S Tiezhu Yang
  -- strict thread matches above, loose matches on Subject: below --
2023-08-29  6:28 Tiezhu Yang
2023-08-29  6:45 ` Xi Ruoyao
2023-08-29  8:46   ` Jinyang He
2023-08-29 12:40   ` Tiezhu Yang
2023-08-29 12:47     ` Huacai Chen
2023-08-29  8:33 ` Jinyang He

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