[PATCH] Fix exception handler for supporting FPU

[sudarshan.rajagopalan]

On 2015-08-28 17:18, Martin Galvan wrote:
> On Thu, Aug 27, 2015 at 3:53 PM, sudarshan.rajagopalan
> <sudarshan.rajagopalan at vecna.com> wrote:
>> The instruction "cmn r2, #3\n" in line 31 basically compares the Link
>> Register(lr) to value 0xFFFFFFFD (negative #3, because CMN negates the 
>> RHS
>> and compares with LHS) and chooses MSP or PSP in the following IT 
>> block.
>> This is pretty cool! But, it will not work if you have the 
>> floating-point
>> unit (FPU) enabled in your processor, which is enabled in mine. With 
>> FPU
>> enabled, the error code returned is either 0xFFFFFFE9 or 0xFFFFFFED, 
>> for
>> which the above assembly instruction will not work out and MSP will be
>> selected always.
>> 
>> Better way to do is to check the 2nd bit of the error code to 
>> determine
>> which stack pointer was used before the exception happened - "tst lr, 
>> #4\n"
>> and change the IT block from "itt ne" to "itt eq" and the "mov" and 
>> "add"
>> within this IT block.
> 
> Hi Sudarshan! First of all, great catch. It looks like there's indeed a 
> bug
> on certain conditions.
> 
> While your method is indeed way less convoluted than the clever 
> "compare to -3"
> the old code did, I think we can actually make it even simpler
> (and save a couple of instructions) by doing the following:
> 
>     "tst lr, #4\n" /* Check if bit 2 of LR is zero */
>     "itte eq\n" /* IF bit 2 of LR is zero... */
>     "mrseq r0, msp\n" /* THEN we were using MSP */
>     "addeq r0, %[cpufsz]\n" /* THEN revert the previous 'sub sp, 
> %[cpufsz]' */
>     "mrsne r0, psp\n" /* ELSE it's not zero; we were using PSP */
> 
> What do you think?

Hi Martin,

Yup, we could definitely save 2 cycles and make it look simpler and 
understandable. Thumps up for the comments explaining LR decoding!

Thanks and Regards,
Sudarshan

-----
Sudarshan Rajagopalan
sudarshan.rajagopalan at vecna.com
www.vecna.com

> 
> Also, I must admit it took me quite a while to understand what was
> going on there.
> If you don't mind, I'd also like to add a brief comment explaining how
> to decode LR
> after taking an exception. The patch would end up looking like this:
> 
> ---
>  cpukit/score/cpu/arm/armv7m-exception-default.c | 27 
> +++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/cpukit/score/cpu/arm/armv7m-exception-default.c
> b/cpukit/score/cpu/arm/armv7m-exception-default.c
> index e890cdf..d04dbea 100644
> --- a/cpukit/score/cpu/arm/armv7m-exception-default.c
> +++ b/cpukit/score/cpu/arm/armv7m-exception-default.c
> @@ -22,16 +22,27 @@
> 
>  void __attribute__((naked)) _ARMV7M_Exception_default( void )
>  {
> +    /* On exception entry, ARMv7M saves context state onto a stack 
> pointed to
> +     * by either MSP or PSP. The value stored in LR indicates whether 
> we were
> +     * in Thread or Handler mode, whether we were using the FPU (if 
> any),
> +     * and which stack pointer we were using.
> +     * In particular, bit 2 of LR will be 0 if we were using MSP.
> +     *
> +     * For a more detailed explanation, see the Exception Entry 
> Behavior
> +     * section of the ARMv7M Architecture Reference Manual.
> +     */
> +
> +    /* As we're in Handler mode here, we'll always operate on MSP.
> +     * However, we need to store the right SP in our 
> CPU_Exception_frame.
> +     */
>    __asm__ volatile (
> -    "sub sp, %[cpufsz]\n"
> +    "sub sp, %[cpufsz]\n" /* Allocate space for a CPU_Exception_frame. 
> */
>      "stm sp, {r0-r12}\n"
> -    "mov r2, lr\n"
> -    "mrs r1, msp\n"
> -    "mrs r0, psp\n"
> -    "cmn r2, #3\n"
> -    "itt ne\n"
> -    "movne r0, r1\n"
> -    "addne r0, %[cpufsz]\n"
> +    "tst lr, #4\n" /* Check if bit 2 of LR is zero. (If so, PSR.Z = 1) 
> */
> +    "itte eq\n" /* IF bit 2 of LR is zero... (PSR.Z == 1) */
> +    "mrseq r0, msp\n" /* THEN we were using MSP. */
> +    "addeq r0, %[cpufsz]\n" /* THEN, set r0 = old MSP. */
> +    "mrsne r0, psp\n" /* ELSE it's not zero; we were using PSP. */
>      "add r2, r0, %[v7mlroff]\n"
>      "add r1, sp, %[cpulroff]\n"
>      "ldm r2, {r3-r5}\n"