linux-mips
[Top] [All Lists]

Re: FP emulator patch

To: Carsten Langgaard <carstenl@mips.com>
Subject: Re: FP emulator patch
From: Daniel Jacobowitz <dan@debian.org>
Date: Wed, 15 Aug 2001 11:06:34 -0700
Cc: linux-mips@oss.sgi.com
In-reply-to: <3B7A70B8.ED92FE4@mips.com>; from carstenl@mips.com on Wed, Aug 15, 2001 at 02:53:12PM +0200
References: <3B7A70B8.ED92FE4@mips.com>
Sender: owner-linux-mips@oss.sgi.com
User-agent: Mutt/1.3.16i
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

<Prev in Thread] Current Thread [Next in Thread>