On Tue, 26 Apr 2022, Stephen Zhang wrote:
> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> index 246c6a6b0261..ef9792261f91 100644
> --- a/arch/mips/kernel/traps.c
> +++ b/arch/mips/kernel/traps.c
> @@ -90,7 +90,9 @@ extern asmlinkage void handle_cpu(void);
> extern asmlinkage void handle_ov(void);
> extern asmlinkage void handle_tr(void);
> extern asmlinkage void handle_msa_fpe(void);
> +#ifdef CONFIG_MIPS_FP_SUPPORT
> extern asmlinkage void handle_fpe(void);
> +#endif
No need to conditionalise declarations ever.
> @@ -2489,8 +2491,10 @@ void __init trap_init(void)
> if (board_nmi_handler_setup)
> board_nmi_handler_setup();
>
> +#ifdef CONFIG_MIPS_FP_SUPPORT
> if (cpu_has_fpu && !cpu_has_nofpuex)
> set_except_vector(EXCCODE_FPE, handle_fpe);
> +#endif
No need to conditionalise this either, because `cpu_has_fpu' is forced 0
(in arch/mips/include/asm/cpu-features.h) if !CONFIG_MIPS_FP_SUPPORT. So
this code translates to:
if (0 && !0)
set_except_vector(15, handle_fpe);
in the preprocessor if CONFIG_MIPS_FP_SUPPORT is unset and is optimised
away. Otherwise it should be written as:
if (IS_ENABLED(CONFIG_MIPS_FP_SUPPORT) && ...
so as not to clutter C code with #ifdef, as per our coding style.
Maciej
Maciej W. Rozycki <[email protected]> 于2022年4月27日周三 08:40写道：
>
> No need to conditionalise this either, because `cpu_has_fpu' is forced 0
> (in arch/mips/include/asm/cpu-features.h) if !CONFIG_MIPS_FP_SUPPORT. So
> this code translates to:
>
> if (0 && !0)
> set_except_vector(15, handle_fpe);
>
> in the preprocessor if CONFIG_MIPS_FP_SUPPORT is unset and is optimised
> away. Otherwise it should be written as:
>
> if (IS_ENABLED(CONFIG_MIPS_FP_SUPPORT) && ...
>
> so as not to clutter C code with #ifdef, as per our coding style.
>
> Maciej
Thanks for your comment. Do you mean the following code:
if (0 && !0)
set_except_vector(15, handle_fpe);
will be optimised away if !CONFIG_MIPS_FP_SUPPORT?
But we did get “undefined reference to `handle_fpe” error when compiled with
!CONFIG_MIPS_FP_SUPPORT.
On Wed, 27 Apr 2022, Stephen Zhang wrote:
> Thanks for your comment. Do you mean the following code:
>
> if (0 && !0)
> set_except_vector(15, handle_fpe);
>
> will be optimised away if !CONFIG_MIPS_FP_SUPPORT?
Yes. Or more specifically the LHS of the conditional expression will be
0 then, as shown above, and the whole statement will be gone.
> But we did get “undefined reference to `handle_fpe” error when compiled with
> !CONFIG_MIPS_FP_SUPPORT.
Please send me .config causing it and tell me what compiler and version
you have seen this error with. We rely on things being optimised away
heavily throughout the Linux kernel, so this is certainly something to
investigate. I have built such a config just fine, but maybe there's a
bug somewhere my setup does not trigger.
Maciej
Maciej W. Rozycki <[email protected]> 于2022年4月27日周三 18:57写道：
> Please send me .config causing it and tell me what compiler and version
> you have seen this error with. We rely on things being optimised away
> heavily throughout the Linux kernel, so this is certainly something to
> investigate. I have built such a config just fine, but maybe there's a
> bug somewhere my setup does not trigger.
>
> Maciej
Okay. The compiler we used is:
Compiler gcc
Compiler version 10
Compiler string mips-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110
Cross-compile mips-linux-gnu
the commit id of kernel is c00c5e1d157bec0ef0b0b59aa5482eb8dc7e8e49
and the .config file is sent as an attachment.
On Thu, 28 Apr 2022, Stephen Zhang wrote:
> > Please send me .config causing it and tell me what compiler and version
> > you have seen this error with. We rely on things being optimised away
> > heavily throughout the Linux kernel, so this is certainly something to
> > investigate. I have built such a config just fine, but maybe there's a
> > bug somewhere my setup does not trigger.
>
> Okay. The compiler we used is:
>
> Compiler gcc
> Compiler version 10
> Compiler string mips-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110
> Cross-compile mips-linux-gnu
>
> the commit id of kernel is c00c5e1d157bec0ef0b0b59aa5482eb8dc7e8e49
>
> and the .config file is sent as an attachment.
Thanks.
The bug is in arch/mips/include/asm/mach-ip27/cpu-feature-overrides.h,
which has:
#define cpu_has_fpu 1
(and similarly arch/mips/include/asm/mach-ip30/cpu-feature-overrides.h).
This is not supported, as noted in arch/mips/include/asm/cpu-features.h:
/* Don't override `cpu_has_fpu' to 1 or the "nofpu" option won't work. */
Perhaps we should explicitly undefine `cpu_has_fpu' if set to 1?
Maciej
Maciej W. Rozycki <[email protected]> 于2022年4月28日周四 17:00写道：
>
> On Thu, 28 Apr 2022, Stephen Zhang wrote:
> Thanks.
>
> The bug is in arch/mips/include/asm/mach-ip27/cpu-feature-overrides.h,
> which has:
>
> #define cpu_has_fpu 1
>
> (and similarly arch/mips/include/asm/mach-ip30/cpu-feature-overrides.h).
> This is not supported, as noted in arch/mips/include/asm/cpu-features.h:
>
> /* Don't override `cpu_has_fpu' to 1 or the "nofpu" option won't work. */
>
> Perhaps we should explicitly undefine `cpu_has_fpu' if set to 1?
>
> Maciej
Thanks for your detailed explanation and suggestion. I will make a v2 patch.