[PATCH v6] MIPS: force use FR=0 for FPXX binary

[PATCH v6] MIPS: force use FR=0 for FPXX binary

[YunQiang Su]

The MIPS FPU may have 2 mode:
  FR=0: MIPS I style, odd-FPR can only be single,
        and even-FPR can be double.
  FR=1: all 32 FPR can be double.

The binary may have 3 mode:
  FP32: can only work with FR=0 mode
  FPXX: can work with both FR=0 and FR=1 mode.
  FP64: can only work with FR=1 mode

Some binary, for example the output of golang, may be mark as FPXX,
while in fact they are FP32.

Currently, FR=1 mode is used for all FPXX binary, it makes some wrong
behivour of the binaries. Since FPXX binary can work with both FR=1 and FR=0,
we force it to use FR=0.

Reference:

https://web.archive.org/web/20180828210612/https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking

https://go-review.googlesource.com/c/go/+/239217
https://go-review.googlesource.com/c/go/+/237058

Signed-off-by: YunQiang Su <yunqiang.su@cipunited.com>
Cc: stable@vger.kernel.org # 4.19+

v5->v6:
	Rollback to V3, aka remove config option.

v4->v5:
	Fix CONFIG_MIPS_O32_FPXX_USE_FR0 usage: if -> ifdef

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
---
 arch/mips/kernel/elf.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
