[PATCH] MIPS: VDSO: Always select -msoft-float

[PATCH] MIPS: VDSO: Always select -msoft-float

[Guenter Roeck]

Some toolchains fail to build mips images with the following build error.

arch/mips/vdso/gettimeofday.c:1:0: error: '-march=r3000' requires '-mfp32'

This is seen, for example, with the 'mipsel-linux-gnu-gcc (Debian 6.1.1-9)
6.1.1 20160705' toolchain as used by the 0Day build robot when building
decstation_defconfig.

Comparison of compile flags suggests that the major difference is a missing
'-soft-float', which is otherwise defined unconditionally.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Cc: James Hogan <james.hogan@imgtec.com>
Fixes: ebb5e78cc634 ("MIPS: Initial implementation of a VDSO")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 arch/mips/vdso/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/vdso/Makefile b/arch/mips/vdso/Makefile
index c3dc12a8b7d9..9bdd6641400f 100644
--- a/arch/mips/vdso/Makefile
+++ b/arch/mips/vdso/Makefile
@@ -6,7 +6,8 @@ ccflags-vdso := \
 	$(filter -I%,$(KBUILD_CFLAGS)) \
 	$(filter -E%,$(KBUILD_CFLAGS)) \
 	$(filter -mmicromips,$(KBUILD_CFLAGS)) \
-	$(filter -march=%,$(KBUILD_CFLAGS))
+	$(filter -march=%,$(KBUILD_CFLAGS)) \
+	-msoft-float
 cflags-vdso := $(ccflags-vdso) \
 	$(filter -W%,$(filter-out -Wa$(comma)%,$(KBUILD_CFLAGS))) \
 	-O2 -g -fPIC -fno-strict-aliasing -fno-common -fno-builtin -G 0 \
-- 
2.5.0

Re: [PATCH] MIPS: VDSO: Always select -msoft-float

[Maciej W. Rozycki]

On Sun, 30 Oct 2016, Guenter Roeck wrote:

> Some toolchains fail to build mips images with the following build error.
> 
> arch/mips/vdso/gettimeofday.c:1:0: error: '-march=r3000' requires '-mfp32'
> 
> This is seen, for example, with the 'mipsel-linux-gnu-gcc (Debian 6.1.1-9)
> 6.1.1 20160705' toolchain as used by the 0Day build robot when building
> decstation_defconfig.
> 
> Comparison of compile flags suggests that the major difference is a missing
> '-soft-float', which is otherwise defined unconditionally.
> 
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Cc: James Hogan <james.hogan@imgtec.com>
> Fixes: ebb5e78cc634 ("MIPS: Initial implementation of a VDSO")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---

 Using `-msoft-float' changes the floating-point ABI with the result being 
incompatible with the rest of the userland.  I think the dynamic loader 
may not be currently enforcing ABI compatibility here, but this may change 
in the future.

 Using `-mno-float' in place of `-msoft-float' might be a safer option, 
because even if we start enforcing floating-point ABI checks in dynamic 
loading, then `-mno-float' DSOs will surely remain compatible with 
everything else, because they guarantee no floating-point code or data 
even to be ever produced by the compiler, be it using the software or the 
hardware ABI.  One problem with that option is however that it is 
apparently not universally accepted, for reasons unclear to me offhand.

 That written not so long ago I actually explicitly tried the config file 
sent by the build bot reporting this issue and I built a kernel thus 
configured with current upstream top-of-tree toolchain components, which 
went just fine.  So what I suspect you've observied is just another sign 
of a bug which has been already fixed, maybe even the very same binutils 
bug I referred to recently.

 If you send me the generated assembly, i.e. `gettimeofday.s', that is 
causing you trouble, then I'll see if I can figure out what is going on 
here.  We may decide to paper a particularly nasty toolchain bug over from 
time to time rather than requesting users to apply the relevant proper fix 
to the toolchain, but before we do so I think we first need to thoroughly 
understand what the issue is so as not to cause more harm than good with 
the workaround.

  Maciej

Re: [PATCH] MIPS: VDSO: Always select -msoft-float

[Maciej W. Rozycki]

On Sun, 30 Oct 2016, Guenter Roeck wrote:

> Some toolchains fail to build mips images with the following build error.
> 
> arch/mips/vdso/gettimeofday.c:1:0: error: '-march=r3000' requires '-mfp32'
> 
> This is seen, for example, with the 'mipsel-linux-gnu-gcc (Debian 6.1.1-9)
> 6.1.1 20160705' toolchain as used by the 0Day build robot when building
> decstation_defconfig.
> 
> Comparison of compile flags suggests that the major difference is a missing
> '-soft-float', which is otherwise defined unconditionally.
> 
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Cc: James Hogan <james.hogan@imgtec.com>
> Fixes: ebb5e78cc634 ("MIPS: Initial implementation of a VDSO")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---

 Using `-msoft-float' changes the floating-point ABI with the result being 
incompatible with the rest of the userland.  I think the dynamic loader 
may not be currently enforcing ABI compatibility here, but this may change 
in the future.

 Using `-mno-float' in place of `-msoft-float' might be a safer option, 
because even if we start enforcing floating-point ABI checks in dynamic 
loading, then `-mno-float' DSOs will surely remain compatible with 
everything else, because they guarantee no floating-point code or data 
even to be ever produced by the compiler, be it using the software or the 
hardware ABI.  One problem with that option is however that it is 
apparently not universally accepted, for reasons unclear to me offhand.

 That written not so long ago I actually explicitly tried the config file 
sent by the build bot reporting this issue and I built a kernel thus 
configured with current upstream top-of-tree toolchain components, which 
went just fine.  So what I suspect you've observied is just another sign 
of a bug which has been already fixed, maybe even the very same binutils 
bug I referred to recently.

 If you send me the generated assembly, i.e. `gettimeofday.s', that is 
causing you trouble, then I'll see if I can figure out what is going on 
here.  We may decide to paper a particularly nasty toolchain bug over from 
time to time rather than requesting users to apply the relevant proper fix 
to the toolchain, but before we do so I think we first need to thoroughly 
understand what the issue is so as not to cause more harm than good with 
the workaround.

  Maciej

Re: [PATCH] MIPS: VDSO: Always select -msoft-float

[Guenter Roeck]

On Tue, Nov 01, 2016 at 10:40:24PM +0000, Maciej W. Rozycki wrote:
> On Sun, 30 Oct 2016, Guenter Roeck wrote:
> 
> > Some toolchains fail to build mips images with the following build error.
> > 
> > arch/mips/vdso/gettimeofday.c:1:0: error: '-march=r3000' requires '-mfp32'
> > 
> > This is seen, for example, with the 'mipsel-linux-gnu-gcc (Debian 6.1.1-9)
> > 6.1.1 20160705' toolchain as used by the 0Day build robot when building
> > decstation_defconfig.
> > 
> > Comparison of compile flags suggests that the major difference is a missing
> > '-soft-float', which is otherwise defined unconditionally.
> > 
> > Reported-by: kbuild test robot <fengguang.wu@intel.com>
> > Cc: James Hogan <james.hogan@imgtec.com>
> > Fixes: ebb5e78cc634 ("MIPS: Initial implementation of a VDSO")
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> 
>  Using `-msoft-float' changes the floating-point ABI with the result being 
> incompatible with the rest of the userland.  I think the dynamic loader 
> may not be currently enforcing ABI compatibility here, but this may change 
> in the future.
> 
>  Using `-mno-float' in place of `-msoft-float' might be a safer option, 
> because even if we start enforcing floating-point ABI checks in dynamic 
> loading, then `-mno-float' DSOs will surely remain compatible with 
> everything else, because they guarantee no floating-point code or data 
> even to be ever produced by the compiler, be it using the software or the 
> hardware ABI.  One problem with that option is however that it is 
> apparently not universally accepted, for reasons unclear to me offhand.
> 
>  That written not so long ago I actually explicitly tried the config file 
> sent by the build bot reporting this issue and I built a kernel thus 
> configured with current upstream top-of-tree toolchain components, which 
> went just fine.  So what I suspect you've observied is just another sign 
> of a bug which has been already fixed, maybe even the very same binutils 
> bug I referred to recently.
> 
>  If you send me the generated assembly, i.e. `gettimeofday.s', that is 
> causing you trouble, then I'll see if I can figure out what is going on 
> here.  We may decide to paper a particularly nasty toolchain bug over from 

