| Message ID | 20220426083259.526685-1-starzhangzsd@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arch/mips/kernel/traps: add CONFIG_MIPS_FP_SUPPORT when using handle_fpe | expand |
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 <macro@orcam.me.uk> 于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
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 <macro@orcam.me.uk> 于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.
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 extern asmlinkage void handle_ftlb(void); extern asmlinkage void handle_gsexc(void); extern asmlinkage void handle_msa(void); @@ -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 if (cpu_has_ftlbparex) set_except_vector(MIPS_EXCCODE_TLBPAR, handle_ftlb);