From fbbe5fde57bdc9d0c7f809931c2addb43bc1bd42 Mon Sep 17 00:00:00 2001 From: Joel Sherrill Date: Fri, 18 Jul 2003 14:47:55 +0000 Subject: 2003-07-18 Till Straumann PR 430/rtems * include/rtems/score/watchdog.h: _Watchdog_Ticks_since_boot should be a VOLATILE variable. * src/watchdoginsert.c: 'restart' algorithm needs to enforce reloading the list head in case a TICK interrupt during ISR_Flash() modified the list. This is achieved by a proper VOLATILE cast. Also _Watchdog_Sync_count++ should be protected by _ISR_Disable (prevent corruption in case ISR calls watchdoginsert) * src/watchdogadjust.c: ISR protection added. * src/watchdogtickle.c: ISR protection added. NOTE: PowerPC BSPs using the new exception processing MUST BE UPDATED to maintain _ISR_Nest_level. See also PR288 which provides fixes for the affected BSPs distributed with RTEMS. --- cpukit/score/ChangeLog | 16 ++++++++++++++++ cpukit/score/include/rtems/score/watchdog.h | 2 +- cpukit/score/src/watchdogadjust.c | 22 ++++++++++++++++++++++ cpukit/score/src/watchdoginsert.c | 22 ++++++++++++++++++---- cpukit/score/src/watchdogtickle.c | 26 +++++++++++++++++++++++--- 5 files changed, 80 insertions(+), 8 deletions(-) (limited to 'cpukit') diff --git a/cpukit/score/ChangeLog b/cpukit/score/ChangeLog index 4676766ed7..713d99db8d 100644 --- a/cpukit/score/ChangeLog +++ b/cpukit/score/ChangeLog @@ -1,3 +1,19 @@ +2003-07-18 Till Straumann + + PR 430/rtems + * include/rtems/score/watchdog.h: _Watchdog_Ticks_since_boot should + be a VOLATILE variable. + * src/watchdoginsert.c: 'restart' algorithm needs to enforce + reloading the list head in case a TICK interrupt during ISR_Flash() + modified the list. This is achieved by a proper VOLATILE cast. + Also _Watchdog_Sync_count++ should be protected by _ISR_Disable + (prevent corruption in case ISR calls watchdoginsert) + * src/watchdogadjust.c: ISR protection added. + * src/watchdogtickle.c: ISR protection added. + NOTE: PowerPC BSPs using the new exception processing MUST BE UPDATED + to maintain _ISR_Nest_level. See also PR288 which provides fixes + for the affected BSPs distributed with RTEMS. + 2003-07-08 Ralf Corsepius * cpu/Makefile.am: Add DIST_SUBDIRS = $(RTEMS_CPU). diff --git a/cpukit/score/include/rtems/score/watchdog.h b/cpukit/score/include/rtems/score/watchdog.h index f24c7a1a69..0a9f033200 100644 --- a/cpukit/score/include/rtems/score/watchdog.h +++ b/cpukit/score/include/rtems/score/watchdog.h @@ -103,7 +103,7 @@ SCORE_EXTERN volatile unsigned32 _Watchdog_Sync_count; * system was booted. */ -SCORE_EXTERN Watchdog_Interval _Watchdog_Ticks_since_boot; +SCORE_EXTERN volatile Watchdog_Interval _Watchdog_Ticks_since_boot; /* * The following defines the watchdog chains which are managed diff --git a/cpukit/score/src/watchdogadjust.c b/cpukit/score/src/watchdogadjust.c index 6926743f32..73b85a7eb4 100644 --- a/cpukit/score/src/watchdogadjust.c +++ b/cpukit/score/src/watchdogadjust.c @@ -37,6 +37,19 @@ void _Watchdog_Adjust( Watchdog_Interval units ) { + ISR_Level level; + + _ISR_Disable( level ); + + /* + * NOTE: It is safe NOT to make 'header' a pointer + * to volatile data (contrast this with watchdoginsert.c) + * because we call _Watchdog_Tickle() below and + * hence the compiler must not assume *header to remain + * unmodified across that call. + * + * Till Straumann, 7/2003 + */ if ( !_Chain_Is_empty( header ) ) { switch ( direction ) { case WATCHDOG_BACKWARD: @@ -50,7 +63,13 @@ void _Watchdog_Adjust( } else { units -= _Watchdog_First( header )->delta_interval; _Watchdog_First( header )->delta_interval = 1; + + _ISR_Enable( level ); + _Watchdog_Tickle( header ); + + _ISR_Disable( level ); + if ( _Chain_Is_empty( header ) ) break; } @@ -58,5 +77,8 @@ void _Watchdog_Adjust( break; } } + + _ISR_Enable( level ); + } diff --git a/cpukit/score/src/watchdoginsert.c b/cpukit/score/src/watchdoginsert.c index 43c1eacd76..0602a0c9b6 100644 --- a/cpukit/score/src/watchdoginsert.c +++ b/cpukit/score/src/watchdoginsert.c @@ -38,13 +38,28 @@ void _Watchdog_Insert( insert_isr_nest_level = _ISR_Nest_level; the_watchdog->state = WATCHDOG_BEING_INSERTED; + _ISR_Disable( level ); + _Watchdog_Sync_count++; + restart: delta_interval = the_watchdog->initial; - _ISR_Disable( level ); - - for ( after = _Watchdog_First( header ) ; + /* + * We CANT use _Watchdog_First() here, because a TICK interrupt + * could modify the chain during the _ISR_Flash() below. Hence, + * the header is pointing to volatile data. The _Watchdog_First() + * INLINE routine (but not the macro - note the subtle difference) + * casts away the 'volatile'... + * + * Also, this is only necessary because we call no other routine + * from this piece of code, hence the compiler thinks it's safe to + * cache *header!! + * + * Till Straumann, 7/2003 (gcc-3.2.2 -O4 on powerpc) + * + */ + for ( after = (Watchdog_Control *) ((volatile Chain_Control *)header)->first ; ; after = _Watchdog_Next( after ) ) { @@ -75,7 +90,6 @@ restart: if ( _Watchdog_Sync_level > insert_isr_nest_level ) { _Watchdog_Sync_level = insert_isr_nest_level; - _ISR_Enable( level ); goto restart; } } diff --git a/cpukit/score/src/watchdogtickle.c b/cpukit/score/src/watchdogtickle.c index fe07f566ff..80b1784152 100644 --- a/cpukit/score/src/watchdogtickle.c +++ b/cpukit/score/src/watchdogtickle.c @@ -33,18 +33,32 @@ void _Watchdog_Tickle( Chain_Control *header ) { + ISR_Level level; Watchdog_Control *the_watchdog; + Watchdog_States watchdog_state; + + /* + * See the comment in watchdoginsert.c and watchdogadjust.c + * about why it's safe not to declare header a pointer to + * volatile data - till, 2003/7 + */ + + _ISR_Disable( level ); if ( _Chain_Is_empty( header ) ) - return; + goto leave; the_watchdog = _Watchdog_First( header ); the_watchdog->delta_interval--; if ( the_watchdog->delta_interval != 0 ) - return; + goto leave; do { - switch( _Watchdog_Remove( the_watchdog ) ) { + watchdog_state = _Watchdog_Remove( the_watchdog ); + + _ISR_Enable( level ); + + switch( watchdog_state ) { case WATCHDOG_ACTIVE: (*the_watchdog->routine)( the_watchdog->id, @@ -71,7 +85,13 @@ void _Watchdog_Tickle( case WATCHDOG_REMOVE_IT: break; } + + _ISR_Disable( level ); + the_watchdog = _Watchdog_First( header ); } while ( !_Chain_Is_empty( header ) && (the_watchdog->delta_interval == 0) ); + +leave: + _ISR_Enable(level); } -- cgit v1.2.3