PowerPC FP handling weakness.

Wed Oct 11 07:58:27 UTC 2000

At 12:30 PM +0400 10/2/00, Sergei Organov wrote:
>Duncan Smith <dds at flavors.com> writes:
>[...]
>>  At Flavors we have been using RTEMS for 4 or 5 years on 604/604e MPC105/106,
>>  VME boards.  Although the version we use is an old one, the BSP requirement
>>  is probably similar.
>>
>>  In our own BSP we do lazy FPU context switching based on actual use of FP
>>  through the FP fault.
>>
>
>As an exercise I also implemented similar FPU context switching for another
>RTOS, and have faced with one subtle problem. The FP enable and interrupts
>enable bits are in the same MSR register. When on-demand context switch is
>used, the FP bit in MSR can be cleared any time if scheduling is enabled. This
>fact makes it impossible to use _ISR_Disable/_ISR_Enable pair in the form they
>are now, because the value of FP bit could be lost: _ISR_Disable reads MSR
>with FP bit set, then interrupts/context switches occur that clear FP bit,
>then interrupts are disabled by writing MSR with old value with cleared
>interrupts bit but set FP bit (here the right value of FP bit is lost), then
>FP instruction is executed and clobbers FP context owned by different task.
>
>One solution to the problem is to always clear FP bit in _ISR_Disable. This
>can lead to unnecessary FP context switches though.
>
>How did you solve the problem?
>
>BR,
>Sergei.

Sorry for the delay.  I am out of town most of the time, and don't 
check this stuff very often...

Here is the part of our SC handler for Enable/Disable interrupts. 
Actually this only used for Disable...

# 
###############################################################################
#
#	System Call (0xc00) Level 0
#
#   Exactly 64 words (max.) -dds 96-02-15
#
#	Because the FP bit can be modified at any time due to an enabled
#	External Exception, it is necessary that MFMSR/MTMSR used to clear
#	the EE bit be atomic. To satisfy this we use the System Call Exception
#	The protocol is:
#		Bit 31 of R3 is inserted into MSR( EE ), Bits 0-30 of 
R3 must be zero.
#		Previous MSR( EE ) is returned in Bit 31 of R3, Bits 
0-30 of R3 are cleared.
#		R4 and R5 are destroyed.
#
#  0,1 - Bit to EE
#

		csect	_SC_Handler{RW}
		export	_SC_Handler{RW}
		# SC Handler
		dc.l	_L0_SC_Handler{PR}
		dc.l	(_L0_SC_Handler_End - _L0_SC_Handler{PR}) >> 2
		dc.l	0x0c00
		dc.l	2
		dc.l	(_L0_SC_Cell_Done_LISORI - _L0_SC_Handler{PR}) >> 1
		dc.l	_L1_CS_Done
		dc.l	(_L0_SC_Cell_Timeout_LISORI - _L0_SC_Handler{PR}) >> 1
		dc.l	_L1_CS_Timeout

		csect	_L0_SC_Handler{PR}
		export	_L0_SC_Handler{PR}

		cmplwi	r3,1
		bgt-	_L0_SC_Cell
		mfsrr1	r4				#Get MSR
		extrwi	r5,r4,1,16		#Extract EE bit
		insrwi	r4,r3,1,16		#Insert new EE bit
		mtsrr1	r4				#Put MSR
		mr		r3,r5			#Copy result
		rfi						#Get done

---------------------------------------

The junk at the top of the csect is a linkage table used to copy 
level 0 interrupt handlers to their low memory addresses, and to set 
up the crawlouts to the level 1 handlers:

FYI, e.g. this is a LISORI and crawlout...:

		mfsrr0	r5
		mfsrr1	r6
		ori		r7,r6,MSR_EE|MSR_FP
		mtsrr1	r7				#Put MSR
_L0_SC_Cell_Timeout_LISORI:
		lis		r7,0
		ori		r7,r7,0
		mtsrr0	r7
		rfi 
	#Transfer to Level 1
