[Bug 1933] New: SPARC: libbsp/sparc/shared/irq_asm.S IRQ handler bugs fixes and optimizations

[bugzilla-daemon at rtems.org]

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.