The problem is seen in 0Day builds, with the toolchain mentioned above.
I don't see the problem myself; I used to see it but switched toolchains
instead of trying to nail down the problem.

I don't think that generated assembly is going to help, though, since the
compiler fails to compile the code in the first place because, as it says,
it doesn't like '-march=r3000' without '-mfp32'.

Guenter

Re: [PATCH] MIPS: VDSO: Always select -msoft-float

[Maciej W. Rozycki]

On Tue, 1 Nov 2016, Guenter Roeck wrote:

> > > Some toolchains fail to build mips images with the following build error.
> > > 
> > > arch/mips/vdso/gettimeofday.c:1:0: error: '-march=r3000' requires '-mfp32'
> > > 
> > > This is seen, for example, with the 'mipsel-linux-gnu-gcc (Debian 6.1.1-9)
> > > 6.1.1 20160705' toolchain as used by the 0Day build robot when building
> > > decstation_defconfig.
> > > 
> > > Comparison of compile flags suggests that the major difference is a missing
> > > '-soft-float', which is otherwise defined unconditionally.
> > > 
> > > Reported-by: kbuild test robot <fengguang.wu@intel.com>
> > > Cc: James Hogan <james.hogan@imgtec.com>
> > > Fixes: ebb5e78cc634 ("MIPS: Initial implementation of a VDSO")
> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > ---
[...]
> >  If you send me the generated assembly, i.e. `gettimeofday.s', that is 
> > causing you trouble, then I'll see if I can figure out what is going on 
> > here.  We may decide to paper a particularly nasty toolchain bug over from 
> 
> The problem is seen in 0Day builds, with the toolchain mentioned above.

 That does not tell me anything -- I have no idea how each of the 
toolchains out there has been configured.  I can only assess information
provided by the bug/patch submitter, and otherwise try guessing.

> I don't think that generated assembly is going to help, though, since the
> compiler fails to compile the code in the first place because, as it says,
> it doesn't like '-march=r3000' without '-mfp32'.

 Indeed I got that confused, even though there is supposed to be a mention 
in the compiler's diagnostics if a warning or error has originated from an 
external tool invoked.  Sorry about that.  The message itself could well 
have come from the assembler though as the same options are accepted by 
that tool.

 And indeed I have just guessed what the cause might be, that is the 
compiler must have been configured with an explicit `--with-fp-32=xx' or 
`--with-fp-32=64' option.

 Here's quite an extensive analysis of what these options do and why: 
<https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking>
and I note that there is a mention of a setup which would have avoided the 
situation we have now:

"Ideally any MIPS I arch/core will default to -mfp32 Any MIPS II -> 
MIPS32r2 arch/core will default to -mfpxx. However, this should be 
controlled via a configure time option to adjust the default ABI from O32 
FP32 to O32 FPXX (or O32 FP64 as needed). The new configure time option is 
--with-fp-32=[32|xx|64] and this affects the FP mode only when targetting 
the O32 ABI."

however this ideal arrangement, which I would expect a configuration 
option like `--with-fp-32=from-isa' (and a corresponding `-mfp=from-isa' 
compiler option) would handle, got lost/forgotten in the works somehow.

 Matthew has been the expert in this area and might be able to add some 
