* [PATCH v2] MIPS: undefine and redefine cpu_has_fpu when it is overrided
@ 2022-04-29 3:26 Stephen Zhang
2022-04-29 9:19 ` Maciej W. Rozycki
2022-04-29 9:51 ` Thomas Bogendoerfer
0 siblings, 2 replies; 8+ messages in thread
From: Stephen Zhang @ 2022-04-29 3:26 UTC (permalink / raw)
To: tsbogend, liam.howlett, ebiederm, dbueso, alobakin, f.fainelli,
paul, linux, anemo
Cc: zhangshida, starzhangzsd, linux-kernel, linux-mips, Maciej W . Rozycki
From: Shida Zhang <zhangshida@kylinos.cn>
Undefine and redefine cpu_has_fpu to 0 when it is overridden with
the "nofpu" option.
Suggested-by: Maciej W. Rozycki <macro@orcam.me.uk>
Signed-off-by: Shida Zhang <zhangshida@kylinos.cn>
---
Changelog in v1 -> v2:
- Choose to redefine cpu_has_fpu to solve the problem.
arch/mips/include/asm/cpu-features.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
index de8cb2ccb781..38eb469008b6 100644
--- a/arch/mips/include/asm/cpu-features.h
+++ b/arch/mips/include/asm/cpu-features.h
@@ -134,6 +134,10 @@
# endif
#else
# define raw_cpu_has_fpu cpu_has_fpu
+# ifndef CONFIG_MIPS_FP_SUPPORT
+# undef cpu_has_fpu
+# define cpu_has_fpu 0
+# endif
#endif
#ifndef cpu_has_32fpr
#define cpu_has_32fpr __isa_ge_or_opt(1, MIPS_CPU_32FPR)
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] MIPS: undefine and redefine cpu_has_fpu when it is overrided
2022-04-29 3:26 [PATCH v2] MIPS: undefine and redefine cpu_has_fpu when it is overrided Stephen Zhang
@ 2022-04-29 9:19 ` Maciej W. Rozycki
2022-04-29 9:51 ` Thomas Bogendoerfer
1 sibling, 0 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2022-04-29 9:19 UTC (permalink / raw)
To: Stephen Zhang
Cc: Thomas Bogendoerfer, liam.howlett, ebiederm, dbueso, alobakin,
f.fainelli, paul, linux, anemo, zhangshida, linux-kernel,
linux-mips
On Fri, 29 Apr 2022, Stephen Zhang wrote:
> Undefine and redefine cpu_has_fpu to 0 when it is overridden with
> the "nofpu" option.
Umm, `nofpu' is a kernel parameter, not the CONFIG_MIPS_FP_SUPPORT config
option; cf. Documentation/admin-guide/kernel-parameters.txt.
Maciej
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] MIPS: undefine and redefine cpu_has_fpu when it is overrided
2022-04-29 3:26 [PATCH v2] MIPS: undefine and redefine cpu_has_fpu when it is overrided Stephen Zhang
2022-04-29 9:19 ` Maciej W. Rozycki
@ 2022-04-29 9:51 ` Thomas Bogendoerfer
2022-04-29 15:11 ` Maciej W. Rozycki
1 sibling, 1 reply; 8+ messages in thread
From: Thomas Bogendoerfer @ 2022-04-29 9:51 UTC (permalink / raw)
To: Stephen Zhang
Cc: liam.howlett, ebiederm, dbueso, alobakin, f.fainelli, paul,
linux, anemo, zhangshida, linux-kernel, linux-mips,
Maciej W . Rozycki
On Fri, Apr 29, 2022 at 11:26:21AM +0800, Stephen Zhang wrote:
> From: Shida Zhang <zhangshida@kylinos.cn>
>
> Undefine and redefine cpu_has_fpu to 0 when it is overridden with
> the "nofpu" option.
>
> Suggested-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Signed-off-by: Shida Zhang <zhangshida@kylinos.cn>
> ---
> Changelog in v1 -> v2:
> - Choose to redefine cpu_has_fpu to solve the problem.
>
> arch/mips/include/asm/cpu-features.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
> index de8cb2ccb781..38eb469008b6 100644
> --- a/arch/mips/include/asm/cpu-features.h
> +++ b/arch/mips/include/asm/cpu-features.h
> @@ -134,6 +134,10 @@
> # endif
> #else
> # define raw_cpu_has_fpu cpu_has_fpu
> +# ifndef CONFIG_MIPS_FP_SUPPORT
> +# undef cpu_has_fpu
> +# define cpu_has_fpu 0
> +# endif
> #endif
> #ifndef cpu_has_32fpr
> #define cpu_has_32fpr __isa_ge_or_opt(1, MIPS_CPU_32FPR)
> --
> 2.30.2
I prefer just removing the #defines from ip27/ip30 cpu-feasture-overrides.h.
Or isn't that enough for fixing the problem ?
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] MIPS: undefine and redefine cpu_has_fpu when it is overrided
2022-04-29 9:51 ` Thomas Bogendoerfer
@ 2022-04-29 15:11 ` Maciej W. Rozycki
2022-04-30 3:36 ` Stephen Zhang
0 siblings, 1 reply; 8+ messages in thread
From: Maciej W. Rozycki @ 2022-04-29 15:11 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Stephen Zhang, liam.howlett, ebiederm, alobakin, f.fainelli,
paul, linux, anemo, zhangshida, linux-kernel, linux-mips
On Fri, 29 Apr 2022, Thomas Bogendoerfer wrote:
> > diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
> > index de8cb2ccb781..38eb469008b6 100644
> > --- a/arch/mips/include/asm/cpu-features.h
> > +++ b/arch/mips/include/asm/cpu-features.h
> > @@ -134,6 +134,10 @@
> > # endif
> > #else
> > # define raw_cpu_has_fpu cpu_has_fpu
> > +# ifndef CONFIG_MIPS_FP_SUPPORT
> > +# undef cpu_has_fpu
> > +# define cpu_has_fpu 0
> > +# endif
> > #endif
> > #ifndef cpu_has_32fpr
> > #define cpu_has_32fpr __isa_ge_or_opt(1, MIPS_CPU_32FPR)
> > --
> > 2.30.2
>
> I prefer just removing the #defines from ip27/ip30 cpu-feasture-overrides.h.
> Or isn't that enough for fixing the problem ?
That's what I've meant, and I have now posted fixes, successfully
build-tested.
Additionally I've thought of adding something like:
#if cpu_has_fpu
# undef cpu_has_fpu
#endif
or maybe even:
#if cpu_has_fpu
# error "Forcing `cpu_has_fpu' to non-zero is not supported"
#endif
to arch/mips/include/asm/cpu-features.h, but maybe that's an overkill.
Maciej
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] MIPS: undefine and redefine cpu_has_fpu when it is overrided
2022-04-29 15:11 ` Maciej W. Rozycki
@ 2022-04-30 3:36 ` Stephen Zhang
2022-04-30 15:38 ` Maciej W. Rozycki
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Zhang @ 2022-04-30 3:36 UTC (permalink / raw)
To: Maciej W. Rozycki, Thomas Bogendoerfer
Cc: liam.howlett, ebiederm, alobakin, f.fainelli, paul, linux, anemo,
zhangshida, linux-kernel, linux-mips
Maciej W. Rozycki <macro@orcam.me.uk> 于2022年4月29日周五 23:11写道:
>
> Additionally I've thought of adding something like:
>
> #if cpu_has_fpu
> # undef cpu_has_fpu
> #endif
>
> or maybe even:
>
> #if cpu_has_fpu
> # error "Forcing `cpu_has_fpu' to non-zero is not supported"
> #endif
>
> to arch/mips/include/asm/cpu-features.h, but maybe that's an overkill.
>
> Maciej
Yeah, but why do you think that's an overkill? There is a great chance
people will ignore the note of 'cpu_has_fpu', and it did happen. When
that happens, there should exist a way to point out or fix that.
Thomas Bogendoerfer <tsbogend@alpha.franken.de> 于2022年4月29日周五 18:01写道:
>
> I prefer just removing the #defines from ip27/ip30 cpu-feasture-overrides.h.
> Or isn't that enough for fixing the problem ?
>
> Thomas.
So maybe that's why I don't think just removing the #defines from
ip27/ip30 cpu-feasture-overrides.h. is enough for fixing the problem.
Stephen.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] MIPS: undefine and redefine cpu_has_fpu when it is overrided
2022-04-30 3:36 ` Stephen Zhang
@ 2022-04-30 15:38 ` Maciej W. Rozycki
2022-05-01 2:54 ` Stephen Zhang
0 siblings, 1 reply; 8+ messages in thread
From: Maciej W. Rozycki @ 2022-04-30 15:38 UTC (permalink / raw)
To: Stephen Zhang
Cc: Thomas Bogendoerfer, liam.howlett, ebiederm, alobakin,
f.fainelli, paul, linux, anemo, zhangshida, linux-kernel,
linux-mips
On Sat, 30 Apr 2022, Stephen Zhang wrote:
> > Additionally I've thought of adding something like:
> >
> > #if cpu_has_fpu
> > # undef cpu_has_fpu
> > #endif
> >
> > or maybe even:
> >
> > #if cpu_has_fpu
> > # error "Forcing `cpu_has_fpu' to non-zero is not supported"
> > #endif
> >
> > to arch/mips/include/asm/cpu-features.h, but maybe that's an overkill.
>
> Yeah, but why do you think that's an overkill? There is a great chance
> people will ignore the note of 'cpu_has_fpu', and it did happen. When
> that happens, there should exist a way to point out or fix that.
Maybe it's the language, but my intent has been to express my uncertainty
here rather than asserting that indeed it is an overkill.
People do make mistakes from time to time, both code writers and
reviewers do. It's not clear to me where to draw the line for safety
checks though.
Here `cpu_has_fpu' is a bit unusual in that unlike with the other feature
override macros we don't want it to expand to a non-zero constant. The
comment didn't work twice, though I suspect both cpu-feature-overrides.h
files may have been written before the comment went in (I'm fairly sure
the IP30 port lived outside the tree for a while). But I have only added
the comment in the first place when I tripped over the `nofpu' option not
working for the machine I needed to run FPU emulator verification with,
and several platforms were fixed alongside.
Given these circumstances it probably makes sense to have such a safety
check after all.
> > I prefer just removing the #defines from ip27/ip30 cpu-feasture-overrides.h.
> > Or isn't that enough for fixing the problem ?
> >
> > Thomas.
>
> So maybe that's why I don't think just removing the #defines from
> ip27/ip30 cpu-feasture-overrides.h. is enough for fixing the problem.
Well, that *is* the fix for the problem at hand, as this macro is not
supposed to be defined such as to expand to a non-zero constant.
Adding a safety check would be a separate improvement. Please feel free
to submit one.
We need to keep fixes and improvements as separate changes. For one
fixes can be candidates for backporting while improvements are never
backported; cf. Documentation/process/stable-kernel-rules.rst.
I hope this clears your concerns. Let me know if you have further
questions.
Maciej
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] MIPS: undefine and redefine cpu_has_fpu when it is overrided
2022-04-30 15:38 ` Maciej W. Rozycki
@ 2022-05-01 2:54 ` Stephen Zhang
2022-05-01 11:31 ` Maciej W. Rozycki
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Zhang @ 2022-05-01 2:54 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Thomas Bogendoerfer, liam.howlett, ebiederm, alobakin,
f.fainelli, paul, linux, anemo, zhangshida, linux-kernel,
linux-mips
Maciej W. Rozycki <macro@orcam.me.uk> 于2022年4月30日周六 23:38写道:
>
> Adding a safety check would be a separate improvement. Please feel free
> to submit one.
>
> We need to keep fixes and improvements as separate changes. For one
> fixes can be candidates for backporting while improvements are never
> backported; cf. Documentation/process/stable-kernel-rules.rst.
>
> I hope this clears your concerns. Let me know if you have further
> questions.
>
> Maciej
Thanks for your elaboration.It helps a lot.
I want to submit a v3 patch like:
#if cpu_has_fpu
# error "Forcing `cpu_has_fpu' to non-zero is not supported"
#endif
but this will cause the link error if not combined with the fix:
MIPS: IP30: Remove incorrect `cpu_has_fpu' override
Maybe I should submit one first, and see how it goes then.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] MIPS: undefine and redefine cpu_has_fpu when it is overrided
2022-05-01 2:54 ` Stephen Zhang
@ 2022-05-01 11:31 ` Maciej W. Rozycki
0 siblings, 0 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2022-05-01 11:31 UTC (permalink / raw)
To: Stephen Zhang
Cc: Thomas Bogendoerfer, liam.howlett, ebiederm, alobakin,
f.fainelli, paul, linux, anemo, zhangshida, linux-kernel,
linux-mips
On Sun, 1 May 2022, Stephen Zhang wrote:
> Thanks for your elaboration.It helps a lot.
> I want to submit a v3 patch like:
>
> #if cpu_has_fpu
> # error "Forcing `cpu_has_fpu' to non-zero is not supported"
> #endif
>
> but this will cause the link error if not combined with the fix:
>
> MIPS: IP30: Remove incorrect `cpu_has_fpu' override
>
> Maybe I should submit one first, and see how it goes then.
Let's wait a couple of days until the fixes required have been queued.
Maciej
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-05-01 11:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-29 3:26 [PATCH v2] MIPS: undefine and redefine cpu_has_fpu when it is overrided Stephen Zhang
2022-04-29 9:19 ` Maciej W. Rozycki
2022-04-29 9:51 ` Thomas Bogendoerfer
2022-04-29 15:11 ` Maciej W. Rozycki
2022-04-30 3:36 ` Stephen Zhang
2022-04-30 15:38 ` Maciej W. Rozycki
2022-05-01 2:54 ` Stephen Zhang
2022-05-01 11:31 ` Maciej W. Rozycki
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).