powerpc fptask issue again

Fri Apr 19 21:38:28 UTC 2002

Joel.

Sorry for the delay - we had networking problems...

Joel Sherrill wrote:

> Till Straumann wrote:
> >
> > Joel Sherrill wrote:
> >
> > > Till Straumann wrote:
> > > >
> > > > Hi.
> > > >
> > > > I'm sure this does not appear on this list for the first time.
> > > >
> > > > Can somebody explain to me why enabling deferred/lazy
> > > > floating point context save/restore (as is currently the default
> > > > for the Powerpc/new_exception_processing) and NOT
> > > > disabling MSR_FP for integer tasks (current behavior)
> > > > is assumed to be safe ???
> > >
> > > In general, if you have the capability to disable the
> > > FPU for integer only tasks, you should do it.  You have
> > > to be careful to honor this in interrupts and context
> > > switches.
> > >
> >
> > I couldn't agree more. However, currently the FPU (on ppc)
> > is _never_ disabled. There is a comment stating that it is
> > left enabled because newlibc vfprintf (around '97) behaved
> > just like my task 'B', i.e. it is not modifying but merely
> > saving/restoring FP regs.
>
> And in the old exceptions port (from memory), all tasks are FP
> because of this.
>
> > My point is:
> >
> > 1) _only_ do lazy FPcontext save/restore if you
> >    switch the FPU off for integer tasks otherwise
> >    there is a risk of corruption that goes undetected.
>
> On lower end CPUs, the benefits are relatively greater
> from integer only tasks and the odds of being able to
> disable an FPU are low.  On the PPC or any other CPU
> with FPU disable, I agree totally.
>
> > 2) if there are integer tasks who push/pop the FPregs
> >    without modifying them, you can leave the FPU enabled
> >    _but_ FPContext must be saved/restored at every context
> >    switch.
>
> Exactly and this is why the old exceptions model declared
> that all tasks were FP.
>
> > Note that powerpc-rtems currently violates 1), i.e. it uses
> > lazy FPsave/restore but does not disable the FPU for integer
> > tasks!
>
> Yes.
>
> > One more interesting thing: if you do 1) you will discover that
> > threaddispatch.c is actually incorrect. PR/patch is in preparation
>
> What is the problem?  I think you might be referring to the
> same thing Greg Menke did.  The FP context save/restore code
> must protect itself and enable the FPU.

right

> But if there is
> an easy way to get this as a side-effect, be our guest. :)
>

If I don't oversee something, it is easy - just make sure you
do the FPsave/restore always in the context of the FP task,
see the diff.

-- Till

-------------- next part --------------
This patch addresses the following FP context switching issues:

- if CPU_USE_DEFERRED_FP_SWITCH, make sure (on the PowerPC only)
  that nonFP tasks have the FPU disabled.

- make CPU_USE_DEFERRED_FP_SWITCH FALSE the default

- fix context switching: to FPsave and FPrestore in the context
  of the FP task itself.

Author: Till Straumann <strauman at slac.stanford.edu>, 4/19/2002

Index: c/src/exec/score/src/threaddispatch.c
===================================================================
RCS file: /afs/slac/g/spear/cvsrep/rtems/src/c/src/exec/score/src/threaddispatch.c,v
retrieving revision 1.1.1.1
diff -c -r1.1.1.1 threaddispatch.c
*** threaddispatch.c	2001/12/14 22:49:52	1.1.1.1
--- threaddispatch.c	2002/04/19 21:16:20
***************
*** 93,115 ****
       */

  #if ( CPU_HARDWARE_FP == TRUE ) || ( CPU_SOFTWARE_FP == TRUE )
  #if ( CPU_USE_DEFERRED_FP_SWITCH == TRUE )
!     if ( (heir->fp_context != NULL) && !_Thread_Is_allocated_fp( heir ) ) {
        if ( _Thread_Allocated_fp != NULL )
          _Context_Save_fp( &_Thread_Allocated_fp->fp_context );
!       _Context_Restore_fp( &heir->fp_context );
!       _Thread_Allocated_fp = heir;
      }
  #else
      if ( executing->fp_context != NULL )
!       _Context_Save_fp( &executing->fp_context );
! 
!     if ( heir->fp_context != NULL )
!       _Context_Restore_fp( &heir->fp_context );
  #endif
  #endif
- 
-     _Context_Switch( &executing->Registers, &heir->Registers );

      executing = _Thread_Executing;