more -- Matthew?  Also shall we call it a compiler defect?  We seem to be 
getting more and more toolchain configurations which break the existing 
setups in a way requiring them to add more and more compiler options to 
Makefiles for preexisting targets even though the Makefiles were 
previously already prepared to choose the right compiler options for the 
target.  This seems the wrong way to go to me as it's causing people 
burden when upgrading their toolchains while it's supposed to be seamless 
(it's one aspect of maintaining backwards compatibility).

 Meanwhile your proposal may be indeed the way to go, or perhaps we could 
use a substitution rule like:

	$(patsubst -march=%,-mfp32,$(filter -march=r3%,$(KBUILD_CFLAGS)))

-- see the change below, it seems to work for me and the option is indeed 
passed with a `-march=r3000' build (as would with a `-march=r3900' build) 
and suppressed otherwise (such as with a `-march=mips32r2' build).  

 I have tried building `arch/mips/vdso/gettimeofday.s' to see how the 
module settings are chosen by the compiler in my successful build, and 
that produced an unexpected result in that the rule for .s targets is 
evidently broken for VDSO as different compiler flags were used from ones 
used for .o targets -- essentially the regular flags rather than the 
special VDSO ones (no `-fPIC', etc.).  So that looks like a bug to me to 
be fixed too.

 So instead I just checked the ABI annotation of a good .o object with 
`readelf':

$ readelf -A arch/mips/vdso/gettimeofday.o
Attribute Section: gnu
File Attributes
  Tag_GNU_MIPS_ABI_FP: Hard float (double precision)

MIPS ABI Flags Version: 0

ISA: MIPS1
GPR size: 32
CPR1 size: 32
CPR2 size: 0
FP ABI: Hard float (double precision)
ISA Extension: None
ASEs:
	None
FLAGS 1: 00000000
FLAGS 2: 00000000
$ 

and I think this would be good to keep.  Obviously this will prevent 
`-mfp64' executables or DSOs from being loaded together with VDSO, but 
they are incompatible with the 32-bit FPU MIPS I processors have anyway, 
so no change in the semantics there.

 On the other hand there's still the issue what the compiler default will 
be for o32 in a non-r3k build -- which could be any of `-mfp32', `-mfpxx' 
or `-mfp64'.  Requesting `-mfpxx' should allow the greatest flexibility, 
but may not be supported as older compilers did not have it, so this would 
have to be added conditionally only, letting those older compilers retain 
their long-established `-mfp32' default.  I have an idea how to implement 
this part if we agree this is indeed the right direction.

 For the record: `-mfp32' has been there since forever, my little research 
indicating this commit:

Wed Jan 15 06:22:34 1992  Michael Meissner  (meissner at osf.org)

	* mips.h (MIPS_VERSION): Bump Meissner version # to 8.
	(fcmp_op): Add function declaration.
	(TARGET_FLOAT64): Support for -mfp64 switch to support the R4000 FR
	psw bit, which doubles the number of floating point registers.
	(TARGET_SWITCHES): Likewise.
	(HARD_REGNO_NREGS): Likewise.
	(CLASS_FCMP_OP): New bit for classifing floating point compares.
	(PREDICATE_CODES): Add cmp2_op and fcmp_op support.
[...]

which corresponds to GCC 2.0 released roughly a month afterwards, although 
the exact diff seems to have been lost in the mist of history.  The oldest 
version of `mips.h' in the modern GCC repository dates back to r297 from 
11 Feb 1992 where it already contains `-mfp32' support; obviously a sign 
of an inaccurate import of those old changes.  Anyway the option is surely 
always safe to use nowadays.

  Maciej

linux-mips-r3k-vdso-fp32.diff
Index: linux-sfr-decstation/arch/mips/vdso/Makefile
===================================================================
--- linux-sfr-decstation.orig/arch/mips/vdso/Makefile	2016-10-23 06:15:11.000000000 +0100
+++ linux-sfr-decstation/arch/mips/vdso/Makefile	2016-11-03 22:44:31.752065000 +0000
@@ -6,7 +6,8 @@ ccflags-vdso := \
 	$(filter -I%,$(KBUILD_CFLAGS)) \
 	$(filter -E%,$(KBUILD_CFLAGS)) \
 	$(filter -mmicromips,$(KBUILD_CFLAGS)) \
-	$(filter -march=%,$(KBUILD_CFLAGS))
+	$(filter -march=%,$(KBUILD_CFLAGS)) \
+	$(patsubst -march=%,-mfp32,$(filter -march=r3%,$(KBUILD_CFLAGS)))
 cflags-vdso := $(ccflags-vdso) \
 	$(filter -W%,$(filter-out -Wa$(comma)%,$(KBUILD_CFLAGS))) \
 	-O2 -g -fPIC -fno-strict-aliasing -fno-common -fno-builtin -G 0 \

Re: [PATCH] MIPS: VDSO: Always select -msoft-float

[Maciej W. Rozycki]

On Tue, 1 Nov 2016, Guenter Roeck wrote:

> > > Some toolchains fail to build mips images with the following build error.
> > > 
> > > arch/mips/vdso/gettimeofday.c:1:0: error: '-march=r3000' requires '-mfp32'
> > > 
> > > This is seen, for example, with the 'mipsel-linux-gnu-gcc (Debian 6.1.1-9)
> > > 6.1.1 20160705' toolchain as used by the 0Day build robot when building
> > > decstation_defconfig.
> > > 
> > > Comparison of compile flags suggests that the major difference is a missing
> > > '-soft-float', which is otherwise defined unconditionally.
> > > 
> > > Reported-by: kbuild test robot <fengguang.wu@intel.com>
> > > Cc: James Hogan <james.hogan@imgtec.com>
> > > Fixes: ebb5e78cc634 ("MIPS: Initial implementation of a VDSO")
> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > ---
[...]
> >  If you send me the generated assembly, i.e. `gettimeofday.s', that is 
> > causing you trouble, then I'll see if I can figure out what is going on 
> > here.  We may decide to paper a particularly nasty toolchain bug over from 
> 
> The problem is seen in 0Day builds, with the toolchain mentioned above.

 That does not tell me anything -- I have no idea how each of the 
toolchains out there has been configured.  I can only assess information
provided by the bug/patch submitter, and otherwise try guessing.

> I don't think that generated assembly is going to help, though, since the
> compiler fails to compile the code in the first place because, as it says,
> it doesn't like '-march=r3000' without '-mfp32'.

 Indeed I got that confused, even though there is supposed to be a mention 
in the compiler's diagnostics if a warning or error has originated from an 
external tool invoked.  Sorry about that.  The message itself could well 
have come from the assembler though as the same options are accepted by 
that tool.

 And indeed I have just guessed what the cause might be, that is the 
compiler must have been configured with an explicit `--with-fp-32=xx' or 
`--with-fp-32=64' option.

 Here's quite an extensive analysis of what these options do and why: 
<https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking>
and I note that there is a mention of a setup which would have avoided the 
situation we have now:

"Ideally any MIPS I arch/core will default to -mfp32 Any MIPS II -> 
MIPS32r2 arch/core will default to -mfpxx. However, this should be 
controlled via a configure time option to adjust the default ABI from O32 
FP32 to O32 FPXX (or O32 FP64 as needed). The new configure time option is 
--with-fp-32=[32|xx|64] and this affects the FP mode only when targetting 
the O32 ABI."

however this ideal arrangement, which I would expect a configuration 
option like `--with-fp-32=from-isa' (and a corresponding `-mfp=from-isa' 
compiler option) would handle, got lost/forgotten in the works somehow.

 Matthew has been the expert in this area and might be able to add some 
more -- Matthew?  Also shall we call it a compiler defect?  We seem to be 
getting more and more toolchain configurations which break the existing 
setups in a way requiring them to add more and more compiler options to 
Makefiles for preexisting targets even though the Makefiles were 
previously already prepared to choose the right compiler options for the 
target.  This seems the wrong way to go to me as it's causing people 
burden when upgrading their toolchains while it's supposed to be seamless 
(it's one aspect of maintaining backwards compatibility).

 Meanwhile your proposal may be indeed the way to go, or perhaps we could 
use a substitution rule like:

	$(patsubst -march=%,-mfp32,$(filter -march=r3%,$(KBUILD_CFLAGS)))

-- see the change below, it seems to work for me and the option is indeed 
passed with a `-march=r3000' build (as would with a `-march=r3900' build) 
and suppressed otherwise (such as with a `-march=mips32r2' build).  

 I have tried building `arch/mips/vdso/gettimeofday.s' to see how the 
module settings are chosen by the compiler in my successful build, and 
that produced an unexpected result in that the rule for .s targets is 
evidently broken for VDSO as different compiler flags were used from ones 
used for .o targets -- essentially the regular flags rather than the 
special VDSO ones (no `-fPIC', etc.).  So that looks like a bug to me to 
be fixed too.

 So instead I just checked the ABI annotation of a good .o object with 
`readelf':

$ readelf -A arch/mips/vdso/gettimeofday.o
Attribute Section: gnu
File Attributes
  Tag_GNU_MIPS_ABI_FP: Hard float (double precision)

MIPS ABI Flags Version: 0

ISA: MIPS1
GPR size: 32
CPR1 size: 32
CPR2 size: 0
FP ABI: Hard float (double precision)
ISA Extension: None
ASEs:
	None
FLAGS 1: 00000000
FLAGS 2: 00000000
$ 

and I think this would be good to keep.  Obviously this will prevent 
`-mfp64' executables or DSOs from being loaded together with VDSO, but 
they are incompatible with the 32-bit FPU MIPS I processors have anyway, 
so no change in the semantics there.

 On the other hand there's still the issue what the compiler default will 
be for o32 in a non-r3k build -- which could be any of `-mfp32', `-mfpxx' 
or `-mfp64'.  Requesting `-mfpxx' should allow the greatest flexibility, 
but may not be supported as older compilers did not have it, so this would 
have to be added conditionally only, letting those older compilers retain 
their long-established `-mfp32' default.  I have an idea how to implement 
this part if we agree this is indeed the right direction.

 For the record: `-mfp32' has been there since forever, my little research 
indicating this commit:

Wed Jan 15 06:22:34 1992  Michael Meissner  (meissner at osf.org)

	* mips.h (MIPS_VERSION): Bump Meissner version # to 8.
	(fcmp_op): Add function declaration.
	(TARGET_FLOAT64): Support for -mfp64 switch to support the R4000 FR
	psw bit, which doubles the number of floating point registers.
	(TARGET_SWITCHES): Likewise.
	(HARD_REGNO_NREGS): Likewise.
	(CLASS_FCMP_OP): New bit for classifing floating point compares.
	(PREDICATE_CODES): Add cmp2_op and fcmp_op support.
[...]

which corresponds to GCC 2.0 released roughly a month afterwards, although 
the exact diff seems to have been lost in the mist of history.  The oldest 
version of `mips.h' in the modern GCC repository dates back to r297 from 
11 Feb 1992 where it already contains `-mfp32' support; obviously a sign 
of an inaccurate import of those old changes.  Anyway the option is surely 
always safe to use nowadays.

  Maciej

linux-mips-r3k-vdso-fp32.diff
Index: linux-sfr-decstation/arch/mips/vdso/Makefile
===================================================================
--- linux-sfr-decstation.orig/arch/mips/vdso/Makefile	2016-10-23 06:15:11.000000000 +0100
+++ linux-sfr-decstation/arch/mips/vdso/Makefile	2016-11-03 22:44:31.752065000 +0000
@@ -6,7 +6,8 @@ ccflags-vdso := \
 	$(filter -I%,$(KBUILD_CFLAGS)) \
 	$(filter -E%,$(KBUILD_CFLAGS)) \
 	$(filter -mmicromips,$(KBUILD_CFLAGS)) \
-	$(filter -march=%,$(KBUILD_CFLAGS))
+	$(filter -march=%,$(KBUILD_CFLAGS)) \
+	$(patsubst -march=%,-mfp32,$(filter -march=r3%,$(KBUILD_CFLAGS)))
 cflags-vdso := $(ccflags-vdso) \
 	$(filter -W%,$(filter-out -Wa$(comma)%,$(KBUILD_CFLAGS))) \
 	-O2 -g -fPIC -fno-strict-aliasing -fno-common -fno-builtin -G 0 \

RE: [PATCH] MIPS: VDSO: Always select -msoft-float

[Matthew Fortune]

Maciej Rozycki <Maciej.Rozycki@imgtec.com> writes:
> On Tue, 1 Nov 2016, Guenter Roeck wrote:
> 
> > > > Some toolchains fail to build mips images with the following build
> error.
> > > >
> > > > arch/mips/vdso/gettimeofday.c:1:0: error: '-march=r3000' requires
> '-mfp32'
> > > >
> > > > This is seen, for example, with the 'mipsel-linux-gnu-gcc (Debian
> > > > 6.1.1-9)
> > > > 6.1.1 20160705' toolchain as used by the 0Day build robot when
> > > > building decstation_defconfig.
> > > >
> > > > Comparison of compile flags suggests that the major difference is
> > > > a missing '-soft-float', which is otherwise defined
> unconditionally.
> > > >
> > > > Reported-by: kbuild test robot <fengguang.wu@intel.com>
> > > > Cc: James Hogan <james.hogan@imgtec.com>
> > > > Fixes: ebb5e78cc634 ("MIPS: Initial implementation of a VDSO")
> > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > > ---
> [...]
> > >  If you send me the generated assembly, i.e. `gettimeofday.s', that
> > > is causing you trouble, then I'll see if I can figure out what is
> > > going on here.  We may decide to paper a particularly nasty
> > > toolchain bug over from
> >
> > The problem is seen in 0Day builds, with the toolchain mentioned
> above.
> 
>  That does not tell me anything -- I have no idea how each of the
> toolchains out there has been configured.  I can only assess information
> provided by the bug/patch submitter, and otherwise try guessing.
> 
> > I don't think that generated assembly is going to help, though, since
> > the compiler fails to compile the code in the first place because, as
> > it says, it doesn't like '-march=r3000' without '-mfp32'.
> 
>  Indeed I got that confused, even though there is supposed to be a
> mention in the compiler's diagnostics if a warning or error has
> originated from an external tool invoked.  Sorry about that.  The
> message itself could well have come from the assembler though as the
> same options are accepted by that tool.
> 
>  And indeed I have just guessed what the cause might be, that is the
> compiler must have been configured with an explicit `--with-fp-32=xx' or
> `--with-fp-32=64' option.

This is almost certainly using --with-fp-32=xx

>  Here's quite an extensive analysis of what these options do and why:
> <https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-
> _FR0_and_FR1_Interlinking>
> and I note that there is a mention of a setup which would have avoided
> the situation we have now:
> 
> "Ideally any MIPS I arch/core will default to -mfp32 Any MIPS II ->
> MIPS32r2 arch/core will default to -mfpxx. However, this should be
> controlled via a configure time option to adjust the default ABI from
> O32
> FP32 to O32 FPXX (or O32 FP64 as needed). The new configure time option
> is --with-fp-32=[32|xx|64] and this affects the FP mode only when
> targetting the O32 ABI."
> 
> however this ideal arrangement, which I would expect a configuration
> option like `--with-fp-32=from-isa' (and a corresponding `-mfp=from-isa'
> compiler option) would handle, got lost/forgotten in the works somehow.

The intention of these comments was about vendor tool configurations that
often do more option inference than standard tools. We could do a
--with-fp-32=from-isa but that would not be used by debian anyway as it
does not meet the goal of reliably generating FP compatible code. There are
also subjective answers to the 'from-isa' question as mips32r1 could be
fp32 or fpxx depending on what the toolchain owner wanted to achieve.

>  Matthew has been the expert in this area and might be able to add some
> more -- Matthew?  Also shall we call it a compiler defect?  We seem to
> be getting more and more toolchain configurations which break the
> existing setups in a way requiring them to add more and more compiler
> options to Makefiles for preexisting targets even though the Makefiles
> were previously already prepared to choose the right compiler options
> for the target.  This seems the wrong way to go to me as it's causing
> people burden when upgrading their toolchains while it's supposed to be
> seamless (it's one aspect of maintaining backwards compatibility).

I agree we don't want to take decisions lightly that end up breaking
builds but we should not shy away from them.

In this specific case a kernel for a MIPS I arch is being built from a
toolchain in a distribution that requires MIPS II as a minimum and is
moving towards requiring MIPS32R2 as a minimum. The decisions made when
configuring this toolchain are there to support the intended users so
as we move forward there will be some things that stop working as
before (by default).

Given that then I think this is reasonable fall-out. The options are:
Build this configuration of the kernel using an older distribution,
Build a custom toolchain, Update the kernel to cope with this new
configuration.

In general I think it is reasonable for people to request continued
support for architectures or configurations that are older and have a
smaller user-base but such users may have to invest a little more effort
in configuring as focus is moved to support the wider used variants.

We are also talking about a relatively new feature in the kernel and I
would go so far as to say this is a great example of why trying to
support every MIPS variant is futile; even those with the greatest
knowledge of the architecture, toolchain and history cannot make it
work in every case.

An even simpler solution to this, and better for long term support, is
to disable the vDSO feature for architectures older than 'x' where we
could choose MIPS II or even MIPS32R2 depending on how much simpler
we would like life to be.

>  Meanwhile your proposal may be indeed the way to go, or perhaps we
> could use a substitution rule like:
> 
> 	$(patsubst -march=%,-mfp32,$(filter -march=r3%,$(KBUILD_CFLAGS)))
> 
> -- see the change below, it seems to work for me and the option is
> indeed passed with a `-march=r3000' build (as would with a `-
> march=r3900' build) and suppressed otherwise (such as with a `-
> march=mips32r2' build).

Does the kernel build both hard and soft float variants of the vDSO? When
originally discussing the design for it I don't think it was supposed to.

I believe the .MIPS.abiflags section for the vDSO is overwritten to
claim it is an 'any' FP ABI so that it is compatible with all possible
userland variants so it better not contain any floating point instructions.

I'd therefore suggest using -msoft-float unconditionally is the way to go
when building the vDSO even if it gets disabled for MIPS I completely as
well.

>  I have tried building `arch/mips/vdso/gettimeofday.s' to see how the
> module settings are chosen by the compiler in my successful build, and
> that produced an unexpected result in that the rule for .s targets is
> evidently broken for VDSO as different compiler flags were used from
> ones used for .o targets -- essentially the regular flags rather than
> the special VDSO ones (no `-fPIC', etc.).  So that looks like a bug to
> me to be fixed too.
> 
>  So instead I just checked the ABI annotation of a good .o object with
> `readelf':
> 
> $ readelf -A arch/mips/vdso/gettimeofday.o Attribute Section: gnu File
> Attributes
>   Tag_GNU_MIPS_ABI_FP: Hard float (double precision)
> 
> MIPS ABI Flags Version: 0
> 
> ISA: MIPS1
> GPR size: 32
> CPR1 size: 32
> CPR2 size: 0
> FP ABI: Hard float (double precision)
> ISA Extension: None
> ASEs:
> 	None
> FLAGS 1: 00000000
> FLAGS 2: 00000000
> $
> 
> and I think this would be good to keep.  Obviously this will prevent `-
> mfp64' executables or DSOs from being loaded together with VDSO, but
> they are incompatible with the 32-bit FPU MIPS I processors have anyway,
> so no change in the semantics there.
> 
>  On the other hand there's still the issue what the compiler default
> will be for o32 in a non-r3k build -- which could be any of `-mfp32', `-
> mfpxx'
> or `-mfp64'.  Requesting `-mfpxx' should allow the greatest flexibility,
> but may not be supported as older compilers did not have it, so this
> would have to be added conditionally only, letting those older compilers
> retain their long-established `-mfp32' default.  I have an idea how to
> implement this part if we agree this is indeed the right direction.

As above, unless absolutely critical to have floating point code then
the vDSO should just avoid all FP related issues and build soft-float.

Thanks,
Matthew

>  For the record: `-mfp32' has been there since forever, my little
> research indicating this commit:
> 
> Wed Jan 15 06:22:34 1992  Michael Meissner  (meissner at osf.org)
> 
> 	* mips.h (MIPS_VERSION): Bump Meissner version # to 8.
> 	(fcmp_op): Add function declaration.
> 	(TARGET_FLOAT64): Support for -mfp64 switch to support the R4000 FR
> 	psw bit, which doubles the number of floating point registers.
> 	(TARGET_SWITCHES): Likewise.
> 	(HARD_REGNO_NREGS): Likewise.
> 	(CLASS_FCMP_OP): New bit for classifing floating point compares.
> 	(PREDICATE_CODES): Add cmp2_op and fcmp_op support.
> [...]
> 
> which corresponds to GCC 2.0 released roughly a month afterwards,
> although the exact diff seems to have been lost in the mist of history.
> The oldest version of `mips.h' in the modern GCC repository dates back
> to r297 from
> 11 Feb 1992 where it already contains `-mfp32' support; obviously a sign
> of an inaccurate import of those old changes.  Anyway the option is
> surely always safe to use nowadays.
> 
>   Maciej
> 
> linux-mips-r3k-vdso-fp32.diff
> Index: linux-sfr-decstation/arch/mips/vdso/Makefile
> ===================================================================
> --- linux-sfr-decstation.orig/arch/mips/vdso/Makefile	2016-10-23
> 06:15:11.000000000 +0100
> +++ linux-sfr-decstation/arch/mips/vdso/Makefile	2016-11-03
> 22:44:31.752065000 +0000
> @@ -6,7 +6,8 @@ ccflags-vdso := \
>  	$(filter -I%,$(KBUILD_CFLAGS)) \
>  	$(filter -E%,$(KBUILD_CFLAGS)) \
>  	$(filter -mmicromips,$(KBUILD_CFLAGS)) \
> -	$(filter -march=%,$(KBUILD_CFLAGS))
> +	$(filter -march=%,$(KBUILD_CFLAGS)) \
> +	$(patsubst -march=%,-mfp32,$(filter -march=r3%,$(KBUILD_CFLAGS)))
>  cflags-vdso := $(ccflags-vdso) \
>  	$(filter -W%,$(filter-out -Wa$(comma)%,$(KBUILD_CFLAGS))) \
>  	-O2 -g -fPIC -fno-strict-aliasing -fno-common -fno-builtin -G 0 \

Re: [PATCH] MIPS: VDSO: Always select -msoft-float

[Guenter Roeck]

On Fri, Nov 04, 2016 at 01:42:40PM +0000, Matthew Fortune wrote:
> Maciej Rozycki <Maciej.Rozycki@imgtec.com> writes:
> > On Tue, 1 Nov 2016, Guenter Roeck wrote:
> > 
> > > > > Some toolchains fail to build mips images with the following build
> > error.
> > > > >
> > > > > arch/mips/vdso/gettimeofday.c:1:0: error: '-march=r3000' requires
> > '-mfp32'
> > > > >
> > > > > This is seen, for example, with the 'mipsel-linux-gnu-gcc (Debian
> > > > > 6.1.1-9)
> > > > > 6.1.1 20160705' toolchain as used by the 0Day build robot when
> > > > > building decstation_defconfig.
> > > > >
> > > > > Comparison of compile flags suggests that the major difference is
> > > > > a missing '-soft-float', which is otherwise defined
> > unconditionally.
> > > > >
> > > > > Reported-by: kbuild test robot <fengguang.wu@intel.com>
> > > > > Cc: James Hogan <james.hogan@imgtec.com>
> > > > > Fixes: ebb5e78cc634 ("MIPS: Initial implementation of a VDSO")
> > > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > > > ---
> > [...]
> > > >  If you send me the generated assembly, i.e. `gettimeofday.s', that
> > > > is causing you trouble, then I'll see if I can figure out what is
> > > > going on here.  We may decide to paper a particularly nasty
> > > > toolchain bug over from
> > >
> > > The problem is seen in 0Day builds, with the toolchain mentioned
> > above.
> > 
> >  That does not tell me anything -- I have no idea how each of the
> > toolchains out there has been configured.  I can only assess information
> > provided by the bug/patch submitter, and otherwise try guessing.
> > 
> > > I don't think that generated assembly is going to help, though, since
> > > the compiler fails to compile the code in the first place because, as
> > > it says, it doesn't like '-march=r3000' without '-mfp32'.
> > 
> >  Indeed I got that confused, even though there is supposed to be a
> > mention in the compiler's diagnostics if a warning or error has
> > originated from an external tool invoked.  Sorry about that.  The
> > message itself could well have come from the assembler though as the
> > same options are accepted by that tool.
> > 
> >  And indeed I have just guessed what the cause might be, that is the
> > compiler must have been configured with an explicit `--with-fp-32=xx' or
> > `--with-fp-32=64' option.
> 
> This is almost certainly using --with-fp-32=xx
> 
> >  Here's quite an extensive analysis of what these options do and why:
> > <https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-
> > _FR0_and_FR1_Interlinking>
> > and I note that there is a mention of a setup which would have avoided
> > the situation we have now:
> > 
> > "Ideally any MIPS I arch/core will default to -mfp32 Any MIPS II ->
> > MIPS32r2 arch/core will default to -mfpxx. However, this should be
> > controlled via a configure time option to adjust the default ABI from
> > O32
> > FP32 to O32 FPXX (or O32 FP64 as needed). The new configure time option
> > is --with-fp-32=[32|xx|64] and this affects the FP mode only when
> > targetting the O32 ABI."
> > 
> > however this ideal arrangement, which I would expect a configuration
> > option like `--with-fp-32=from-isa' (and a corresponding `-mfp=from-isa'
> > compiler option) would handle, got lost/forgotten in the works somehow.
> 
> The intention of these comments was about vendor tool configurations that
> often do more option inference than standard tools. We could do a
> --with-fp-32=from-isa but that would not be used by debian anyway as it
> does not meet the goal of reliably generating FP compatible code. There are
> also subjective answers to the 'from-isa' question as mips32r1 could be
> fp32 or fpxx depending on what the toolchain owner wanted to achieve.
> 
> >  Matthew has been the expert in this area and might be able to add some
> > more -- Matthew?  Also shall we call it a compiler defect?  We seem to
> > be getting more and more toolchain configurations which break the
> > existing setups in a way requiring them to add more and more compiler
> > options to Makefiles for preexisting targets even though the Makefiles
> > were previously already prepared to choose the right compiler options
> > for the target.  This seems the wrong way to go to me as it's causing
> > people burden when upgrading their toolchains while it's supposed to be
> > seamless (it's one aspect of maintaining backwards compatibility).
> 
> I agree we don't want to take decisions lightly that end up breaking
> builds but we should not shy away from them.
> 
> In this specific case a kernel for a MIPS I arch is being built from a
> toolchain in a distribution that requires MIPS II as a minimum and is
> moving towards requiring MIPS32R2 as a minimum. The decisions made when
> configuring this toolchain are there to support the intended users so
> as we move forward there will be some things that stop working as
> before (by default).
> 
> Given that then I think this is reasonable fall-out. The options are:
> Build this configuration of the kernel using an older distribution,
> Build a custom toolchain, Update the kernel to cope with this new
> configuration.
> 
> In general I think it is reasonable for people to request continued
> support for architectures or configurations that are older and have a
> smaller user-base but such users may have to invest a little more effort
> in configuring as focus is moved to support the wider used variants.
> 
> We are also talking about a relatively new feature in the kernel and I
> would go so far as to say this is a great example of why trying to
> support every MIPS variant is futile; even those with the greatest
> knowledge of the architecture, toolchain and history cannot make it
> work in every case.
> 
> An even simpler solution to this, and better for long term support, is
> to disable the vDSO feature for architectures older than 'x' where we
> could choose MIPS II or even MIPS32R2 depending on how much simpler
> we would like life to be.
> 
> >  Meanwhile your proposal may be indeed the way to go, or perhaps we
> > could use a substitution rule like:
> > 
> > 	$(patsubst -march=%,-mfp32,$(filter -march=r3%,$(KBUILD_CFLAGS)))
> > 
> > -- see the change below, it seems to work for me and the option is
> > indeed passed with a `-march=r3000' build (as would with a `-
> > march=r3900' build) and suppressed otherwise (such as with a `-
> > march=mips32r2' build).
> 
> Does the kernel build both hard and soft float variants of the vDSO? When
> originally discussing the design for it I don't think it was supposed to.
> 
> I believe the .MIPS.abiflags section for the vDSO is overwritten to
> claim it is an 'any' FP ABI so that it is compatible with all possible
> userland variants so it better not contain any floating point instructions.
> 
> I'd therefore suggest using -msoft-float unconditionally is the way to go
> when building the vDSO even if it gets disabled for MIPS I completely as
> well.
> 
> >  I have tried building `arch/mips/vdso/gettimeofday.s' to see how the
> > module settings are chosen by the compiler in my successful build, and
> > that produced an unexpected result in that the rule for .s targets is
> > evidently broken for VDSO as different compiler flags were used from
> > ones used for .o targets -- essentially the regular flags rather than
> > the special VDSO ones (no `-fPIC', etc.).  So that looks like a bug to
> > me to be fixed too.
> > 
> >  So instead I just checked the ABI annotation of a good .o object with
> > `readelf':
> > 
> > $ readelf -A arch/mips/vdso/gettimeofday.o Attribute Section: gnu File
> > Attributes
> >   Tag_GNU_MIPS_ABI_FP: Hard float (double precision)
> > 
> > MIPS ABI Flags Version: 0
> > 
> > ISA: MIPS1
> > GPR size: 32
> > CPR1 size: 32
> > CPR2 size: 0
> > FP ABI: Hard float (double precision)
> > ISA Extension: None
> > ASEs:
> > 	None
> > FLAGS 1: 00000000
> > FLAGS 2: 00000000
> > $
> > 
> > and I think this would be good to keep.  Obviously this will prevent `-
> > mfp64' executables or DSOs from being loaded together with VDSO, but
> > they are incompatible with the 32-bit FPU MIPS I processors have anyway,
> > so no change in the semantics there.
> > 
> >  On the other hand there's still the issue what the compiler default
> > will be for o32 in a non-r3k build -- which could be any of `-mfp32', `-
> > mfpxx'
> > or `-mfp64'.  Requesting `-mfpxx' should allow the greatest flexibility,
> > but may not be supported as older compilers did not have it, so this
> > would have to be added conditionally only, letting those older compilers
> > retain their long-established `-mfp32' default.  I have an idea how to
> > implement this part if we agree this is indeed the right direction.
> 
> As above, unless absolutely critical to have floating point code then
> the vDSO should just avoid all FP related issues and build soft-float.
> 

FWIW, my logic was quite simple: The rest of the kernel builds with
-msoft-float, thus vDSO should do the same. Of course, I don't know the
entire context, so there may well be a reason to handle it differently
than the rest of the kernel.

If building with -msoft-float is not acceptable for vDSO, maybe there
is a means to detect if the toolchain supports the problematic case,
ie '-march=r3000' without '-mfp32', and not build vDSO in that case ?

Anyway, isn't the kernel supposed to not use floating point operations
in the first place ? Is this different for vDSO ?

Thanks,
Guenter

> Thanks,
> Matthew
> 
> >  For the record: `-mfp32' has been there since forever, my little
> > research indicating this commit:
> > 
> > Wed Jan 15 06:22:34 1992  Michael Meissner  (meissner at osf.org)
> > 
> > 	* mips.h (MIPS_VERSION): Bump Meissner version # to 8.
> > 	(fcmp_op): Add function declaration.
> > 	(TARGET_FLOAT64): Support for -mfp64 switch to support the R4000 FR
> > 	psw bit, which doubles the number of floating point registers.
> > 	(TARGET_SWITCHES): Likewise.
> > 	(HARD_REGNO_NREGS): Likewise.
> > 	(CLASS_FCMP_OP): New bit for classifing floating point compares.
> > 	(PREDICATE_CODES): Add cmp2_op and fcmp_op support.
> > [...]
> > 
> > which corresponds to GCC 2.0 released roughly a month afterwards,
> > although the exact diff seems to have been lost in the mist of history.
> > The oldest version of `mips.h' in the modern GCC repository dates back
> > to r297 from
> > 11 Feb 1992 where it already contains `-mfp32' support; obviously a sign
> > of an inaccurate import of those old changes.  Anyway the option is
> > surely always safe to use nowadays.
> > 
> >   Maciej
> > 
> > linux-mips-r3k-vdso-fp32.diff
> > Index: linux-sfr-decstation/arch/mips/vdso/Makefile
> > ===================================================================
> > --- linux-sfr-decstation.orig/arch/mips/vdso/Makefile	2016-10-23
> > 06:15:11.000000000 +0100
> > +++ linux-sfr-decstation/arch/mips/vdso/Makefile	2016-11-03
> > 22:44:31.752065000 +0000
> > @@ -6,7 +6,8 @@ ccflags-vdso := \
> >  	$(filter -I%,$(KBUILD_CFLAGS)) \
> >  	$(filter -E%,$(KBUILD_CFLAGS)) \
> >  	$(filter -mmicromips,$(KBUILD_CFLAGS)) \
> > -	$(filter -march=%,$(KBUILD_CFLAGS))
> > +	$(filter -march=%,$(KBUILD_CFLAGS)) \
> > +	$(patsubst -march=%,-mfp32,$(filter -march=r3%,$(KBUILD_CFLAGS)))
> >  cflags-vdso := $(ccflags-vdso) \
> >  	$(filter -W%,$(filter-out -Wa$(comma)%,$(KBUILD_CFLAGS))) \
> >  	-O2 -g -fPIC -fno-strict-aliasing -fno-common -fno-builtin -G 0 \

Re: [PATCH] MIPS: VDSO: Always select -msoft-float

[Maciej W. Rozycki]

On Fri, 4 Nov 2016, Guenter Roeck wrote:

> > As above, unless absolutely critical to have floating point code then
> > the vDSO should just avoid all FP related issues and build soft-float.
> 
> FWIW, my logic was quite simple: The rest of the kernel builds with
> -msoft-float, thus vDSO should do the same. Of course, I don't know the
> entire context, so there may well be a reason to handle it differently
> than the rest of the kernel.

 VDSO is not a part of the kernel, it's user mode code, made visible in 
the user virtual memory, and implicitly loaded along the rest of the DSOs 
on program startup by the dynamic loader (ld.so).  It has to be PIC for 
that reason, too, causing all the hassle we had with making it such that 
it does not need a GOT.

> Anyway, isn't the kernel supposed to not use floating point operations
> in the first place ? Is this different for vDSO ?

 This code is executed in the user mode so while floating-point code may 
not be needed there, not at least right now, there's actually nothing 
which should stop us from from adding some should such a need arise.

  Maciej

Re: [PATCH] MIPS: VDSO: Always select -msoft-float

[Guenter Roeck]

On Fri, Nov 04, 2016 at 04:09:37PM +0000, Maciej W. Rozycki wrote:
> On Fri, 4 Nov 2016, Guenter Roeck wrote:
> 
> > > As above, unless absolutely critical to have floating point code then
> > > the vDSO should just avoid all FP related issues and build soft-float.
> > 
> > FWIW, my logic was quite simple: The rest of the kernel builds with
> > -msoft-float, thus vDSO should do the same. Of course, I don't know the
> > entire context, so there may well be a reason to handle it differently
> > than the rest of the kernel.
> 
>  VDSO is not a part of the kernel, it's user mode code, made visible in 
> the user virtual memory, and implicitly loaded along the rest of the DSOs 
> on program startup by the dynamic loader (ld.so).  It has to be PIC for 
> that reason, too, causing all the hassle we had with making it such that 
> it does not need a GOT.
> 
> > Anyway, isn't the kernel supposed to not use floating point operations
> > in the first place ? Is this different for vDSO ?
> 
>  This code is executed in the user mode so while floating-point code may 
> not be needed there, not at least right now, there's actually nothing 
> which should stop us from from adding some should such a need arise.
> 
Just for my understanding - so the code is compiled with the kernel and part
of the kernel source but executed in user mode ?

If you ever add real floating point code, doesn't that also mean that you'll
have to implement the necessary linker helper functions or wrappers (such
as the wrappers needed for 64-bit integer divide operations in 32 bit code) ?

Thanks,
Guenter

RE: [PATCH] MIPS: VDSO: Always select -msoft-float

[Matthew Fortune]

Maciej Rozycki <Maciej.Rozycki@imgtec.com> writes:
> On Fri, 4 Nov 2016, Guenter Roeck wrote:
> 
> > > As above, unless absolutely critical to have floating point code
> > > then the vDSO should just avoid all FP related issues and build
> soft-float.
...
> > Anyway, isn't the kernel supposed to not use floating point operations
> > in the first place ? Is this different for vDSO ?
> 
>  This code is executed in the user mode so while floating-point code may
> not be needed there, not at least right now, there's actually nothing
> which should stop us from from adding some should such a need arise.

Indeed. For now though the switch to -msoft-float is the simplest solution
isn't it?

Matthew

Re: [PATCH] MIPS: VDSO: Always select -msoft-float

[Maciej W. Rozycki]

On Fri, 4 Nov 2016, Guenter Roeck wrote:

> >  This code is executed in the user mode so while floating-point code may 
> > not be needed there, not at least right now, there's actually nothing 
> > which should stop us from from adding some should such a need arise.
> > 
> Just for my understanding - so the code is compiled with the kernel and part
> of the kernel source but executed in user mode ?

 Yes, that's correct.

 The idea behind VDSO is to export some kernel data to the userland in a 
way making it possible to avoid the considerable overhead of making a 
syscall, i.e. the whole dance required to switch to the kernel mode, make 
the necessary arrangements there for kernel mode execution such as stack 
switching (see the SAVE_SOME macro), actually retrieve the data requested, 
undo the kernel mode execution arrangements (RESTORE_SOME) and finally 
resume user mode execution.

 So instead a page is mapped by the kernel in the user virtual memory with 
designated entry points comprising the public API and the actual 
implementation which retrieves the data requested in a varying way, 
depending on the kernel configuration, hardware features, etc., so it is 
tightly coupled with the kernel and has to be built along it.  These entry 
points are then used by the C library instead of their corresponding 
syscalls.

 A good use example is a replacement for gettimeofday(2).  This syscall 
retrieves a tiny amount of data which is frequently requested e.g. by X 
servers which want to individually timestamp their events.  So the gain 
from avoiding making this syscall and instead retrieving the data 
requested straight in the user mode is enormous.

> If you ever add real floating point code, doesn't that also mean that you'll
> have to implement the necessary linker helper functions or wrappers (such
> as the wrappers needed for 64-bit integer divide operations in 32 bit code) ?

 No, you could just link the VDSO with `-lgcc' instead and get all the 
necessary bits from there, as usually in user code.  Although if building 
for compat ABIs as well you'd have to ensure you have GCC libraries built 
and installed for all the ABIs required, which in turn depends on the 
configuration chosen for the compiler at its build time.  Not a problem 
right now though as we don't need any of this stuff.

 HTH,

  Maciej

Re: [PATCH] MIPS: VDSO: Always select -msoft-float

[Guenter Roeck]

On Fri, Nov 04, 2016 at 04:55:05PM +0000, Matthew Fortune wrote:
> Maciej Rozycki <Maciej.Rozycki@imgtec.com> writes:
> > On Fri, 4 Nov 2016, Guenter Roeck wrote:
> > 
> > > > As above, unless absolutely critical to have floating point code
> > > > then the vDSO should just avoid all FP related issues and build
> > soft-float.
> ...
> > > Anyway, isn't the kernel supposed to not use floating point operations
> > > in the first place ? Is this different for vDSO ?
> > 
> >  This code is executed in the user mode so while floating-point code may
> > not be needed there, not at least right now, there's actually nothing
> > which should stop us from from adding some should such a need arise.
> 
> Indeed. For now though the switch to -msoft-float is the simplest solution
> isn't it?
> 
One should think so, especially since 1) the code has to be built as part of the
kernel build anyway, and 2) there would be other implications besides just
adding floating point operations into the kernel (see the other e-mail).
Overall I am glad that I don't have to maintain this code.

Either case, it would be great if we can come up with a solution to avoid build
failures due to toolchain configurations. If that means that vDSO won't be
compilable with debian toolchains, so be it. After all, that would not be
different to the current situation, only it would be official instead of causing
build failures.

Thanks,
Guenter

RE: [PATCH] MIPS: VDSO: Always select -msoft-float

[Maciej W. Rozycki]

On Fri, 4 Nov 2016, Matthew Fortune wrote:

> >  This code is executed in the user mode so while floating-point code may
> > not be needed there, not at least right now, there's actually nothing
> > which should stop us from from adding some should such a need arise.
> 
> Indeed. For now though the switch to -msoft-float is the simplest solution
> isn't it?

 As I previously noted I am leaning towards accepting this solution, but 
please let me do some further research before I answer your question.  
I'll reply to your original response when I am ready.

  Maciej

RE: [PATCH] MIPS: VDSO: Always select -msoft-float

[Maciej W. Rozycki]

On Fri, 4 Nov 2016, Matthew Fortune wrote:

> > > I don't think that generated assembly is going to help, though, since
> > > the compiler fails to compile the code in the first place because, as
> > > it says, it doesn't like '-march=r3000' without '-mfp32'.
> > 
> >  Indeed I got that confused, even though there is supposed to be a
> > mention in the compiler's diagnostics if a warning or error has
> > originated from an external tool invoked.  Sorry about that.  The
> > message itself could well have come from the assembler though as the
> > same options are accepted by that tool.
> > 
> >  And indeed I have just guessed what the cause might be, that is the
> > compiler must have been configured with an explicit `--with-fp-32=xx' or
> > `--with-fp-32=64' option.
> 
> This is almost certainly using --with-fp-32=xx

 Thanks for reconfirming.

> >  Here's quite an extensive analysis of what these options do and why:
> > <https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-
> > _FR0_and_FR1_Interlinking>
> > and I note that there is a mention of a setup which would have avoided
> > the situation we have now:
> > 
> > "Ideally any MIPS I arch/core will default to -mfp32 Any MIPS II ->
> > MIPS32r2 arch/core will default to -mfpxx. However, this should be
> > controlled via a configure time option to adjust the default ABI from
> > O32
> > FP32 to O32 FPXX (or O32 FP64 as needed). The new configure time option
> > is --with-fp-32=[32|xx|64] and this affects the FP mode only when
> > targetting the O32 ABI."
> > 
> > however this ideal arrangement, which I would expect a configuration
> > option like `--with-fp-32=from-isa' (and a corresponding `-mfp=from-isa'
> > compiler option) would handle, got lost/forgotten in the works somehow.
> 
> The intention of these comments was about vendor tool configurations that
> often do more option inference than standard tools. We could do a
> --with-fp-32=from-isa but that would not be used by debian anyway as it
> does not meet the goal of reliably generating FP compatible code. There are
> also subjective answers to the 'from-isa' question as mips32r1 could be
> fp32 or fpxx depending on what the toolchain owner wanted to achieve.

 Why?

 With the use of `--with-fp-32=xx' the lone effect is a requirement for 
`-march=mips1' (or an equivalent CPU setting) compilations to pass an 
explicit `-mfp32' option or otherwise they fail.  Likewise you need to 
pass `-mfp64' for `-march=mips32r6' compilations.

 The effect on `-march=mips32' is I think irrelevant because it can just 
use `-mfpxx' as being MIPS II compatible the ISA does support LDC1/SDC1 
instructions unconditionally, even though the FPU implementation option is 
limited to 32 bits (not that any MIPS32r1 FPU hardware exist anyway 
AFAIK).  So we can just document the intent for `from-isa' to be maximum 
flexibility (link/load-time compatibility) and explicitly state that for 
`-march=mips32' `from-isa' means `xx'.

 By having `--with-fp-32=from-isa' you'd keep the current `-mfpxx' default 
for the currently working ISAs whereas those which now fail to build would 
not require the user to stick extra options which are implied by the ISA 
constraints anyway.  So you'd have a consistent compatible code unless the 
ISA contradicts it.

 Debian should not care as it's supposed to build all the packages using 
the same `-march=' setting (be it an ISA or CPU); presumably also wired as 
the default by using `--with-arch=' or `--with-arch-32='/`--with-arch-64=' 
as applicable.  Unless there's a packaging error that is, but I don't 
think checking for such a situation should be addressed in the compiler 
which is a generic tool.

> >  Matthew has been the expert in this area and might be able to add some
> > more -- Matthew?  Also shall we call it a compiler defect?  We seem to
> > be getting more and more toolchain configurations which break the
> > existing setups in a way requiring them to add more and more compiler
> > options to Makefiles for preexisting targets even though the Makefiles
> > were previously already prepared to choose the right compiler options
> > for the target.  This seems the wrong way to go to me as it's causing
> > people burden when upgrading their toolchains while it's supposed to be
> > seamless (it's one aspect of maintaining backwards compatibility).
> 
> I agree we don't want to take decisions lightly that end up breaking
> builds but we should not shy away from them.
> 
> In this specific case a kernel for a MIPS I arch is being built from a
> toolchain in a distribution that requires MIPS II as a minimum and is
> moving towards requiring MIPS32R2 as a minimum. The decisions made when
> configuring this toolchain are there to support the intended users so
> as we move forward there will be some things that stop working as
> before (by default).

 The Linux kernel has always been considered a special compilation case, 
starting from that you use a hosted compiler configuration to build a 
freestanding (bare-metal) program that the Linux kernel is.  So we have 
always strived to support building the kernel for ISAs and ABIs different 
from ones configured for userland apps, so that people do not have to 
struggle and get a separate bare-metal cross-compiler just to be able to 
build the kernel.  I think we want to maintain this situation; at least 
this is something I am going to strongly defend myself and neither I have 
heard of anyone else actually wanting to change it.

> Given that then I think this is reasonable fall-out. The options are:
> Build this configuration of the kernel using an older distribution,
> Build a custom toolchain, Update the kernel to cope with this new
> configuration.
> 
> In general I think it is reasonable for people to request continued
> support for architectures or configurations that are older and have a
> smaller user-base but such users may have to invest a little more effort
> in configuring as focus is moved to support the wider used variants.
> 
> We are also talking about a relatively new feature in the kernel and I
> would go so far as to say this is a great example of why trying to
> support every MIPS variant is futile; even those with the greatest
> knowledge of the architecture, toolchain and history cannot make it
> work in every case.

 That I think is a matter of resource availability rather than an inherent 
problem with the architecture.

> An even simpler solution to this, and better for long term support, is
> to disable the vDSO feature for architectures older than 'x' where we
> could choose MIPS II or even MIPS32R2 depending on how much simpler
> we would like life to be.

 That might well be an option, for instance because we don't have the CP0 
Count register anyway below MIPS III and have no RDHWR to read it from the 
user mode before MIPSr2 either, and also we have no XI bit in the TLB 
across these older implementations (the earliest incarnation of XI is 4KS 
or MIPSr1+SmartMIPS), so there's no benefit from avoiding signal return 
stack trampolines.

 OTOH `clock_gettime' can still see benefit with its coarse modes, 
although I believe they are not as frequently used.

> >  Meanwhile your proposal may be indeed the way to go, or perhaps we
> > could use a substitution rule like:
> > 
> > 	$(patsubst -march=%,-mfp32,$(filter -march=r3%,$(KBUILD_CFLAGS)))
> > 
> > -- see the change below, it seems to work for me and the option is
> > indeed passed with a `-march=r3000' build (as would with a `-
> > march=r3900' build) and suppressed otherwise (such as with a `-
> > march=mips32r2' build).
> 
> Does the kernel build both hard and soft float variants of the vDSO? When
> originally discussing the design for it I don't think it was supposed to.
> 
> I believe the .MIPS.abiflags section for the vDSO is overwritten to
> claim it is an 'any' FP ABI so that it is compatible with all possible
> userland variants so it better not contain any floating point instructions.
> 
> I'd therefore suggest using -msoft-float unconditionally is the way to go
> when building the vDSO even if it gets disabled for MIPS I completely as
> well.

 Indeed, the linker script generated from arch/mips/vdso/vdso.lds.S and 
used to link the embedded image of the vDSO strips any original input 
`.MIPS.abiflags' sections produced by GAS with the object files and then 
code in arch/mips/vdso/genvdso.c postprocesses the image generated, 
substituting hardcoded data from arch/mips/vdso/elf.S as the ultimate 
contents of `.MIPS.abiflags'.  So whether we use `-msoft-float' or any 
other FP ABI selection option in compilation, it will ultimately be 
substituted with what `-mno-float' would produce.  I wasn't aware of this 
arrangement, so thanks for the mention of it.

 Also I've looked through startup code of the dynamic loader and static 
libc, and it appears to me the vDSO is accepted as it is by the generic 
`setup_vdso' handler in elf/setup-vdso.h, based solely on the presence of 
an auxiliary vector's SYSINFO_EHDR entry and no further machine-specific 
checks, indeed as I previously suspected.

> >  I have tried building `arch/mips/vdso/gettimeofday.s' to see how the
> > module settings are chosen by the compiler in my successful build, and
> > that produced an unexpected result in that the rule for .s targets is
> > evidently broken for VDSO as different compiler flags were used from
> > ones used for .o targets -- essentially the regular flags rather than
> > the special VDSO ones (no `-fPIC', etc.).  So that looks like a bug to
> > me to be fixed too.
> > 
> >  So instead I just checked the ABI annotation of a good .o object with
> > `readelf':
> > 
> > $ readelf -A arch/mips/vdso/gettimeofday.o Attribute Section: gnu File
> > Attributes
> >   Tag_GNU_MIPS_ABI_FP: Hard float (double precision)
> > 
> > MIPS ABI Flags Version: 0
> > 
> > ISA: MIPS1
> > GPR size: 32
> > CPR1 size: 32
> > CPR2 size: 0
> > FP ABI: Hard float (double precision)
> > ISA Extension: None
> > ASEs:
> > 	None
> > FLAGS 1: 00000000
> > FLAGS 2: 00000000
> > $
> > 
> > and I think this would be good to keep.  Obviously this will prevent `-
> > mfp64' executables or DSOs from being loaded together with VDSO, but
> > they are incompatible with the 32-bit FPU MIPS I processors have anyway,
> > so no change in the semantics there.
> > 
> >  On the other hand there's still the issue what the compiler default
> > will be for o32 in a non-r3k build -- which could be any of `-mfp32', `-
> > mfpxx'
> > or `-mfp64'.  Requesting `-mfpxx' should allow the greatest flexibility,
> > but may not be supported as older compilers did not have it, so this
> > would have to be added conditionally only, letting those older compilers
> > retain their long-established `-mfp32' default.  I have an idea how to
> > implement this part if we agree this is indeed the right direction.
> 
> As above, unless absolutely critical to have floating point code then
> the vDSO should just avoid all FP related issues and build soft-float.

 Using `-msoft-float' though does not prevent code from including FP 
arithmetic inadvertently, possibly using an incompatible ABI.  What would 
indeed prevent FP code from being produced is the `-mno-float' option I 
mentioned before, which regrettably is not universally enabled across MIPS 
GCC targets though.

 Therefore based on the observations made here and previously I recommend 
using `$(call cc-option,-mno-float,-msoft-float)' rather than plain 
`-msoft-float' so that we don't have to fiddle with this piece again in 
the future, and we need to enable `-mno-float' for `mips*-*-linux*' and 
maybe also other MIPS targets in GCC.  I think the patch description needs 
to state both why we want `-mno-float' (see above) and why `-msoft-float' 
is an acceptable fallback (see further above).

 I think we want to prefer `-mno-float' for the kernel proper as well, as 
the last we want to have in the kernel is soft-float code, even though it 
is indeed FPU user context compatible.  That I think qualifies for a 
separate patch though.

  Maciej