[Bug 1933] New: SPARC: libbsp/sparc/shared/irq_asm.S IRQ handler bugs fixes and optimizations
bugzilla-daemon at rtems.org
bugzilla-daemon at rtems.org
Fri Oct 7 12:49:43 UTC 2011
https://www.rtems.org/bugzilla/show_bug.cgi?id=1933
Summary: SPARC: libbsp/sparc/shared/irq_asm.S IRQ handler bugs
fixes and optimizations
Product: RTEMS
Version: HEAD
Platform: sparc
OS/Version: RTEMS
Status: NEW
Severity: normal
Priority: P3
Component: bsps
AssignedTo: joel.sherrill at oarcorp.com
ReportedBy: daniel at gaisler.com
Created attachment 1351
--> https://www.rtems.org/bugzilla/attachment.cgi?id=1351
irq_asm.S SPARC patches
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)
--
Configure bugmail: https://www.rtems.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
More information about the bugs
mailing list