preempt safe fpu-emulator

preempt safe fpu-emulator

[Atsushi Nemoto]

Hi.  Here is a patch to make the fpu-emulator preempt-safe.  It would
be SMP-safe also.

The 'ieee754_csr' global variable is removed.  Now the 'ieee754_csr'
is an alias of current->thread.fpu.soft.fcr31.  While the fpu-emulator
uses different mapping for RM bits (FPU_CSR_Rm vs. IEEE754_Rm), RM
bits are converted before (and after) calling of cop1Emulate().  If we
adjusted IEEE754_Rm to match with FPU_CSR_Rm, we can remove ieee_rm[]
and mips_rm[].  Should we do it?

With this patch, whole fpu-emulator can be run without disabling
preempt.  I will post a patch to fix preemption issue soon.

diff -u linux-mips/arch/mips/math-emu/cp1emu.c linux/arch/mips/math-emu/cp1emu.c
--- linux-mips/arch/mips/math-emu/cp1emu.c	2005-03-04 10:19:33.000000000 +0900
+++ linux/arch/mips/math-emu/cp1emu.c	2005-04-27 10:45:08.000000000 +0900
@@ -79,7 +79,17 @@
 
 /* Convert Mips rounding mode (0..3) to IEEE library modes. */
 static const unsigned char ieee_rm[4] = {
-	IEEE754_RN, IEEE754_RZ, IEEE754_RU, IEEE754_RD
+	[FPU_CSR_RN] = IEEE754_RN,
+	[FPU_CSR_RZ] = IEEE754_RZ,
+	[FPU_CSR_RU] = IEEE754_RU,
+	[FPU_CSR_RD] = IEEE754_RD,
+};
+/* Convert IEEE library modes to Mips rounding mode (0..3). */
+static const unsigned char mips_rm[4] = {
+	[IEEE754_RN] = FPU_CSR_RN,
+	[IEEE754_RZ] = FPU_CSR_RZ,
+	[IEEE754_RD] = FPU_CSR_RD,
+	[IEEE754_RU] = FPU_CSR_RU,
 };
 
 #if __mips >= 4
@@ -368,6 +378,7 @@
 			}
 			if (MIPSInst_RD(ir) == FPCREG_CSR) {
 				value = ctx->fcr31;
+				value = (value & ~0x3) | mips_rm[value & 0x3];
 #ifdef CSRTRACE
 				printk("%p gpr[%d]<-csr=%08x\n",
 					(void *) (xcp->cp0_epc),
@@ -400,11 +411,10 @@
 					(void *) (xcp->cp0_epc),
 					MIPSInst_RT(ir), value);
 #endif
-				ctx->fcr31 = value;
-				/* copy new rounding mode and
-				   flush bit to ieee library state! */
-				ieee754_csr.nod = (ctx->fcr31 & 0x1000000) != 0;
-				ieee754_csr.rm = ieee_rm[value & 0x3];
+				value &= (FPU_CSR_FLUSH | FPU_CSR_ALL_E | FPU_CSR_ALL_S | 0x03);
+				ctx->fcr31 &= ~(FPU_CSR_FLUSH | FPU_CSR_ALL_E | FPU_CSR_ALL_S | 0x03);
+				/* convert to ieee library modes */
+				ctx->fcr31 |= (value & ~0x3) | ieee_rm[value & 0x3];
 			}
 			if ((ctx->fcr31 >> 5) & ctx->fcr31 & FPU_CSR_ALL_E) {
 				return SIGFPE;
@@ -570,7 +580,7 @@
 static ieee754##p fpemu_##p##_##name (ieee754##p r, ieee754##p s, \
     ieee754##p t) \
 { \
-	struct ieee754_csr ieee754_csr_save; \
+	struct _ieee754_csr ieee754_csr_save; \
 	s = f1 (s, t); \
 	ieee754_csr_save = ieee754_csr; \
 	s = f2 (s, r); \
@@ -699,8 +709,6 @@
 				rcsr |= FPU_CSR_INV_X | FPU_CSR_INV_S;
 
 			ctx->fcr31 = (ctx->fcr31 & ~FPU_CSR_ALL_X) | rcsr;
-			if (ieee754_csr.nod)
-				ctx->fcr31 |= 0x1000000;
 			if ((ctx->fcr31 >> 5) & ctx->fcr31 & FPU_CSR_ALL_E) {
 				/*printk ("SIGFPE: fpu csr = %08x\n",
 				   ctx->fcr31); */
@@ -1297,12 +1305,17 @@
 		if (insn == 0)
 			xcp->cp0_epc += 4;	/* skip nops */
 		else {
-			/* Update ieee754_csr. Only relevant if we have a
-			   h/w FPU */
-			ieee754_csr.nod = (ctx->fcr31 & 0x1000000) != 0;
-			ieee754_csr.rm = ieee_rm[ctx->fcr31 & 0x3];
-			ieee754_csr.cx = (ctx->fcr31 >> 12) & 0x1f;
+			/*
+			 * The 'ieee754_csr' is an alias of
+			 * ctx->fcr31.  No need to copy ctx->fcr31 to
+			 * ieee754_csr.  But ieee754_csr.rm is ieee
+			 * library modes. (not mips rounding mode)
+			 */
+			/* convert to ieee library modes */
+			ieee754_csr.rm = ieee_rm[ieee754_csr.rm];
 			sig = cop1Emulate(xcp, ctx);
+			/* revert to mips rounding mode */
+			ieee754_csr.rm = mips_rm[ieee754_csr.rm];
 		}
 
 		if (cpu_has_fpu)
diff -u linux-mips/arch/mips/math-emu/dp_sqrt.c linux/arch/mips/math-emu/dp_sqrt.c
--- linux-mips/arch/mips/math-emu/dp_sqrt.c	2005-01-31 11:05:18.000000000 +0900
+++ linux/arch/mips/math-emu/dp_sqrt.c	2005-04-27 10:04:18.000000000 +0900
@@ -37,7 +37,7 @@
 
 ieee754dp ieee754dp_sqrt(ieee754dp x)
 {
-	struct ieee754_csr oldcsr;
+	struct _ieee754_csr oldcsr;
 	ieee754dp y, z, t;
 	unsigned scalx, yh;
 	COMPXDP;
diff -u linux-mips/arch/mips/math-emu/ieee754.c linux/arch/mips/math-emu/ieee754.c
--- linux-mips/arch/mips/math-emu/ieee754.c	2005-01-31 11:05:18.000000000 +0900
+++ linux/arch/mips/math-emu/ieee754.c	2005-04-26 18:24:27.000000000 +0900
@@ -50,10 +50,6 @@
 	"SNaN",
 };
 
-/* the control status register
-*/
-struct ieee754_csr ieee754_csr;
-
 /* special constants
 */
 
diff -u linux-mips/arch/mips/math-emu/ieee754.h linux/arch/mips/math-emu/ieee754.h
--- linux-mips/arch/mips/math-emu/ieee754.h	2005-01-31 11:05:18.000000000 +0900
+++ linux/arch/mips/math-emu/ieee754.h	2005-04-27 10:04:57.000000000 +0900
@@ -35,6 +35,7 @@
 #ifdef __KERNEL__
 /* Going from Algorithmics to Linux native environment, add this */
 #include <linux/types.h>
+#include <linux/sched.h>	/* for 'current' */
 
 /*
  * Not very pretty, but the Linux kernel's normal va_list definition
@@ -323,15 +324,28 @@
 
 /* the control status register
 */
-struct ieee754_csr {
-	unsigned pad:13;
+struct _ieee754_csr {
+#if (defined(BYTE_ORDER) && BYTE_ORDER == BIG_ENDIAN) || defined(__MIPSEB__)
+	unsigned pad0:7;
 	unsigned nod:1;		/* set 1 for no denormalised numbers */
-	unsigned cx:5;		/* exceptions this operation */
+	unsigned c:1;		/* condition */
+	unsigned pad1:5;
+	unsigned cx:6;		/* exceptions this operation */
 	unsigned mx:5;		/* exception enable  mask */
 	unsigned sx:5;		/* exceptions total */
 	unsigned rm:2;		/* current rounding mode */
+#else
+	unsigned rm:2;		/* current rounding mode */
+	unsigned sx:5;		/* exceptions total */
+	unsigned mx:5;		/* exception enable  mask */
+	unsigned cx:6;		/* exceptions this operation */
+	unsigned pad1:5;
+	unsigned c:1;		/* condition */
+	unsigned nod:1;		/* set 1 for no denormalised numbers */
+	unsigned pad0:7;
+#endif
 };
-extern struct ieee754_csr ieee754_csr;
+#define ieee754_csr (*(struct _ieee754_csr *)(&current->thread.fpu.soft.fcr31))
 
 static __inline unsigned ieee754_getrm(void)
 {

Re: preempt safe fpu-emulator

[Atsushi Nemoto]

>>>>> On Wed, 27 Apr 2005 14:36:22 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> said:
anemo> With this patch, whole fpu-emulator can be run without
anemo> disabling preempt.  I will post a patch to fix preemption issue
anemo> soon.

Here it is.

It also fixes these issues:

* The process might lose fpu BEFORE calling preempt_disable() in
  do_fpe().
* The saved fp context might be overwritten if another process took
  fpu.


--- linux-mips/arch/mips/kernel/traps.c	2005-04-18 11:42:47.000000000 +0900
+++ linux/arch/mips/kernel/traps.c	2005-04-27 14:04:53.407006244 +0900
@@ -551,6 +551,14 @@
 
 		preempt_disable();
 
+#ifdef CONFIG_PREEMPT
+		if (!is_fpu_owner()) {
+			/* We might lose fpu before disabling preempt... */
+			own_fpu();
+			BUG_ON(!used_math());
+			restore_fp(current);
+		}
+#endif
 		/*
 	 	 * Unimplemented operation exception.  If we've got the full
 		 * software emulator on-board, let's use it...
@@ -562,11 +570,18 @@
 		 * a bit extreme for what should be an infrequent event.
 		 */
 		save_fp(current);
+		/* Ensure 'resume' not overwrite saved fp context again. */
+		lose_fpu();
+
+		preempt_enable();
 
 		/* Run the emulator */
 		sig = fpu_emulator_cop1Handler (0, regs,
 			&current->thread.fpu.soft);
 
+		preempt_disable();
+
+		own_fpu();	/* Using the FPU again.  */
 		/*
 		 * We can't allow the emulated instruction to leave any of
 		 * the cause bit set in $fcr31.
@@ -712,6 +727,8 @@
 			set_used_math();
 		}
 
+		preempt_enable();
+
 		if (!cpu_has_fpu) {
 			int sig = fpu_emulator_cop1Handler(0, regs,
 						&current->thread.fpu.soft);
@@ -719,8 +736,6 @@
 				force_sig(sig, current);
 		}
 
-		preempt_enable();
-
 		return;
 
 	case 2:

New Forum for support of RB500 (MIPS board)

[John Tully]

<http://forum.routerboard.com/>http://forum.routerboard.com/

One interesting topic is that openWRT is being ported to the RB500.

John

Re: preempt safe fpu-emulator

[Ralf Baechle]

On Wed, Apr 27, 2005 at 02:36:22PM +0900, Atsushi Nemoto wrote:

> Hi.  Here is a patch to make the fpu-emulator preempt-safe.  It would
> be SMP-safe also.
> 
> The 'ieee754_csr' global variable is removed.  Now the 'ieee754_csr'
> is an alias of current->thread.fpu.soft.fcr31.  While the fpu-emulator
> uses different mapping for RM bits (FPU_CSR_Rm vs. IEEE754_Rm), RM
> bits are converted before (and after) calling of cop1Emulate().  If we
> adjusted IEEE754_Rm to match with FPU_CSR_Rm, we can remove ieee_rm[]
> and mips_rm[].  Should we do it?
> 
> With this patch, whole fpu-emulator can be run without disabling
> preempt.  I will post a patch to fix preemption issue soon.

I applied both your patches with some slight cleanup for the endianess
stuff in arch/mips/math-emu/ieee754.h and non-Linux stuff.

  Ralf

Re: preempt safe fpu-emulator

[Kevin D. Kissell]

When I first integrated the Algorithmics emulator with the Linux kernel
several years back, I tried doing something like this but ran into some
problem that I cannot recall exactly - there may have been some case
where the system expected threads to "inherit" FCSR changes.  I agree
that this is an obviously cleaner approach, but be careful.

            Regards,

            Kevin K.

----- Original Message ----- 
From: "Ralf Baechle" <ralf@linux-mips.org>
To: "Atsushi Nemoto" <anemo@mba.ocn.ne.jp>
Cc: <linux-mips@linux-mips.org>
Sent: Thursday, April 28, 2005 6:41 AM
Subject: Re: preempt safe fpu-emulator


> On Wed, Apr 27, 2005 at 02:36:22PM +0900, Atsushi Nemoto wrote:
> 
> > Hi.  Here is a patch to make the fpu-emulator preempt-safe.  It would
> > be SMP-safe also.
> > 
> > The 'ieee754_csr' global variable is removed.  Now the 'ieee754_csr'
> > is an alias of current->thread.fpu.soft.fcr31.  While the fpu-emulator
> > uses different mapping for RM bits (FPU_CSR_Rm vs. IEEE754_Rm), RM
> > bits are converted before (and after) calling of cop1Emulate().  If we
> > adjusted IEEE754_Rm to match with FPU_CSR_Rm, we can remove ieee_rm[]
> > and mips_rm[].  Should we do it?
> > 
> > With this patch, whole fpu-emulator can be run without disabling
> > preempt.  I will post a patch to fix preemption issue soon.
> 
> I applied both your patches with some slight cleanup for the endianess
> stuff in arch/mips/math-emu/ieee754.h and non-Linux stuff.
> 
>   Ralf
> 
> 

Re: preempt safe fpu-emulator

[Kevin D. Kissell]

When I first integrated the Algorithmics emulator with the Linux kernel
several years back, I tried doing something like this but ran into some
problem that I cannot recall exactly - there may have been some case
where the system expected threads to "inherit" FCSR changes.  I agree
that this is an obviously cleaner approach, but be careful.

            Regards,

            Kevin K.

----- Original Message ----- 
From: "Ralf Baechle" <ralf@linux-mips.org>
To: "Atsushi Nemoto" <anemo@mba.ocn.ne.jp>
Cc: <linux-mips@linux-mips.org>
Sent: Thursday, April 28, 2005 6:41 AM
Subject: Re: preempt safe fpu-emulator


> On Wed, Apr 27, 2005 at 02:36:22PM +0900, Atsushi Nemoto wrote:
> 
> > Hi.  Here is a patch to make the fpu-emulator preempt-safe.  It would
> > be SMP-safe also.
> > 
> > The 'ieee754_csr' global variable is removed.  Now the 'ieee754_csr'
> > is an alias of current->thread.fpu.soft.fcr31.  While the fpu-emulator
> > uses different mapping for RM bits (FPU_CSR_Rm vs. IEEE754_Rm), RM
> > bits are converted before (and after) calling of cop1Emulate().  If we
> > adjusted IEEE754_Rm to match with FPU_CSR_Rm, we can remove ieee_rm[]
> > and mips_rm[].  Should we do it?
> > 
> > With this patch, whole fpu-emulator can be run without disabling
> > preempt.  I will post a patch to fix preemption issue soon.
> 
> I applied both your patches with some slight cleanup for the endianess
> stuff in arch/mips/math-emu/ieee754.h and non-Linux stuff.
> 
>   Ralf
> 
> 

Re: preempt safe fpu-emulator

[Ralf Baechle]

On Thu, Apr 28, 2005 at 06:58:28AM -0700, Kevin D. Kissell wrote:

> When I first integrated the Algorithmics emulator with the Linux kernel
> several years back, I tried doing something like this but ran into some
> problem that I cannot recall exactly - there may have been some case
> where the system expected threads to "inherit" FCSR changes.  I agree
> that this is an obviously cleaner approach, but be careful.

The global variables definately won't fly anymore in preemptable and SMP
kernels.  Or rather any attempt to get that to work would only make things
worse, so they had to go.

  Ralf

Re: preempt safe fpu-emulator

[Maciej W. Rozycki]

On Thu, 28 Apr 2005, Ralf Baechle wrote:

> > When I first integrated the Algorithmics emulator with the Linux kernel
> > several years back, I tried doing something like this but ran into some
> > problem that I cannot recall exactly - there may have been some case
> > where the system expected threads to "inherit" FCSR changes.  I agree
> > that this is an obviously cleaner approach, but be careful.
> 
> The global variables definately won't fly anymore in preemptable and SMP
> kernels.  Or rather any attempt to get that to work would only make things
> worse, so they had to go.

 It depends on how they were actually used -- real FPU circuitry is 
"global", too, and somehow it works or at least it has to.

  Maciej

Re: preempt safe fpu-emulator

[Ralf Baechle]

On Thu, Apr 28, 2005 at 04:25:30PM +0100, Maciej W. Rozycki wrote:

>  It depends on how they were actually used -- real FPU circuitry is 
> "global", too, and somehow it works or at least it has to.

But it's not shared across processes ;-)

  Ralf

Re: preempt safe fpu-emulator

[Ralf Baechle]

On Thu, Apr 28, 2005 at 04:33:56PM +0100, Ralf Baechle wrote:

> >  It depends on how they were actually used -- real FPU circuitry is 
> > "global", too, and somehow it works or at least it has to.
> 
> But it's not shared across processes ;-)

Processors, of course.

  Ralf

Re: preempt safe fpu-emulator

[Kevin D. Kissell]

> On Thu, Apr 28, 2005 at 06:58:28AM -0700, Kevin D. Kissell wrote:
> 
> > When I first integrated the Algorithmics emulator with the Linux kernel
> > several years back, I tried doing something like this but ran into some
> > problem that I cannot recall exactly - there may have been some case
> > where the system expected threads to "inherit" FCSR changes.  I agree
> > that this is an obviously cleaner approach, but be careful.
> 
> The global variables definately won't fly anymore in preemptable and SMP
> kernels.  Or rather any attempt to get that to work would only make things
> worse, so they had to go.

The global variable thing was clearly not SMP safe - but then again, the
32-bit MIPS kernel we were working with wasn't SMP safe either, in
those days.  ;o)  But *if* - and it may not really (or no longer) be the case - 
there is an implicit assumption that some FCSR state is preserved on a
context switch, it would be more correct to map the ieee754_csr symbol
to a per-CPU variable than a per-thread variable.

            Regards,

            Kevin K.

Re: preempt safe fpu-emulator

[Kevin D. Kissell]

> On Thu, Apr 28, 2005 at 06:58:28AM -0700, Kevin D. Kissell wrote:
> 
> > When I first integrated the Algorithmics emulator with the Linux kernel
> > several years back, I tried doing something like this but ran into some
> > problem that I cannot recall exactly - there may have been some case
> > where the system expected threads to "inherit" FCSR changes.  I agree
> > that this is an obviously cleaner approach, but be careful.
> 
> The global variables definately won't fly anymore in preemptable and SMP
> kernels.  Or rather any attempt to get that to work would only make things
> worse, so they had to go.

The global variable thing was clearly not SMP safe - but then again, the
32-bit MIPS kernel we were working with wasn't SMP safe either, in
those days.  ;o)  But *if* - and it may not really (or no longer) be the case - 
there is an implicit assumption that some FCSR state is preserved on a
context switch, it would be more correct to map the ieee754_csr symbol
to a per-CPU variable than a per-thread variable.

            Regards,

            Kevin K.

Re: preempt safe fpu-emulator

[Atsushi Nemoto]

>>>>> On Thu, 28 Apr 2005 18:06:53 +0200, "Kevin D. Kissell" <kevink@mips.com> said:

kevink> The global variable thing was clearly not SMP safe - but then
kevink> again, the 32-bit MIPS kernel we were working with wasn't SMP
kevink> safe either, in those days.  ;o)

Also, IIRC, old FPU ownership management ("lazy fpu switch") was
somewhat broken (or fragile at least) in those days.  The current code
is more robust (and simple, I suppose).

kevink> But *if* - and it may not really (or no longer) be the case -
kevink> there is an implicit assumption that some FCSR state is
kevink> preserved on a context switch, it would be more correct to map
kevink> the ieee754_csr symbol to a per-CPU variable than a per-thread
kevink> variable.

Thanks for your advise.  I believe there is not a such assumption now.
Any newly created process always start with CP1 disabled, and on the
first CP1 unusable exception FCSR will be initialized to 0.

---
Atsushi Nemoto

Re: preempt safe fpu-emulator

[Atsushi Nemoto]

>>>>> On Thu, 28 Apr 2005 14:41:18 +0100, Ralf Baechle <ralf@linux-mips.org> said:

ralf> I applied both your patches with some slight cleanup for the
ralf> endianess stuff in arch/mips/math-emu/ieee754.h and non-Linux
ralf> stuff.

Thanks.  Since the fpu emulator run without FPU ownership now, the
patch I posted a few weeks ago is necessary to emulate cp1 branch
instructions.

http://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips&i=20050420.165320.42204293.nemoto%40toshiba-tops.co.jp

Please apply this also.  Thank you.
---
Atsushi Nemoto