--- 93,119 ----
       */

  #if ( CPU_HARDWARE_FP == TRUE ) || ( CPU_SOFTWARE_FP == TRUE )
+ #if ( CPU_USE_DEFERRED_FP_SWITCH != TRUE )
+     if ( executing->fp_context != NULL )
+       _Context_Save_fp( &executing->fp_context );
+ #endif
+ #endif
+ 
+     _Context_Switch( &executing->Registers, &heir->Registers );
+ 
+ #if ( CPU_HARDWARE_FP == TRUE ) || ( CPU_SOFTWARE_FP == TRUE )
  #if ( CPU_USE_DEFERRED_FP_SWITCH == TRUE )
!     if ( (executing->fp_context != NULL) && !_Thread_Is_allocated_fp( executing ) ) {
        if ( _Thread_Allocated_fp != NULL )
          _Context_Save_fp( &_Thread_Allocated_fp->fp_context );
!       _Context_Restore_fp( &executing->fp_context );
!       _Thread_Allocated_fp = executing;
      }
  #else
      if ( executing->fp_context != NULL )
!       _Context_Restore_fp( &executing->fp_context );
  #endif
  #endif

      executing = _Thread_Executing;

Index: c/src/lib/libbsp/powerpc/support//new_exception_processing/cpu.c
===================================================================
RCS file: /afs/slac/g/spear/cvsrep/rtems/src/c/src/lib/libbsp/powerpc/support/new_exception_processing/cpu.c,v
retrieving revision 1.1.1.1
diff -c -r1.1.1.1 cpu.c
*** cpu.c	2001/12/15 00:14:11	1.1.1.1
--- cpu.c	2002/04/19 21:29:25
***************
*** 93,100 ****
     *  time (7 July 1997), this restructuring is not being done.
     */

!   /*if ( is_fp ) */
      the_context->msr |= PPC_MSR_FP;

    the_context->pc = (unsigned32)entry_point;
  }
--- 93,112 ----
     *  time (7 July 1997), this restructuring is not being done.
     */

!   /* Till Straumann: For deferred FPContext save/restore, make sure integer
!    *                 tasks have no FPU access in order to catch violations.
!    *                 Otherwise, the FP registers may be corrupted.
!    *				 Since we set the_contex->msr using our current MSR,
!    *				 we must make sure MSR_FP is off if (!is_fp)...
!    */
! #if defined(CPU_USE_DEFERRED_FP_SWITCH) && (CPU_USE_DEFERRED_FP_SWITCH==TRUE)
!   if ( is_fp )
! #endif
      the_context->msr |= PPC_MSR_FP;
+ #if defined(CPU_USE_DEFERRED_FP_SWITCH) && (CPU_USE_DEFERRED_FP_SWITCH==TRUE)
+   else
+ 	the_context->msr &= ~PPC_MSR_FP;
+ #endif

    the_context->pc = (unsigned32)entry_point;
  }
Index: c/src/lib/libbsp/powerpc/support//new_exception_processing/rtems/score/cpu.h
===================================================================
RCS file: /afs/slac/g/spear/cvsrep/rtems/src/c/src/lib/libbsp/powerpc/support/new_exception_processing/rtems/score/cpu.h,v
retrieving revision 1.1.1.1
diff -c -r1.1.1.1 cpu.h
*** cpu.h	2001/12/15 00:14:11	1.1.1.1
--- cpu.h	2002/04/19 21:29:25
***************
*** 229,240 ****
   *  until a context switch is made to another, different FP task.
   *  Thus in a system with only one FP task, the FP context will never
   *  be saved or restored.
   */
  /*
   *  ACB Note:  This could make debugging tricky..
   */

! #define CPU_USE_DEFERRED_FP_SWITCH       TRUE

  /*
   *  Does this port provide a CPU dependent IDLE task implementation?
--- 229,247 ----
   *  until a context switch is made to another, different FP task.
   *  Thus in a system with only one FP task, the FP context will never
   *  be saved or restored.
+  *
+  *  Note, however that compilers may use floating point registers/
+  *  instructions for optimization or they may save/restore FP registers
+  *  on the stack. You must not use deferred switching in these cases
+  *  and on the PowerPC attempting to do so will raise a "FP unavailable"
+  *  exception.
   */
  /*
   *  ACB Note:  This could make debugging tricky..
   */

! /* conservative setting; probably doesn't affect performance too much */
! #define CPU_USE_DEFERRED_FP_SWITCH       FALSE

  /*
   *  Does this port provide a CPU dependent IDLE task implementation?