MIPS: Fix MIPS_O32_FP64_SUPPORT for 64bit CPUs before R2

Message ID 20230519163023.70542-1-jiaxun.yang@flygoat.com
State New
Headers
Series MIPS: Fix MIPS_O32_FP64_SUPPORT for 64bit CPUs before R2 |

Commit Message

Jiaxun Yang May 19, 2023, 4:30 p.m. UTC
  MIPS_O32_FP64_SUPPORT enables possibility of using all 32 FPRs on 32bit
kernel in case CPU implemented FR1. As FR1 is present on all 64bit CPUs
following R4000's priviliged spec, there is no reason to limit such support
to R2+ CPUs.

Fix it by remove architecture reversion conditions in FR1 context switch
code and enable relevant bits in kernel's FIR copy.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/include/asm/asmmacro.h |  6 ++----
 arch/mips/include/asm/fpu.h      |  3 +--
 arch/mips/kernel/elf.c           |  5 ++---
 arch/mips/kernel/fpu-probe.c     | 17 +++++++++++++++++
 arch/mips/kernel/r4k_fpu.S       | 16 ++++++----------
 5 files changed, 28 insertions(+), 19 deletions(-)
  

Comments

Maciej W. Rozycki May 19, 2023, 8:10 p.m. UTC | #1
On Fri, 19 May 2023, Jiaxun Yang wrote:

> MIPS_O32_FP64_SUPPORT enables possibility of using all 32 FPRs on 32bit
> kernel in case CPU implemented FR1. As FR1 is present on all 64bit CPUs
> following R4000's priviliged spec, there is no reason to limit such support
> to R2+ CPUs.

 I guess one can do it and still run FPXX software, but I fail to see what 
gain it provides.  For FP32 it breaks things as accesses to odd-numbered 
FPRs will no longer get at the high part of a double value and for FP64 
there are no MTHC1/MFHC1 instructions required to access the high part.

 What problem are you trying to solve?  And how did you verify this patch?

