Skip to content

arch: arm: move stack switching back to ASM code #16099

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 16, 2019
Merged

arch: arm: move stack switching back to ASM code #16099

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed682b9
arch: arm: switching stack pointer with assembly code
wentongwu May 16, 2019
481a118
tests: kernel: increase stack buffer when code coverage enabled
wentongwu May 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 0 additions & 47 deletions arch/arm/core/cortex_m/prep_c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include <linker/linker-defs.h>
#include <kernel_internal.h>
#include <arch/arm/cortex_m/cmsis.h>
#include <cortex_m/stack.h>

#if defined(__GNUC__)
/*
Expand All @@ -37,44 +36,6 @@

#include <string.h>

static inline void switch_sp_to_psp(void)
{
__set_CONTROL(__get_CONTROL() | CONTROL_SPSEL_Msk);
/*
* When changing the stack pointer, software must use an ISB instruction
* immediately after the MSR instruction. This ensures that instructions
* after the ISB instruction execute using the new stack pointer.
*/
__ISB();
}

static inline void set_and_switch_to_psp(void)
{
u32_t process_sp;

process_sp = (u32_t)&_interrupt_stack + CONFIG_ISR_STACK_SIZE;
__set_PSP(process_sp);
switch_sp_to_psp();
}

void lock_interrupts(void)
{
#if defined(CONFIG_ARMV6_M_ARMV8_M_BASELINE)
__disable_irq();
#elif defined(CONFIG_ARMV7_M_ARMV8_M_MAINLINE)
__set_BASEPRI(_EXC_IRQ_DEFAULT_PRIO);
#else
#error Unknown ARM architecture
#endif /* CONFIG_ARMV6_M_ARMV8_M_BASELINE */
}

#ifdef CONFIG_INIT_STACKS
static inline void init_stacks(void)
{
memset(&_interrupt_stack, 0xAA, CONFIG_ISR_STACK_SIZE);
}
#endif

#ifdef CONFIG_CPU_CORTEX_M_HAS_VTOR

#ifdef CONFIG_XIP
Expand Down Expand Up @@ -191,14 +152,6 @@ extern void z_IntLibInit(void);
#endif
void _PrepC(void)
{
#ifdef CONFIG_INIT_STACKS
init_stacks();
#endif
/*
* Set PSP and use it to boot without using MSP, so that it
* gets set to _interrupt_stack during initialization.
*/
set_and_switch_to_psp();
relocate_vector_table();
enable_floating_point();
z_bss_zero();
Expand Down
38 changes: 37 additions & 1 deletion arch/arm/core/cortex_m/reset.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
_ASM_FILE_PROLOGUE

GTEXT(__reset)
GTEXT(memset)
GDATA(_interrupt_stack)
#if defined(CONFIG_PLATFORM_SPECIFIC_INIT)
GTEXT(_PlatformInit)
#endif
Expand Down Expand Up @@ -61,12 +63,46 @@ SECTION_SUBSEC_FUNC(TEXT,_reset_section,__start)
#endif

/* lock interrupts: will get unlocked when switch to main task */
bl lock_interrupts
#if defined(CONFIG_ARMV6_M_ARMV8_M_BASELINE)
cpsid i
#elif defined(CONFIG_ARMV7_M_ARMV8_M_MAINLINE)
movs.n r0, #_EXC_IRQ_DEFAULT_PRIO
msr BASEPRI, r0
#else
#error Unknown ARM architecture
#endif

#ifdef CONFIG_WDOG_INIT
/* board-specific watchdog initialization is necessary */
bl _WdogInit
#endif

#ifdef CONFIG_INIT_STACKS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is OK with me, I think we can just keep it as is. Although, still the C code should work, cause I don't see any SP manipulation.

ldr r0, =_interrupt_stack
ldr r1, =0xaa
ldr r2, =CONFIG_ISR_STACK_SIZE
bl memset
#endif

/*
* Set PSP and use it to boot without using MSP, so that it
* gets set to _interrupt_stack during initialization.
*/
ldr r0, =_interrupt_stack
ldr r1, =CONFIG_ISR_STACK_SIZE
adds r0, r0, r1
msr PSP, r0
mrs r0, CONTROL
movs r1, #2
Copy link
Member

@ioannisg ioannisg May 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls, add the /* CONTROL_SPSEL_Msk */ comment in this line: orrs r0, r1
Actually, probably, you can do "orrs" directly with an immediate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it seems ARMV7-m can orrs immediate, but ARMV6-M can not (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0662a/CIHBDBJH.html )

orrs r0, r1 /* CONTROL_SPSEL_Msk */
msr CONTROL, r0
/*
* When changing the stack pointer, software must use an ISB instruction
* immediately after the MSR instruction. This ensures that instructions
* after the ISB instruction execute using the new stack pointer.
*/
isb

/*
* 'bl' jumps the furthest of the branch instructions that are
* supported on all platforms. So it is used when jumping to _PrepC
Expand Down
2 changes: 1 addition & 1 deletion tests/kernel/mem_protect/userspace/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,7 @@ static void domain_remove_part_context_switch(void)
*/

#define NUM_STACKS 3
#define STEST_STACKSIZE 1024
#define STEST_STACKSIZE (1024 + CONFIG_TEST_EXTRA_STACKSIZE)
K_THREAD_STACK_DEFINE(stest_stack, STEST_STACKSIZE);
K_THREAD_STACK_ARRAY_DEFINE(stest_stack_array, NUM_STACKS, STEST_STACKSIZE);

Expand Down