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]

[ARM] Buggy handling of spurious interrupts


Hi,

I think I have spotted some problems w.r.t. handling of spurious
interrupts on the ARM architecture, all in the architecture-
specific vectors.S file:

|
|          mov     r0,v1                   // vector #
|
|  #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
|  #ifndef CYGIMP_HAL_COMMON_INTERRUPTS_IGNORE_SPURIOUS
|          bl      hal_spurious_IRQ

*  hal_spurious_IRQ() is called with the interrupt vector number as
   first argument (in r0), but the C function expects a
   HAL_SavedRegisters*, that is, a pointer to the saved register frame.

   Aside from that, the "vector" element of the HAL_SavedRegisters
   structure is undefined in case of an IRQ/FIQ. I don't know if this
   is disastrous; the stub uses the vector element at least for
   informative purposes. If it isn't set right, interrupts don't get
   reported as SIGINT in gdb.

|  #else
|          mov     lr,pc                   // invoke handler (call
indirect
|          mov     pc,v3                   // thru v3)
|  #endif
|
|  spurious_IRQ:
|
|  #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
|  #ifdef CYGFUN_HAL_COMMON_KERNEL_SUPPORT
|          // 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.
|

*  I'd suggest to skip the following code for spurious interrupts.
   In that case, the vector (in v1) is CYGNUM_HAL_INTERRUPT_NONE (which
   happens to be -1) so the shift-indexed ldr instruction below steps
   past the beginning of the hal_interrupt_objects array, yielding a
   bogus Cyg_Interrupt* pointer (the second arg to interrupt_end()).
   In interrupt_end(), then, if this pointer is non-zero, it gets
   dereferenced _before_ the handler return value (first arg) is
checked.

|          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
|                                  // threads and call scheduler



I have attached a context diff against the June 2nd snapshot:


Index: vectors.S
===================================================================
RCS file:
/d0/cvs/ecos/ecos/packages/hal/arm/arch/current/src/vectors.S,v
retrieving revision 1.1.1.3
diff -c -b -r1.1.1.3 vectors.S
*** vectors.S   2001/06/05 07:04:39     1.1.1.3
--- vectors.S   2001/06/05 07:36:00
***************
*** 707,712 ****
--- 706,713 ----
          str     r4,[sp,#armreg_pc]      // PC at time of interrupt
          str     lr,[sp,#armreg_lr]      // LR at time of interrupt
          str     fp,[sp,#armreg_sp]      // SP at time of interrupt
+       mov     r0,#CYGNUM_HAL_VECTOR_IRQ       // FIXME: we don't
distinguish
+       str     r0,[sp,#armreg_vector]          // FIQ and IRQ here
          mov     v6,sp                   // Save pointer to register
frame

  //      mov     r0,sp
***************
*** 768,773 ****
--- 769,775 ----
          cmp     r0,#CYGNUM_HAL_INTERRUPT_NONE   // spurious interrupt
          bne     10f
  #ifndef CYGIMP_HAL_COMMON_INTERRUPTS_IGNORE_SPURIOUS
+       mov     r0,v6                   // register frame
          bl      hal_spurious_IRQ
  #endif // CYGIMP_HAL_COMMON_INTERRUPTS_IGNORE_SPURIOUS
          b       spurious_IRQ
***************
*** 814,819 ****
--- 816,824 ----
          // 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
          ldr     r1,.hal_interrupt_objects
          ldr     r1,[r1,v1,lsl #2]
          mov     r2,v6           // register frame
***************
*** 822,830 ****

          bl      interrupt_end   // post any bottom layer handler
                                  // threads and call scheduler
- #endif
-
          ARM_MODE(r1,10)

  //      mov     r0,sp
  //      bl      show_frame_out
--- 827,835 ----

          bl      interrupt_end   // post any bottom layer handler
                                  // threads and call scheduler
          ARM_MODE(r1,10)
+ 17:
+ #endif

  //      mov     r0,sp
  //      bl      show_frame_out


Regards,
Thomas

-- 
Thomas Fähnle					thomas.faehnle@tst-ag.de
Digital Signal Processing			+49 731 80018-28
Touchless Sensor Technology AG			89231 Neu-Ulm · Germany


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