#1933 closed defect (fixed)

SPARC: libbsp/sparc/shared/irq_asm.S IRQ handler bugs fixes and optimizations

Reported by: Daniel Hellstrom Owned by: Joel Sherrill
Priority: normal Milestone: 4.11
Component: bsps Version: 4.11
Severity: normal Keywords:
Cc: jennifer.averett@…, jiri@…, daniel@… Blocked By:
Blocking:

Description

Hello,

From code inspection I have found the following issues (most SMP), and some improvements in irq_asm.S. I would need a long test with interrupts to verify the interrupt handler better, however I can not see that these patches hurt. Please see comment per hunk below,

One should go through the file to indent delay-slots correctly, I have fixed some in the patch areas. An extra space is added in front of delay slots to indicate a delay slot.

Daniel

Index: c/src/lib/libbsp/sparc/shared/irq_asm.S
===================================================================
RCS file: /home/cvspserver/SMP_Repository/rtems-smp/c/src/lib/libbsp/sparc/shared/irq_asm.S,v
retrieving revision 1.2
diff -u -r1.2 irq_asm.S
--- c/src/lib/libbsp/sparc/shared/irq_asm.S 20 May 2011 15:30:20 -0000 1.2
+++ c/src/lib/libbsp/sparc/shared/irq_asm.S 7 Oct 2011 12:20:23 -0000
@@ -267,8 +267,6 @@

add %l5, %l7, %l5

#endif

ld [%l5], %l5 /* l5 = pointer to per CPU */

  • nop
  • nop

/*

  • On multi-core system, we need to use SMP safe versions

### Daniel: No point in two extra instructions. (optimization)

@@ -277,9 +275,8 @@

  • _ISR_SMP_Enter returns the interrupt nest level. If we are
  • outermost interrupt, then we need to switch stacks. */
  • mov %sp, %fp

call SYM(_ISR_SMP_Enter), 0

  • nop ! delay slot

+ mov %sp, %fp ! delay slot

cmp %o0, 0

#else

/*

### Daniel: Optimize away one instruction by using delay slot to set %fp
### (optimization)

@@ -321,8 +318,8 @@

/*

  • Do we need to switch to the interrupt stack? */
  • bnz dont_switch_stacks ! No, then do not switch stacks
  • ld [%l5 + PER_CPU_INTERRUPT_STACK_HIGH], %sp

+ beq,a dont_switch_stacks ! No, then do not switch stacks
+ ld [%l5 + PER_CPU_INTERRUPT_STACK_HIGH], %sp

dont_switch_stacks:

/*

### Daniel: This is a bug, the branch as no effect since LD is always executed.
###
### The _ISR_Handler determine if the IRQ stack is to be switched
### depending on nest level, however the stack is always switched
### because the delay slot on SPARC is always executed on calls and
### branches if the branch is taken, one can use the annul bit when
### the branch is not taken to avoid the delay-slot instruction beeing
### executed , the branch address (dont_switch_stacks) must follow
### directly.

@@ -358,6 +355,7 @@

nop ! delay slot
cmp %o0, 0
bz simple_return

+ nop

#else

!sethi %hi(SYM(_Thread_Dispatch_disable_level)), %l4
!ld [%l5 + PER_CPU_ISR_NEST_LEVEL], %l7

### Daniel: This is a bug, since PSR is set in the delay-slot when branch taken

@@ -405,7 +403,7 @@

ld [%l6 + %lo(SYM(_CPU_ISR_Dispatch_disable))], %l7
orcc %l7, %g0, %g0 ! Is this thread already doing an ISR?
bnz simple_return ! Yes, then do a "simple" exit

  • nop

+ nop

/*

  • If a context switch is necessary, then do fudge stack to

### Daniel: added space to indicate delay-slot (code-style)

@@ -413,11 +411,9 @@

*/


ldub [%l5 + PER_CPU_DISPATCH_NEEDED], %l5

  • nop
  • nop

-

### Daniel: No point in two extra instructions (optimization)

orcc %l5, %g0, %g0 ! Is thread switch necessary?
bz simple_return ! No, then return

+ nop

#endif

/*

  • Invoke interrupt dispatcher.

### Daniel: delay-slot executed over label, bad coding style and dangerous when
### function _ISR_Dispatch() is changed. (coding-style, future bug)

@@ -479,16 +475,11 @@

nop

#endif

ld [%l5], %l5 /* l5 = pointer to per CPU */

  • nop
  • nop #else

sethi %hi(_Per_CPU_Information), %l5
add %l5, %lo(_Per_CPU_Information), %l5

#endif

ldub [%l5 + PER_CPU_DISPATCH_NEEDED], %l5

  • nop
  • nop

-

orcc %l5, %g0, %g0 ! Is thread switch necessary?
bz allow_nest_again
nop

### Daniel: 2 x No point in two extra instructions. (optimization)

Attachments (1)

irq_asm1.patch (2.7 KB) - added by Daniel Hellstrom on Oct 7, 2011 at 11:49:42 AM.
irq_asm.S SPARC patches

Download all attachments as: .zip

Change History (4)

Changed on Oct 7, 2011 at 11:49:42 AM by Daniel Hellstrom

Attachment: irq_asm1.patch added

irq_asm.S SPARC patches

comment:1 Changed on Oct 7, 2011 at 11:50:12 AM by Daniel Hellstrom

Cc: daniel@… jennifer.averett@… jiri@… added

comment:2 Changed on Oct 7, 2011 at 1:39:57 PM by Joel Sherrill

Resolution: fixed
Status: newclosed

Applied. Thanks.

comment:3 Changed on Nov 24, 2014 at 6:58:28 PM by Gedare Bloom

Version: HEAD4.11

Replace Version=HEAD with Version=4.11 for the tickets with Milestone >= 4.11

Note: See TracTickets for help on using tickets.