_L0_SC_Handler_End:

---------------------------------------

/*
  *  rtems_interrupt_disable
  *
  *  DESCRIPTION:
  *
  *  This routine disables all maskable interrupts and returns the
  *  previous level in _isr_cookie.
  */

#define rtems_interrupt_disable( _isr_cookie ) \
     _ISR_Disable(_isr_cookie)

/*
  *  rtems_interrupt_enable
  *
  *  DESCRIPTION:
  *
  *  This routine enables maskable interrupts to the level indicated
  *  _isr_cookie.
  */

#define rtems_interrupt_enable( _isr_cookie ) \
     _ISR_Enable(_isr_cookie)

/*
  *  rtems_interrupt_flash
  *
  *  DESCRIPTION:
  *
  *  This routine temporarily enables maskable interrupts to the
  *  level in _isr_cookie before redisabling them.
  */

#define rtems_interrupt_flash( _isr_cookie ) \
     _ISR_Flash(_isr_cookie)

-------------------------------------

/*
  *  _ISR_Disable
  *
  *  DESCRIPTION:
  *
  *  This routine disables all interrupts so that a critical section
  *  of code can be executing without being interrupted.  Upon return,
  *  the argument _level will contain the previous interrupt mask level.
  */

#define _ISR_Disable( _level ) \
         _CPU_ISR_Disable( _level )

/*
  *  _ISR_Enable
  *
  *  DESCRIPTION:
  *
  *  This routine enables interrupts to the previous interrupt mask
  *  LEVEL.  It is used at the end of a critical section of code to
  *  enable interrupts so they can be processed again.
  */

#define _ISR_Enable( _level ) \
         _CPU_ISR_Enable( _level )

/*
  *  _ISR_Flash
  *
  *  DESCRIPTION:
  *
  *  This routine temporarily enables interrupts to the previous
  *  interrupt mask level and then disables all interrupts so that
  *  the caller can continue into the second part of a critical
  *  section.  This routine is used to temporarily enable interrupts
  *  during a long critical section.  It is used in long sections of
  *  critical code when a point is reached at which interrupts can
  *  be temporarily enabled.  Deciding where to flash interrupts
  *  in a long critical section is often difficult and the point
  *  must be selected with care to insure that the critical section
  *  properly protects itself.
  */

#define _ISR_Flash( _level ) \
         _CPU_ISR_Flash( _level )

-------------------------------------

/*
  *  Disable all interrupts for an RTEMS critical section.  The previous
  *  level is returned in _level.
  */

#define _CPU_ISR_Disable( _isr_cookie ) \
   { \
     (_isr_cookie) = __sc( 0 ); \
   }

/*
  *  Enable interrupts to the previous level (returned by _CPU_ISR_Disable).
  *  This indicates the end of an RTEMS critical section.  The parameter
  *  _level is not modified.
  */

#define _CPU_ISR_Enable( _isr_cookie )  \
   { \
  	__mtmsr_isr_cookie( __mfmsr(), (_isr_cookie) ); \
   }

/*
  *  This temporarily restores the interrupt to _level before immediately
  *  disabling them again.  This is used to divide long RTEMS critical
  *  sections into two or more parts.  The parameter _level is not
  * modified.
  */

#define _CPU_ISR_Flash( _isr_cookie ) \
   { \
  	__mtmsr_isr_cookie( __mfmsr(), (_isr_cookie) ); \
  	__sc( 0 ); \
   }

---------------------------------------

These get inlined by our Builder(locator):

...
.__mfmsr:
		mfmsr	r3
		blr
.__mtmsr:
		mtmsr	r3
		blr
...
.__mtmsr_isr_cookie:
		insrwi	r3,r4,1,16		#Insert new EE bit
		mtmsr	r3
		blr
...
# Other
.__sc:	sc
		blr