[PATCH] ISR<->Task stack switch failure

Vasili Galka

Other Parts Discussed in Thread: AM3359, SYSBIOS

Hi,

The attached patch solves an issue that I bumped into with the SYS/BIOS ISR handling code.

My environment & target: GCC 4.8.1 compiler, AM3359 chip (Cortex-A8).

Problem description: When the SYS/BIOS code is compiled with no optimizations, GCC restores stack-pointer from frame-pointer in function epilogue. Thus, ti_sysbios_family_xxx_Hwi_switchToIsrStack() and ti_sysbios_family_xxx_Hwi_switchToTaskStack() functions (implemented in C) effectively fail to switch the stack.

Solution: I doubt that C language standard allows us to write C code that would correctly switch the stack and would not lead to unpredictable results depending on compiler and its optimizations. Thus, to be on the safe side and prevent potential breakages with future compiler versions, I propose to implement the stack switching functions in assembly.

Patch: 8787.0003 - Hwi ISR stack switch bug fix.txt
(Change the extension to "patch", for some reason the forum forbids files with this extension...)

P.S. Moreover, I see that these functions already implemented in assembly for TI compiler, I wonder why this wasn't already done for GCC build too...

P.P.S. I'm intentionally using MOVW/MOVT instead of LDR in my code. This is more performance efficient (http://blogs.arm.com/software-enablement/251-how-to-load-constants-in-assembly-for-arm-architecture/)

Best regards,
Vasili

over 11 years ago

0 Ashish Kapania over 11 years ago

TI__Mastermind 21645 points

Hi Vasili,

Thanks for finding the problem. We recently found a similar issue with A15 Hwi module but did not realize the same problem exists with A8 Hwi module too. I have filed a bug for this:

https://cqweb.ext.ti.com/cqweb/main?command=GenerateMainFrame&service=CQ&schema=SDO-Web&contextid=SDOWP&entityID=SDOCM00104170&entityDefName=IncidentReport&username=readonly&password=readonly

For A15 we fixed this issue by marking the C function that contains inline assembly to switch the stack with "naked" attribute and suppressing the prologue/epilogue generation.

Best,

Ashish

0 Vasili Galka over 11 years ago in reply to Ashish Kapania

Expert 2690 points

Hi Ashish,

1. Glad to help :-)
2. I don't seem to have access to SDOCM00104170, is there some kind of permission system?
3. Indeed, the "naked" attribute solution sounds even better as I had to add an additional indirection to access the struct members from assembly (which has a penalty of additional instruction and possible cache load).

Best,
Vasili

0 Vasili Galka over 11 years ago in reply to Vasili Galka

Expert 2690 points

Just looked on the A15 solution:
    __asm__ __volatile__ (
            "ldr r1, hwiModState\n\t"
            "ldr r0, [r1]\n\t"                /* Old taskSP */
            "cmp r0, #0\n\t"                  /* On ISR stack already ? */
            "bxne lr\n\t"                     /* Return if yes */
            "str r13, [r1]\n\t"               /* save SP into taskSP */
            "ldr r13, [r1, #4]\n\t"           /* switch to isr Stack */
            "bx lr\n\t"
            "hwiModState: .word ti_sysbios_family_arm_gic_Hwi_Module__state__V"
                 );

I only can propose replacing the "ldr r1, hwiModState" by movw/movt for above mentioned reasons.
Also, use the power of in line assembly to automatically deduce the isrStack offset in the struct instead of coding #4 directly.

0 Ashish Kapania over 11 years ago in reply to Vasili Galka

TI__Mastermind 21645 points

Hi Vasili,

I think there is a delay between me submitting a bug and it becoming visible externally. If you check after an hour, it should be visible externally.

And I will make sure we replace ldr with movt/movw instructions.

BEst,

Ashish

Processors

Processors forum

[PATCH] ISR<->Task stack switch failure