* [PATCH v4] MIPS: introduce config option to force use FR=0 for FPXX binary
@ 2021-02-22 3:43 YunQiang Su
2021-02-23 6:29 ` YunQiang Su
0 siblings, 1 reply; 7+ messages in thread
From: YunQiang Su @ 2021-02-22 3:43 UTC (permalink / raw)
To: tsbogend, jiaxun.yang, macro; +Cc: linux-mips, YunQiang Su, stable
some binary, for example the output of golang, may be mark as FPXX,
while in fact they are still FP32.
Since FPXX binary can work with both FR=1 and FR=0, we introduce a
config option CONFIG_MIPS_O32_FPXX_USE_FR0 to force it to use FR=0 here.
https://go-review.googlesource.com/c/go/+/239217
https://go-review.googlesource.com/c/go/+/237058
v3->v4:
introduce a config option: CONFIG_MIPS_O32_FPXX_USE_FR0
v2->v3:
commit message: add Signed-off-by and Cc to stable.
v1->v2:
Fix bad commit message: in fact, we are switching to FR=0
Signed-off-by: YunQiang Su <yunqiang.su@cipunited.com>
Cc: stable@vger.kernel.org # 4.19+
---
arch/mips/Kconfig | 11 +++++++++++
arch/mips/kernel/elf.c | 13 ++++++++++---
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 0a17bedf4f0d..442db620636f 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -3100,6 +3100,17 @@ config MIPS_O32_FP64_SUPPORT
If unsure, say N.
+config MIPS_O32_FPXX_USE_FR0
+ bool "Use FR=0 mode for O32 FPXX binaries" if !CPU_MIPSR6
+ depends on MIPS_O32_FP64_SUPPORT
+ help
+ O32 FPXX can works on both FR=0 and FR=1 mode, so by default, the
+ mode preferred by hardware is used.
+
+ While some binaries may be marked as FPXX by mistake, for example
+ output of golang: they are in fact FP32 mode. To compatiable with
+ these binaries, we should use FR=0 mode for them.
+
config USE_OF
bool
select OF
diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
index 7b045d2a0b51..443ced26ee60 100644
--- a/arch/mips/kernel/elf.c
+++ b/arch/mips/kernel/elf.c
@@ -234,9 +234,10 @@ int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr,
* fpxx case. This is because, in any-ABI (or no-ABI) we have no FPU
* instructions so we don't care about the mode. We will simply use
* the one preferred by the hardware. In fpxx case, that ABI can
- * handle both FR=1 and FR=0, so, again, we simply choose the one
- * preferred by the hardware. Next, if we only use single-precision
- * FPU instructions, and the default ABI FPU mode is not good
+ * handle both FR=1 and FR=0. Here, we may need to use FR=0, because
+ * some binaries may be mark as FPXX by mistake (ie, output of golang).
+ * - If we only use single-precision FPU instructions,
+ * and the default ABI FPU mode is not good
* (ie single + any ABI combination), we set again the FPU mode to the
* one is preferred by the hardware. Next, if we know that the code
* will only use single-precision instructions, shown by single being
@@ -248,8 +249,14 @@ int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr,
*/
if (prog_req.fre && !prog_req.frdefault && !prog_req.fr1)
state->overall_fp_mode = FP_FRE;
+#if CONFIG_MIPS_O32_FPXX_USE_FR0
+ else if (prog_req.fr1 && prog_req.frdefault)
+ state->overall_fp_mode = FP_FR0;
+ else if (prog_req.single && !prog_req.frdefault)
+#else
else if ((prog_req.fr1 && prog_req.frdefault) ||
(prog_req.single && !prog_req.frdefault))
+#endif
/* Make sure 64-bit MIPS III/IV/64R1 will not pick FR1 */
state->overall_fp_mode = ((raw_current_cpu_data.fpu_id & MIPS_FPIR_F64) &&
cpu_has_mips_r2_r6) ?
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4] MIPS: introduce config option to force use FR=0 for FPXX binary
2021-02-22 3:43 [PATCH v4] MIPS: introduce config option to force use FR=0 for FPXX binary YunQiang Su
@ 2021-02-23 6:29 ` YunQiang Su
2021-02-28 1:47 ` Maciej W. Rozycki
0 siblings, 1 reply; 7+ messages in thread
From: YunQiang Su @ 2021-02-23 6:29 UTC (permalink / raw)
To: YunQiang Su
Cc: Thomas Bogendoerfer, Jiaxun Yang, Maciej W. Rozycki, linux-mips, stable
YunQiang Su <yunqiang.su@cipunited.com> 于2021年2月22日周一 上午11:45写道:
>
> some binary, for example the output of golang, may be mark as FPXX,
> while in fact they are still FP32.
>
> Since FPXX binary can work with both FR=1 and FR=0, we introduce a
> config option CONFIG_MIPS_O32_FPXX_USE_FR0 to force it to use FR=0 here.
As the diffination, .gnu.attribution 4,0 is the same as no
.gnu.attribution section.
Its meaning is that the binary has no float operation at all.
I worry about that if we force 4,0 as 4,1, it may cause some
compatible problems.
>
> https://go-review.googlesource.com/c/go/+/239217
> https://go-review.googlesource.com/c/go/+/237058
>
> v3->v4:
> introduce a config option: CONFIG_MIPS_O32_FPXX_USE_FR0
>
> v2->v3:
> commit message: add Signed-off-by and Cc to stable.
>
> v1->v2:
> Fix bad commit message: in fact, we are switching to FR=0
>
> Signed-off-by: YunQiang Su <yunqiang.su@cipunited.com>
> Cc: stable@vger.kernel.org # 4.19+
> ---
> arch/mips/Kconfig | 11 +++++++++++
> arch/mips/kernel/elf.c | 13 ++++++++++---
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 0a17bedf4f0d..442db620636f 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -3100,6 +3100,17 @@ config MIPS_O32_FP64_SUPPORT
>
> If unsure, say N.
>
> +config MIPS_O32_FPXX_USE_FR0
> + bool "Use FR=0 mode for O32 FPXX binaries" if !CPU_MIPSR6
> + depends on MIPS_O32_FP64_SUPPORT
> + help
> + O32 FPXX can works on both FR=0 and FR=1 mode, so by default, the
> + mode preferred by hardware is used.
> +
> + While some binaries may be marked as FPXX by mistake, for example
> + output of golang: they are in fact FP32 mode. To compatiable with
> + these binaries, we should use FR=0 mode for them.
> +
> config USE_OF
> bool
> select OF
> diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
> index 7b045d2a0b51..443ced26ee60 100644
> --- a/arch/mips/kernel/elf.c
> +++ b/arch/mips/kernel/elf.c
> @@ -234,9 +234,10 @@ int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr,
> * fpxx case. This is because, in any-ABI (or no-ABI) we have no FPU
> * instructions so we don't care about the mode. We will simply use
> * the one preferred by the hardware. In fpxx case, that ABI can
> - * handle both FR=1 and FR=0, so, again, we simply choose the one
> - * preferred by the hardware. Next, if we only use single-precision
> - * FPU instructions, and the default ABI FPU mode is not good
> + * handle both FR=1 and FR=0. Here, we may need to use FR=0, because
> + * some binaries may be mark as FPXX by mistake (ie, output of golang).
> + * - If we only use single-precision FPU instructions,
> + * and the default ABI FPU mode is not good
> * (ie single + any ABI combination), we set again the FPU mode to the
> * one is preferred by the hardware. Next, if we know that the code
> * will only use single-precision instructions, shown by single being
> @@ -248,8 +249,14 @@ int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr,
> */
> if (prog_req.fre && !prog_req.frdefault && !prog_req.fr1)
> state->overall_fp_mode = FP_FRE;
> +#if CONFIG_MIPS_O32_FPXX_USE_FR0
> + else if (prog_req.fr1 && prog_req.frdefault)
> + state->overall_fp_mode = FP_FR0;
> + else if (prog_req.single && !prog_req.frdefault)
> +#else
> else if ((prog_req.fr1 && prog_req.frdefault) ||
> (prog_req.single && !prog_req.frdefault))
> +#endif
> /* Make sure 64-bit MIPS III/IV/64R1 will not pick FR1 */
> state->overall_fp_mode = ((raw_current_cpu_data.fpu_id & MIPS_FPIR_F64) &&
> cpu_has_mips_r2_r6) ?
> --
> 2.20.1
>
--
YunQiang Su
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] MIPS: introduce config option to force use FR=0 for FPXX binary
2021-02-23 6:29 ` YunQiang Su
@ 2021-02-28 1:47 ` Maciej W. Rozycki
2021-02-28 7:40 ` 回复: " yunqiang.su
2021-02-28 12:49 ` Maciej W. Rozycki
0 siblings, 2 replies; 7+ messages in thread
From: Maciej W. Rozycki @ 2021-02-28 1:47 UTC (permalink / raw)
To: YunQiang Su
Cc: YunQiang Su, Thomas Bogendoerfer, Jiaxun Yang, linux-mips, stable
On Tue, 23 Feb 2021, YunQiang Su wrote:
> YunQiang Su <yunqiang.su@cipunited.com> 于2021年2月22日周一 上午11:45写道:
> >
> > some binary, for example the output of golang, may be mark as FPXX,
> > while in fact they are still FP32.
> >
> > Since FPXX binary can work with both FR=1 and FR=0, we introduce a
> > config option CONFIG_MIPS_O32_FPXX_USE_FR0 to force it to use FR=0 here.
>
> As the diffination, .gnu.attribution 4,0 is the same as no
> .gnu.attribution section.
> Its meaning is that the binary has no float operation at all.
Nope, quoting include/elf/mips.h from GNU binutils:
/* Not tagged or not using any ABIs affected by the differences. */
Val_GNU_MIPS_ABI_FP_ANY = 0,
which means that the object file *either* does not use FP *or* has not
been tagged at all, as the GNU linker does not tell these two cases apart
internally (yes, quite useless semantics, but there you go; I think the
enumeration constant of 0 shouldn't have been chosen for any explicit tag,
or possibly for double float instead, so this is an ABI design mistake).
Any object produced before mid 2007, specifically this GNU binutils
commit:
commit 2cf19d5cb991a88bdc71d0852de8206d9510678f
Author: Joseph Myers <joseph@codesourcery.com>
Date: Fri Jun 29 16:41:32 2007 +0000
will not have been tagged for FP use and therefore the traditional
MIPS/Linux psABI of double float has to be assumed.
This is also the correct interpretation for objects produced by Golang,
which I have concluded are actually just fine according to the traditional
psABI definition. It looks to me like the bug is solely in the linker,
due to this weird interpretation quoted above and unforeseen consequences
for FPXX links invented much later.
Yes, the two cases (not tagged vs tagged as 0) ought to be told apart and
I outlined a solution (needed for different reasons, but with the same
motivation) for the GNU linker in the course of the discussion around NaN
interlinking design, but that has never materialised before I effectively
left the MIPS world, only remaining active in a limited manner for legacy
MIPS platforms (that is ISAs I-IV).
Maciej
^ permalink raw reply [flat|nested] 7+ messages in thread
* 回复: [PATCH v4] MIPS: introduce config option to force use FR=0 for FPXX binary
2021-02-28 1:47 ` Maciej W. Rozycki
@ 2021-02-28 7:40 ` yunqiang.su
2021-02-28 13:39 ` Maciej W. Rozycki
2021-02-28 12:49 ` Maciej W. Rozycki
1 sibling, 1 reply; 7+ messages in thread
From: yunqiang.su @ 2021-02-28 7:40 UTC (permalink / raw)
To: 'Maciej W. Rozycki', 'YunQiang Su'
Cc: 'Thomas Bogendoerfer', 'Jiaxun Yang',
'linux-mips',
stable
> -----邮件原件-----
> 发件人: Maciej W. Rozycki <macro@orcam.me.uk>
> 发送时间: 2021年2月28日 9:48
> 收件人: YunQiang Su <wzssyqa@gmail.com>
> 抄送: YunQiang Su <yunqiang.su@cipunited.com>; Thomas Bogendoerfer
> <tsbogend@alpha.franken.de>; Jiaxun Yang <jiaxun.yang@flygoat.com>;
> linux-mips <linux-mips@vger.kernel.org>; stable@vger.kernel.org
> 主题: Re: [PATCH v4] MIPS: introduce config option to force use FR=0 for FPXX
> binary
>
> On Tue, 23 Feb 2021, YunQiang Su wrote:
>
> > YunQiang Su <yunqiang.su@cipunited.com> 于2021年2月22日周一 上
> 午11:45写道:
> > >
> > > some binary, for example the output of golang, may be mark as FPXX,
> > > while in fact they are still FP32.
> > >
> > > Since FPXX binary can work with both FR=1 and FR=0, we introduce a
> > > config option CONFIG_MIPS_O32_FPXX_USE_FR0 to force it to use FR=0
> here.
> >
> > As the diffination, .gnu.attribution 4,0 is the same as no
> > .gnu.attribution section.
> > Its meaning is that the binary has no float operation at all.
>
> Nope, quoting include/elf/mips.h from GNU binutils:
>
> /* Not tagged or not using any ABIs affected by the differences. */
> Val_GNU_MIPS_ABI_FP_ANY = 0,
>
> which means that the object file *either* does not use FP *or* has not been
> tagged at all, as the GNU linker does not tell these two cases apart internally
> (yes, quite useless semantics, but there you go; I think the enumeration
> constant of 0 shouldn't have been chosen for any explicit tag, or possibly for
> double float instead, so this is an ABI design mistake).
>
> Any object produced before mid 2007, specifically this GNU binutils
> commit:
>
> commit 2cf19d5cb991a88bdc71d0852de8206d9510678f
> Author: Joseph Myers <joseph@codesourcery.com>
> Date: Fri Jun 29 16:41:32 2007 +0000
>
> will not have been tagged for FP use and therefore the traditional MIPS/Linux
> psABI of double float has to be assumed.
>
> This is also the correct interpretation for objects produced by Golang, which I
> have concluded are actually just fine according to the traditional psABI
> definition. It looks to me like the bug is solely in the linker, due to this weird
> interpretation quoted above and unforeseen consequences for FPXX links
> invented much later.
>
Yes. This a bug of linker, and we should fix it.
While for pre-existing binaries, we need a solution to make it workable, especially for the
generic Linux distributions, just like Debian.
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=962485
> Yes, the two cases (not tagged vs tagged as 0) ought to be told apart and I
> outlined a solution (needed for different reasons, but with the same
> motivation) for the GNU linker in the course of the discussion around NaN
> interlinking design, but that has never materialised before I effectively left the
> MIPS world, only remaining active in a limited manner for legacy MIPS
> platforms (that is ISAs I-IV).
>
> Maciej
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] MIPS: introduce config option to force use FR=0 for FPXX binary
2021-02-28 1:47 ` Maciej W. Rozycki
2021-02-28 7:40 ` 回复: " yunqiang.su
@ 2021-02-28 12:49 ` Maciej W. Rozycki
1 sibling, 0 replies; 7+ messages in thread
From: Maciej W. Rozycki @ 2021-02-28 12:49 UTC (permalink / raw)
To: YunQiang Su
Cc: YunQiang Su, Thomas Bogendoerfer, Jiaxun Yang, linux-mips, stable
On Sun, 28 Feb 2021, Maciej W. Rozycki wrote:
> Nope, quoting include/elf/mips.h from GNU binutils:
>
> /* Not tagged or not using any ABIs affected by the differences. */
> Val_GNU_MIPS_ABI_FP_ANY = 0,
>
> which means that the object file *either* does not use FP *or* has not
> been tagged at all, as the GNU linker does not tell these two cases apart
> internally (yes, quite useless semantics, but there you go; I think the
> enumeration constant of 0 shouldn't have been chosen for any explicit tag,
> or possibly for double float instead, so this is an ABI design mistake).
FAOD I think the original intent was to make non-tagged legacy objects
link-compatible with any FP ABI under the assumption that the user knows
what he's doing. While that is acceptable, it shouldn't have implied the
absence of FP code in such legacy objects. Instead legacy properties
should have been implied, that is double FP and likewise legacy NaN. It
would have been easier if a non-zero enumeration constant was assigned to
Val_GNU_MIPS_ABI_FP_ANY, as generic GNU linker code considers the absence
of a given tag equivalent to that tag being equal zero. This still can be
handled, but complicates matters.
Maciej
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 回复: [PATCH v4] MIPS: introduce config option to force use FR=0 for FPXX binary
2021-02-28 7:40 ` 回复: " yunqiang.su
@ 2021-02-28 13:39 ` Maciej W. Rozycki
2021-03-02 2:04 ` YunQiang Su
0 siblings, 1 reply; 7+ messages in thread
From: Maciej W. Rozycki @ 2021-02-28 13:39 UTC (permalink / raw)
To: yunqiang.su
Cc: 'YunQiang Su', 'Thomas Bogendoerfer',
'Jiaxun Yang', 'linux-mips',
stable
On Sun, 28 Feb 2021, yunqiang.su@cipunited.com wrote:
> > This is also the correct interpretation for objects produced by Golang, which I
> > have concluded are actually just fine according to the traditional psABI
> > definition. It looks to me like the bug is solely in the linker, due to this weird
> > interpretation quoted above and unforeseen consequences for FPXX links
> > invented much later.
> >
>
> Yes. This a bug of linker, and we should fix it.
> While for pre-existing binaries, we need a solution to make it workable, especially for the
> generic Linux distributions, just like Debian.
>
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=962485
Thanks for the pointer.
After a bit of thinking and having fully understood what the issue
actually is I conclude a change like your original one (with no
configuration option; we've got too many of them already) will be OK so
long as it keeps the current arrangement for R6, which has the FR mode
hardwired, because, as you say, for genuine FPXX binaries the actual FR
setting does not matter, so the change in the fixed form won't break what
hasn't been broken already.
Please keep the history of changes in the comment section rather that the
change description though. Also I think the change description needs to
be more elaborate on the motivation, so that someone who looks at it say
10 years from now can figure out what is going on here. You can reuse
bits of our discussion for that purpose.
Sadly I can see many changes going in where the description hardly says
anything, and while the matter may seem obvious right now, it surely won't
be for someone trying to unbreak things years from now while keeping the
intent of the original change where it did the right thing. Especially as
secondary sources of information may not be easily available (anymore) and
the test environment may not be easily reproducible. Notice how often I
need to refer to changes that were made many years ago and were not always
correct.
NB the real problem are not programs included with the distribution
(which as I say can and ought to be fixed up with a script automatically;
a distribution needs to have provisions for such workarounds as problems
with the toolchain inevitably do happen from time to time), but programs
built by users of the distribution who we cannot reasonably expect to be
aware of every single quirk out there.
Observe however that this does not solve the issue of a link-time or
load-time incompatibility between FP32 modules incorrectly marked FPXX and
FP64 or FP64A modules. These will be let through and depending on usage
likely eventually fail.
You might be able to come up with a wrapper script in place of whatever
the Golang invocation command is to postprocess modules produced in user
compilations as well, and have it distributed until the linker issue has
been fixed upstream and the changes propagated back to the distribution.
Maciej
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 回复: [PATCH v4] MIPS: introduce config option to force use FR=0 for FPXX binary
2021-02-28 13:39 ` Maciej W. Rozycki
@ 2021-03-02 2:04 ` YunQiang Su
0 siblings, 0 replies; 7+ messages in thread
From: YunQiang Su @ 2021-03-02 2:04 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: YunQiang Su, Thomas Bogendoerfer, Jiaxun Yang, linux-mips, stable
Maciej W. Rozycki <macro@orcam.me.uk> 于2021年2月28日周日 下午9:39写道:
>
> On Sun, 28 Feb 2021, yunqiang.su@cipunited.com wrote:
>
> > > This is also the correct interpretation for objects produced by Golang, which I
> > > have concluded are actually just fine according to the traditional psABI
> > > definition. It looks to me like the bug is solely in the linker, due to this weird
> > > interpretation quoted above and unforeseen consequences for FPXX links
> > > invented much later.
> > >
> >
> > Yes. This a bug of linker, and we should fix it.
> > While for pre-existing binaries, we need a solution to make it workable, especially for the
> > generic Linux distributions, just like Debian.
> >
> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=962485
>
> Thanks for the pointer.
>
> After a bit of thinking and having fully understood what the issue
> actually is I conclude a change like your original one (with no
> configuration option; we've got too many of them already) will be OK so
> long as it keeps the current arrangement for R6, which has the FR mode
> hardwired, because, as you say, for genuine FPXX binaries the actual FR
> setting does not matter, so the change in the fixed form won't break what
> hasn't been broken already.
>
> Please keep the history of changes in the comment section rather that the
> change description though. Also I think the change description needs to
> be more elaborate on the motivation, so that someone who looks at it say
> 10 years from now can figure out what is going on here. You can reuse
> bits of our discussion for that purpose.
>
> Sadly I can see many changes going in where the description hardly says
> anything, and while the matter may seem obvious right now, it surely won't
> be for someone trying to unbreak things years from now while keeping the
> intent of the original change where it did the right thing. Especially as
> secondary sources of information may not be easily available (anymore) and
> the test environment may not be easily reproducible. Notice how often I
> need to refer to changes that were made many years ago and were not always
> correct.
>
> NB the real problem are not programs included with the distribution
> (which as I say can and ought to be fixed up with a script automatically;
> a distribution needs to have provisions for such workarounds as problems
> with the toolchain inevitably do happen from time to time), but programs
> built by users of the distribution who we cannot reasonably expect to be
> aware of every single quirk out there.
>
The "stable" branch of distribution is in the same situation as the user.
Normally, we cannot modify the binary in the "stable" branch.
> Observe however that this does not solve the issue of a link-time or
> load-time incompatibility between FP32 modules incorrectly marked FPXX and
> FP64 or FP64A modules. These will be let through and depending on usage
> likely eventually fail.
>
Yes, that's a problem. the patch of golang has been merged now.
https://go-review.googlesource.com/c/go/+/239217
https://go-review.googlesource.com/c/go/+/237058
And we will continue to try to fix binutils/llvm etc.
> You might be able to come up with a wrapper script in place of whatever
> the Golang invocation command is to postprocess modules produced in user
> compilations as well, and have it distributed until the linker issue has
> been fixed upstream and the changes propagated back to the distribution.
>
We fixed the golang itself, and the packages has been in Debian bullseye now.
> Maciej
--
YunQiang Su
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-03-03 0:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-22 3:43 [PATCH v4] MIPS: introduce config option to force use FR=0 for FPXX binary YunQiang Su
2021-02-23 6:29 ` YunQiang Su
2021-02-28 1:47 ` Maciej W. Rozycki
2021-02-28 7:40 ` 回复: " yunqiang.su
2021-02-28 13:39 ` Maciej W. Rozycki
2021-03-02 2:04 ` YunQiang Su
2021-02-28 12:49 ` Maciej W. Rozycki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.