index 7b045d2a0b51..d1aa907e9864 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,9 @@ 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;
-	else if ((prog_req.fr1 && prog_req.frdefault) ||
-		 (prog_req.single && !prog_req.frdefault))
+	else if (prog_req.fr1 && prog_req.frdefault)
+		state->overall_fp_mode = FP_FR0;
+	else if (prog_req.single && !prog_req.frdefault)
 		/* 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

Re: [PATCH v6] MIPS: force use FR=0 for FPXX binary

[Maciej W. Rozycki]

On Tue, 2 Mar 2021, YunQiang Su wrote:

> The MIPS FPU may have 2 mode:
>   FR=0: MIPS I style, odd-FPR can only be single,
>         and even-FPR can be double.

 Depending on the ISA level FR=0 may or may not allow single arithmetic 
with odd-numbered FPRs.  Compare the FP64A ABI.

>   FR=1: all 32 FPR can be double.

 I think it's best to describe the FR=0 mode as one where the FP registers 
are 32-bit and the FR=1 mode as one where the FP registers are 64-bit 
(this mode is also needed for the paired-single format).  See:

<https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking#1._Introduction>

> The binary may have 3 mode:
>   FP32: can only work with FR=0 mode
>   FPXX: can work with both FR=0 and FR=1 mode.
>   FP64: can only work with FR=1 mode
> 
> Some binary, for example the output of golang, may be mark as FPXX,
> while in fact they are FP32.
> 
> Currently, FR=1 mode is used for all FPXX binary, it makes some wrong
> behivour of the binaries. Since FPXX binary can work with both FR=1 and FR=0,
> we force it to use FR=0.

 I think you need to document here what we discussed, that is the linker 
bug exposed in the context of FPXX annotation by legacy modules that lack 
FP mode annotation, which is the underlying problem.

> v5->v6:
> 	Rollback to V3, aka remove config option.

 You can't reuse v3 as it stands because it breaks R6 as we previously 
discussed.  You need to tell R6 and earlier ISAs apart and set the FR bit 
accordingly.

 It would be more proper I suppose if we actually checked at the boot time 
whether the bit is writable, just like we handle NAN2008, but I don't see 
it as a prerequisite for this workaround since we currently don't do this 
either (it would also be good to check if the FP emulation code gets the 
read-only FR bit right for R6 for FPU-less operation).

 Also you need to put the history in the comment section and not the 
commit description, so that the change can be directly applied and does 
not have to be hand-edited by the maintainer.  You don't want to overload 
him with mechanical work.

  Maciej

回复: [PATCH v6] MIPS: force use FR=0 for FPXX binary

[yunqiang.su]

> On Tue, 2 Mar 2021, YunQiang Su wrote:
> 
> > The MIPS FPU may have 2 mode:
> >   FR=0: MIPS I style, odd-FPR can only be single,
> >         and even-FPR can be double.
> 
>  Depending on the ISA level FR=0 may or may not allow single arithmetic
with
> odd-numbered FPRs.  Compare the FP64A ABI.
> 
> >   FR=1: all 32 FPR can be double.
> 
>  I think it's best to describe the FR=0 mode as one where the FP registers
are
> 32-bit and the FR=1 mode as one where the FP registers are 64-bit (this
mode
> is also needed for the paired-single format).  See:
> 
> <https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlin
> king#1._Introduction>
> 

Thank you. I will update it.

> > The binary may have 3 mode:
> >   FP32: can only work with FR=0 mode
> >   FPXX: can work with both FR=0 and FR=1 mode.
> >   FP64: can only work with FR=1 mode
> >
> > Some binary, for example the output of golang, may be mark as FPXX,
> > while in fact they are FP32.
> >
> > Currently, FR=1 mode is used for all FPXX binary, it makes some wrong
> > behivour of the binaries. Since FPXX binary can work with both FR=1
> > and FR=0, we force it to use FR=0.
> 
>  I think you need to document here what we discussed, that is the linker
bug
> exposed in the context of FPXX annotation by legacy modules that lack FP
> mode annotation, which is the underlying problem.

I will do it.

> 
> > v5->v6:
> > 	Rollback to V3, aka remove config option.
> 
>  You can't reuse v3 as it stands because it breaks R6 as we previously
> discussed.  You need to tell R6 and earlier ISAs apart and set the FR bit
> accordingly.
> 

It won't break r6, as all of r6 binary is FP64, and on r6
   `frdefault' is always false, and `fr1' is always true.
It won't trigger this mode switch.

Oh, you are right, there may be a case that to run legacy app on r6 CPU.
For this case, on r6, we need to set the CPU to FRE mode.

>  It would be more proper I suppose if we actually checked at the boot time
> whether the bit is writable, just like we handle NAN2008, but I don't see
it as
> a prerequisite for this workaround since we currently don't do this either
(it
> would also be good to check if the FP emulation code gets the read-only FR
bit
> right for R6 for FPU-less operation).
> 

Check writable is not needed here.

>  Also you need to put the history in the comment section and not the
commit
> description, so that the change can be directly applied and does not have
to
> be hand-edited by the maintainer.  You don't want to overload him with
> mechanical work.
> 

Ok, I will add more comment.

>   Maciej

Re: 回复: [PATCH v6] MIPS: force use FR=0 for FPXX binary

[Maciej W. Rozycki]

On Wed, 3 Mar 2021, yunqiang.su@cipunited.com wrote:

> > > v5->v6:
> > > 	Rollback to V3, aka remove config option.
> > 
> >  You can't reuse v3 as it stands because it breaks R6 as we previously
> > discussed.  You need to tell R6 and earlier ISAs apart and set the FR bit
> > accordingly.
> > 
> 
> It won't break r6, as all of r6 binary is FP64, and on r6
>    `frdefault' is always false, and `fr1' is always true.
> It won't trigger this mode switch.
> 
> Oh, you are right, there may be a case that to run legacy app on r6 CPU.
> For this case, on r6, we need to set the CPU to FRE mode.

 The FRE mode causes a severe performance regression for single FP 
operations, so we shouldn't use it for FPXX software.

 As a matter of interest: do you have figures available as to how many 
software packages are affected in Debian?

 Also it has now struck me that another userland workaround should be 
possible, by setting LD_PRELOAD in the environment system-wide to a 
dummy FR=0 DSO (e.g. via /etc/environment or /etc/initscript; I reckon 
systemd has its own way too), which will force the right mode the normal 
way.  All the distribution has been built for FPXX I presume, right?

 You can distribute a package with the dummy along with the environment 
entry to all the people who require it.  I fail to see how it could be 
more problematic than getting a questionable hack included in the kernel 
forever and then requiring everyone to upgrade the relevant packages 
anyway, which you will have to supply for stable releases too.

 Or I guess you could just rebuild libc as FR=0 instead, or is there a
Golang standard library that every Golang program uses?  And then have 
people upgrade that package instead.

 It seems to me like there are still a couple of alternatives available.  
You might be able to come up with yet more if you continued looking for 
them.  I consider putting any workaround into the kernel the last resort 
really.  The problem is in the userland, so let's try hard to deal with 
it there.

  Maciej

回复: 回复: [PATCH v6] MIPS: force use FR=0 for FPXX binary

[yunqiang.su]

> On Wed, 3 Mar 2021, yunqiang.su@cipunited.com wrote:
> 
> > > > v5->v6:
> > > > 	Rollback to V3, aka remove config option.
> > >
> > >  You can't reuse v3 as it stands because it breaks R6 as we
> > > previously discussed.  You need to tell R6 and earlier ISAs apart
> > > and set the FR bit accordingly.
> > >
> >
> > It won't break r6, as all of r6 binary is FP64, and on r6
> >    `frdefault' is always false, and `fr1' is always true.
> > It won't trigger this mode switch.
> >
> > Oh, you are right, there may be a case that to run legacy app on r6 CPU.
> > For this case, on r6, we need to set the CPU to FRE mode.
> 
>  The FRE mode causes a severe performance regression for single FP
> operations, so we shouldn't use it for FPXX software.
> 

If we need to run pre-R6 FPXX/FP32 app on r6 CPU, it may be the only choice
for us.
Any way, in this case we need lots of T&E, the problem of FRE won't be a big
problem.

>  As a matter of interest: do you have figures available as to how many
> software packages are affected in Debian?
> 

Almost all packages built with Golang in buster.

>  Also it has now struck me that another userland workaround should be
> possible, by setting LD_PRELOAD in the environment system-wide to a
> dummy FR=0 DSO (e.g. via /etc/environment or /etc/initscript; I reckon
> systemd has its own way too), which will force the right mode the normal
way.
> All the distribution has been built for FPXX I presume, right?
> 

It is not acceptable for "stable" branch of distributions.
For developing branch, we have fixed this problem on Golang itself.

>  You can distribute a package with the dummy along with the environment
> entry to all the people who require it.  I fail to see how it could be
more
> problematic than getting a questionable hack included in the kernel
forever
> and then requiring everyone to upgrade the relevant packages anyway, which
> you will have to supply for stable releases too.
> 

So, I'd prefer to introduce a new CONFIG option.

>  Or I guess you could just rebuild libc as FR=0 instead, or is there a
Golang
> standard library that every Golang program uses?  And then have people
> upgrade that package instead.
> 

Rebuiding libc to FP32 is not acceptable, since we want to do is to support
MSA,
Which require FR=1 and all the result is FP64.

In fact we found this problem when we try to enable MIPS_O32_FP64_SUPPORT,
Without this option is enabled, all FPXX binaries are still use FR=0 mode:
See: function mips_set_personality_fp()

So, here, we doesn't introduce the rollback to FR=0.

>  It seems to me like there are still a couple of alternatives available.
> You might be able to come up with yet more if you continued looking for
them.
> I consider putting any workaround into the kernel the last resort really.
The
> problem is in the userland, so let's try hard to deal with it there.
> 

Yes. It is problem of userland, while it has no way to fix in for the
pre-exist binaries in userland.

>   Maciej

Re: 回复: 回复: [PATCH v6] MIPS: force use FR=0 for FPXX binary

[Maciej W. Rozycki]

On Wed, 3 Mar 2021, yunqiang.su@cipunited.com wrote:

> >  The FRE mode causes a severe performance regression for single FP
> > operations, so we shouldn't use it for FPXX software.
> > 
> 
> If we need to run pre-R6 FPXX/FP32 app on r6 CPU, it may be the only choice
> for us.

 Nope, FPXX doesn't require FRE, and FPXX is all this change is about.

> Any way, in this case we need lots of T&E, the problem of FRE won't be a big
> problem.

 The R6 instruction set has been designed such as to minimise traps and 
emulations, so there is no point to make it worse for everyone for the 
sake of a broken corner case.

> >  As a matter of interest: do you have figures available as to how many
> > software packages are affected in Debian?
> > 
> 
> Almost all packages built with Golang in buster.

 How many is that though?  Two?  Ten?  A thousand?

> >  Also it has now struck me that another userland workaround should be
> > possible, by setting LD_PRELOAD in the environment system-wide to a
> > dummy FR=0 DSO (e.g. via /etc/environment or /etc/initscript; I reckon
> > systemd has its own way too), which will force the right mode the normal
> way.
> > All the distribution has been built for FPXX I presume, right?
> > 
> 
> It is not acceptable for "stable" branch of distributions.

 I'd say the chosen policy of any distribution is said distribution's 
problem, not the upstream kernel's.  You can have a local patch for the 
kernel too if you consider a kernel solution the only one that works for 
you.  From the discussion so far it looks to me like the least involving 
solution which will make everyone happy.

> >  Or I guess you could just rebuild libc as FR=0 instead, or is there a
> Golang
> > standard library that every Golang program uses?  And then have people
> > upgrade that package instead.
> > 
> 
> Rebuiding libc to FP32 is not acceptable, since we want to do is to support
> MSA,
> Which require FR=1 and all the result is FP64.

 Do you have any software build for MSA with your distribution already, 
or do you just plan it?  How is it expected work with non-MSA hardware, 
which I believe is still predominant?

 Also I'll repeat myself: is there a Golang standard library that every 
Golang program uses?

> In fact we found this problem when we try to enable MIPS_O32_FP64_SUPPORT,
> Without this option is enabled, all FPXX binaries are still use FR=0 mode:
> See: function mips_set_personality_fp()
> 
> So, here, we doesn't introduce the rollback to FR=0.

 So keep MIPS_O32_FP64_SUPPORT disabled then until the environment has 
been fixed?

> >  It seems to me like there are still a couple of alternatives available.
> > You might be able to come up with yet more if you continued looking for
> them.
> > I consider putting any workaround into the kernel the last resort really.
> The
> > problem is in the userland, so let's try hard to deal with it there.
> > 
> 
> Yes. It is problem of userland, while it has no way to fix in for the
> pre-exist binaries in userland.

 I gave you examples.  It appears the problem instead is with the 
distribution's policy, and the kernel is not there to work it around, 
sorry.

  Maciej

回复: 回复: 回复: [PATCH v6] MIPS: force use FR=0 for FPXX binary

[yunqiang.su]

> On Wed, 3 Mar 2021, yunqiang.su@cipunited.com wrote:
> 
> > >  The FRE mode causes a severe performance regression for single FP
> > > operations, so we shouldn't use it for FPXX software.
> > >
> >
> > If we need to run pre-R6 FPXX/FP32 app on r6 CPU, it may be the only
> > choice for us.
> 
>  Nope, FPXX doesn't require FRE, and FPXX is all this change is about.
> 
> > Any way, in this case we need lots of T&E, the problem of FRE won't be
> > a big problem.
> 
>  The R6 instruction set has been designed such as to minimise traps and
> emulations, so there is no point to make it worse for everyone for the
sake of
> a broken corner case.
> 
> > >  As a matter of interest: do you have figures available as to how
> > > many software packages are affected in Debian?
> > >
> >
> > Almost all packages built with Golang in buster.
> 
>  How many is that though?  Two?  Ten?  A thousand?

syq@m530-2:~$ cat
/var/lib/apt/lists/mirrors.aliyun.com_debian_dists_sid_main_source_Sources |
grep 'Build-Depends' | grep golang | wc
   2039   21384  344099

> 
> > >  Also it has now struck me that another userland workaround should
> > > be possible, by setting LD_PRELOAD in the environment system-wide to
> > > a dummy FR=0 DSO (e.g. via /etc/environment or /etc/initscript; I
> > > reckon systemd has its own way too), which will force the right mode
> > > the normal
> > way.
> > > All the distribution has been built for FPXX I presume, right?
> > >
> >
> > It is not acceptable for "stable" branch of distributions.
> 
>  I'd say the chosen policy of any distribution is said distribution's
problem, not
> the upstream kernel's.  You can have a local patch for the kernel too if
you
> consider a kernel solution the only one that works for you.  From the
> discussion so far it looks to me like the least involving solution which
will
> make everyone happy.
> 

It is not only about one distribution, instead of all distribution, since
this is caused by the bug of Golang distribution.

> > >  Or I guess you could just rebuild libc as FR=0 instead, or is there
> > > a
> > Golang
> > > standard library that every Golang program uses?  And then have
> > > people upgrade that package instead.
> > >
> >
> > Rebuiding libc to FP32 is not acceptable, since we want to do is to
> > support MSA, Which require FR=1 and all the result is FP64.
> 
>  Do you have any software build for MSA with your distribution already, or
do
> you just plan it?  How is it expected work with non-MSA hardware, which I
> believe is still predominant?
> 

I am just plan it for Debian. While currently there do be some downstream
user of
mipsel/Debian are using MSA on it.

>  Also I'll repeat myself: is there a Golang standard library that every
Golang
> program uses?
> 

Yes. It even effect /usr/bin/go itself, and all of binaries its generate may
be effected.

> > In fact we found this problem when we try to enable
> > MIPS_O32_FP64_SUPPORT, Without this option is enabled, all FPXX binaries
> are still use FR=0 mode:
> > See: function mips_set_personality_fp()
> >
> > So, here, we doesn't introduce the rollback to FR=0.
> 
>  So keep MIPS_O32_FP64_SUPPORT disabled then until the environment has
> been fixed?
> 

That is really a solution, while we still need a solution to compatible with
the pre-exists binaries. 

> > >  It seems to me like there are still a couple of alternatives
available.
> > > You might be able to come up with yet more if you continued looking
> > > for
> > them.
> > > I consider putting any workaround into the kernel the last resort
really.
> > The
> > > problem is in the userland, so let's try hard to deal with it there.
> > >
> >
> > Yes. It is problem of userland, while it has no way to fix in for the
> > pre-exist binaries in userland.
> 
>  I gave you examples.  It appears the problem instead is with the
> distribution's policy, and the kernel is not there to work it around,
sorry.
> 

It effects all distributions not only one.
I am not want to change the default behavior of upstream kernel, that's why
I prefer to introduce a new config option.

>   Maciej

Re: 回复: 回复: 回复: [PATCH v6] MIPS: force use FR=0 for FPXX binary

[Maciej W. Rozycki]

On Thu, 4 Mar 2021, yunqiang.su@cipunited.com wrote:

> >  How many is that though?  Two?  Ten?  A thousand?
> 
> syq@m530-2:~$ cat
> /var/lib/apt/lists/mirrors.aliyun.com_debian_dists_sid_main_source_Sources |
> grep 'Build-Depends' | grep golang | wc
>    2039   21384  344099

 Ack, that's quite a lot indeed.

> >  I'd say the chosen policy of any distribution is said distribution's
> problem, not
> > the upstream kernel's.  You can have a local patch for the kernel too if
> you
> > consider a kernel solution the only one that works for you.  From the
> > discussion so far it looks to me like the least involving solution which
> will
> > make everyone happy.
> > 
> 
> It is not only about one distribution, instead of all distribution, since
> this is caused by the bug of Golang distribution.

 I think every distribution is free to handle the problem their own way.  
You have asked on behalf of Debian, so we're discussing what Debian can 
do.  Other distributions may or may not follow.

> >  Also I'll repeat myself: is there a Golang standard library that every
> Golang
> > program uses?
> > 
> 
> Yes. It even effect /usr/bin/go itself, and all of binaries its generate may
> be effected.

 Sigh, you did not answer my question again, but I have lost my patience 
and figured it out myself, as noted with v7 (libgo.so.17 is the Golang 
standard library as at GCC 11).  The obvious userland solution goes along 
with it.  You don't even have to do anything really.

 NB please keep the subject proper of the cover letter the same with every 
iteration of a patch series, as in that case mail user agents (at least 
the sane ones) will group all iterations together in the thread sorting 
mode.  With the subject changed every time the link is lost and 
submissions are scattered all over the mail folder.

  Maciej

Re: 回复: 回复: 回复: [PATCH v6] MIPS: force use FR=0 for FPXX binary

[YunQiang Su]

Maciej W. Rozycki <macro@orcam.me.uk> 于2021年3月18日周四 上午7:30写道：
>
> On Thu, 4 Mar 2021, yunqiang.su@cipunited.com wrote:
>
> > >  How many is that though?  Two?  Ten?  A thousand?
> >
> > syq@m530-2:~$ cat
> > /var/lib/apt/lists/mirrors.aliyun.com_debian_dists_sid_main_source_Sources |
> > grep 'Build-Depends' | grep golang | wc
> >    2039   21384  344099
>
>  Ack, that's quite a lot indeed.
>
> > >  I'd say the chosen policy of any distribution is said distribution's
> > problem, not
> > > the upstream kernel's.  You can have a local patch for the kernel too if
> > you
> > > consider a kernel solution the only one that works for you.  From the
> > > discussion so far it looks to me like the least involving solution which
> > will
> > > make everyone happy.
> > >
> >
> > It is not only about one distribution, instead of all distribution, since
> > this is caused by the bug of Golang distribution.
>
>  I think every distribution is free to handle the problem their own way.
> You have asked on behalf of Debian, so we're discussing what Debian can
> do.  Other distributions may or may not follow.
>
> > >  Also I'll repeat myself: is there a Golang standard library that every
> > Golang
> > > program uses?
> > >
> >
> > Yes. It even effect /usr/bin/go itself, and all of binaries its generate may
> > be effected.
>
>  Sigh, you did not answer my question again, but I have lost my patience
> and figured it out myself, as noted with v7 (libgo.so.17 is the Golang
> standard library as at GCC 11).  The obvious userland solution goes along
> with it.  You don't even have to do anything really.
>

I am sorry for not clear enough.
Here, Golang means the Google's Go, aka https://golang.org/.
It is 2 different software with Go in gcc, which we call it gccgo.

Golang (golang.org, not gccgo) has no runtime normally, since it seems
prefer static linking.
So, Lots of Go programs produced by golang (golang.org, not gccgo) are
static linked.
Especially for Linux distributions `stable' branch.

>  NB please keep the subject proper of the cover letter the same with every
> iteration of a patch series, as in that case mail user agents (at least
> the sane ones) will group all iterations together in the thread sorting
> mode.  With the subject changed every time the link is lost and
> submissions are scattered all over the mail folder.
>

I will keep it as the last one. Thank your.

>   Maciej



-- 
YunQiang Su