diff --git a/arch/arm/core/cortex_m/pm_s2ram.S b/arch/arm/core/cortex_m/pm_s2ram.S index d15bfea0432..3e797c749c3 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,15 +212,18 @@ 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 - -resume: + push {r0, lr} + bl pm_s2ram_mark_check_and_clear + cmp r0, #0x1 + pop {r0, lr} + beq .L_resume + bx lr + +.L_resume: /* * Restore the CPU context */ 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/arch/arm/core/cortex_m/swap_helper.S b/arch/arm/core/cortex_m/swap_helper.S index c6207084b5e..f1bbbfa2a13 100644 --- a/arch/arm/core/cortex_m/swap_helper.S +++ b/arch/arm/core/cortex_m/swap_helper.S @@ -98,7 +98,7 @@ SECTION_FUNC(TEXT, z_arm_pendsv) #ifdef CONFIG_FPU_SHARING /* Assess whether switched-out thread had been using the FP registers. */ tst lr, #_EXC_RETURN_FTYPE_Msk - bne out_fp_endif + bne .L_out_fp_endif /* FP context active: set FP state and store callee-saved registers. * Note: if Lazy FP stacking is enabled, storing the callee-saved @@ -108,7 +108,7 @@ SECTION_FUNC(TEXT, z_arm_pendsv) add r0, r2, #_thread_offset_to_preempt_float vstmia r0, {s16-s31} -out_fp_endif: +.L_out_fp_endif: /* At this point FPCCR.LSPACT is guaranteed to be cleared, * regardless of whether the thread has an active FP context. */ @@ -204,9 +204,9 @@ out_fp_endif: * were enabled before irq_lock was called. */ cmp r0, #0 - bne _thread_irq_disabled + bne .L_thread_irq_disabled cpsie i -_thread_irq_disabled: +.L_thread_irq_disabled: #if defined(CONFIG_MPU_STACK_GUARD) || defined(CONFIG_USERSPACE) /* Re-program dynamic memory map */ @@ -259,7 +259,7 @@ _thread_irq_disabled: #ifdef CONFIG_FPU_SHARING /* Assess whether switched-in thread had been using the FP registers. */ tst lr, #_EXC_RETURN_FTYPE_Msk - beq in_fp_active + beq .L_in_fp_active /* FP context inactive for swapped-in thread: * - reset FPSCR to 0 * - set EXC_RETURN.F_Type (prevents FP frame un-stacking when returning @@ -267,9 +267,9 @@ _thread_irq_disabled: */ movs.n r3, #0 vmsr fpscr, r3 - b in_fp_endif + b .L_in_fp_endif -in_fp_active: +.L_in_fp_active: /* FP context active: * - clear EXC_RETURN.F_Type * - FPSCR and caller-saved registers will be restored automatically @@ -277,7 +277,7 @@ in_fp_active: */ add r0, r2, #_thread_offset_to_preempt_float vldmia r0, {s16-s31} -in_fp_endif: +.L_in_fp_endif: /* Clear CONTROL.FPCA that may have been set by FP instructions */ mrs r3, CONTROL bic r3, #_CONTROL_FPCA_Msk @@ -361,12 +361,12 @@ SECTION_FUNC(TEXT, z_arm_svc) movs r0, #_EXC_RETURN_SPSEL_Msk mov r1, lr tst r1, r0 - beq _stack_frame_msp + beq .L_stack_frame_msp mrs r0, PSP - bne _stack_frame_endif -_stack_frame_msp: + bne .L_stack_frame_endif +.L_stack_frame_msp: mrs r0, MSP -_stack_frame_endif: +.L_stack_frame_endif: #elif defined(CONFIG_ARMV7_M_ARMV8_M_MAINLINE) tst lr, #_EXC_RETURN_SPSEL_Msk /* did we come from thread mode ? */ ite eq /* if zero (equal), came from handler mode */ @@ -399,7 +399,7 @@ _stack_frame_endif: mrs r2, CONTROL cmp r1, #3 - beq _do_syscall + beq .L_do_syscall /* * check that we are privileged before invoking other SVCs @@ -411,12 +411,12 @@ _stack_frame_endif: #elif defined(CONFIG_ARMV7_M_ARMV8_M_MAINLINE) tst r2, #0x1 #endif - bne _oops + bne .L_oops #endif /* CONFIG_USERSPACE */ cmp r1, #2 - beq _oops + beq .L_oops #if defined(CONFIG_IRQ_OFFLOAD) push {r0, lr} @@ -434,7 +434,7 @@ _stack_frame_endif: #endif -_oops: +.L_oops: push {r0, lr} #if defined(CONFIG_EXTRA_EXCEPTION_INFO) #if defined(CONFIG_ARMV7_M_ARMV8_M_MAINLINE) @@ -484,7 +484,7 @@ _oops: * r6 - call_id * r8 - saved link register */ -_do_syscall: +.L_do_syscall: #if defined(CONFIG_ARMV6_M_ARMV8_M_BASELINE) movs r3, #24 ldr r1, [r0, r3] /* grab address of PC from stack frame */ @@ -510,7 +510,7 @@ _do_syscall: /* The supplied syscall_id must be lower than the limit * (Requires unsigned integer comparison) */ - blo valid_syscall_id + blo .L_valid_syscall_id /* bad syscall id. Set arg1 to bad id and set call_id to SYSCALL_BAD */ str r6, [r0] @@ -518,7 +518,7 @@ _do_syscall: /* Bad syscalls treated as valid syscalls with ID K_SYSCALL_BAD. */ -valid_syscall_id: +.L_valid_syscall_id: ldr r0, =_kernel ldr r0, [r0, #_kernel_offset_to_current] #if defined(CONFIG_ARMV6_M_ARMV8_M_BASELINE) 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. diff --git a/soc/nordic/nrf54h/pm_s2ram.c b/soc/nordic/nrf54h/pm_s2ram.c index a85572f3d7b..c339039045e 100644 --- a/soc/nordic/nrf54h/pm_s2ram.c +++ b/soc/nordic/nrf54h/pm_s2ram.c @@ -177,51 +177,23 @@ int soc_s2ram_suspend(pm_s2ram_system_off_fn_t system_off) return ret; } -void __attribute__((naked)) pm_s2ram_mark_set(void) +void pm_s2ram_mark_set(void) { /* empty */ - __asm__ volatile("bx lr\n"); } -bool __attribute__((naked)) pm_s2ram_mark_check_and_clear(void) +bool pm_s2ram_mark_check_and_clear(void) { - register uint32_t link_reg __asm__("r14"); + bool restore_valid; + uint32_t reset_reason = nrf_resetinfo_resetreas_local_get(NRF_RESETINFO); - __asm__ volatile( - /* Set return value to 0 */ - "mov r0, #0\n" - - /* Load and check RESETREAS register */ - "ldr r3, [%[resetinfo_addr], %[resetreas_offs]]\n" - "cmp r3, %[resetreas_unretained_mask]\n" - - "bne exit\n" - - /* Clear RESETREAS register */ - "str r0, [%[resetinfo_addr], %[resetreas_offs]]\n" - - /* Load RESTOREVALID register */ - "ldr r3, [%[resetinfo_addr], %[restorevalid_offs]]\n" - - /* Clear RESTOREVALID */ - "str r0, [%[resetinfo_addr], %[restorevalid_offs]]\n" - - /* Check RESTOREVALID register */ - "cmp r3, %[restorevalid_present_mask]\n" - "bne exit\n" - - /* Set return value to 1 */ - "mov r0, #1\n" + if (reset_reason != NRF_RESETINFO_RESETREAS_LOCAL_UNRETAINED_MASK) { + return false; + } + nrf_resetinfo_resetreas_local_set(NRF_RESETINFO, 0); - "exit:\n" - "bx %[link_reg]\n" - : - : [resetinfo_addr] "r"(NRF_RESETINFO), - [resetreas_offs] "r"(offsetof(NRF_RESETINFO_Type, RESETREAS.LOCAL)), - [resetreas_unretained_mask] "r"(NRF_RESETINFO_RESETREAS_LOCAL_UNRETAINED_MASK), - [restorevalid_offs] "r"(offsetof(NRF_RESETINFO_Type, RESTOREVALID)), - [restorevalid_present_mask] "r"(RESETINFO_RESTOREVALID_RESTOREVALID_Msk), - [link_reg] "r"(link_reg) + restore_valid = nrf_resetinfo_restore_valid_check(NRF_RESETINFO); + nrf_resetinfo_restore_valid_set(NRF_RESETINFO, false); - : "r0", "r1", "r3", "r4", "cc", "memory"); + return restore_valid ? true : false; }