> --- a/arch/mips/kernel/fpu-probe.c
> +++ b/arch/mips/kernel/fpu-probe.c
> @@ -289,6 +289,23 @@ void cpu_set_fpu_opts(struct cpuinfo_mips *c)
>  			c->options |= MIPS_CPU_FRE;
>  	}
>  
> +	/* Fix up FIR for FPU earlier than R2 */
> +	if (!cpu_has_mips_r2_r6) {
> +		c->fpu_id |= MIPS_FPIR_S;
> +#ifdef CONFIG_CPU_R4K_FPU
> +		/* All known R4000 class FPU implemented double */
> +		c->fpu_id |= MIPS_FPIR_D;
> +#endif

 Currently all FPUs we support implement double and we require that, so no 
need to make this piece conditional (I would use IS_ENABLED otherwise, so 
as not to clutter the source with #ifdef), but `c->fpu_id' is also exposed 
to the user via ptrace(2), so this has to reflect hardware and not give a 
synthesized value.

  Maciej
  
Jiaxun Yang May 19, 2023, 10:02 p.m. UTC | #2
> 2023年5月19日 21:10,Maciej W. Rozycki <macro@orcam.me.uk> 写道:
> 
> On Fri, 19 May 2023, Jiaxun Yang wrote:
> 
>> MIPS_O32_FP64_SUPPORT enables possibility of using all 32 FPRs on 32bit
>> kernel in case CPU implemented FR1. As FR1 is present on all 64bit CPUs
>> following R4000's priviliged spec, there is no reason to limit such support
>> to R2+ CPUs.
> 
> I guess one can do it and still run FPXX software, but I fail to see what 
> gain it provides.  For FP32 it breaks things as accesses to odd-numbered 
> FPRs will no longer get at the high part of a double value and for FP64 
> there are no MTHC1/MFHC1 instructions required to access the high part.

Actually software may access the high part by SDC1/LDC1.
FP32 binaries are still going to run at FR0 mode.

> 
> What problem are you trying to solve?  And how did you verify this patch?

Was trying to deal a proprietary JIT software who want to enable FR1 via prctl
on Loongson-2F with 32 bit kernel.

> 
>> --- a/arch/mips/kernel/fpu-probe.c
>> +++ b/arch/mips/kernel/fpu-probe.c
>> @@ -289,6 +289,23 @@ void cpu_set_fpu_opts(struct cpuinfo_mips *c)
>> c->options |= MIPS_CPU_FRE;
>> }
>> 
>> + /* Fix up FIR for FPU earlier than R2 */
>> + if (!cpu_has_mips_r2_r6) {
>> + c->fpu_id |= MIPS_FPIR_S;
>> +#ifdef CONFIG_CPU_R4K_FPU
>> + /* All known R4000 class FPU implemented double */
>> + c->fpu_id |= MIPS_FPIR_D;
>> +#endif
> 
> Currently all FPUs we support implement double and we require that, so no 
> need to make this piece conditional (I would use IS_ENABLED otherwise, so 
> as not to clutter the source with #ifdef), but `c->fpu_id' is also exposed 
> to the user via ptrace(2), so this has to reflect hardware and not give a 
> synthesized value.

Alas, I thought R2030 class FPU does not have double? Since MIPS-IV spec
says SDC1 is introduced in MIPS II.

Thanks
Jiaxun

> 
>  Maciej
  
Maciej W. Rozycki May 20, 2023, 8:32 p.m. UTC | #3
On Fri, 19 May 2023, Jiaxun Yang wrote:

> > I guess one can do it and still run FPXX software, but I fail to see what 
> > gain it provides.  For FP32 it breaks things as accesses to odd-numbered 
> > FPRs will no longer get at the high part of a double value and for FP64 
> > there are no MTHC1/MFHC1 instructions required to access the high part.
> 
> Actually software may access the high part by SDC1/LDC1.

 I'm aware of that, but you'd need a new psABI variation really to handle 
such an arrangement.  None of the existing FP32, FP64, FPXX handles it.

> > What problem are you trying to solve?  And how did you verify this patch?
> 
> Was trying to deal a proprietary JIT software who want to enable FR1 via prctl
> on Loongson-2F with 32 bit kernel.

 There may be a better way: rather than avoiding MTHC1/MFHC1, handle them 
in the FPU emulator where unavailable in FR=1 mode while leaving the rest 
to hardware.  That would make regular FR64 software work.

 I'd expect such a JIT to have other issues with pre-R2 hardware though, 
with missing machine instructions.  I had a similar situation a few years 
ago with FireFox's JIT making assumptions above the MIPS ISA level the 
piece of software was itself compiled for and opted for just disabling the 
JIT, as fixing FireFox and rebuilding it would be more effort than it was 
worth in my view.

 This might be the best way for you to move forward too, and I'm all but 
enthusiastic about adding a workaround in the kernel for a broken piece of 
proprietary user software.  Sorry.

 Also I seem to remember there was a pitfall with running 32-bit software 
on pre-R2 hardware in the FR=1 mode, but maybe I'm making up things here.  
It's been so long since I last looked into this.

 In any case you do need to verify this somehow, like by running the math 
part of the glibc testsuite with o32 in the FR=1 mode on pre-R2 hardware.  
Running the GDB test suite to make sure ptrace(2) works fine with the new 
FPU configuration would make sense too.

> > Currently all FPUs we support implement double and we require that, so no 
> > need to make this piece conditional (I would use IS_ENABLED otherwise, so 
> > as not to clutter the source with #ifdef), but `c->fpu_id' is also exposed 
> > to the user via ptrace(2), so this has to reflect hardware and not give a 
> > synthesized value.
> 
> Alas, I thought R2030 class FPU does not have double? Since MIPS-IV spec
> says SDC1 is introduced in MIPS II.

 There's no SDC1/LDC1, but the usual MIPS I FP machine instructions (of 
which there are fewer than in MIPS II, e.g. there's no SQRT.fmt or direct 
conversions) do support the double format/encoding.  It's just that double 
FP data has to be transferred piecemeal; other supported operations will 
execute just fine.  Otherwise the existence of the odd-numbered FPRs would 
make no sense in the first place.

 Plain single floating-point units are extremely rare, e.g. the R4650 has 
one (it does support CP0.Status.FR though, for 32 singles), and we do not 
support them (e.g. the R4650 has a simple base-bounds MMU only, no TLB).  
Another one is the R5900, but its FPU is not an IEEE 754 device even.

  Maciej
  
Jiaxun Yang May 21, 2023, 2:06 a.m. UTC | #4
> 2023年5月20日 21:32,Maciej W. Rozycki <macro@orcam.me.uk> 写道:
> 
> On Fri, 19 May 2023, Jiaxun Yang wrote:
> 
>>> I guess one can do it and still run FPXX software, but I fail to see what 
>>> gain it provides.  For FP32 it breaks things as accesses to odd-numbered 
>>> FPRs will no longer get at the high part of a double value and for FP64 
>>> there are no MTHC1/MFHC1 instructions required to access the high part.
>> 
>> Actually software may access the high part by SDC1/LDC1.
> 
> I'm aware of that, but you'd need a new psABI variation really to handle 
> such an arrangement.  None of the existing FP32, FP64, FPXX handles it.
> 
>>> What problem are you trying to solve?  And how did you verify this patch?
>> 
>> Was trying to deal a proprietary JIT software who want to enable FR1 via prctl
>> on Loongson-2F with 32 bit kernel.
> 
> There may be a better way: rather than avoiding MTHC1/MFHC1, handle them 
> in the FPU emulator where unavailable in FR=1 mode while leaving the rest 
> to hardware.  That would make regular FR64 software work.
> 
> I'd expect such a JIT to have other issues with pre-R2 hardware though, 
> with missing machine instructions.  I had a similar situation a few years 
> ago with FireFox's JIT making assumptions above the MIPS ISA level the 
> piece of software was itself compiled for and opted for just disabling the 
> JIT, as fixing FireFox and rebuilding it would be more effort than it was 
> worth in my view.
> 
> This might be the best way for you to move forward too, and I'm all but 
> enthusiastic about adding a workaround in the kernel for a broken piece of 
> proprietary user software.  Sorry.

Thanks for all those information. I was just entertaining by poking around some
old software blobs from Loongson so I’d give up upstreaming those changes.

My limited test works fine with this patch though.

- Jiaxun

> 
> Also I seem to remember there was a pitfall with running 32-bit software 
> on pre-R2 hardware in the FR=1 mode, but maybe I'm making up things here.  
> It's been so long since I last looked into this.
> 
> In any case you do need to verify this somehow, like by running the math 
> part of the glibc testsuite with o32 in the FR=1 mode on pre-R2 hardware.  
> Running the GDB test suite to make sure ptrace(2) works fine with the new 
> FPU configuration would make sense too.
> 
>>> Currently all FPUs we support implement double and we require that, so no 
>>> need to make this piece conditional (I would use IS_ENABLED otherwise, so 
>>> as not to clutter the source with #ifdef), but `c->fpu_id' is also exposed 
>>> to the user via ptrace(2), so this has to reflect hardware and not give a 
>>> synthesized value.
>> 
>> Alas, I thought R2030 class FPU does not have double? Since MIPS-IV spec
>> says SDC1 is introduced in MIPS II.
> 
> There's no SDC1/LDC1, but the usual MIPS I FP machine instructions (of 
> which there are fewer than in MIPS II, e.g. there's no SQRT.fmt or direct 
> conversions) do support the double format/encoding.  It's just that double 
> FP data has to be transferred piecemeal; other supported operations will 
> execute just fine.  Otherwise the existence of the odd-numbered FPRs would 
> make no sense in the first place.
> 
> Plain single floating-point units are extremely rare, e.g. the R4650 has 
> one (it does support CP0.Status.FR though, for 32 singles), and we do not 
> support them (e.g. the R4650 has a simple base-bounds MMU only, no TLB).  
> Another one is the R5900, but its FPU is not an IEEE 754 device even.
> 
>  Maciej
  

Patch

diff --git a/arch/mips/include/asm/asmmacro.h b/arch/mips/include/asm/asmmacro.h
index 067a635d3bc8..3e7c19ee6a90 100644
--- a/arch/mips/include/asm/asmmacro.h
+++ b/arch/mips/include/asm/asmmacro.h
@@ -130,8 +130,7 @@ 
 	.endm
 
 	.macro	fpu_save_double thread status tmp
-#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
-    defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
+#if defined(CONFIG_64BIT) || defined(CONFIG_MIPS_O32_FP64_SUPPORT)
 	sll	\tmp, \status, 5
 	bgez	\tmp, 10f
 	fpu_save_16odd \thread
@@ -189,8 +188,7 @@ 
 	.endm
 
 	.macro	fpu_restore_double thread status tmp
-#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
-    defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
+#if defined(CONFIG_64BIT) || defined(CONFIG_MIPS_O32_FP64_SUPPORT)
 	sll	\tmp, \status, 5
 	bgez	\tmp, 10f				# 16 register mode?
 
diff --git a/arch/mips/include/asm/fpu.h b/arch/mips/include/asm/fpu.h
index 86310d6e1035..1cd820e79f77 100644
--- a/arch/mips/include/asm/fpu.h
+++ b/arch/mips/include/asm/fpu.h
@@ -71,8 +71,7 @@  static inline int __enable_fpu(enum fpu_mode mode)
 		goto fr_common;
 
 	case FPU_64BIT:
-#if !(defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5) || \
-      defined(CONFIG_CPU_MIPSR6) || defined(CONFIG_64BIT))
+#if !(defined(CONFIG_64BIT) || defined(CONFIG_MIPS_O32_FP64_SUPPORT))
 		/* we only have a 32-bit FPU */
 		return SIGFPE;
 #endif
diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
index 5582a4ca1e9e..0895bc9b5dd7 100644
--- a/arch/mips/kernel/elf.c
+++ b/arch/mips/kernel/elf.c
@@ -250,9 +250,8 @@  int arch_check_elf(void *_ehdr, bool has_interpreter, void *_interp_ehdr,
 		state->overall_fp_mode = FP_FRE;
 	else if ((prog_req.fr1 && prog_req.frdefault) ||
 		 (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) ?
+		/* Make sure CPUs without FR1 will not pick FR1 */
+		state->overall_fp_mode = (raw_current_cpu_data.fpu_id & MIPS_FPIR_F64) ?
 					  FP_FR1 : FP_FR0;
 	else if (prog_req.fr1)
 		state->overall_fp_mode = FP_FR1;
diff --git a/arch/mips/kernel/fpu-probe.c b/arch/mips/kernel/fpu-probe.c
index e689d6a83234..383c43e5c86a 100644
--- a/arch/mips/kernel/fpu-probe.c
+++ b/arch/mips/kernel/fpu-probe.c
@@ -289,6 +289,23 @@  void cpu_set_fpu_opts(struct cpuinfo_mips *c)
 			c->options |= MIPS_CPU_FRE;
 	}
 
+	/* Fix up FIR for FPU earlier than R2 */
+	if (!cpu_has_mips_r2_r6) {
+		c->fpu_id |= MIPS_FPIR_S;
+#ifdef CONFIG_CPU_R4K_FPU
+		/* All known R4000 class FPU implemented double */
+		c->fpu_id |= MIPS_FPIR_D;
+#endif
+		if (cpu_has_64bits) {
+			/* Try to enable FR1 */
+			change_c0_status(ST0_FR, ST0_FR);
+			if (read_c0_status() & ST0_FR) {
+				c->fpu_id |= MIPS_FPIR_F64;
+				c->options |= MIPS_CPU_32FPR;
+			}
+		}
+	}
+
 	cpu_set_fpu_fcsr_mask(c);
 	cpu_set_fpu_2008(c);
 	cpu_set_nan_2008(c);
diff --git a/arch/mips/kernel/r4k_fpu.S b/arch/mips/kernel/r4k_fpu.S
index 4e8c98517d9d..eeef115fc39e 100644
--- a/arch/mips/kernel/r4k_fpu.S
+++ b/arch/mips/kernel/r4k_fpu.S
@@ -40,8 +40,7 @@ 
  */
 LEAF(_save_fp)
 EXPORT_SYMBOL(_save_fp)
-#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
-    defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
+#if defined(CONFIG_64BIT) || defined(CONFIG_MIPS_O32_FP64_SUPPORT)
 	mfc0	t0, CP0_STATUS
 #endif
 	fpu_save_double a0 t0 t1		# clobbers t1
@@ -52,8 +51,7 @@  EXPORT_SYMBOL(_save_fp)
  * Restore a thread's fp context.
  */
 LEAF(_restore_fp)
-#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
-    defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
+#if defined(CONFIG_64BIT) || defined(CONFIG_MIPS_O32_FP64_SUPPORT)
 	mfc0	t0, CP0_STATUS
 #endif
 	fpu_restore_double a0 t0 t1		# clobbers t1
@@ -102,11 +100,10 @@  LEAF(_save_fp_context)
 	cfc1	t1, fcr31
 	.set	pop
 
-#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
-    defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
+#if defined(CONFIG_64BIT) || defined(CONFIG_MIPS_O32_FP64_SUPPORT)
 	.set	push
 	.set	hardfloat
-#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5)
+#if MIPS_ISA_REV < 6
 	.set	mips32r2
 	.set	fp=64
 	mfc0	t0, CP0_STATUS
@@ -170,11 +167,10 @@  LEAF(_save_fp_context)
 LEAF(_restore_fp_context)
 	EX	lw t1, 0(a1)
 
-#if defined(CONFIG_64BIT) || defined(CONFIG_CPU_MIPSR2) || \
-    defined(CONFIG_CPU_MIPSR5) || defined(CONFIG_CPU_MIPSR6)
+#if defined(CONFIG_64BIT) || defined(CONFIG_MIPS_O32_FP64_SUPPORT)
 	.set	push
 	.set	hardfloat
-#if defined(CONFIG_CPU_MIPSR2) || defined(CONFIG_CPU_MIPSR5)
+#if MIPS_ISA_REV < 6
 	.set	mips32r2
 	.set	fp=64
 	mfc0	t0, CP0_STATUS