Minor fix for CortexM vectors.S

Minor fix for CortexM vectors.S

[Chris Holgate]

Hi folks,

This is a minor fix that I found while building the ROM based target for
my STM32 board.  The hal_switch_state_vsr can generate invalid
EXC_RETURN values in the link register for certain alignments - the fix
is to ensure that bit 1 is always cleared.  It's also possible to
reclaim a bit of RAM for the interrupt stack:

*** cvs-14.11.08/ecos/packages/hal/cortexm/arch/current/src/vectors.S
2008-11-03 14:53:51.000000000 +0000
---
working-14.11.08/ecos/packages/hal/cortexm/arch/current/src/vectors.S
2008-11-18 11:10:31.000000000 +0000
***************
*** 129,140 ****
--- 129,142 ----
          isb                             // Insert a barrier
          
          msr     psp,sp                  // Copy SP to PSP
+         ldr     sp,=hal_startup_stack   // Reset SP to top of RAM
  
  #if !defined(CYGPKG_KERNEL)
          sub     sp,#(CYGNUM_HAL_COMMON_INTERRUPTS_STACK_SIZE/2)
  #endif        
          
          orr     lr,#0xD                 // Adjust return link
+         bic     lr,#0x2
          bx      lr                      // Return to init code on PSP

Chris.

Re: Minor fix for CortexM vectors.S

[Nick Garnett]

Chris Holgate <chris@zynaptic.com> writes:

> Hi folks,
> 
> This is a minor fix that I found while building the ROM based target for
> my STM32 board.  The hal_switch_state_vsr can generate invalid
> EXC_RETURN values in the link register for certain alignments - the fix
> is to ensure that bit 1 is always cleared.  It's also possible to
> reclaim a bit of RAM for the interrupt stack:

I'm not sure about either of these.

The stack reset is benign. However, on other targets we often assign a
few words padding at the top of the interrupt stack to allow for
errant ISRs. There's no proof that this has ever been necessary, so
it is just a safety margin. By not adjusting the stack here, we got
that buffer zone implicitly.

I'm not sure I understand the change to LR. Normally the SWI from the
reset VSR should set LR to 0xFFFFFFF1, a return to handler mode on the
main stack. The state switch VSR sets it to 0xFFFFFFFD, a return to
thread mode on the process stack. The only other valid value would be
0xFFFFFFF9, a return to thread mode on the main stack, which we never
use. None of these has bit 0x2 set, so I'm not sure what problem you
are seeing that requires this bit to be cleared. I don't see how
alignment can affect this bit, or the LR value here at all.



> 
> *** cvs-14.11.08/ecos/packages/hal/cortexm/arch/current/src/vectors.S
> 2008-11-03 14:53:51.000000000 +0000
> ---
> working-14.11.08/ecos/packages/hal/cortexm/arch/current/src/vectors.S
> 2008-11-18 11:10:31.000000000 +0000
> ***************
> *** 129,140 ****
> --- 129,142 ----
>           isb                             // Insert a barrier
>           
>           msr     psp,sp                  // Copy SP to PSP
> +         ldr     sp,=hal_startup_stack   // Reset SP to top of RAM
>   
>   #if !defined(CYGPKG_KERNEL)
>           sub     sp,#(CYGNUM_HAL_COMMON_INTERRUPTS_STACK_SIZE/2)
>   #endif        
>           
>           orr     lr,#0xD                 // Adjust return link
> +         bic     lr,#0x2
>           bx      lr                      // Return to init code on PSP
> 
> Chris.
> 

-- 
Nick Garnett                                      eCos Kernel Architect
eCosCentric Limited    http://www.eCosCentric.com      The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.     Tel: +44 1223 245571
Registered in England and Wales:                        Reg No: 4422071

Re: Minor fix for CortexM vectors.S

[Chris Holgate]

On Wed, 2008-11-19 at 16:30 +0000, Nick Garnett wrote:
> Chris Holgate <chris@zynaptic.com> writes:
> 
> > Hi folks,
> > 
> > This is a minor fix that I found while building the ROM based target for
> > my STM32 board.  The hal_switch_state_vsr can generate invalid
> > EXC_RETURN values in the link register for certain alignments - the fix
> > is to ensure that bit 1 is always cleared.  It's also possible to
> > reclaim a bit of RAM for the interrupt stack:
> 
> I'm not sure about either of these.
> 
> The stack reset is benign. However, on other targets we often assign a
> few words padding at the top of the interrupt stack to allow for
> errant ISRs. There's no proof that this has ever been necessary, so
> it is just a safety margin. By not adjusting the stack here, we got
> that buffer zone implicitly.

OK - I was just looking at the 20k RAM on my '103RB device and thinking
that any saving is worthwhile!

> I'm not sure I understand the change to LR. Normally the SWI from the
> reset VSR should set LR to 0xFFFFFFF1, a return to handler mode on the
> main stack. The state switch VSR sets it to 0xFFFFFFFD, a return to
> thread mode on the process stack. The only other valid value would be
> 0xFFFFFFF9, a return to thread mode on the main stack, which we never
> use. None of these has bit 0x2 set, so I'm not sure what problem you
> are seeing that requires this bit to be cleared. I don't see how
> alignment can affect this bit, or the LR value here at all.

On the alignment issue, I'd assumed (obviously incorrectly) that the
value held in the LR inside the state switch VSR would be the address
after the SWI opcode that called it (just as it is with the old ARM
exception model), so a marginal change in the location of the SWI
instruction would affect bit 1.  The CortexM 'application' reference
manual is a bit vague on the matter - but now you mention it, it makes
sense that a valid EXC_RETURN value should be placed there on entry.

Ideally, I'd have noticed this from a register dump from inside the VSR,
but the new toolchain and my J-Link GDB server seem to be having
disagreements.  All I know is that the additional BIC instruction seems
to be the difference between working correctly and failing at that
point.  Since the BIC doesn't do anything useful in itself, maybe it's
just acting as pipeline padding.  I'll do some more tests and report
back.

Chris.

Re: Minor fix for CortexM vectors.S

[Nick Garnett]

Chris Holgate <chris@zynaptic.com> writes:

> On Wed, 2008-11-19 at 16:30 +0000, Nick Garnett wrote:
> > Chris Holgate <chris@zynaptic.com> writes:
> > 
> > > Hi folks,
> > > 
> > > This is a minor fix that I found while building the ROM based target for
> > > my STM32 board.  The hal_switch_state_vsr can generate invalid
> > > EXC_RETURN values in the link register for certain alignments - the fix
> > > is to ensure that bit 1 is always cleared.  It's also possible to
> > > reclaim a bit of RAM for the interrupt stack:
> > 
> > I'm not sure about either of these.
> > 
> > The stack reset is benign. However, on other targets we often assign a
> > few words padding at the top of the interrupt stack to allow for
> > errant ISRs. There's no proof that this has ever been necessary, so
> > it is just a safety margin. By not adjusting the stack here, we got
> > that buffer zone implicitly.
> 
> OK - I was just looking at the 20k RAM on my '103RB device and thinking
> that any saving is worthwhile!

If that is all you have then maybe. But you would have to shave the
interrupt stack down to the absolute minimum for it to make much of a
difference. 

> 
> > I'm not sure I understand the change to LR. Normally the SWI from the
> > reset VSR should set LR to 0xFFFFFFF1, a return to handler mode on the
> > main stack. The state switch VSR sets it to 0xFFFFFFFD, a return to
> > thread mode on the process stack. The only other valid value would be
> > 0xFFFFFFF9, a return to thread mode on the main stack, which we never
> > use. None of these has bit 0x2 set, so I'm not sure what problem you
> > are seeing that requires this bit to be cleared. I don't see how
> > alignment can affect this bit, or the LR value here at all.
> 
> On the alignment issue, I'd assumed (obviously incorrectly) that the
> value held in the LR inside the state switch VSR would be the address
> after the SWI opcode that called it (just as it is with the old ARM
> exception model), so a marginal change in the location of the SWI
> instruction would affect bit 1.  The CortexM 'application' reference
> manual is a bit vague on the matter - but now you mention it, it makes
> sense that a valid EXC_RETURN value should be placed there on entry.
> 
> Ideally, I'd have noticed this from a register dump from inside the VSR,
> but the new toolchain and my J-Link GDB server seem to be having
> disagreements.  All I know is that the additional BIC instruction seems
> to be the difference between working correctly and failing at that
> point.  Since the BIC doesn't do anything useful in itself, maybe it's
> just acting as pipeline padding.  I'll do some more tests and report
> back.
>

That is all very odd, it shouldn't make any difference. Do keep us
informed if you find anything out. You could try replacing the BIC
with a NOP, or perhaps a barrier instruction and see if the behaviour
stays the same. My experience is that in some situations the Cortex-M
behaves somewhat non-intuitively, until you work out what the
designers intended. So it is possible there is some unexpected
behaviour here.


-- 
Nick Garnett                                      eCos Kernel Architect
eCosCentric Limited    http://www.eCosCentric.com      The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.     Tel: +44 1223 245571
Registered in England and Wales:                        Reg No: 4422071

Re: Minor fix for CortexM vectors.S

[Chris Holgate]

On Wed, 2008-11-19 at 18:11 +0000, Nick Garnett wrote:
> Chris Holgate <chris@zynaptic.com> writes:
> 
> > OK - I was just looking at the 20k RAM on my '103RB device and thinking
> > that any saving is worthwhile!
> 
> If that is all you have then maybe. But you would have to shave the
> interrupt stack down to the absolute minimum for it to make much of a
> difference. 

Well, it's pretty academic now, as I'm going to have to move up to one
of the larger devices for flash space.  However, it does mean that
there's a basic port for the Olimex-P103 board going begging if anyone
wants it.
 
> That is all very odd, it shouldn't make any difference. Do keep us
> informed if you find anything out. You could try replacing the BIC
> with a NOP, or perhaps a barrier instruction and see if the behaviour
> stays the same. My experience is that in some situations the Cortex-M
> behaves somewhat non-intuitively, until you work out what the
> designers intended. So it is possible there is some unexpected
> behaviour here.

Unexpected and clearly hard to reproduce, since my current build now
runs perfectly well after rolling back to the original file.
Unfortunately it's going to be difficult to go back to the build that
was failing, so I'll just have to keep my eyes open for a recurrence.

Chris.