From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10171 invoked by alias); 9 Apr 2006 12:33:11 -0000 Received: (qmail 10162 invoked by uid 22791); 9 Apr 2006 12:33:10 -0000 X-Spam-Check-By: sourceware.org Received: from londo.lunn.ch (HELO londo.lunn.ch) (80.238.139.98) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sun, 09 Apr 2006 12:33:08 +0000 Received: from lunn by londo.lunn.ch with local (Exim 3.36 #1 (Debian)) id 1FSZ5q-0008NO-00; Sun, 09 Apr 2006 14:32:38 +0200 Date: Sun, 09 Apr 2006 12:33:00 -0000 To: Joe Porthouse Cc: ecos-discuss@ecos.sourceware.org Message-ID: <20060409123238.GS12221@lunn.ch> Mail-Followup-To: Joe Porthouse , ecos-discuss@ecos.sourceware.org References: <20060406211847.GH12221@lunn.ch> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.11+cvs20060126 From: Andrew Lunn X-IsSubscribed: yes Mailing-List: contact ecos-discuss-help@ecos.sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: ecos-discuss-owner@ecos.sourceware.org Subject: Re: [ECOS] DSR stops running after heavy interrupts. Bug found? X-SW-Source: 2006-04/txt/msg00103.txt.bz2 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