Re: FP emulator patch

Re: FP emulator patch

[Kevin D. Kissell]

> >
> >         if (current->used_math) {               /* Using the FPU again.
> */
> >                 lazy_fpu_switch(last_task_used_math);
> >         } else {                                /* First time FPU user.
> */
> >                 init_fpu();
> >                 current->used_math = 1;
> >         }
> >         last_task_used_math = current;
> >
> > Clearly the second path is logically the correct one.
>
> Not really.  See below.
>
> > BTW, do I see another bug here in do_cpu()?  It seems that before we
call
> > init_fpu(), we should check last_task_used_math.  If it is not NULL, we
> should
> > save the FP state to the last_task_used_math.  Hmm, strange ...
>
> Strange indeed.  And note that if the code were correct, your
> surmise about the init_fpu() path being "logically the correct"
> one would no longer be true - we'd be saving the FPU state of
> the current process for no good reason.

And note further that, by forcing current->used_math to
zero, the old code was in fact driving the signal handler
needlessly into the broken code...

            Kevin K.

Re: FP emulator patch

[Kevin D. Kissell]

> >
> >         if (current->used_math) {               /* Using the FPU again.
> */
> >                 lazy_fpu_switch(last_task_used_math);
> >         } else {                                /* First time FPU user.
> */
> >                 init_fpu();
> >                 current->used_math = 1;
> >         }
> >         last_task_used_math = current;
> >
> > Clearly the second path is logically the correct one.
>
> Not really.  See below.
>
> > BTW, do I see another bug here in do_cpu()?  It seems that before we
call
> > init_fpu(), we should check last_task_used_math.  If it is not NULL, we
> should
> > save the FP state to the last_task_used_math.  Hmm, strange ...
>
> Strange indeed.  And note that if the code were correct, your
> surmise about the init_fpu() path being "logically the correct"
> one would no longer be true - we'd be saving the FPU state of
> the current process for no good reason.

And note further that, by forcing current->used_math to
zero, the old code was in fact driving the signal handler
needlessly into the broken code...

            Kevin K.

Re: FP emulator patch

[Jun Sun]

"Kevin D. Kissell" wrote:
> 
> > >
> > >         if (current->used_math) {               /* Using the FPU again.
> > */
> > >                 lazy_fpu_switch(last_task_used_math);
> > >         } else {                                /* First time FPU user.
> > */
> > >                 init_fpu();
> > >                 current->used_math = 1;
> > >         }
> > >         last_task_used_math = current;
> > >
> > > Clearly the second path is logically the correct one.
> >
> > Not really.  See below.
> >
> > > BTW, do I see another bug here in do_cpu()?  It seems that before we
> call
> > > init_fpu(), we should check last_task_used_math.  If it is not NULL, we
> > should
> > > save the FP state to the last_task_used_math.  Hmm, strange ...
> >
> > Strange indeed.  And note that if the code were correct, your
> > surmise about the init_fpu() path being "logically the correct"
> > one would no longer be true - we'd be saving the FPU state of
> > the current process for no good reason.
> 
> And note further that, by forcing current->used_math to
> zero, the old code was in fact driving the signal handler
> needlessly into the broken code...
> 

By not clearing current->used_math bit, you are in fact restoring an FPU
context unnecessarily.

Jun

Re: FP emulator patch

[Kevin D. Kissell]

> > > Strange indeed.  And note that if the code were correct, your
> > > surmise about the init_fpu() path being "logically the correct"
> > > one would no longer be true - we'd be saving the FPU state of
> > > the current process for no good reason.
> > 
> > And note further that, by forcing current->used_math to
> > zero, the old code was in fact driving the signal handler
> > needlessly into the broken code...
> > 
> 
> By not clearing current->used_math bit, you are in fact restoring an FPU
> context unnecessarily.

And by clearing it, you are destroying an FPU context unnecessarily.
I'll take the overhead, thanks! ;-)  Seriously, if that optimization
is really that important, let's find some other mechanism for communicating
to do_cpu() the fact that we're doing a signal.

            Regards,

            Kevin K.

Re: FP emulator patch

[Kevin D. Kissell]

> > > Strange indeed.  And note that if the code were correct, your
> > > surmise about the init_fpu() path being "logically the correct"
> > > one would no longer be true - we'd be saving the FPU state of
> > > the current process for no good reason.
> > 
> > And note further that, by forcing current->used_math to
> > zero, the old code was in fact driving the signal handler
> > needlessly into the broken code...
> > 
> 
> By not clearing current->used_math bit, you are in fact restoring an FPU
> context unnecessarily.

And by clearing it, you are destroying an FPU context unnecessarily.
I'll take the overhead, thanks! ;-)  Seriously, if that optimization
is really that important, let's find some other mechanism for communicating
to do_cpu() the fact that we're doing a signal.

            Regards,

            Kevin K.

FP emulator patch

[Carsten Langgaard]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

This patch fix a bug in the FP emulator, when used in the 64-bit kernel.

Ralf, could you please apply.

/Carsten



--
_    _ ____  ___   Carsten Langgaard   Mailto:carstenl@mips.com
|\  /|||___)(___   MIPS Denmark        Direct: +45 4486 5527
| \/ |||    ____)  Lautrupvang 4B      Switch: +45 4486 5555
  TECHNOLOGIES     2750 Ballerup       Fax...: +45 4486 5556
                   Denmark             http://www.mips.com



[-- Attachment #2: fpe.patch --]
[-- Type: text/plain, Size: 717 bytes --]

Index: arch/mips/math-emu/cp1emu.c
===================================================================
RCS file: /cvs/linux/arch/mips/math-emu/cp1emu.c,v
retrieving revision 1.13.2.9
diff -u -r1.13.2.9 cp1emu.c
--- arch/mips/math-emu/cp1emu.c	2002/08/05 23:53:34	1.13.2.9
+++ arch/mips/math-emu/cp1emu.c	2002/09/10 11:47:22
@@ -168,8 +168,8 @@
 
 #define SIFROMREG(si,x)	((si) = \
 			(xcp->cp0_status & FR_BIT) || !(x & 1) ? \
-			ctx->regs[x] : \
-			ctx->regs[x & ~1] >> 32 )
+			(int)ctx->regs[x] : \
+			(int)(ctx->regs[x & ~1] >> 32 ))
 #define SITOREG(si,x)	(ctx->regs[x & ~((xcp->cp0_status & FR_BIT) == 0)] = \
 			(xcp->cp0_status & FR_BIT) || !(x & 1) ? \
 			ctx->regs[x & ~1] >> 32 << 32 | (u32)(si) : \

Re: FP emulator patch

[Kevin D. Kissell]

