This is the mail archive of the ecos-discuss@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: RM7000 interrupt handling



We were discussing the eCos requirement of being able to mask individual
interrupt sources within an ISR, and the difficulty in doing that in MIPS
variants where the individual mask bits are in the SR along with the global
"disable interrupts" bit.  There's always a race condition in the
read-modify-write op that's needed to disable all interrupts.

Chris Morrow <cmorrow@YottaYotta.com> spotted that race condition in
conseqence of looking at the code I added to allow ISR changes to the SR to
"stick" when the interrupt returns, so thanks Chris, you saved us some
possibly painful and elongated debugging!

There is a neat enough solution, which I have implemented in the target we
have that does have individual mask bits in the SR.

We need to keep a shadow copy of the SR mask bits in RAM, and the RAM copy
is the truth.  The race condition in HAL_DISABLE_INTERRUPTS() is avoided by
disabling interrupts as before, and then, *once interrupts are disabled*
reading the RAM copy and injecting it into the SR.  That way, even if an
ISR's manipulation of the SR bits is discarded, the RAM copy still holds
and is poked into the SR.  HAL_RESTORE_INTERRUPTS() also injects the true
value.  The RAM copy needs to be at a fixed memory location so that it is
shared with RedBoot, since intercalling between the app and RedBoot can
enable and disable the interrupts on the debug channel, which can only work
correctly if the same RAM copy is used in both contexts.  That's dealt with
in the linker script which places hal_interrupt_sr_mask_shadow_base[].

Anyway, there are some minor changes to the MIPS arch HAL to allow the
variant to define HAL_DISABLE_INTERRUPTS() et al, they'll be out in anoncvs
soon.  The variant HAL is not (yet) published, so I'll put the snippets of
code from the variant HAL's var_intr.h on the end of this message to help
anyone else who's dealing with the same issues.

        - Huge

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

// This is placed in memory at a fixed location because we must share it
// with RedBoot, along with the VSR table and Virtual Vector table.
// It has to be an array to get the correct code generation to access it
// over all that distance.

externC volatile cyg_uint32 hal_interrupt_sr_mask_shadow_base[];

#define hal_interrupt_sr_mask_shadow (hal_interrupt_sr_mask_shadow_base[0])

// We have to have local versions of these to preserve the mask bits in the
// SR correctly when an interrupt occurs within one of these code sequences
// which are doing a read-modify-write to the main interrupt bit of the SR.

// Disable, it doesn't matter what the SR IM bits are - but it is possible
// for control to return with interrupts enabled if a context switch occurs
// away from the thread that disabled interrupts.  Therefore we also make
// sure the contents of the SR match the shadow variable at the end.

#define HAL_DISABLE_INTERRUPTS(_old_)                                   \
CYG_MACRO_START                                                         \
    register int _tmp;                                                  \
    asm volatile (                                                      \
        "mfc0   $8,$12; nop;"                                           \
        "move   %0,$8;"                                                 \
        "and    $8,$8,0xfffffffe;"                                      \
        "mtc0   $8,$12;"                                                \
        "nop; nop; nop;"                                                \
        : "=r"(_tmp)                                                    \
        :                                                               \
        : "$8"                                                          \
        );                                                              \
    /* interrupts disabled so can now inject the correct IM bits */     \
    (_old_) = _tmp & 1;                                                 \
    _tmp &= 0xffff00fe;                                                 \
    _tmp |= (hal_interrupt_sr_mask_shadow & 0xff00);                    \
    asm volatile (                                                      \
        "mtc0   %0,$12;"                                                \
        "nop; nop; nop;"                                                \
        :                                                               \
        : "r"(_tmp)                                                     \
        );                                                              \
CYG_MACRO_END

// Enable and restore, we must pick up hal_interrupt_sr_mask_shadow because
// it contains the truth.  This is also for the convenience of the
// mask/unmask macros below.
#define HAL_ENABLE_INTERRUPTS() HAL_RESTORE_INTERRUPTS(1)

#define HAL_RESTORE_INTERRUPTS(_old_)                                   \
CYG_MACRO_START                                                         \
    asm volatile (                                                      \
        "mfc0   $8,$12; nop;"                                           \
        "or     $8,$8,%0;"         /* inject IE bit  */                 \
        "and    $8,$8,0xffff00ff;" /* clear IM bits  */                 \
        "or     $8,$8,%1;"         /* insert true IM */                 \
        "mtc0   $8,$12;"                                                \
        "nop; nop; nop;"                                                \
        :                                                               \
        : "r"((_old_) & 1),"r"(hal_interrupt_sr_mask_shadow & 0xff00)   \
        : "$8"                                                          \
        );                                                              \
CYG_MACRO_END

#define CYGHWR_HAL_INTERRUPT_ENABLE_DISABLE_RESTORE_DEFINED

// ------------------------------------------------------------------------

// For the bits which are in the SR, we only need to diddle the shadow
// variable; restore interrupts will pick that up at the end of the macro.

#define HAL_INTERRUPT_MASK( _vector_ )                                  \
CYG_MACRO_START                                                         \
register int _intstate;                                                 \
register int _shift;                                                    \
HAL_DISABLE_INTERRUPTS( _intstate );                                    \
if ( CYGNUM_HAL_INTERRUPT_SYSCTL_LOW > (_vector_) ) {                   \
    /* mask starts at bit 8 */                                          \
    _shift = 8 + (_vector_) - CYGNUM_HAL_INTERRUPT_STATUS_CAUSE_LOW;    \
    hal_interrupt_sr_mask_shadow &=~(1 << _shift);                      \
}                                                                       \
else {                                                                  \
    _shift = (_vector_) - CYGNUM_HAL_INTERRUPT_SYSCTL_LOW;              \
    *S_IMR &=~(1 << _shift);                                            \
}                                                                       \
HAL_RESTORE_INTERRUPTS( _intstate );                                    \
CYG_MACRO_END


#define HAL_INTERRUPT_UNMASK( _vector_ )                                \
CYG_MACRO_START                                                         \
register int _intstate;                                                 \
register int _shift;                                                    \
HAL_DISABLE_INTERRUPTS( _intstate );                                    \
if ( CYGNUM_HAL_INTERRUPT_SYSCTL_LOW > (_vector_) ) {                   \
    /* mask starts at bit 8 */                                          \
    _shift = 8 + (_vector_) - CYGNUM_HAL_INTERRUPT_STATUS_CAUSE_LOW;    \
    hal_interrupt_sr_mask_shadow |= (1 << _shift);                      \
}                                                                       \
else {                                                                  \
    _shift = (_vector_) - CYGNUM_HAL_INTERRUPT_SYSCTL_LOW;              \
    *S_IMR |= (1 << _shift);                                            \
}                                                                       \
HAL_RESTORE_INTERRUPTS( _intstate );                                    \
CYG_MACRO_END



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]