diff --git a/arch/arm/core/cortex_m/pm_s2ram.S b/arch/arm/core/cortex_m/pm_s2ram.S index d15bfea0432..1722e32136e 100644 --- a/arch/arm/core/cortex_m/pm_s2ram.S +++ b/arch/arm/core/cortex_m/pm_s2ram.S @@ -178,9 +178,7 @@ SECTION_FUNC(TEXT, arch_pm_s2ram_suspend) /* * Mark entering suspend to RAM. */ - mov r1, lr - bl pm_s2ram_mark_set - mov lr, r1 + bl pm_s2ram_mark_set /* * Call the system_off function passed as parameter. This should never @@ -199,9 +197,7 @@ SECTION_FUNC(TEXT, arch_pm_s2ram_suspend) /* * Reset the marking of suspend to RAM, return is ignored. */ - mov r1, lr - bl pm_s2ram_mark_check_and_clear - mov lr, r1 + bl pm_s2ram_mark_check_and_clear /* Move the stored return value of system_off back to r0, * setting it as return value for this function. @@ -216,13 +212,16 @@ GTEXT(arch_pm_s2ram_resume) SECTION_FUNC(TEXT, arch_pm_s2ram_resume) /* * Check if reset occurred after suspending to RAM. + * Store LR to ensure we can continue boot when we are not suspended + * to RAM. In addition to LR, R0 is pushed too, to ensure "SP mod 8 = 0", + * as stated by ARM rule 6.2.1.2 for AAPCS32. */ - mov r1, lr - bl pm_s2ram_mark_check_and_clear - mov lr, r1 - cmp r0, #0x1 - beq resume - bx lr + push {r0, lr} + bl pm_s2ram_mark_check_and_clear + cmp r0, #0x1 + pop {r0, lr} + beq resume + bx lr resume: /* diff --git a/arch/arm/core/cortex_m/pm_s2ram.c b/arch/arm/core/cortex_m/pm_s2ram.c index b7fe5d9b626..2657d48dc32 100644 --- a/arch/arm/core/cortex_m/pm_s2ram.c +++ b/arch/arm/core/cortex_m/pm_s2ram.c @@ -22,44 +22,20 @@ __noinit _cpu_context_t _cpu_context; */ static __noinit uint32_t marker; -void __attribute__((naked)) pm_s2ram_mark_set(void) +void pm_s2ram_mark_set(void) { - __asm__ volatile( - /* Set the marker to MAGIC value */ - "str %[_magic_val], [%[_marker]]\n" - - "bx lr\n" - : - : [_magic_val] "r"(MAGIC), [_marker] "r"(&marker) - : "r1", "r4", "memory"); + marker = MAGIC; } -bool __attribute__((naked)) pm_s2ram_mark_check_and_clear(void) +bool pm_s2ram_mark_check_and_clear(void) { - __asm__ volatile( - /* Set return value to 0 */ - "mov r0, #0\n" - - /* Check the marker */ - "ldr r3, [%[_marker]]\n" - "cmp r3, %[_magic_val]\n" - "bne exit\n" - - /* - * Reset the marker - */ - "str r0, [%[_marker]]\n" + if (marker == MAGIC) { + marker = 0; - /* - * Set return value to 1 - */ - "mov r0, #1\n" + return true; + } - "exit:\n" - "bx lr\n" - : - : [_magic_val] "r"(MAGIC), [_marker] "r"(&marker) - : "r0", "r1", "r3", "r4", "memory"); + return false; } #endif /* CONFIG_PM_S2RAM_CUSTOM_MARKING */ diff --git a/arch/arm/core/cortex_m/reset.S b/arch/arm/core/cortex_m/reset.S index bc75ccfceaa..c3f9362eca8 100644 --- a/arch/arm/core/cortex_m/reset.S +++ b/arch/arm/core/cortex_m/reset.S @@ -21,6 +21,7 @@ _ASM_FILE_PROLOGUE GTEXT(z_arm_reset) GTEXT(z_early_memset) GDATA(z_interrupt_stacks) +GDATA(z_main_stack) #if defined(CONFIG_DEBUG_THREAD_INFO) GDATA(z_sys_post_kernel) #endif @@ -29,7 +30,6 @@ GTEXT(soc_reset_hook) #endif #if defined(CONFIG_INIT_ARCH_HW_AT_BOOT) GTEXT(z_arm_init_arch_hw_at_boot) -GDATA(z_main_stack) #endif #if defined(CONFIG_PM_S2RAM) GTEXT(arch_pm_s2ram_resume) @@ -68,13 +68,6 @@ SECTION_SUBSEC_FUNC(TEXT,_reset_section,z_arm_reset) */ SECTION_SUBSEC_FUNC(TEXT,_reset_section,__start) -#if defined(CONFIG_DEBUG_THREAD_INFO) - /* Clear z_sys_post_kernel flag for RTOS aware debuggers */ - movs.n r0, #0 - ldr r1, =z_sys_post_kernel - strb r0, [r1] -#endif /* CONFIG_DEBUG_THREAD_INFO */ - #if defined(CONFIG_INIT_ARCH_HW_AT_BOOT) /* Reset CONTROL register */ movs.n r0, #0 @@ -90,9 +83,56 @@ SECTION_SUBSEC_FUNC(TEXT,_reset_section,__start) #endif /* CONFIG_INIT_ARCH_HW_AT_BOOT */ #if defined(CONFIG_PM_S2RAM) + /* + * Temporarily set MSP to interrupt stack so that arch_pm_s2ram_resume can + * use stack for calling pm_s2ram_mark_check_and_clear. + * This is safe because suspend never being called from an ISR ensures that + * interrupt stack was not live during suspend. + * Note: + * if resuming from suspend, MSP is restored from cpu context + * if not resuming, MSP will be set back to z_main_stack for proper init + * And, apart from arch_pm_s2ram_resume being able to use the stack for + * a short while, there is no change in behavior in either of the paths. + */ + ldr r0, =z_interrupt_stacks + CONFIG_ISR_STACK_SIZE + MPU_GUARD_ALIGN_AND_SIZE + msr msp, r0 + bl arch_pm_s2ram_resume #endif /* CONFIG_PM_S2RAM */ + /* + * MSP needs to be set to main stack for following scenarios: + * - If CONFIG_PM_S2RAM is enabled, MSP is set to z_interrupt_stacks + * for resume functions to make use of a stack. However, + * if the device was not suspended and if resume functions return, + * MSP needs to be set back to z_main_stack to ensure proper + * initialization. + * - If CONFIG_PM_S2RAM is not enabled but CONFIG_INIT_ARCH_HW_AT_BOOT is, + * MSP needs to be set to z_main_stack for proper initialization in case + * device was loaded through chain loading or a debugger, as the initial + * value of MSP would be anything that the previous image loaded. + * - If CONFIG_INIT_STACKS is enabled, we need to ensure MSP is not set + * to z_interrupt_stacks, so we set it to z_main_stack. + * + * Since these scenarios cover most of the cases, we set MSP to + * z_main_stack here. + * + */ + ldr r0, =z_main_stack + CONFIG_MAIN_STACK_SIZE + msr msp, r0 + + /* Note: Make sure that variables like z_sys_post_kernel + * are set after the call to arch_pm_s2ram_resume + * to avoid any issues with suspend/resume path. + * Refer issue #83660 for more details. + */ +#if defined(CONFIG_DEBUG_THREAD_INFO) + /* Clear z_sys_post_kernel flag for RTOS aware debuggers */ + movs.n r0, #0 + ldr r1, =z_sys_post_kernel + strb r0, [r1] +#endif /* CONFIG_DEBUG_THREAD_INFO */ + #if defined(CONFIG_SOC_RESET_HOOK) bl soc_reset_hook #endif @@ -105,8 +145,6 @@ SECTION_SUBSEC_FUNC(TEXT,_reset_section,__start) str r0, [r1] dsb #endif /* CONFIG_CPU_HAS_ARM_MPU */ - ldr r0, =z_main_stack + CONFIG_MAIN_STACK_SIZE - msr msp, r0 /* Initialize core architecture registers and system blocks */ bl z_arm_init_arch_hw_at_boot diff --git a/include/zephyr/arch/common/pm_s2ram.h b/include/zephyr/arch/common/pm_s2ram.h index ad9ab8ad7a4..34c544c769b 100644 --- a/include/zephyr/arch/common/pm_s2ram.h +++ b/include/zephyr/arch/common/pm_s2ram.h @@ -65,11 +65,6 @@ int arch_pm_s2ram_suspend(pm_s2ram_system_off_fn_t system_off); * * Default implementation is setting a magic word in RAM. CONFIG_PM_S2RAM_CUSTOM_MARKING * allows custom implementation. - * The following requirements must be fulfilled: - * - the function cannot use stack (asm function or function with 'naked' attribute) - * - the content of the R1 and R4 registers must remain unchanged - * - returning from the function should be performed with the `bx lr` instruction - * */ void pm_s2ram_mark_set(void); @@ -81,11 +76,6 @@ void pm_s2ram_mark_set(void); * * Default implementation is checking a magic word in RAM. CONFIG_PM_S2RAM_CUSTOM_MARKING * allows custom implementation. - * The following requirements must be fulfilled: - * - the function cannot use stack (most likely asm function) - * - the content of the R1 and R4 registers must remain unchanged - * - the function's return value is passed by R0 register - * - returning from the function should be performed with the `bx lr` instruction * * @retval true if marking is found which indicates resuming after suspend-to-RAM. * @retval false if marking is not found which indicates standard boot.