[-- Attachment #1: Type: text/plain, Size: 2158 bytes --]

As I wrote last night, it looks to me as if FPU context
management in signals was broken in all versions and
proposed patches that I've seen.  The way I see it is this:

If the current thread "owns" the FPU, we need to save the
FPU state in the sigcontext and restore it on return.  If
the signal handler uses floating point, it already has the
FPU, and can do what it wants with it.  If it doesn't use
the FPU, then we'll save and restore FPU state for
nothing, but better safe than sorry.  In either case,
the current thread retains ownership of the FPU.
There is no reason to muck with the ownership data
or the Cp0.Status.CU1 enables, apart from the
questionable bit of enabling it before the FP context
save in case it wasn't enabled.  I think that should go
away, but I don't have time to test exhastively.  In
any case, CU1 should have it's pre-signal state
going into the signal handler.

If the current thread does *not* own the FPU, we don't
need to save the thread FP state. If the signal handler
does no FP, so much the better, there's nothing to
be done.   If the signal handler uses FP, it will acquire 
the FPU by normal means. The FP context will be saved 
into the thread context of the previous owner, the signalling 
thread will acquire the FPU, and the signal handler will do 
it's FP. On return from the signal, we *must* de-allocate the
FPU and clear the CU1 bit.  If that's done, and the
thread (which had not *owned* the FPU prior to the
signal) starts doing FP again, normal mechanisms
will cause it's FP context to be restored.  If we don't,
it will start exectuing with a bogus FP context.

The code I sketched last night is essentially correct,
though it used a macro that doesn't exist.  I attach a
patch relative to the current OSS repository's signal.c.
The patch includes the stack frame tweak for the FPU
emulator that was part of previous patches, but which
is orthogonal to the problem under discussion.  I have
built a kernel using this code and run 20 simultaneous
copies of the MontaVista "stressor" program with no
problems (though I also had the "v1.5" FPU emulator
code).

            Regards,

            Kevin K. 

[-- Attachment #2: signal.c.patch --]
[-- Type: application/octet-stream, Size: 2041 bytes --]

Index: signal.c
===================================================================
RCS file: /cvs/linux/arch/mips/kernel/signal.c,v
retrieving revision 1.36
diff -u -p -r1.36 signal.c
--- signal.c	2001/06/18 22:43:35	1.36
+++ signal.c	2001/08/16 18:01:58
@@ -221,7 +221,13 @@ restore_sigcontext(struct pt_regs *regs,
 	err |= __get_user(owned_fp, &sc->sc_ownedfp);
 	if (owned_fp) {
 		err |= restore_fp_context(sc);
-		last_task_used_math = current;
+	} else {
+		if (current == last_task_used_math) {
+		/* Signal handler acquired FPU - give it back */
+			last_task_used_math = NULL;
+			current->used_math = 0;
+			regs->cp0_status &= ~ST0_CU1;
+		}
 	}
 
 	return err;
@@ -326,6 +332,7 @@ setup_sigcontext(struct pt_regs *regs, s
 	int owned_fp;
 	int err = 0;
 	u64 reg;
+	u32 tstat;
 
 	err |= __put_user(regs->cp0_epc, &sc->sc_pc);
 	err |= __put_user(regs->cp0_status, &sc->sc_status);
@@ -353,12 +360,12 @@ setup_sigcontext(struct pt_regs *regs, s
 	owned_fp = (current == last_task_used_math);
 	err |= __put_user(owned_fp, &sc->sc_ownedfp);
 
-	if (current->used_math) {	/* fp is active.  */
+	if (owned_fp) {	/* fp is active.  */
+		tstat = regs->cp0_status;
+		/* This enable should not really be necessary */
 		set_cp0_status(ST0_CU1);
 		err |= save_fp_context(sc);
-		last_task_used_math = NULL;
-		regs->cp0_status &= ~ST0_CU1;
-		current->used_math = 0;
+		regs->cp0_status = tstat;
 	}
 
 	return err;
@@ -374,6 +381,16 @@ get_sigframe(struct k_sigaction *ka, str
 
 	/* Default to using normal stack */
 	sp = regs->regs[29];
+
+#ifdef CONFIG_MIPS_FPU_EMULATOR
+	/*
+	 * FPU emulator may have it's own trampoline active just
+	 * above the user stack, 16-bytes before the next lowest
+	 * 16 byte boundary.  Try to avoid trashing it.
+	 */
+	sp -= 32;	/* 32 ought to be enough, but... */
+
+#endif /* CONFIG_MIPS_FPU_EMULATOR */
 
 	/* This is the X/Open sanctioned signal stack switching.  */
 	if ((ka->sa.sa_flags & SA_ONSTACK) && ! on_sig_stack(sp))

Re: FP emulator patch

[Kevin D. Kissell]

[-- Attachment #1: Type: text/plain, Size: 2158 bytes --]

As I wrote last night, it looks to me as if FPU context
management in signals was broken in all versions and
proposed patches that I've seen.  The way I see it is this:

If the current thread "owns" the FPU, we need to save the
FPU state in the sigcontext and restore it on return.  If
the signal handler uses floating point, it already has the
FPU, and can do what it wants with it.  If it doesn't use
the FPU, then we'll save and restore FPU state for
nothing, but better safe than sorry.  In either case,
the current thread retains ownership of the FPU.
There is no reason to muck with the ownership data
or the Cp0.Status.CU1 enables, apart from the
questionable bit of enabling it before the FP context
save in case it wasn't enabled.  I think that should go
away, but I don't have time to test exhastively.  In
any case, CU1 should have it's pre-signal state
going into the signal handler.

If the current thread does *not* own the FPU, we don't
need to save the thread FP state. If the signal handler
does no FP, so much the better, there's nothing to
be done.   If the signal handler uses FP, it will acquire 
the FPU by normal means. The FP context will be saved 
into the thread context of the previous owner, the signalling 
thread will acquire the FPU, and the signal handler will do 
it's FP. On return from the signal, we *must* de-allocate the
FPU and clear the CU1 bit.  If that's done, and the
thread (which had not *owned* the FPU prior to the
signal) starts doing FP again, normal mechanisms
will cause it's FP context to be restored.  If we don't,
it will start exectuing with a bogus FP context.

The code I sketched last night is essentially correct,
though it used a macro that doesn't exist.  I attach a
patch relative to the current OSS repository's signal.c.
The patch includes the stack frame tweak for the FPU
emulator that was part of previous patches, but which
is orthogonal to the problem under discussion.  I have
built a kernel using this code and run 20 simultaneous
copies of the MontaVista "stressor" program with no
problems (though I also had the "v1.5" FPU emulator
code).

            Regards,

            Kevin K. 

[-- Attachment #2: signal.c.patch --]
[-- Type: application/octet-stream, Size: 2041 bytes --]

Index: signal.c
===================================================================
RCS file: /cvs/linux/arch/mips/kernel/signal.c,v
retrieving revision 1.36
diff -u -p -r1.36 signal.c
--- signal.c	2001/06/18 22:43:35	1.36
+++ signal.c	2001/08/16 18:01:58
@@ -221,7 +221,13 @@ restore_sigcontext(struct pt_regs *regs,
 	err |= __get_user(owned_fp, &sc->sc_ownedfp);
 	if (owned_fp) {
 		err |= restore_fp_context(sc);
-		last_task_used_math = current;
+	} else {
+		if (current == last_task_used_math) {
+		/* Signal handler acquired FPU - give it back */
+			last_task_used_math = NULL;
+			current->used_math = 0;
+			regs->cp0_status &= ~ST0_CU1;
+		}
 	}
 
 	return err;
@@ -326,6 +332,7 @@ setup_sigcontext(struct pt_regs *regs, s
 	int owned_fp;
 	int err = 0;
 	u64 reg;
+	u32 tstat;
 
 	err |= __put_user(regs->cp0_epc, &sc->sc_pc);
 	err |= __put_user(regs->cp0_status, &sc->sc_status);
@@ -353,12 +360,12 @@ setup_sigcontext(struct pt_regs *regs, s
 	owned_fp = (current == last_task_used_math);
 	err |= __put_user(owned_fp, &sc->sc_ownedfp);
 
-	if (current->used_math) {	/* fp is active.  */
+	if (owned_fp) {	/* fp is active.  */
+		tstat = regs->cp0_status;
+		/* This enable should not really be necessary */
 		set_cp0_status(ST0_CU1);
 		err |= save_fp_context(sc);
-		last_task_used_math = NULL;
-		regs->cp0_status &= ~ST0_CU1;
-		current->used_math = 0;
+		regs->cp0_status = tstat;
 	}
 
 	return err;
@@ -374,6 +381,16 @@ get_sigframe(struct k_sigaction *ka, str
 
 	/* Default to using normal stack */
 	sp = regs->regs[29];
+
+#ifdef CONFIG_MIPS_FPU_EMULATOR
+	/*
+	 * FPU emulator may have it's own trampoline active just
+	 * above the user stack, 16-bytes before the next lowest
+	 * 16 byte boundary.  Try to avoid trashing it.
+	 */
+	sp -= 32;	/* 32 ought to be enough, but... */
+
+#endif /* CONFIG_MIPS_FPU_EMULATOR */
 
 	/* This is the X/Open sanctioned signal stack switching.  */
 	if ((ka->sa.sa_flags & SA_ONSTACK) && ! on_sig_stack(sp))

Re: FP emulator patch

[Pete Popov]

Kevin D. Kissell wrote:
> As I wrote last night, it looks to me as if FPU context
> management in signals was broken in all versions and
> proposed patches that I've seen.  The way I see it is this:
> 
> If the current thread "owns" the FPU, we need to save the
> FPU state in the sigcontext and restore it on return.  If
> the signal handler uses floating point, it already has the
> FPU, and can do what it wants with it.  If it doesn't use
> the FPU, then we'll save and restore FPU state for
> nothing, but better safe than sorry.  In either case,
> the current thread retains ownership of the FPU.
> There is no reason to muck with the ownership data
> or the Cp0.Status.CU1 enables, apart from the
> questionable bit of enabling it before the FP context
> save in case it wasn't enabled.  I think that should go
> away, but I don't have time to test exhastively.  In
> any case, CU1 should have it's pre-signal state
> going into the signal handler.
> 
> If the current thread does *not* own the FPU, we don't
> need to save the thread FP state. If the signal handler
> does no FP, so much the better, there's nothing to
> be done.   If the signal handler uses FP, it will acquire 
> the FPU by normal means. The FP context will be saved 
> into the thread context of the previous owner, the signalling 
> thread will acquire the FPU, and the signal handler will do 
> it's FP. On return from the signal, we *must* de-allocate the
> FPU and clear the CU1 bit.  If that's done, and the
> thread (which had not *owned* the FPU prior to the
> signal) starts doing FP again, normal mechanisms
> will cause it's FP context to be restored.  If we don't,
> it will start exectuing with a bogus FP context.
> 
> The code I sketched last night is essentially correct,
> though it used a macro that doesn't exist.  I attach a
> patch relative to the current OSS repository's signal.c.
> The patch includes the stack frame tweak for the FPU
> emulator that was part of previous patches, but which
> is orthogonal to the problem under discussion.  I have
> built a kernel using this code and run 20 simultaneous
> copies of the MontaVista "stressor" program with no
> problems (though I also had the "v1.5" FPU emulator
> code).

I've lost track of all the patches now :-) Can I trouble you to create a 
complete patch of your arch/mips/math-emu, and whatever other files you've 
modifed, against Ralf's tree?

Pete

Re: FP emulator patch

[Daniel Jacobowitz]

On Thu, Aug 16, 2001 at 08:23:32PM +0200, Kevin D. Kissell wrote:
> As I wrote last night, it looks to me as if FPU context
> management in signals was broken in all versions and
> proposed patches that I've seen.  The way I see it is this:
> 
> If the current thread "owns" the FPU, we need to save the
> FPU state in the sigcontext and restore it on return.  If
> the signal handler uses floating point, it already has the
> FPU, and can do what it wants with it.  If it doesn't use
> the FPU, then we'll save and restore FPU state for
> nothing, but better safe than sorry.  In either case,
> the current thread retains ownership of the FPU.
> There is no reason to muck with the ownership data
> or the Cp0.Status.CU1 enables, apart from the
> questionable bit of enabling it before the FP context
> save in case it wasn't enabled.  I think that should go
> away, but I don't have time to test exhastively.  In
> any case, CU1 should have it's pre-signal state
> going into the signal handler.

This bit sounds fine to me.

> If the current thread does *not* own the FPU, we don't
> need to save the thread FP state. If the signal handler
> does no FP, so much the better, there's nothing to
> be done.   If the signal handler uses FP, it will acquire 
> the FPU by normal means. The FP context will be saved 
> into the thread context of the previous owner, the signalling 
> thread will acquire the FPU, and the signal handler will do 
> it's FP. On return from the signal, we *must* de-allocate the
> FPU and clear the CU1 bit.  If that's done, and the
> thread (which had not *owned* the FPU prior to the
> signal) starts doing FP again, normal mechanisms
> will cause it's FP context to be restored.  If we don't,
> it will start exectuing with a bogus FP context.

No, this is wrong.

 - Current thread has an FPU state saved but does not own the FPU.
 - Signal handler uses FP
 - Acquires FP through a lazy switch
 - (FPU) Context switch occurs while in signal handler.

Where do we put the state now?  We save it in the process's thread
structure, of course.  Overwriting what was there.

If the process has ever used math, the sigcontext must contain a saved
FP state.

current->used_math should never be set to zero in this sort of
situation.  It's not an ownership flag!  It marks whether the FP state
in the thread structure is valid.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: FP emulator patch

[Jun Sun]

Daniel Jacobowitz wrote:
> 
> On Thu, Aug 16, 2001 at 08:23:32PM +0200, Kevin D. Kissell wrote:
> > As I wrote last night, it looks to me as if FPU context
> > management in signals was broken in all versions and
> > proposed patches that I've seen.  The way I see it is this:
> >
> > If the current thread "owns" the FPU, we need to save the
> > FPU state in the sigcontext and restore it on return.  If
> > the signal handler uses floating point, it already has the
> > FPU, and can do what it wants with it.  If it doesn't use
> > the FPU, then we'll save and restore FPU state for
> > nothing, but better safe than sorry.  In either case,
> > the current thread retains ownership of the FPU.
> > There is no reason to muck with the ownership data
> > or the Cp0.Status.CU1 enables, apart from the
> > questionable bit of enabling it before the FP context
> > save in case it wasn't enabled.  I think that should go
> > away, but I don't have time to test exhastively.  In
> > any case, CU1 should have it's pre-signal state
> > going into the signal handler.
> 
> This bit sounds fine to me.
> 
> > If the current thread does *not* own the FPU, we don't
> > need to save the thread FP state. If the signal handler
> > does no FP, so much the better, there's nothing to
> > be done.   If the signal handler uses FP, it will acquire
> > the FPU by normal means. The FP context will be saved
> > into the thread context of the previous owner, the signalling
> > thread will acquire the FPU, and the signal handler will do
> > it's FP. On return from the signal, we *must* de-allocate the
> > FPU and clear the CU1 bit.  If that's done, and the
> > thread (which had not *owned* the FPU prior to the
> > signal) starts doing FP again, normal mechanisms
> > will cause it's FP context to be restored.  If we don't,
> > it will start exectuing with a bogus FP context.
> 
> No, this is wrong.
> 
>  - Current thread has an FPU state saved but does not own the FPU.
>  - Signal handler uses FP
>  - Acquires FP through a lazy switch
>  - (FPU) Context switch occurs while in signal handler.
> 
> Where do we put the state now?  We save it in the process's thread
> structure, of course.  Overwriting what was there.
> 
> If the process has ever used math, the sigcontext must contain a saved
> FP state.
> 

We discovered this bug a while back.  The right fix is to copy FP state from
thread structure to signal stack *when* the current thread used FPU BUT not
own it right now.

> current->used_math should never be set to zero in this sort of
> situation.  It's not an ownership flag!  It marks whether the FP state
> in the thread structure is valid.
>

Daniel, it is funny that I agree with your last statement but cannot agree
with your first one.  

Under the above mentioned situation, after we make the copy of FPU state from
thread structure to the saved signal context, we need to set used_math bit to
zero.  This way when the signal handler uses FPU for the first time - if it
ever uses it -, the normal lazy FPU switch mechanism can kick in smoothly.

Jun

Jun

Re: FP emulator patch

[Kevin D. Kissell]

> > current->used_math should never be set to zero in this sort of
> > situation.  It's not an ownership flag!  It marks whether the FP state
> > in the thread structure is valid.
>
> Daniel, it is funny that I agree with your last statement but cannot agree
> with your first one.
>
> Under the above mentioned situation, after we make the copy of FPU state
from
> thread structure to the saved signal context, we need to set used_math bit
to
> zero.  This way when the signal handler uses FPU for the first time - if
it
> ever uses it -, the normal lazy FPU switch mechanism can kick in smoothly.

We've had a lot of messages crossing here, but please
explain yourself here why clearing used_math helps in
this case.  If, as has been proposed, the current
thread does not own the FPU, and thus CU1 is not
enabled, the FPU switch mechanism should kick in
during the signal handler regardless. The signal
handler will inherit the thread's FPU state from
the thread context, and will muck with it, but
if, as has been noted, the sigcontext has
been loaded from the thread context before
the handler is dispatched, and is restored after
the handler executes, we're fine.  The only thing
I can see that clearing used_math would achieve
would be to guarantee the signal handler a virgin
FPU context.

            Regards,

            Kevin K.

Re: FP emulator patch

[Kevin D. Kissell]

> > current->used_math should never be set to zero in this sort of
> > situation.  It's not an ownership flag!  It marks whether the FP state
> > in the thread structure is valid.
>
> Daniel, it is funny that I agree with your last statement but cannot agree
> with your first one.
>
> Under the above mentioned situation, after we make the copy of FPU state
from
> thread structure to the saved signal context, we need to set used_math bit
to
> zero.  This way when the signal handler uses FPU for the first time - if
it
> ever uses it -, the normal lazy FPU switch mechanism can kick in smoothly.

We've had a lot of messages crossing here, but please
explain yourself here why clearing used_math helps in
this case.  If, as has been proposed, the current
thread does not own the FPU, and thus CU1 is not
enabled, the FPU switch mechanism should kick in
during the signal handler regardless. The signal
handler will inherit the thread's FPU state from
the thread context, and will muck with it, but
if, as has been noted, the sigcontext has
been loaded from the thread context before
the handler is dispatched, and is restored after
the handler executes, we're fine.  The only thing
I can see that clearing used_math would achieve
would be to guarantee the signal handler a virgin
FPU context.

            Regards,

            Kevin K.

Re: FP emulator patch

[Jun Sun]

"Kevin D. Kissell" wrote:
> 
> > > current->used_math should never be set to zero in this sort of
> > > situation.  It's not an ownership flag!  It marks whether the FP state
> > > in the thread structure is valid.
> >
> > Daniel, it is funny that I agree with your last statement but cannot agree
> > with your first one.
> >
> > Under the above mentioned situation, after we make the copy of FPU state
> from
> > thread structure to the saved signal context, we need to set used_math bit
> to
> > zero.  This way when the signal handler uses FPU for the first time - if
> it
> > ever uses it -, the normal lazy FPU switch mechanism can kick in smoothly.
> 
> We've had a lot of messages crossing here, but please
> explain yourself here why clearing used_math helps in
> this case.  If, as has been proposed, the current
> thread does not own the FPU, and thus CU1 is not
> enabled, the FPU switch mechanism should kick in
> during the signal handler regardless. The signal
> handler will inherit the thread's FPU state from
> the thread context, and will muck with it, but
> if, as has been noted, the sigcontext has
> been loaded from the thread context before
> the handler is dispatched, and is restored after
> the handler executes, we're fine.  The only thing
> I can see that clearing used_math would achieve
> would be to guarantee the signal handler a virgin
> FPU context.
> 


Yes, that is somewhat the purpose.  Essentially we want to see, at the
beginning of a signal handler execution, the process appears to have not used
FPU at all.

This requirement might be a must, because whether clearing current->used_math
bit determine which patch we will take in the do_cpu(), when signal handler
uses FPU for the first time.  See the code below.

        if (current->used_math) {               /* Using the FPU again.  */
                lazy_fpu_switch(last_task_used_math);
        } else {                                /* First time FPU user.  */
                init_fpu();
                current->used_math = 1;
        }
        last_task_used_math = current;

Clearly the second path is logically the correct one.

BTW, do I see another bug here in do_cpu()?  It seems that before we call
init_fpu(), we should check last_task_used_math.  If it is not NULL, we should
save the FP state to the last_task_used_math.  Hmm, strange ...

Jun

Re: FP emulator patch

[Kevin D. Kissell]

> > We've had a lot of messages crossing here, but please
> > explain yourself here why clearing used_math helps in
> > this case.  If, as has been proposed, the current
> > thread does not own the FPU, and thus CU1 is not
> > enabled, the FPU switch mechanism should kick in
> > during the signal handler regardless. The signal
> > handler will inherit the thread's FPU state from
> > the thread context, and will muck with it, but
> > if, as has been noted, the sigcontext has
> > been loaded from the thread context before
> > the handler is dispatched, and is restored after
> > the handler executes, we're fine.  The only thing
> > I can see that clearing used_math would achieve
> > would be to guarantee the signal handler a virgin
> > FPU context.
>
> Yes, that is somewhat the purpose.  Essentially we want to see, at the
> beginning of a signal handler execution, the process appears to have not
used
> FPU at all.
>
> This requirement might be a must, because whether clearing
current->used_math
> bit determine which patch we will take in the do_cpu(), when signal
handler
> uses FPU for the first time.  See the code below.
>
>         if (current->used_math) {               /* Using the FPU again.
*/
>                 lazy_fpu_switch(last_task_used_math);
>         } else {                                /* First time FPU user.
*/
>                 init_fpu();
>                 current->used_math = 1;
>         }
>         last_task_used_math = current;
>
> Clearly the second path is logically the correct one.

Not really.  See below.

> BTW, do I see another bug here in do_cpu()?  It seems that before we call
> init_fpu(), we should check last_task_used_math.  If it is not NULL, we
should
> save the FP state to the last_task_used_math.  Hmm, strange ...

Strange indeed.  And note that if the code were correct, your
surmise about the init_fpu() path being "logically the correct"
one would no longer be true - we'd be saving the FPU state of
the current process for no good reason.

The more I look at the FPU management code, the more I marvel
that it even gives an appearance of working...

            Regards,

            Kevin K.

Re: FP emulator patch

[Kevin D. Kissell]

> > We've had a lot of messages crossing here, but please
> > explain yourself here why clearing used_math helps in
> > this case.  If, as has been proposed, the current
> > thread does not own the FPU, and thus CU1 is not
> > enabled, the FPU switch mechanism should kick in
> > during the signal handler regardless. The signal
> > handler will inherit the thread's FPU state from
> > the thread context, and will muck with it, but
> > if, as has been noted, the sigcontext has
> > been loaded from the thread context before
> > the handler is dispatched, and is restored after
> > the handler executes, we're fine.  The only thing
> > I can see that clearing used_math would achieve
> > would be to guarantee the signal handler a virgin
> > FPU context.
>
> Yes, that is somewhat the purpose.  Essentially we want to see, at the
> beginning of a signal handler execution, the process appears to have not
used
> FPU at all.
>
> This requirement might be a must, because whether clearing
current->used_math
> bit determine which patch we will take in the do_cpu(), when signal
handler
> uses FPU for the first time.  See the code below.
>
>         if (current->used_math) {               /* Using the FPU again.
*/
>                 lazy_fpu_switch(last_task_used_math);
>         } else {                                /* First time FPU user.
*/
>                 init_fpu();
>                 current->used_math = 1;
>         }
>         last_task_used_math = current;
>
> Clearly the second path is logically the correct one.

Not really.  See below.

> BTW, do I see another bug here in do_cpu()?  It seems that before we call
> init_fpu(), we should check last_task_used_math.  If it is not NULL, we
should
> save the FP state to the last_task_used_math.  Hmm, strange ...

Strange indeed.  And note that if the code were correct, your
surmise about the init_fpu() path being "logically the correct"
one would no longer be true - we'd be saving the FPU state of
the current process for no good reason.

The more I look at the FPU management code, the more I marvel
that it even gives an appearance of working...

            Regards,

            Kevin K.

Re: FP emulator patch

[Pete Popov]

<cut>

> Strange indeed.  And note that if the code were correct, your
> surmise about the init_fpu() path being "logically the correct"
> one would no longer be true - we'd be saving the FPU state of
> the current process for no good reason.
> 
> The more I look at the FPU management code, the more I marvel
> that it even gives an appearance of working...

Maybe no one has stress tested it enough up till now. As soon as we
put he mips boards in QA we found a few fpu problems immediately.  I hope
those were it and we weren't just scratching the surface.

Pete

Re: FP emulator patch

[Daniel Jacobowitz]

On Thu, Aug 16, 2001 at 02:34:45PM -0700, Jun Sun wrote:
> Yes, that is somewhat the purpose.  Essentially we want to see, at the
> beginning of a signal handler execution, the process appears to have not used
> FPU at all.

Why?

> This requirement might be a must, because whether clearing current->used_math
> bit determine which patch we will take in the do_cpu(), when signal handler
> uses FPU for the first time.  See the code below.
> 
>         if (current->used_math) {               /* Using the FPU again.  */
>                 lazy_fpu_switch(last_task_used_math);
>         } else {                                /* First time FPU user.  */
>                 init_fpu();
>                 current->used_math = 1;
>         }
>         last_task_used_math = current;
> 
> Clearly the second path is logically the correct one.

Not really.  Why should it get a clean set of FP registers?  I think
the CORRECT thing would actually be for it to have the app's FP
registers.  Changes should not propogate back to the app, that's all.

> BTW, do I see another bug here in do_cpu()?  It seems that before we call
> init_fpu(), we should check last_task_used_math.  If it is not NULL, we should
> save the FP state to the last_task_used_math.  Hmm, strange ...

I thought I got all of these... <sigh>

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: FP emulator patch

[Jun Sun]

Daniel Jacobowitz wrote:
> 
> On Thu, Aug 16, 2001 at 02:34:45PM -0700, Jun Sun wrote:
> > Yes, that is somewhat the purpose.  Essentially we want to see, at the
> > beginning of a signal handler execution, the process appears to have not used
> > FPU at all.
> 
> Why?
> 
> > This requirement might be a must, because whether clearing current->used_math
> > bit determine which patch we will take in the do_cpu(), when signal handler
> > uses FPU for the first time.  See the code below.
> >
> >         if (current->used_math) {               /* Using the FPU again.  */
> >                 lazy_fpu_switch(last_task_used_math);
> >         } else {                                /* First time FPU user.  */
> >                 init_fpu();
> >                 current->used_math = 1;
> >         }
> >         last_task_used_math = current;
> >
> > Clearly the second path is logically the correct one.
> 
> Not really.  Why should it get a clean set of FP registers?  I think
> the CORRECT thing would actually be for it to have the app's FP
> registers.  Changes should not propogate back to the app, that's all.
> 

Ah, it seems I need to defend an "obvious" point. :-)

I am from the school which view signal handler as an interrupt context to a
normal process.  (In fact, some OSes implement signal handling by using a
separate thread/process).  Under this view, it is natural to provide a fresh
new context when a signal handler starts.

In other words, I am thinking in the following semantics: a signal handler
starting execution is equivalent to another process starting execution with
the same address space, except that the original process won't resume until
the "signal handler process" exits.

Thinking from that, I feel we should clear the current->used_math bit.

I think it is not correct for signal handler to inherit the FPU registers.  If
it wants to check the register values when the signal happens, it should do so
by examing the signal context.

Apparently a well code signal handler does not depend on whether we clear
current->used_math bit or not, because it should always set a FPU register
before it uses it.

Jun

> > BTW, do I see another bug here in do_cpu()?  It seems that before we call
> > init_fpu(), we should check last_task_used_math.  If it is not NULL, we should
> > save the FP state to the last_task_used_math.  Hmm, strange ...
> 
> I thought I got all of these... <sigh>
> 
> --
> Daniel Jacobowitz                           Carnegie Mellon University
> MontaVista Software                         Debian GNU/Linux Developer

Re: FP emulator patch

[Kevin D. Kissell]

> >         if (current->used_math) {               /* Using the FPU again.
*/
> >                 lazy_fpu_switch(last_task_used_math);
> >         } else {                                /* First time FPU user.
*/
> >                 init_fpu();
> >                 current->used_math = 1;
> >         }
> >         last_task_used_math = current;
> >
...
> > BTW, do I see another bug here in do_cpu()?  It seems that before we
call
> > init_fpu(), we should check last_task_used_math.  If it is not NULL, we
should
> > save the FP state to the last_task_used_math.  Hmm, strange ...
>
> I thought I got all of these... <sigh>

Looks like that should be:

           } else {
                if (last_task_used_math != NULL)
save_fp(last_task_used_math);
                init_fpu()
                current->used_math = 1;
            }

And things will be OK.  What's wierd is that I could have sworn
that I looked at this code long ago, and that there was a save
there.  But even in the 2.2.12-based tree, there is none.

It's quite late here in France.  If one (or more) of you guys could
sketch a code fragment that would copy FP context back and
forth between a thread structure and a sigcontext without passing
through the FPU while I'm sleeping, I'll put together an integrated
patch tomorrow.

            Regards,

            Kevin K.

Re: FP emulator patch

[Kevin D. Kissell]

> >         if (current->used_math) {               /* Using the FPU again.
*/
> >                 lazy_fpu_switch(last_task_used_math);
> >         } else {                                /* First time FPU user.
*/
> >                 init_fpu();
> >                 current->used_math = 1;
> >         }
> >         last_task_used_math = current;
> >
...
> > BTW, do I see another bug here in do_cpu()?  It seems that before we
call
> > init_fpu(), we should check last_task_used_math.  If it is not NULL, we
should
> > save the FP state to the last_task_used_math.  Hmm, strange ...
>
> I thought I got all of these... <sigh>

Looks like that should be:

           } else {
                if (last_task_used_math != NULL)
save_fp(last_task_used_math);
                init_fpu()
                current->used_math = 1;
            }

And things will be OK.  What's wierd is that I could have sworn
that I looked at this code long ago, and that there was a save
there.  But even in the 2.2.12-based tree, there is none.

It's quite late here in France.  If one (or more) of you guys could
sketch a code fragment that would copy FP context back and
forth between a thread structure and a sigcontext without passing
through the FPU while I'm sleeping, I'll put together an integrated
patch tomorrow.

            Regards,

            Kevin K.

Re: FP emulator patch

[Kevin D. Kissell]

> If the current thread does *not* own the FPU, we don't
> need to save the thread FP state. If the signal handler
> does no FP, so much the better, there's nothing to
> be done.   If the signal handler uses FP, it will acquire 
> the FPU by normal means. The FP context will be saved 
> into the thread context of the previous owner, the signalling 
> thread will acquire the FPU, and the signal handler will do 
> it's FP. On return from the signal, we *must* de-allocate the
> FPU and clear the CU1 bit.  If that's done, and the
> thread (which had not *owned* the FPU prior to the
> signal) starts doing FP again, normal mechanisms
> will cause it's FP context to be restored.  If we don't,
> it will start exectuing with a bogus FP context.

There is, actually, one conceptual hole in this
scheme (and in every other one we've seen) if the
following scenario occurs.  The current thread does
not own the FPU, the signal handler executes FP
instructions and runs for so long that it gets
switched out.  The thread's FP state will then
become that of the signal handler, and, since the
thread didn't have the FPU when the signal context
was set up, it the pre-signal state will be lost.

>From that standpoint, the old code that saved the
contex if current->used_math was almost correct.
Almost correct, because the naive dump-the-FPU
code is correct only if the FPU belongs to the thread.
If it doesn't, the last saved thead FPU context, and not
the FPU contents, needs to be copied into the signal 
context.  Symmetrically, in restore_sigcontext(), the current 
code restores the FPU if we *owned* the fp prior to the 
signal, but we must likewise restore the thread FPU 
context from the sigcontext if we *didn't* own the FPU, but
the signal handler subsequently acquired it.

The alternative to doing direct sigcontext<->thread context
saves/restores without passing through the FPU would
be to force a fpu context switch on every signal dispatch,
which I find rather inelegant.

I'll see if I can't come up with a new patch that also
takes this into account, but may not have time before
I take off on vacation this weekend...

            Kevin K.

Re: FP emulator patch

[Kevin D. Kissell]

> If the current thread does *not* own the FPU, we don't
> need to save the thread FP state. If the signal handler
> does no FP, so much the better, there's nothing to
> be done.   If the signal handler uses FP, it will acquire 
> the FPU by normal means. The FP context will be saved 
> into the thread context of the previous owner, the signalling 
> thread will acquire the FPU, and the signal handler will do 
> it's FP. On return from the signal, we *must* de-allocate the
> FPU and clear the CU1 bit.  If that's done, and the
> thread (which had not *owned* the FPU prior to the
> signal) starts doing FP again, normal mechanisms
> will cause it's FP context to be restored.  If we don't,
> it will start exectuing with a bogus FP context.

There is, actually, one conceptual hole in this
scheme (and in every other one we've seen) if the
following scenario occurs.  The current thread does
not own the FPU, the signal handler executes FP
instructions and runs for so long that it gets
switched out.  The thread's FP state will then
become that of the signal handler, and, since the
thread didn't have the FPU when the signal context
was set up, it the pre-signal state will be lost.

From that standpoint, the old code that saved the
contex if current->used_math was almost correct.
Almost correct, because the naive dump-the-FPU
code is correct only if the FPU belongs to the thread.
If it doesn't, the last saved thead FPU context, and not
the FPU contents, needs to be copied into the signal 
context.  Symmetrically, in restore_sigcontext(), the current 
code restores the FPU if we *owned* the fp prior to the 
signal, but we must likewise restore the thread FPU 
context from the sigcontext if we *didn't* own the FPU, but
the signal handler subsequently acquired it.

The alternative to doing direct sigcontext<->thread context
saves/restores without passing through the FPU would
be to force a fpu context switch on every signal dispatch,
which I find rather inelegant.

I'll see if I can't come up with a new patch that also
takes this into account, but may not have time before
I take off on vacation this weekend...

            Kevin K.

Re: FP emulator patch

[Jun Sun]

"Kevin D. Kissell" wrote:
> 
> > If the current thread does *not* own the FPU, we don't
> > need to save the thread FP state. If the signal handler
> > does no FP, so much the better, there's nothing to
> > be done.   If the signal handler uses FP, it will acquire
> > the FPU by normal means. The FP context will be saved
> > into the thread context of the previous owner, the signalling
> > thread will acquire the FPU, and the signal handler will do
> > it's FP. On return from the signal, we *must* de-allocate the
> > FPU and clear the CU1 bit.  If that's done, and the
> > thread (which had not *owned* the FPU prior to the
> > signal) starts doing FP again, normal mechanisms
> > will cause it's FP context to be restored.  If we don't,
> > it will start exectuing with a bogus FP context.
> 
> There is, actually, one conceptual hole in this
> scheme (and in every other one we've seen) if the
> following scenario occurs.  The current thread does
> not own the FPU, the signal handler executes FP
> instructions and runs for so long that it gets
> switched out.  The thread's FP state will then
> become that of the signal handler, and, since the
> thread didn't have the FPU when the signal context
> was set up, it the pre-signal state will be lost.
> 
> From that standpoint, the old code that saved the
> contex if current->used_math was almost correct.
> Almost correct, because the naive dump-the-FPU
> code is correct only if the FPU belongs to the thread.
> If it doesn't, the last saved thead FPU context, and not
> the FPU contents, needs to be copied into the signal
> context.  Symmetrically, in restore_sigcontext(), the current
> code restores the FPU if we *owned* the fp prior to the
> signal, but we must likewise restore the thread FPU
> context from the sigcontext if we *didn't* own the FPU, but
> the signal handler subsequently acquired it.
> 
> The alternative to doing direct sigcontext<->thread context
> saves/restores without passing through the FPU would
> be to force a fpu context switch on every signal dispatch,
> which I find rather inelegant.
> 
> I'll see if I can't come up with a new patch that also
> takes this into account, but may not have time before
> I take off on vacation this weekend...
> 
>             Kevin K.

Kevin,

The following pseudo code should take care of that bug.


setup_sigcontext():

...

if (owned_fp) ASSERT(current->used_math);	/* my assumption, obvious */

err |= __put_user(owned_fp, &sc->sc_ownedfp);
err |= __put_user(current->used_math, &sc->sc_used_math);

if (owned_math) {
	/* save current FP state to signal context */
	current->used_math = 0;
	/* perhaps some other bits need to be modified etc */
} else if (current->used_math) {
	/* copy FP state from thread structure to signal context */
	current->used_math = 0;
	/* perhaps some other bits need to be modified etc */
}

....

restore_context():

..
__get_user(owned_fp, &sc->sc_ownedfp);
__get_user(&current->used_math, &sc->sc_used_math);

if (owned_fp) ASSERT(current->used_math);	/* my assumption, obvious */

if (owned_fp) {
	restore_fp_context(sc);
	last_task_used_math = current;
} else if (current->used_math) {
	/* copy FP state from signal context to thread structure */
}


(I meant to do that myself, of course, but yeah, yeah, ... :-0)

Jun

FP emulator patch

[Carsten Langgaard]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

There has been some reports regarding FP emulator failures, which the
attached patch should solve.
The patch include a fix for emulation of instructions in a COP1
delay-slot, a fix for FP context switching and some additional stuff ,
which was needed to pass our torture test.

Ralf could you please apply this patch.

/Carsten


--
_    _ ____  ___   Carsten Langgaard   Mailto:carstenl@mips.com
|\  /|||___)(___   MIPS Denmark        Direct: +45 4486 5527
| \/ |||    ____)  Lautrupvang 4B      Switch: +45 4486 5555
  TECHNOLOGIES     2750 Ballerup       Fax...: +45 4486 5556
                   Denmark             http://www.mips.com



[-- Attachment #2: fp_emulator.patch --]
[-- Type: text/plain, Size: 12232 bytes --]

Index: linux/arch/mips/kernel/signal.c
===================================================================
RCS file: /cvs/linux/arch/mips/kernel/signal.c,v
retrieving revision 1.36
diff -u -r1.36 signal.c
--- linux/arch/mips/kernel/signal.c	2001/06/18 22:43:35	1.36
+++ linux/arch/mips/kernel/signal.c	2001/08/15 12:33:38
@@ -220,6 +220,8 @@
 
 	err |= __get_user(owned_fp, &sc->sc_ownedfp);
 	if (owned_fp) {
+		if (last_task_used_math && (last_task_used_math != current))
+			last_task_used_math->thread.cp0_status &= ~ST0_CU1;
 		err |= restore_fp_context(sc);
 		last_task_used_math = current;
 	}
@@ -353,12 +355,11 @@
 	owned_fp = (current == last_task_used_math);
 	err |= __put_user(owned_fp, &sc->sc_ownedfp);
 
-	if (current->used_math) {	/* fp is active.  */
+	if (owned_fp) { /* fp is active.  */
 		set_cp0_status(ST0_CU1);
 		err |= save_fp_context(sc);
 		last_task_used_math = NULL;
 		regs->cp0_status &= ~ST0_CU1;
-		current->used_math = 0;
 	}
 
 	return err;
@@ -375,6 +376,13 @@
 	/* Default to using normal stack */
 	sp = regs->regs[29];
 
+	/* 
+	 * FPU emulator may have it's own trampoline active just
+	 * above the user stack, 16-bytes before the next lowest
+	 * 16 byte boundary.  Try to avoid trashing it.
+	 */
+	sp -= 32;
+
 	/* This is the X/Open sanctioned signal stack switching.  */
 	if ((ka->sa.sa_flags & SA_ONSTACK) && ! on_sig_stack(sp))
                 sp = current->sas_ss_sp + current->sas_ss_size;
@@ -435,7 +443,7 @@
 
 #if DEBUG_SIG
 	printk("SIG deliver (%s:%d): sp=0x%p pc=0x%p ra=0x%p\n",
-	       current->comm, current->pid, frame, regs->cp0_epc, frame->code);
+	       current->comm, current->pid, frame, regs->cp0_epc, frame->sf_code);
 #endif
         return;
 
Index: linux/arch/mips/kernel/unaligned.c
===================================================================
RCS file: /cvs/linux/arch/mips/kernel/unaligned.c,v
retrieving revision 1.12
diff -u -r1.12 unaligned.c
--- linux/arch/mips/kernel/unaligned.c	2001/07/11 22:05:11	1.12
+++ linux/arch/mips/kernel/unaligned.c	2001/08/15 12:33:38
@@ -365,15 +365,12 @@
 		return;
 	}
 
-	die_if_kernel ("Unhandled kernel unaligned access", regs);
 	send_sig(SIGSEGV, current, 1);
 	return;
 sigbus:
-	die_if_kernel ("Unhandled kernel unaligned access", regs);
 	send_sig(SIGBUS, current, 1);
 	return;
 sigill:
-	die_if_kernel ("Unhandled kernel unaligned access or invalid instruction", regs);
 	send_sig(SIGILL, current, 1);
 	return;
 }
@@ -391,11 +388,10 @@
 	 * of the CPU after executing the instruction
 	 * in the delay slot of an emulated branch.
 	 */
+	/* Terminate if exception was recognized as a delay slot return */
+	if(do_dsemulret(regs)) return;
 
-	if ((unsigned long)regs->cp0_epc == current->thread.dsemul_aerpc) {
-		do_dsemulret(regs);
-		return;
-	}
+	/* Otherwise handle as normal */
 
 	/*
 	 * Did we catch a fault trying to load an instruction?
@@ -417,7 +413,6 @@
 	return;
 
 sigbus:
-	die_if_kernel ("Kernel unaligned instruction access", regs);
 	force_sig(SIGBUS, current);
 
 	return;
Index: linux/arch/mips/math-emu/cp1emu.c
===================================================================
RCS file: /cvs/linux/arch/mips/math-emu/cp1emu.c,v
retrieving revision 1.7
diff -u -r1.7 cp1emu.c
--- linux/arch/mips/math-emu/cp1emu.c	2001/08/02 21:55:26	1.7
+++ linux/arch/mips/math-emu/cp1emu.c	2001/08/15 12:33:38
@@ -6,7 +6,7 @@
  * http://www.algor.co.uk
  *
  * Kevin D. Kissell, kevink@mips.com and Carsten Langgaard, carstenl@mips.com
- * Copyright (C) 2000  MIPS Technologies, Inc.
+ * Copyright (C) 2000-2001  MIPS Technologies, Inc.
  *
  *  This program is free software; you can distribute it and/or modify it
  *  under the terms of the GNU General Public License (Version 2) as
@@ -273,7 +273,10 @@
 			fpuemuprivate.stats.errors++;
 			return SIGBUS;
 		}
+		/* __computer_return_epc() will have updated cp0_epc */
 		contpc = REG_TO_VA xcp->cp0_epc;
+		/* In order not to confuse ptrace() et al, tweak context */
+		xcp->cp0_epc = VA_TO_REG emulpc - 4;
 	} else {
 		emulpc = REG_TO_VA xcp->cp0_epc;
 		contpc = REG_TO_VA xcp->cp0_epc + 4;
@@ -753,29 +756,73 @@
  *  execution of delay-slot instruction execution.
  */
 
+* Instruction inserted following delay slot instruction to force trap */
+
+#define AdELOAD 0x8c000001     /* lw $0,1($0) */
+
+/* Instruction inserted following the AdELOAD to further tag the sequence */
+
+#define BD_COOKIE 0x0000bd36 /* tne $0,$0 with baggage */
+
 int do_dsemulret(struct pt_regs *xcp)
 {
+	unsigned long *pinst;
+	unsigned long stackitem;
+	int err = 0;
+
+	/* See if this trap was deliberate. First check the instruction */
+
+	pinst = (unsigned long *) REG_TO_VA(xcp->cp0_epc);
+
+	/* 
+	 * If we can't even access the area, something is very
+	 * wrong, but we'll leave that to the default handling
+	 */
+	if (verify_area(VERIFY_READ, pinst, sizeof(unsigned long) * 3))
+		return 0;
+
+	/* Is the instruction pointed to by the EPC an AdELOAD? */
+	stackitem = mips_get_word(xcp, pinst, &err);
+	if (err || (stackitem != AdELOAD)) return 0;
+	/* Is the following memory word the BD_COOKIE? */
+	stackitem = mips_get_word(xcp, pinst+1, &err);
+	if (err || (stackitem != BD_COOKIE)) return 0;
+	
+	/* 
+	 * At this point, we are satisfied that it's a BD emulation 
+	 * trap.  Yes, a user might have deliberately put two
+	 * malformed and useless instructions in a row in his program,
+	 * in which case he's in for a nasty surprise - the
+	 * next instruction will be treated as a continuation
+	 * address!  Alas, this seems to be the only way that
+	 * we can handle signals, recursion, and longjmps()
+	 * in the context of emulating the branch delay instruction.
+	 */
+
 #ifdef DSEMUL_TRACE
 	printk("desemulret\n");
 #endif
+
+	/* Fetch the Saved EPC to Resume */
+ 
+	stackitem = mips_get_word(xcp, pinst+2, &err);
+	if (err) {
+		/* This is not a good situation to be in */
+		fpuemuprivate.stats.errors++;
+		force_sig(SIGBUS, current);
+		return(1);
+	}
+ 
 	/* Set EPC to return to post-branch instruction */
-	xcp->cp0_epc = current->thread.dsemul_epc;
-	/*
-	 * Clear the state that got us here.
-	 */
-	current->thread.dsemul_aerpc = (unsigned long) 0;
+	xcp->cp0_epc = stackitem;
 
-	return 0;
+	return 1;
 }
 
-
-#define AdELOAD 0x8c000001	/* lw $0,1($0) */
-
 static int
 mips_dsemul(struct pt_regs *xcp, mips_instruction ir, vaddr_t cpc)
 {
 	mips_instruction *dsemul_insns;
-	mips_instruction forcetrap;
 	extern asmlinkage void handle_dsemulret(void);
 
 	if (ir == 0) {		/* a nop is easy */
@@ -791,13 +838,22 @@
 	 * and put a trap after it which we can catch and jump to 
 	 * the required address any alternative apart from full 
 	 * instruction emulation!!.
+	 * 
+	 * Algorithmics used a system call instruction, and
+	 * borrowed that vector.  MIPS/Linux version is a bit
+	 * more heavyweight in the interests of portability and
+	 * multiprocessor support.  We flag the thread for special
+	 * handling in the unaligned access handler and force an
+	 * address error excpetion.
 	 */
-	dsemul_insns = (mips_instruction *) (xcp->regs[29] & ~3);
-	dsemul_insns -= 3;	/* Two instructions, plus one for luck ;-) */
+
+	/* Ensure that the two instructions are in the same cache line */
+	dsemul_insns = (mips_instruction *) (xcp->regs[29] & ~0xf);
+	dsemul_insns -= 4;      /* Retain 16-byte alignment */
 
 	/* Verify that the stack pointer is not competely insane */
 	if (verify_area(VERIFY_WRITE, dsemul_insns,
-	                sizeof(mips_instruction) * 2))
+	                sizeof(mips_instruction) * 4))
 		return SIGBUS;
 
 	if (mips_put_word(xcp, &dsemul_insns[0], ir)) {
@@ -805,29 +861,22 @@
 		return SIGBUS;
 	}
 
-	/* 
-	 * Algorithmics used a system call instruction, and
-	 * borrowed that vector.  MIPS/Linux version is a bit
-	 * more heavyweight in the interests of portability and
-	 * multiprocessor support.  We flag the thread for special
-	 * handling in the unaligned access handler and force an
-	 * address error excpetion.
-	 */
+	if (mips_put_word(xcp, &dsemul_insns[1], (mips_instruction)AdELOAD)) {
+		fpuemuprivate.stats.errors++;
+		return (SIGBUS);
+	}
 
-	/* If one is *really* paranoid, one tests for a bad stack pointer */
-	if ((xcp->regs[29] & 0x3) == 0x3)
-		forcetrap = AdELOAD - 1;
-	else
-		forcetrap = AdELOAD;
+	if (mips_put_word(xcp, &dsemul_insns[2], 
+			  (mips_instruction)BD_COOKIE)) {
+		fpuemuprivate.stats.errors++;
+		return (SIGBUS);
+	}
 
-	if (mips_put_word(xcp, &dsemul_insns[1], forcetrap)) {
+	if (mips_put_word(xcp, &dsemul_insns[3], (mips_instruction)cpc)) {
 		fpuemuprivate.stats.errors++;
 		return (SIGBUS);
 	}
 
-	/* Set thread state to catch and handle the exception */
-	current->thread.dsemul_epc = (unsigned long) cpc;
-	current->thread.dsemul_aerpc = (unsigned long) &dsemul_insns[1];
 	xcp->cp0_epc = VA_TO_REG & dsemul_insns[0];
 	flush_cache_sigtramp((unsigned long) dsemul_insns);
 
Index: linux/arch/mips/math-emu/kernel_linkage.c
===================================================================
RCS file: /cvs/linux/arch/mips/math-emu/kernel_linkage.c,v
retrieving revision 1.3
diff -u -r1.3 kernel_linkage.c
--- linux/arch/mips/math-emu/kernel_linkage.c	2001/01/13 18:17:58	1.3
+++ linux/arch/mips/math-emu/kernel_linkage.c	2001/08/15 12:33:38
@@ -3,7 +3,7 @@
  *  arch/mips/math_emu/kernel_linkage.c
  *
  *  Kevin D. Kissell, kevink@mips and Carsten Langgaard, carstenl@mips.com
- *  Copyright (C) 2000 MIPS Technologies, Inc.  All rights reserved.
+ *  Copyright (C) 2000-2001 MIPS Technologies, Inc.  All rights reserved.
  *
  * ########################################################################
  *
@@ -45,7 +45,7 @@
  
 	if (first) {
 		first = 0;
-		printk("Algorithmics/MIPS FPU Emulator v1.4\n");
+		printk("Algorithmics/MIPS FPU Emulator v1.5\n");
 	}
 
 	current->thread.fpu.soft.sr = 0;
Index: linux/arch/mips/tools/offset.c
===================================================================
RCS file: /cvs/linux/arch/mips/tools/offset.c,v
retrieving revision 1.16
diff -u -r1.16 offset.c
--- linux/arch/mips/tools/offset.c	2000/12/10 07:56:02	1.16
+++ linux/arch/mips/tools/offset.c	2001/08/15 12:33:39
@@ -121,10 +121,6 @@
 	       thread.irix_trampoline);
 	offset("#define THREAD_OLDCTX  ", struct task_struct, \
 	       thread.irix_oldctx);
-	offset("#define THREAD_DSEEPC  ", struct task_struct, \
-	       thread.dsemul_epc);
-	offset("#define THREAD_DSEAERPC ", struct task_struct, \
-	       thread.dsemul_aerpc);
 	linefeed;
 }
 
Index: linux/include/asm-mips/processor.h
===================================================================
RCS file: /cvs/linux/include/asm-mips/processor.h,v
retrieving revision 1.37
diff -u -r1.37 processor.h
--- linux/include/asm-mips/processor.h	2001/07/23 00:17:39	1.37
+++ linux/include/asm-mips/processor.h	2001/08/15 12:33:42
@@ -151,21 +151,6 @@
 	mm_segment_t current_ds;
 	unsigned long irix_trampoline;  /* Wheee... */
 	unsigned long irix_oldctx;
-
-	/*
-	 * These are really only needed if the full FPU emulator is configured.
-	 * Would be made conditional on MIPS_FPU_EMULATOR if it weren't for the
-	 * fact that having offset.h rebuilt differently for different config
-	 * options would be asking for trouble.
-	 *
-	 * Saved EPC during delay-slot emulation (see math-emu/cp1emu.c)
-	 */
-	unsigned long dsemul_epc;
-
-	/*
-	 * Pointer to instruction used to induce address error
-	 */
-	unsigned long dsemul_aerpc;
 };
 
 #endif /* !defined (_LANGUAGE_ASSEMBLY) */
@@ -195,11 +180,6 @@
 	 * For now the default is to fix address errors \
 	 */ \
 	MF_FIXADE, { 0 }, 0, 0, \
-	/* \
-	 * dsemul_epc and dsemul_aerpc should never be used uninitialized, \
-	 * but... \
-	 */ \
-	0 ,0 \
 }
 
 #ifdef __KERNEL__
@@ -235,8 +215,8 @@
  * Do necessary setup to start up a newly executed thread.
  */
 #define start_thread(regs, new_pc, new_sp) do {				\
-	/* New thread looses kernel privileges. */			\
-	regs->cp0_status = (regs->cp0_status & ~(ST0_CU0|ST0_KSU)) | KU_USER;\
+	/* New thread loses kernel and FPU privileges. */               \
+        regs->cp0_status = (regs->cp0_status & ~(ST0_CU0|ST0_KSU|ST0_CU1)) | KU_USER;\
 	regs->cp0_epc = new_pc;						\
 	regs->regs[29] = new_sp;					\
 	current->thread.current_ds = USER_DS;				\

Re: FP emulator patch

[Daniel Jacobowitz]

On Wed, Aug 15, 2001 at 02:53:12PM +0200, Carsten Langgaard wrote:
> There has been some reports regarding FP emulator failures, which the
> attached patch should solve.
> The patch include a fix for emulation of instructions in a COP1
> delay-slot, a fix for FP context switching and some additional stuff ,
> which was needed to pass our torture test.
> 
> Ralf could you please apply this patch.

Two comments, especially since parts of this seem to be the patch I
posted here over a month ago.

> Index: linux/arch/mips/kernel/signal.c

> @@ -353,12 +355,11 @@
>  	owned_fp = (current == last_task_used_math);
>  	err |= __put_user(owned_fp, &sc->sc_ownedfp);
>  
> -	if (current->used_math) {	/* fp is active.  */
> +	if (owned_fp) { /* fp is active.  */
>  		set_cp0_status(ST0_CU1);
>  		err |= save_fp_context(sc);
>  		last_task_used_math = NULL;
>  		regs->cp0_status &= ~ST0_CU1;
> -		current->used_math = 0;
>  	}
>  
>  	return err;

This is absolutely not right.  It's righter than the status quo.  If we
don't own the FP, you don't save the FP.  Then we can use FP in the
signal handler, corrupting the process's original floating point
context.


> Index: linux/include/asm-mips/processor.h


> @@ -235,8 +215,8 @@
>   * Do necessary setup to start up a newly executed thread.
>   */
>  #define start_thread(regs, new_pc, new_sp) do {				\
> -	/* New thread looses kernel privileges. */			\
> -	regs->cp0_status = (regs->cp0_status & ~(ST0_CU0|ST0_KSU)) | KU_USER;\
> +	/* New thread loses kernel and FPU privileges. */               \
> +        regs->cp0_status = (regs->cp0_status & ~(ST0_CU0|ST0_KSU|ST0_CU1)) | KU_USER;\
>  	regs->cp0_epc = new_pc;						\
>  	regs->regs[29] = new_sp;					\
>  	current->thread.current_ds = USER_DS;				\

I could be misremembering, but I believe that Ralf said this should be
unnecessary and the problem was somewhere else.  On the other hand, I
still think it's a good idea.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: FP emulator patch

[Kevin D. Kissell]

> Two comments, especially since parts of this seem to be the patch I
> posted here over a month ago.
> 
> > Index: linux/arch/mips/kernel/signal.c
> 
> > @@ -353,12 +355,11 @@
> >  owned_fp = (current == last_task_used_math);
> >  err |= __put_user(owned_fp, &sc->sc_ownedfp);
> >  
> > - if (current->used_math) { /* fp is active.  */
> > + if (owned_fp) { /* fp is active.  */
> >  set_cp0_status(ST0_CU1);
> >  err |= save_fp_context(sc);
> >  last_task_used_math = NULL;
> >  regs->cp0_status &= ~ST0_CU1;
> > - current->used_math = 0;
> >  }
> >  
> >  return err;
> 
> This is absolutely not right.  It's righter than the status quo.  If we
> don't own the FP, you don't save the FP.  Then we can use FP in the
> signal handler, corrupting the process's original floating point
> context.

I only worked on the FP branch emulation fix, and hadn't looked
at the stack signal context problem until now.   If the current thread
has any FPU state, it needs to be saved ahead of signal delivery
for the reasons you suggest above.  I assume that's the reason
for the previous "if (current->used_math)" condition.  If there's
been a problem with FP register corruption in the face of signals
and context switches, it's presumably because, while we're
saving the state in the sigcontext so that it will be restored
on the signal return, we're also clearing last_task_used_math
and curren->used_math.  The former means that if
we invoke the signal handler, it returns, and we will restore the
FP context to the register file.  But if the signal handler 
*doesn't* do any FP, we've got a "dirty" FPU and no owner
for the state, and Bad Things happen.  And the way the current
code is written, if the signal handler does happen to acquire
the FPU, the thread inherits the enable, the register contents,
and an obligation to save the FPU state uselessly on the next
context switch.

I don't understand why there is any manipulation of the
FPU ownership status going on in setup_sigcontext(), nor 
any persistent modification of the Cp0.Status state.  
Shouldn't the code in setup_sigcontext() look more like:

    if(owned_fp) {
        /* Not clear to me how we could have owned_fp 
           true with CU1 off.  Double check this... */
        someaccursednewtemp = regs->cp0_status;
        set_cp0_status(ST0_CU1);
        err |= save_fp_context(sc);
        regs->cp0_status = someaccursednewtemp;
    }

Then, in the symmetric code fragment in restore_sigcontext()

    if(owned_fp) {
        err |= restore_fp_context(sc);
    } else {
        if (current == last_task_used_math) {
        /* signal handler acquired FPU - give it back */
            last_task_used_math = NULL;
            current->used_math = 0;
            clear_cp0_status(ST0_CU1);
        }
    }

Disclaimer:  The above was typed into Outlook Express
and may contain typos, but I think it expresses the idea
well enough for someone to tell me if I'm completely off
base.

            Regards,

            Kevin K.


 

Re: FP emulator patch

[Kevin D. Kissell]

> Two comments, especially since parts of this seem to be the patch I
> posted here over a month ago.
> 
> > Index: linux/arch/mips/kernel/signal.c
> 
> > @@ -353,12 +355,11 @@
> >  owned_fp = (current == last_task_used_math);
> >  err |= __put_user(owned_fp, &sc->sc_ownedfp);
> >  
> > - if (current->used_math) { /* fp is active.  */
> > + if (owned_fp) { /* fp is active.  */
> >  set_cp0_status(ST0_CU1);
> >  err |= save_fp_context(sc);
> >  last_task_used_math = NULL;
> >  regs->cp0_status &= ~ST0_CU1;
> > - current->used_math = 0;
> >  }
> >  
> >  return err;
> 
> This is absolutely not right.  It's righter than the status quo.  If we
> don't own the FP, you don't save the FP.  Then we can use FP in the
> signal handler, corrupting the process's original floating point
> context.

I only worked on the FP branch emulation fix, and hadn't looked
at the stack signal context problem until now.   If the current thread
has any FPU state, it needs to be saved ahead of signal delivery
for the reasons you suggest above.  I assume that's the reason
for the previous "if (current->used_math)" condition.  If there's
been a problem with FP register corruption in the face of signals
and context switches, it's presumably because, while we're
saving the state in the sigcontext so that it will be restored
on the signal return, we're also clearing last_task_used_math
and curren->used_math.  The former means that if
we invoke the signal handler, it returns, and we will restore the
FP context to the register file.  But if the signal handler 
*doesn't* do any FP, we've got a "dirty" FPU and no owner
for the state, and Bad Things happen.  And the way the current
code is written, if the signal handler does happen to acquire
the FPU, the thread inherits the enable, the register contents,
and an obligation to save the FPU state uselessly on the next
context switch.

I don't understand why there is any manipulation of the
FPU ownership status going on in setup_sigcontext(), nor 
any persistent modification of the Cp0.Status state.  
Shouldn't the code in setup_sigcontext() look more like:

    if(owned_fp) {
        /* Not clear to me how we could have owned_fp 
           true with CU1 off.  Double check this... */
        someaccursednewtemp = regs->cp0_status;
        set_cp0_status(ST0_CU1);
        err |= save_fp_context(sc);
        regs->cp0_status = someaccursednewtemp;
    }

Then, in the symmetric code fragment in restore_sigcontext()

    if(owned_fp) {
        err |= restore_fp_context(sc);
    } else {
        if (current == last_task_used_math) {
        /* signal handler acquired FPU - give it back */
            last_task_used_math = NULL;
            current->used_math = 0;
            clear_cp0_status(ST0_CU1);
        }
    }

Disclaimer:  The above was typed into Outlook Express
and may contain typos, but I think it expresses the idea
well enough for someone to tell me if I'm completely off
base.

            Regards,

            Kevin K.


 

Re: FP emulator patch

[Atsushi Nemoto]

[-- Attachment #1: Type: Text/Plain, Size: 962 bytes --]

>>>>> On Wed, 15 Aug 2001 11:06:34 -0700, Daniel Jacobowitz <dan@debian.org> said:
>> Index: linux/arch/mips/kernel/signal.c
>
>> @@ -353,12 +355,11 @@
>>  	owned_fp = (current == last_task_used_math);
>>  	err |= __put_user(owned_fp, &sc->sc_ownedfp);
>>  
>> -	if (current->used_math) {	/* fp is active.  */
>> +	if (owned_fp) { /* fp is active.  */
>>  		set_cp0_status(ST0_CU1);
>>  		err |= save_fp_context(sc);
>>  		last_task_used_math = NULL;
>>  		regs->cp0_status &= ~ST0_CU1;
>> -		current->used_math = 0;
>>  	}
>>  
>>  	return err;

dan> This is absolutely not right.  It's righter than the status quo.
dan> If we don't own the FP, you don't save the FP.  Then we can use
dan> FP in the signal handler, corrupting the process's original
dan> floating point context.

I also am trying to fix this problem.  How about my patch?

restore_sigcontext() can be more optimized, but I think this is a
smallest patch to fix the problem.

---
Atsushi Nemoto

[-- Attachment #2: signal.c.patch --]
[-- Type: Text/Plain, Size: 920 bytes --]

diff -ur linux.sgi/arch/mips/kernel/signal.c linux/arch/mips/kernel/signal.c
--- linux.sgi/arch/mips/kernel/signal.c	Mon Jun 25 22:56:56 2001
+++ linux/arch/mips/kernel/signal.c	Thu Aug 16 13:09:28 2001
@@ -350,11 +350,18 @@
 	err |= __put_user(regs->cp0_cause, &sc->sc_cause);
 	err |= __put_user(regs->cp0_badvaddr, &sc->sc_badvaddr);
 
-	owned_fp = (current == last_task_used_math);
+	/* restore_sigcontext must restore the fp context even if this
+	   process was not last_task_used_math. */
+	owned_fp = current->used_math;
 	err |= __put_user(owned_fp, &sc->sc_ownedfp);
 
 	if (current->used_math) {	/* fp is active.  */
+#if 0
+		/* Do not set CU1 here.  If this process does not
+		   owned fp, save_fp_context causes lazy_fpu_switch
+		   (and fp-owner's context will saved). */
 		set_cp0_status(ST0_CU1);
+#endif
 		err |= save_fp_context(sc);
 		last_task_used_math = NULL;
 		regs->cp0_status &= ~ST0_CU1;

Re: FP emulator patch

[Kevin D. Kissell]

> I also am trying to fix this problem.  How about my patch?
> 
> restore_sigcontext() can be more optimized, but I think this is a
> smallest patch to fix the problem.

I beleive that your patch might fix a symptom of the bugs
that I talked about in my mail last night, but it doesn't solve
the underlying problem.  With the signal setup/restore 
changes I posted last night, your patch should not be
necessary.

            Kevin K.

Re: FP emulator patch

[Kevin D. Kissell]

> I also am trying to fix this problem.  How about my patch?
> 
> restore_sigcontext() can be more optimized, but I think this is a
> smallest patch to fix the problem.

I beleive that your patch might fix a symptom of the bugs
that I talked about in my mail last night, but it doesn't solve
the underlying problem.  With the signal setup/restore 
changes I posted last night, your patch should not be
necessary.

            Kevin K.

Re: FP emulator patch

[Carsten Langgaard]

Daniel Jacobowitz wrote:

> On Wed, Aug 15, 2001 at 02:53:12PM +0200, Carsten Langgaard wrote:
> > There has been some reports regarding FP emulator failures, which the
> > attached patch should solve.
> > The patch include a fix for emulation of instructions in a COP1
> > delay-slot, a fix for FP context switching and some additional stuff ,
> > which was needed to pass our torture test.
> >
> > Ralf could you please apply this patch.
>
> Two comments, especially since parts of this seem to be the patch I
> posted here over a month ago.

You are absolutely right, it's your old patch.

>
> > Index: linux/arch/mips/kernel/signal.c
>
> > @@ -353,12 +355,11 @@
> >       owned_fp = (current == last_task_used_math);
> >       err |= __put_user(owned_fp, &sc->sc_ownedfp);
> >
> > -     if (current->used_math) {       /* fp is active.  */
> > +     if (owned_fp) { /* fp is active.  */
> >               set_cp0_status(ST0_CU1);
> >               err |= save_fp_context(sc);
> >               last_task_used_math = NULL;
> >               regs->cp0_status &= ~ST0_CU1;
> > -             current->used_math = 0;
> >       }
> >
> >       return err;
>
> This is absolutely not right.  It's righter than the status quo.  If we
> don't own the FP, you don't save the FP.  Then we can use FP in the
> signal handler, corrupting the process's original floating point
> context.

You are probably right, this is not a proper fix, but at least it solves the problems people
has been seeing.
So until we come up with a better fix, this is still better than the current sources.

>
> > Index: linux/include/asm-mips/processor.h
>
> > @@ -235,8 +215,8 @@
> >   * Do necessary setup to start up a newly executed thread.
> >   */
> >  #define start_thread(regs, new_pc, new_sp) do {                              \
> > -     /* New thread looses kernel privileges. */                      \
> > -     regs->cp0_status = (regs->cp0_status & ~(ST0_CU0|ST0_KSU)) | KU_USER;\
> > +     /* New thread loses kernel and FPU privileges. */               \
> > +        regs->cp0_status = (regs->cp0_status & ~(ST0_CU0|ST0_KSU|ST0_CU1)) | KU_USER;\
> >       regs->cp0_epc = new_pc;                                         \
> >       regs->regs[29] = new_sp;                                        \
> >       current->thread.current_ds = USER_DS;                           \
>
> I could be misremembering, but I believe that Ralf said this should be
> unnecessary and the problem was somewhere else.  On the other hand, I
> still think it's a good idea.
>
> --
> Daniel Jacobowitz                           Carnegie Mellon University
> MontaVista Software                         Debian GNU/Linux Developer

--
_    _ ____  ___   Carsten Langgaard   Mailto:carstenl@mips.com
|\  /|||___)(___   MIPS Denmark        Direct: +45 4486 5527
| \/ |||    ____)  Lautrupvang 4B      Switch: +45 4486 5555
  TECHNOLOGIES     2750 Ballerup       Fax...: +45 4486 5556
                   Denmark             http://www.mips.com

Re: FP emulator patch

[Atsushi Nemoto]

[-- Attachment #1: Type: Text/Plain, Size: 783 bytes --]

>>>>> On Wed, 15 Aug 2001 14:53:12 +0200, Carsten Langgaard <carstenl@mips.com> said:
carstenl> There has been some reports regarding FP emulator failures,
carstenl> which the attached patch should solve.  The patch include a
carstenl> fix for emulation of instructions in a COP1 delay-slot, a
carstenl> fix for FP context switching and some additional stuff ,
carstenl> which was needed to pass our torture test.

There is another bug in FPU emulator.  An instruction in delay slot of
bc1tl/bc1fl executed(or emulated) even if the branch not taken.  Here
is a patch to fix this.

Since current kernel calls FPU emulator on FP exception and FPU
emulator handles continuous FP instructions in one call, this bug
affects CPUs with FPU (not only CPUs without FPU).

---
Atsushi Nemoto


[-- Attachment #2: cp1emu.c.patch --]
[-- Type: Text/Plain, Size: 420 bytes --]

diff -ur linux.sgi/arch/mips/math-emu/cp1emu.c linux/arch/mips/math-emu/cp1emu.c
--- linux.sgi/arch/mips/math-emu/cp1emu.c	Sun Aug  5 23:39:27 2001
+++ linux/arch/mips/math-emu/cp1emu.c	Thu Aug 16 12:21:07 2001
@@ -686,7 +686,7 @@
 					 * branch likely nullifies dslot if not
 					 * taken
 					 */
-					xcp->cp0_epc += 4;
+					contpc += 4;
 					/* else continue & execute dslot as normal insn */
 			}
 			break;