This is the mail archive of the
ecos-discuss@sourceware.org
mailing list for the eCos project.
Re: DSR stops running after heavy interrupts. Bug found?
On Sat, Apr 08, 2006 at 12:18:24AM -0400, Joe Porthouse wrote:
> Found it!!!
>
> It took two days to figure out what was happening but I think I have a
> handle on it. See if this sounds right.
>
> After an ISR executes, if there is an associated DSR to execute, the DSR is
> added to the DSR list and a scheduler lock is made. Since DSRs are run with
> interrupts enabled, the scheduler lock will prevent the application code
> from running until all DSRs finish and release each of the scheduler locks.
> After adding the DSR to the list, if there is only one scheduler lock (the
> one just added), then a call must be made to start the first DSR executing.
> If more then one scheduler lock is in place, then execution must resume from
> where it left off (DSR or other critical section). The DSR will start after
> the next scheduler unlock is called.
>
> If the ISR does not have an associated DSR, nothing is added to the DSR list
> and the scheduler lock is not made, allowing the application or DSR to
> resume when the ISR finishes.
>
> The problem is in the /hal/arm/arch/current/src/vectors.S file at line 951.
>
> // The return value from the handler (in r0) will indicate whether a
> // DSR is to be posted. Pass this together with a pointer to the
> // interrupt object we have just used to the interrupt tidy up routine.
>
> // don't run this for spurious interrupts!
> cmp v1,#CYGNUM_HAL_INTERRUPT_NONE <-- Incorrectly references R4
>
> cmp r0,#CYGNUM_HAL_INTERRUPT_NONE <-- Change to this
>
> The wrong register is referenced to determine if the ISR has a DSR to add to
> the DSR list. Since any value in R4 other then 0x0001 will call the
> routines to add a DSR, and I assume most ISRs have a DSR, the default
> behavior seems to works by chance in most configurations.
I don't think this is the correct interprtation of this code.
Line 914"
bl hal_IRQ_handler // determine interrupt source
mov v1,r0 // returned vector #
So the vector is now in both r0 and v1( == r4)
#if defined(CYGPKG_KERNEL_INSTRUMENT) && \
defined(CYGDBG_KERNEL_INSTRUMENT_INTR)
ldr r0,=RAISE_INTR // arg0 = type = INTR,RAISE
mov r1,v1 // arg1 = vector
mov r2,#0 // arg2 = 0
bl cyg_instrument // call instrument function
#endif
ARM_MODE(r0,10)
mov r0,v1 // vector #
The code above destroys the vector value in r0. Reload it from v1.
#if defined(CYGDBG_HAL_DEBUG_GDB_CTRLC_SUPPORT) \
|| defined(CYGDBG_HAL_DEBUG_GDB_BREAK_SUPPORT)
// If we are supporting Ctrl-C interrupts from GDB, we must squirrel
// away a pointer to the save interrupt state here so that we can
// plant a breakpoint at some later time.
.extern hal_saved_interrupt_state
ldr r2,=hal_saved_interrupt_state
str v6,[r2]
#endif
cmp r0,#CYGNUM_HAL_INTERRUPT_NONE // spurious interrupt
bne 10f
Here we check if the vector returned indicates there is a spurious
interrupt. If it is not we jump over this next bit of code.
#ifdef CYGIMP_HAL_COMMON_INTERRUPTS_IGNORE_SPURIOUS
// Acknowledge the interrupt
THUMB_CALL(r1,12,hal_interrupt_acknowledge)
#else
mov r0,v6 // register frame
THUMB_CALL(r1,12,hal_spurious_IRQ)
#endif // CYGIMP_HAL_COMMON_INTERRUPTS_IGNORE_SPURIOUS
b spurious_IRQ
Cleans up the interrupt controller after the spurious interrupt. v1
still contains the interrupt vector, ie #CYGNUM_HAL_INTERRUPT_NONE
10: ldr r1,.hal_interrupt_data
ldr r1,[r1,v1,lsl #2] // handler data
ldr r2,.hal_interrupt_handlers
ldr v3,[r2,v1,lsl #2] // handler (indexed by vector #)
mov r2,v6 // register frame (this is necessary
// for the ISR too, for ^C detection)
Calculates the address of the function to call.
#ifdef __thumb__
ldr lr,=10f
bx v3 // invoke handler (thumb mode)
.pool
.code 16
.thumb_func
IRQ_10T:
10: ldr r2,=15f
bx r2 // switch back to ARM mode
.pool
.code 32
15:
IRQ_15A:
#else
mov lr,pc // invoke handler (call indirect
mov pc,v3 // thru v3)
#endif
Calls the vector, either as thumb of ARM ISA. v1 (==r4) still contains the
vector.
#ifdef CYGIMP_HAL_COMMON_INTERRUPTS_USE_INTERRUPT_STACK
// If we are returning from the last nested interrupt, move back
// to the thread stack. interrupt_end() must be called on the
// thread stack since it potentially causes a context switch.
ldr r2,.irq_level
ldr r3,[r2]
subs r1,r3,#1
str r1,[r2]
ldreq sp,[sp] // This should be the saved stack pointer
#endif
// The return value from the handler (in r0) will indicate whether a
// DSR is to be posted. Pass this together with a pointer to the
// interrupt object we have just used to the interrupt tidy up routine.
// don't run this for spurious interrupts!
cmp v1,#CYGNUM_HAL_INTERRUPT_NONE
beq 17f
v1 is still the vector. If the vector indicates a spurious interrupt
we don't have a DSR to call so skip the next bit of code.
ldr r1,.hal_interrupt_objects
ldr r1,[r1,v1,lsl #2]
mov r2,v6 // register frame
THUMB_MODE(r3,10)
bl interrupt_end // post any bottom layer handler
Now run the DSRs if possible etc.
// threads and call scheduler
ARM_MODE(r1,10)
17:
// mov r0,sp
// bl show_frame_out
// return from IRQ is same as return from exception
b return_from_exception
So i think the comparison with v1 is correct. However, from what you
are saying it sounds like there needs to be another comparison
afterwards. Something like:
and r0,r0,#2 // CYG_ISR_CALL_DSR
beq 17f
Andrew
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss