From bb801857eaf21365402a4748296c05cb3c6e861f Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Tue, 21 Jan 2025 11:41:46 +0000 Subject: [PATCH 1/9] feat(cpus): add sysreg_bit_toggle Introduce a new helper to toggle bits in assembly. This allows us to call the workaround twice, with the first call setting the workaround and second undoing it. This allows the (errata) workaround functions to be used to both apply and undo the mitigation. This is applied to functions where the undo part will be required in follow-up patches. Change-Id: I058bad58f5949b2d5fe058101410e33b6be1b8ba Signed-off-by: Boyan Karatotev --- include/lib/cpus/aarch64/cpu_macros.S | 14 +++++++++++++- lib/cpus/aarch64/cortex_a710.S | 7 ++++--- lib/cpus/aarch64/cortex_gelas.S | 7 ++++--- lib/cpus/aarch64/cortex_x3.S | 4 +++- lib/cpus/aarch64/neoverse_n2.S | 7 ++++--- lib/cpus/aarch64/travis.S | 7 ++++--- 6 files changed, 32 insertions(+), 14 deletions(-) diff --git a/include/lib/cpus/aarch64/cpu_macros.S b/include/lib/cpus/aarch64/cpu_macros.S index 0ce9c3cbc..ab6041289 100644 --- a/include/lib/cpus/aarch64/cpu_macros.S +++ b/include/lib/cpus/aarch64/cpu_macros.S @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014-2024, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2014-2025, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -402,6 +402,18 @@ msr \_reg, x1 .endm +/* + * Toggle a bit in a system register. Can toggle multiple bits but is limited by + * the way the EOR instrucion encodes them. + * + * see sysreg_bit_set for usage + */ +.macro sysreg_bit_toggle _reg:req, _bit:req, _assert=1 + mrs x1, \_reg + eor x1, x1, #\_bit + msr \_reg, x1 +.endm + .macro override_vector_table _table:req adr x1, \_table msr vbar_el3, x1 diff --git a/lib/cpus/aarch64/cortex_a710.S b/lib/cpus/aarch64/cortex_a710.S index dce9c7354..830771aa7 100644 --- a/lib/cpus/aarch64/cortex_a710.S +++ b/lib/cpus/aarch64/cortex_a710.S @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021-2024, Arm Limited. All rights reserved. + * Copyright (c) 2021-2025, Arm Limited. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -163,8 +163,9 @@ workaround_reset_end cortex_a710, ERRATUM(2282622) check_erratum_ls cortex_a710, ERRATUM(2282622), CPU_REV(2, 1) workaround_runtime_start cortex_a710, ERRATUM(2291219), ERRATA_A710_2291219 - /* Set bit 36 in ACTLR2_EL1 */ - sysreg_bit_set CORTEX_A710_CPUACTLR2_EL1, CORTEX_A710_CPUACTLR2_EL1_BIT_36 + /* Set/unset bit 36 in ACTLR2_EL1. The first call will set it, applying + * the workaround. Second call clears it to undo it. */ + sysreg_bit_toggle CORTEX_A710_CPUACTLR2_EL1, CORTEX_A710_CPUACTLR2_EL1_BIT_36 workaround_runtime_end cortex_a710, ERRATUM(2291219), NO_ISB check_erratum_ls cortex_a710, ERRATUM(2291219), CPU_REV(2, 0) diff --git a/lib/cpus/aarch64/cortex_gelas.S b/lib/cpus/aarch64/cortex_gelas.S index 891e9a653..43608e4fb 100644 --- a/lib/cpus/aarch64/cortex_gelas.S +++ b/lib/cpus/aarch64/cortex_gelas.S @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023-2024, Arm Limited. All rights reserved. + * Copyright (c) 2023-2025, Arm Limited. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -49,10 +49,11 @@ func cortex_gelas_core_pwr_dwn 1: #endif /* --------------------------------------------------- - * Enable CPU power down bit in power control register + * Flip CPU power down bit in power control register. + * It will be set on powerdown and cleared on wakeup * --------------------------------------------------- */ - sysreg_bit_set CORTEX_GELAS_CPUPWRCTLR_EL1, \ + sysreg_bit_toggle CORTEX_GELAS_CPUPWRCTLR_EL1, \ CORTEX_GELAS_CPUPWRCTLR_EL1_CORE_PWRDN_BIT isb ret diff --git a/lib/cpus/aarch64/cortex_x3.S b/lib/cpus/aarch64/cortex_x3.S index 4a0212e20..7a339b489 100644 --- a/lib/cpus/aarch64/cortex_x3.S +++ b/lib/cpus/aarch64/cortex_x3.S @@ -53,7 +53,9 @@ workaround_runtime_end cortex_x3, ERRATUM(2302506), NO_ISB check_erratum_ls cortex_x3, ERRATUM(2302506), CPU_REV(1, 1) workaround_runtime_start cortex_x3, ERRATUM(2313909), ERRATA_X3_2313909 - sysreg_bit_set CORTEX_X3_CPUACTLR2_EL1, CORTEX_X3_CPUACTLR2_EL1_BIT_36 + /* Set/unset bit 36 in ACTLR2_EL1. The first call will set it, applying + * the workaround. Second call clears it to undo it. */ + sysreg_bit_toggle CORTEX_X3_CPUACTLR2_EL1, CORTEX_X3_CPUACTLR2_EL1_BIT_36 workaround_runtime_end cortex_x3, ERRATUM(2313909), NO_ISB check_erratum_ls cortex_x3, ERRATUM(2313909), CPU_REV(1, 0) diff --git a/lib/cpus/aarch64/neoverse_n2.S b/lib/cpus/aarch64/neoverse_n2.S index 69aa8abbc..5cccff311 100644 --- a/lib/cpus/aarch64/neoverse_n2.S +++ b/lib/cpus/aarch64/neoverse_n2.S @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020-2024, Arm Limited. All rights reserved. + * Copyright (c) 2020-2025, Arm Limited. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -166,8 +166,9 @@ workaround_reset_end neoverse_n2, ERRATUM(2280757) check_erratum_ls neoverse_n2, ERRATUM(2280757), CPU_REV(0, 0) workaround_runtime_start neoverse_n2, ERRATUM(2326639), ERRATA_N2_2326639 - /* Set bit 36 in ACTLR2_EL1 */ - sysreg_bit_set NEOVERSE_N2_CPUACTLR2_EL1, NEOVERSE_N2_CPUACTLR2_EL1_BIT_36 + /* Set/unset bit 36 in ACTLR2_EL1. The first call will set it, applying + * the workaround. Second call clears it to undo it. */ + sysreg_bit_toggle NEOVERSE_N2_CPUACTLR2_EL1, NEOVERSE_N2_CPUACTLR2_EL1_BIT_36 workaround_runtime_end neoverse_n2, ERRATUM(2326639) check_erratum_ls neoverse_n2, ERRATUM(2326639), CPU_REV(0, 0) diff --git a/lib/cpus/aarch64/travis.S b/lib/cpus/aarch64/travis.S index e8b3860b0..695e7d86b 100644 --- a/lib/cpus/aarch64/travis.S +++ b/lib/cpus/aarch64/travis.S @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023-2024, Arm Limited. All rights reserved. + * Copyright (c) 2023-2025, Arm Limited. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -45,10 +45,11 @@ func travis_core_pwr_dwn 1: #endif /* --------------------------------------------------- - * Enable CPU power down bit in power control register + * Flip CPU power down bit in power control register. + * It will be set on powerdown and cleared on wakeup * --------------------------------------------------- */ - sysreg_bit_set TRAVIS_IMP_CPUPWRCTLR_EL1, \ + sysreg_bit_toggle TRAVIS_IMP_CPUPWRCTLR_EL1, \ TRAVIS_IMP_CPUPWRCTLR_EL1_CORE_PWRDN_EN_BIT isb ret From cc94e71b3ac5233d5ff6bc0156ded8ff03408c24 Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Thu, 26 Sep 2024 17:00:09 +0100 Subject: [PATCH 2/9] refactor(cpus): undo errata mitigations The workarounds introduced in the three patches starting at 888eafa00b99aa06b4ff688407336811a7ff439a assumed that any powerdown request will be (forced to be) terminal. This assumption can no longer be the case for new CPUs so there is a need to revisit these older cores. Since we may wake up, we now need to respect the workaround's recommendation that the workaround needs to be reverted on wakeup. So do exactly that. Introduce a new helper to toggle bits in assembly. This allows us to call the workaround twice, with the first call setting the workaround and second undoing it. This is also used for gelas' an travis' powerdown routines. This is so the same function can be called again Also fix the condition in the cpu helper macro as it was subtly wrong Change-Id: Iff9e5251dc9d8670d085d88c070f78991955e7c3 Signed-off-by: Boyan Karatotev --- include/lib/cpus/aarch64/cpu_macros.S | 2 +- lib/cpus/aarch64/cortex_a710.S | 1 + lib/cpus/aarch64/cortex_x3.S | 1 + lib/cpus/aarch64/neoverse_n2.S | 1 + lib/psci/aarch64/psci_helpers.S | 10 +++++++++- 5 files changed, 13 insertions(+), 2 deletions(-) diff --git a/include/lib/cpus/aarch64/cpu_macros.S b/include/lib/cpus/aarch64/cpu_macros.S index ab6041289..ac26fd74b 100644 --- a/include/lib/cpus/aarch64/cpu_macros.S +++ b/include/lib/cpus/aarch64/cpu_macros.S @@ -463,7 +463,7 @@ * clobbers: x0-x10 (PCS compliant) */ .macro apply_erratum _cpu:req, _cve:req, _id:req, _chosen:req, _get_rev=GET_CPU_REV - .if (\_chosen & \_get_rev) + .if (\_chosen && \_get_rev) mov x9, x30 bl cpu_get_rev_var mov x10, x0 diff --git a/lib/cpus/aarch64/cortex_a710.S b/lib/cpus/aarch64/cortex_a710.S index 830771aa7..fef240c1c 100644 --- a/lib/cpus/aarch64/cortex_a710.S +++ b/lib/cpus/aarch64/cortex_a710.S @@ -162,6 +162,7 @@ workaround_reset_end cortex_a710, ERRATUM(2282622) check_erratum_ls cortex_a710, ERRATUM(2282622), CPU_REV(2, 1) +.global erratum_cortex_a710_2291219_wa workaround_runtime_start cortex_a710, ERRATUM(2291219), ERRATA_A710_2291219 /* Set/unset bit 36 in ACTLR2_EL1. The first call will set it, applying * the workaround. Second call clears it to undo it. */ diff --git a/lib/cpus/aarch64/cortex_x3.S b/lib/cpus/aarch64/cortex_x3.S index 7a339b489..7c6af8cb4 100644 --- a/lib/cpus/aarch64/cortex_x3.S +++ b/lib/cpus/aarch64/cortex_x3.S @@ -52,6 +52,7 @@ workaround_runtime_end cortex_x3, ERRATUM(2302506), NO_ISB check_erratum_ls cortex_x3, ERRATUM(2302506), CPU_REV(1, 1) +.global erratum_cortex_x3_2313909_wa workaround_runtime_start cortex_x3, ERRATUM(2313909), ERRATA_X3_2313909 /* Set/unset bit 36 in ACTLR2_EL1. The first call will set it, applying * the workaround. Second call clears it to undo it. */ diff --git a/lib/cpus/aarch64/neoverse_n2.S b/lib/cpus/aarch64/neoverse_n2.S index 5cccff311..b9818cd64 100644 --- a/lib/cpus/aarch64/neoverse_n2.S +++ b/lib/cpus/aarch64/neoverse_n2.S @@ -165,6 +165,7 @@ workaround_reset_end neoverse_n2, ERRATUM(2280757) check_erratum_ls neoverse_n2, ERRATUM(2280757), CPU_REV(0, 0) +.global erratum_neoverse_n2_2326639_wa workaround_runtime_start neoverse_n2, ERRATUM(2326639), ERRATA_N2_2326639 /* Set/unset bit 36 in ACTLR2_EL1. The first call will set it, applying * the workaround. Second call clears it to undo it. */ diff --git a/lib/psci/aarch64/psci_helpers.S b/lib/psci/aarch64/psci_helpers.S index cca08c132..088ab43f4 100644 --- a/lib/psci/aarch64/psci_helpers.S +++ b/lib/psci/aarch64/psci_helpers.S @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014-2024, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2014-2025, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -131,4 +131,12 @@ func psci_power_down_wfi 1: wfi b 1b + + /* + * in case the WFI wasn't terminal, we have to undo errata mitigations. + * These will be smart enough to handle being called the same way + */ + apply_erratum cortex_a710, ERRATUM(2291219), ERRATA_A710_2291219 + apply_erratum cortex_x3, ERRATUM(2313909), ERRATA_X3_2313909, NO_GET_CPU_REV + apply_erratum neoverse_n2, ERRATUM(2326639), ERRATA_N2_2326639, NO_GET_CPU_REV endfunc psci_power_down_wfi From 2bd3b39767019a2c49f4d6b0d8d316dfe8e1e6b7 Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Mon, 21 Oct 2024 07:36:31 +0100 Subject: [PATCH 3/9] refactor: panic after calling psci_power_down_wfi() This function doesn't return and its callers that don't return either rely on this. Drop the dead attribute and add a panic() after it to make this expectation explicit. Calling `wfi` in the powerdown sequence is terminal so even if the function was made to return, there would be no functional change. This is useful for a following patch that makes psci_power_down_wfi() return. Change-Id: I62ca1ee058b1eaeb046966c795081e01bf45a2eb Signed-off-by: Boyan Karatotev --- include/lib/psci/psci.h | 4 ++-- plat/allwinner/common/sunxi_native_pm.c | 2 ++ plat/allwinner/common/sunxi_scpi_pm.c | 6 ++++++ plat/mediatek/include/lib/pm/mtk_pm.h | 2 +- plat/mediatek/lib/pm/armv9_0/pwr_ctrl.c | 4 +++- plat/qti/common/src/qti_pm.c | 1 + plat/rockchip/common/plat_pm.c | 4 ++++ plat/rockchip/px30/drivers/pmu/pmu.c | 4 ++++ plat/rockchip/rk3328/drivers/pmu/pmu.c | 2 ++ plat/rockchip/rk3588/drivers/pmu/pmu.c | 8 ++++++++ 10 files changed, 33 insertions(+), 4 deletions(-) diff --git a/include/lib/psci/psci.h b/include/lib/psci/psci.h index f12a4d62c..8ea4c277f 100644 --- a/include/lib/psci/psci.h +++ b/include/lib/psci/psci.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2019, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2013-2024, Arm Limited and Contributors. All rights reserved. * Copyright (c) 2023, NVIDIA Corporation. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause @@ -376,7 +376,7 @@ int psci_features(unsigned int psci_fid); #if PSCI_OS_INIT_MODE int psci_set_suspend_mode(unsigned int mode); #endif -void __dead2 psci_power_down_wfi(void); +void psci_power_down_wfi(void); void psci_arch_setup(void); #endif /*__ASSEMBLER__*/ diff --git a/plat/allwinner/common/sunxi_native_pm.c b/plat/allwinner/common/sunxi_native_pm.c index 148f50e2a..558b0bb0f 100644 --- a/plat/allwinner/common/sunxi_native_pm.c +++ b/plat/allwinner/common/sunxi_native_pm.c @@ -49,6 +49,8 @@ static void __dead2 sunxi_system_off(void) sunxi_cpu_power_off_others(); sunxi_cpu_power_off_self(); psci_power_down_wfi(); + /* should never reach here */ + panic(); } static void __dead2 sunxi_system_reset(void) diff --git a/plat/allwinner/common/sunxi_scpi_pm.c b/plat/allwinner/common/sunxi_scpi_pm.c index 6a0e96701..8870a71d9 100644 --- a/plat/allwinner/common/sunxi_scpi_pm.c +++ b/plat/allwinner/common/sunxi_scpi_pm.c @@ -108,6 +108,8 @@ static void __dead2 sunxi_system_off(void) } psci_power_down_wfi(); + /* should never reach here */ + panic(); } static void __dead2 sunxi_system_reset(void) @@ -123,6 +125,8 @@ static void __dead2 sunxi_system_reset(void) } psci_power_down_wfi(); + /* should never reach here */ + panic(); } static int sunxi_system_reset2(int is_vendor, int reset_type, u_register_t cookie) @@ -142,6 +146,8 @@ static int sunxi_system_reset2(int is_vendor, int reset_type, u_register_t cooki } psci_power_down_wfi(); + /* should never reach here */ + panic(); /* * Should not reach here. diff --git a/plat/mediatek/include/lib/pm/mtk_pm.h b/plat/mediatek/include/lib/pm/mtk_pm.h index 14d005dcd..05293e95d 100644 --- a/plat/mediatek/include/lib/pm/mtk_pm.h +++ b/plat/mediatek/include/lib/pm/mtk_pm.h @@ -78,7 +78,7 @@ struct plat_pm_pwr_ctrl { psci_power_state_t *req_state); void (*get_sys_suspend_power_state)( psci_power_state_t *req_state); - __dead2 void (*pwr_domain_pwr_down_wfi)( + void (*pwr_domain_pwr_down_wfi)( const psci_power_state_t *req_state); }; diff --git a/plat/mediatek/lib/pm/armv9_0/pwr_ctrl.c b/plat/mediatek/lib/pm/armv9_0/pwr_ctrl.c index 73b1f6855..fbaa3f437 100644 --- a/plat/mediatek/lib/pm/armv9_0/pwr_ctrl.c +++ b/plat/mediatek/lib/pm/armv9_0/pwr_ctrl.c @@ -371,7 +371,7 @@ static void get_sys_suspend_power_state(psci_power_state_t *req_state) } #endif -static void __dead2 pwr_domain_pwr_down_wfi(const psci_power_state_t *req_state) +static void pwr_domain_pwr_down_wfi(const psci_power_state_t *req_state) { unsigned int cpu = plat_my_core_pos(); int ret = MTK_CPUPM_E_NOT_SUPPORT; @@ -382,6 +382,8 @@ static void __dead2 pwr_domain_pwr_down_wfi(const psci_power_state_t *req_state) plat_panic_handler(); else psci_power_down_wfi(); + /* should never reach here */ + panic(); } static void pm_smp_init(unsigned int cpu_id, uintptr_t entry_point) diff --git a/plat/qti/common/src/qti_pm.c b/plat/qti/common/src/qti_pm.c index 1113efcfa..4400e40ea 100644 --- a/plat/qti/common/src/qti_pm.c +++ b/plat/qti/common/src/qti_pm.c @@ -217,6 +217,7 @@ __dead2 void qti_domain_power_down_wfi(const psci_power_state_t *target_state) /* For now just do WFI - add any target specific handling if needed */ psci_power_down_wfi(); /* We should never reach here */ + panic(); } static __dead2 void assert_ps_hold(void) diff --git a/plat/rockchip/common/plat_pm.c b/plat/rockchip/common/plat_pm.c index 69268870d..c3dc23474 100644 --- a/plat/rockchip/common/plat_pm.c +++ b/plat/rockchip/common/plat_pm.c @@ -118,11 +118,15 @@ void __dead2 rockchip_soc_cores_pd_pwr_dn_wfi( const psci_power_state_t *target_state) { psci_power_down_wfi(); + /* should never reach here */ + panic(); } void __dead2 rockchip_soc_sys_pd_pwr_dn_wfi(void) { psci_power_down_wfi(); + /* should never reach here */ + panic(); } /******************************************************************************* diff --git a/plat/rockchip/px30/drivers/pmu/pmu.c b/plat/rockchip/px30/drivers/pmu/pmu.c index 0d8e8b689..6200cac07 100644 --- a/plat/rockchip/px30/drivers/pmu/pmu.c +++ b/plat/rockchip/px30/drivers/pmu/pmu.c @@ -1000,6 +1000,8 @@ void __dead2 rockchip_soc_soft_reset(void) * so we do not hope the core to execute valid codes. */ psci_power_down_wfi(); + /* should never reach here */ + panic(); } void __dead2 rockchip_soc_system_off(void) @@ -1025,6 +1027,8 @@ void __dead2 rockchip_soc_system_off(void) * so we do not hope the core to execute valid codes. */ psci_power_down_wfi(); + /* should never reach here */ + panic(); } void rockchip_plat_mmu_el3(void) diff --git a/plat/rockchip/rk3328/drivers/pmu/pmu.c b/plat/rockchip/rk3328/drivers/pmu/pmu.c index 597db978f..41660e2c0 100644 --- a/plat/rockchip/rk3328/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3328/drivers/pmu/pmu.c @@ -619,6 +619,8 @@ void __dead2 rockchip_soc_sys_pd_pwr_dn_wfi(void) /* should never reach here */ psci_power_down_wfi(); + /* should never reach here */ + panic(); } int rockchip_soc_sys_pwr_dm_suspend(void) diff --git a/plat/rockchip/rk3588/drivers/pmu/pmu.c b/plat/rockchip/rk3588/drivers/pmu/pmu.c index a4128b214..16436dd13 100644 --- a/plat/rockchip/rk3588/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3588/drivers/pmu/pmu.c @@ -1319,12 +1319,16 @@ void __dead2 rockchip_soc_cores_pd_pwr_dn_wfi(const psci_power_state_t *target_state) { psci_power_down_wfi(); + /* should never reach here */ + panic(); } void __dead2 rockchip_soc_sys_pd_pwr_dn_wfi(void) { cpus_pd_req_enter_wfi(); psci_power_down_wfi(); + /* should never reach here */ + panic(); } void __dead2 rockchip_soc_soft_reset(void) @@ -1352,6 +1356,8 @@ void __dead2 rockchip_soc_soft_reset(void) * so we do not hope the core to execute valid codes. */ psci_power_down_wfi(); + /* should never reach here */ + panic(); } void __dead2 rockchip_soc_system_off(void) @@ -1373,6 +1379,8 @@ void __dead2 rockchip_soc_system_off(void) * so we do not hope the core to execute valid codes. */ psci_power_down_wfi(); + /* should never reach here */ + panic(); } static void rockchip_pmu_pd_init(void) From 2b5e00d4eacbac4b315c1c2925882d0b77bc9205 Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Thu, 19 Dec 2024 16:07:29 +0000 Subject: [PATCH 4/9] feat(psci): allow cores to wake up from powerdown The simplistic view of a core's powerdown sequence is that power is atomically cut upon calling `wfi`. However, it turns out that it has lots to do - it has to talk to the interconnect to exit coherency, clean caches, check for RAS errors, etc. These take significant amounts of time and are certainly not atomic. As such there is a significant window of opportunity for external events to happen. Many of these steps are not destructive to context, so theoretically, the core can just "give up" half way (or roll certain actions back) and carry on running. The point in this sequence after which roll back is not possible is called the point of no return. One of these actions is the checking for RAS errors. It is possible for one to happen during this lengthy sequence, or at least remain undiscovered until that point. If the core were to continue powerdown when that happens, there would be no (easy) way to inform anyone about it. Rejecting the powerdown and letting software handle the error is the best way to implement this. Arm cores since at least the a510 have included this exact feature. So far it hasn't been deemed necessary to account for it in firmware due to the low likelihood of this happening. However, events like GIC wakeup requests are much more probable. Older cores will powerdown and immediately power back up when this happens. Travis and Gelas include a feature similar to the RAS case above, called powerdown abandon. The idea is that this will improve the latency to service the interrupt by saving on work which the core and software need to do. So far firmware has relied on the `wfi` being the point of no return and if it doesn't explicitly detect a pending interrupt quite early on, it will embark onto a sequence that it expects to end with shutdown. To accommodate for it not being a point of no return, we must undo all of the system management we did, just like in the warm boot entrypoint. To achieve that, the pwr_domain_pwr_down_wfi hook must not be terminal. Most recent platforms do some platform management and finish on the standard `wfi`, followed by a panic or an endless loop as this is expected to not return. To make this generic, any platform that wishes to support wakeups must instead let common code call `psci_power_down_wfi()` right after. Besides wakeups, this lets common code handle powerdown errata better as well. Then, the CPU_OFF case is simple - PSCI does not allow it to return. So the best that can be done is to attempt the `wfi` a few times (the choice of 32 is arbitrary) in the hope that the wakeup is transient. If it isn't, the only choice is to panic, as the system is likely to be in a bad state, eg. interrupts weren't routed away. The same applies for SYSTEM_OFF, SYSTEM_RESET, and SYSTEM_RESET2. There the panic won't matter as the system is going offline one way or another. The RAS case will be considered in a separate patch. Now, the CPU_SUSPEND case is more involved. First, to powerdown it must wipe its context as it is not written on warm boot. But it cannot be overwritten in case of a wakeup. To avoid the catch 22, save a copy that will only be used if powerdown fails. That is about 500 bytes on the stack so it hopefully doesn't tip anyone over any limits. In future that can be avoided by having a core manage its own context. Second, when the core wakes up, it must undo anything it did to prepare for poweroff, which for the cores we care about, is writing CPUPWRCTLR_EL1.CORE_PWRDN_EN. The least intrusive for the cpu library way of doing this is to simply call the power off hook again and have the hook toggle the bit. If in the future there need to be more complex sequences, their direction can be advised on the value of this bit. Third, do the actual "resume". Most of the logic is already there for the retention suspend, so that only needs a small touch up to apply to the powerdown case as well. The missing bit is the powerdown specific state management. Luckily, the warmboot entrypoint does exactly that already too, so steal that and we're done. All of this is hidden behind a FEAT_PABANDON flag since it has a large memory and runtime cost that we don't want to burden non pabandon cores with. Finally, do some function renaming to better reflect their purpose and make names a little bit more consistent. Change-Id: I2405b59300c2e24ce02e266f91b7c51474c1145f Signed-off-by: Boyan Karatotev --- Makefile | 2 + docs/design/firmware-design.rst | 13 +++ docs/getting_started/build-options.rst | 8 +- docs/porting-guide.rst | 29 +++--- drivers/arm/css/scp/css_pm_scmi.c | 2 +- include/lib/psci/psci.h | 6 +- include/lib/psci/psci_lib.h | 5 +- lib/cpus/aarch64/cortex_gelas.S | 4 + lib/cpus/aarch64/travis.S | 4 + lib/psci/aarch64/psci_helpers.S | 11 +-- lib/psci/psci_common.c | 64 +++++++++++- lib/psci/psci_off.c | 14 +-- lib/psci/psci_private.h | 2 +- lib/psci/psci_suspend.c | 130 +++++++++++++++++-------- lib/psci/psci_system_off.c | 15 +-- make_helpers/defaults.mk | 5 +- plat/arm/board/fvp/platform.mk | 1 + plat/arm/board/tc/platform.mk | 3 +- plat/arm/css/common/css_pm.c | 2 +- 19 files changed, 230 insertions(+), 90 deletions(-) diff --git a/Makefile b/Makefile index 58c98a1fe..cd763db3f 100644 --- a/Makefile +++ b/Makefile @@ -1247,6 +1247,7 @@ $(eval $(call assert_booleans,\ PSA_FWU_SUPPORT \ PSA_FWU_METADATA_FW_STORE_DESC \ ENABLE_MPMM \ + FEAT_PABANDON \ ENABLE_MPMM_FCONF \ FEATURE_DETECTION \ TRNG_SUPPORT \ @@ -1446,6 +1447,7 @@ $(eval $(call add_defines,\ ENABLE_TRF_FOR_NS \ ENABLE_FEAT_HCX \ ENABLE_MPMM \ + FEAT_PABANDON \ ENABLE_MPMM_FCONF \ ENABLE_FEAT_FGT \ ENABLE_FEAT_FGT2 \ diff --git a/docs/design/firmware-design.rst b/docs/design/firmware-design.rst index 2ba54ea8e..cda80ca4c 100644 --- a/docs/design/firmware-design.rst +++ b/docs/design/firmware-design.rst @@ -1504,6 +1504,19 @@ At runtime the platform hooks for power down are invoked by the PSCI service to perform platform specific operations during a power down sequence, for example turning off CCI coherency during a cluster power down. +Newer CPUs include a feature called "powerdown abandon". The feature is based on +the observation that events like GIC wakeups have a high likelihood of happening +while the core is in the middle of its powerdown sequence (at ``wfi``). Older +cores will powerdown and immediately power back up when this happens. To save on +the work and latency involved, the newer cores will "give up" mid way through if +no context has been lost yet. This is possible as the powerdown operation is +lengthy and a large part of it does not lose context. + +To cater for this possibility, the powerdown hook will be called a second time +after a wakeup. The expectation is that the first call will operate as before, +while the second call will undo anything the first call did. This should be done +statelessly, for example by toggling the relevant bits. + CPU specific register reporting during crash ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/getting_started/build-options.rst b/docs/getting_started/build-options.rst index 58321e707..e6d8a1d15 100644 --- a/docs/getting_started/build-options.rst +++ b/docs/getting_started/build-options.rst @@ -526,6 +526,12 @@ Common build options power domain dynamic power budgeting and limit the triggering of whole-rail (i.e. clock chopping) responses to overcurrent conditions. Defaults to ``0``. + - ``FEAT_PABANDON``: Boolean option to enable support for powerdown abandon on + Arm cores that support it (currently Gelas and Travis). Extends the PSCI + implementation to expect waking up after the terminal ``wfi``. Currently, + introduces a performance penalty. Once this is removed, this option will be + removed and the feature will be enabled by default. Defaults to ``0``. + - ``ENABLE_MPMM_FCONF``: Enables configuration of MPMM through FCONF, which allows platforms with cores supporting MPMM to describe them via the ``HW_CONFIG`` device tree blob. Default is 0. @@ -1474,7 +1480,7 @@ Firmware update options -------------- -*Copyright (c) 2019-2024, Arm Limited. All rights reserved.* +*Copyright (c) 2019-2025, Arm Limited. All rights reserved.* .. _DEN0115: https://developer.arm.com/docs/den0115/latest .. _PSA FW update specification: https://developer.arm.com/documentation/den0118/latest/ diff --git a/docs/porting-guide.rst b/docs/porting-guide.rst index 6d03f4413..f0da2aa94 100644 --- a/docs/porting-guide.rst +++ b/docs/porting-guide.rst @@ -2901,19 +2901,21 @@ plat_psci_ops.pwr_domain_pwr_down_wfi() ....................................... This is an optional function and, if implemented, is expected to perform -platform specific actions including the ``wfi`` invocation which allows the -CPU to powerdown. Since this function is invoked outside the PSCI locks, -the actions performed in this hook must be local to the CPU or the platform -must ensure that races between multiple CPUs cannot occur. +platform specific actions before the CPU is powered down. Since this function is +invoked outside the PSCI locks, the actions performed in this hook must be local +to the CPU or the platform must ensure that races between multiple CPUs cannot +occur. The ``target_state`` has a similar meaning as described in the ``pwr_domain_off()`` operation and it encodes the platform coordinated target local power states for -the CPU power domain and its parent power domain levels. This function must -not return back to the caller (by calling wfi in an infinite loop to ensure -some CPUs power down mitigations work properly). +the CPU power domain and its parent power domain levels. -If this function is not implemented by the platform, PSCI generic -implementation invokes ``psci_power_down_wfi()`` for power down. +It is preferred that this function returns. The caller will invoke +``psci_power_down_wfi()`` to powerdown the CPU, mitigate any powerdown errata, +and handle any wakeups that may arise. Previously, this function did not return +and instead called ``wfi`` (in an infinite loop) directly. This is still +possible on platforms where this is guaranteed to be terminal, however, it is +strongly discouraged going forward. plat_psci_ops.pwr_domain_on_finish() .................................... @@ -2965,14 +2967,16 @@ plat_psci_ops.system_off() This function is called by PSCI implementation in response to a ``SYSTEM_OFF`` call. It performs the platform-specific system poweroff sequence after -notifying the Secure Payload Dispatcher. +notifying the Secure Payload Dispatcher. The caller will call ``wfi`` if this +function returns, similar to `plat_psci_ops.pwr_domain_pwr_down_wfi()`_. plat_psci_ops.system_reset() ............................ This function is called by PSCI implementation in response to a ``SYSTEM_RESET`` call. It performs the platform-specific system reset sequence after -notifying the Secure Payload Dispatcher. +notifying the Secure Payload Dispatcher. The caller will call ``wfi`` if this +function returns, similar to `plat_psci_ops.pwr_domain_pwr_down_wfi()`_. plat_psci_ops.validate_power_state() .................................... @@ -3060,7 +3064,8 @@ reset information. If the ``reset_type`` is not supported, the function must return ``PSCI_E_NOT_SUPPORTED``. For architectural resets, all failures must return ``PSCI_E_INVALID_PARAMETERS`` and vendor reset can return other PSCI error codes as defined -in `PSCI`_. On success this function will not return. +in `PSCI`_. If this function returns success, the caller will call +``wfi`` similar to `plat_psci_ops.pwr_domain_pwr_down_wfi()`_. plat_psci_ops.write_mem_protect() ................................. diff --git a/drivers/arm/css/scp/css_pm_scmi.c b/drivers/arm/css/scp/css_pm_scmi.c index fbcbdc709..84b8bd2be 100644 --- a/drivers/arm/css/scp/css_pm_scmi.c +++ b/drivers/arm/css/scp/css_pm_scmi.c @@ -339,7 +339,7 @@ void __dead2 css_scp_system_off(int state) } /* Powerdown of primary core */ - psci_pwrdown_cpu(PLAT_MAX_PWR_LVL); + psci_pwrdown_cpu_start(PLAT_MAX_PWR_LVL); wfi(); ERROR("CSS set power state: operation not handled.\n"); panic(); diff --git a/include/lib/psci/psci.h b/include/lib/psci/psci.h index 8ea4c277f..3a5bdbaba 100644 --- a/include/lib/psci/psci.h +++ b/include/lib/psci/psci.h @@ -331,10 +331,10 @@ typedef struct plat_psci_ops { const psci_power_state_t *target_state); void (*pwr_domain_suspend_finish)( const psci_power_state_t *target_state); - void __dead2 (*pwr_domain_pwr_down_wfi)( + void (*pwr_domain_pwr_down_wfi)( const psci_power_state_t *target_state); - void __dead2 (*system_off)(void); - void __dead2 (*system_reset)(void); + void (*system_off)(void); + void (*system_reset)(void); int (*validate_power_state)(unsigned int power_state, psci_power_state_t *req_state); int (*validate_ns_entrypoint)(uintptr_t ns_entrypoint); diff --git a/include/lib/psci/psci_lib.h b/include/lib/psci/psci_lib.h index 9950a6e66..8c9296b49 100644 --- a/include/lib/psci/psci_lib.h +++ b/include/lib/psci/psci_lib.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017-2023, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2017-2024, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -94,6 +94,9 @@ int psci_stop_other_cores(unsigned int this_cpu_idx, unsigned int wait_ms, bool psci_is_last_on_cpu_safe(unsigned int this_core); bool psci_are_all_cpus_on_safe(unsigned int this_core); void psci_pwrdown_cpu(unsigned int power_level); +void psci_pwrdown_cpu_start(unsigned int power_level); +void __dead2 psci_pwrdown_cpu_end_terminal(void); +void psci_pwrdown_cpu_end_wakeup(unsigned int power_level); void psci_do_manage_extensions(void); #endif /* __ASSEMBLER__ */ diff --git a/lib/cpus/aarch64/cortex_gelas.S b/lib/cpus/aarch64/cortex_gelas.S index 43608e4fb..df73a895e 100644 --- a/lib/cpus/aarch64/cortex_gelas.S +++ b/lib/cpus/aarch64/cortex_gelas.S @@ -21,6 +21,10 @@ #error "Gelas supports only AArch64. Compile with CTX_INCLUDE_AARCH32_REGS=0" #endif +#if FEAT_PABANDON == 0 +#error "Gelas must be compiled with FEAT_PABANDON enabled" +#endif + cpu_reset_func_start cortex_gelas /* ---------------------------------------------------- * Disable speculative loads diff --git a/lib/cpus/aarch64/travis.S b/lib/cpus/aarch64/travis.S index 695e7d86b..3edd298b5 100644 --- a/lib/cpus/aarch64/travis.S +++ b/lib/cpus/aarch64/travis.S @@ -21,6 +21,10 @@ #error "Travis supports only AArch64. Compile with CTX_INCLUDE_AARCH32_REGS=0" #endif +#if FEAT_PABANDON == 0 +#error "Travis must be compiled with FEAT_PABANDON enabled" +#endif + cpu_reset_func_start travis /* ---------------------------------------------------- * Disable speculative loads diff --git a/lib/psci/aarch64/psci_helpers.S b/lib/psci/aarch64/psci_helpers.S index 088ab43f4..b297f9b6a 100644 --- a/lib/psci/aarch64/psci_helpers.S +++ b/lib/psci/aarch64/psci_helpers.S @@ -118,19 +118,16 @@ func psci_do_pwrup_cache_maintenance endfunc psci_do_pwrup_cache_maintenance /* ----------------------------------------------------------------------- - * void psci_power_down_wfi(void); - * This function is called to indicate to the power controller that it - * is safe to power down this cpu. It should not exit the wfi and will - * be released from reset upon power up. + * void psci_power_down_wfi(void); This function is called to indicate to the + * power controller that it is safe to power down this cpu. It may exit if the + * request was denied and reset did not occur * ----------------------------------------------------------------------- */ func psci_power_down_wfi apply_erratum cortex_a510, ERRATUM(2684597), ERRATA_A510_2684597 dsb sy // ensure write buffer empty -1: wfi - b 1b /* * in case the WFI wasn't terminal, we have to undo errata mitigations. @@ -139,4 +136,6 @@ func psci_power_down_wfi apply_erratum cortex_a710, ERRATUM(2291219), ERRATA_A710_2291219 apply_erratum cortex_x3, ERRATUM(2313909), ERRATA_X3_2313909, NO_GET_CPU_REV apply_erratum neoverse_n2, ERRATUM(2326639), ERRATA_N2_2326639, NO_GET_CPU_REV + + ret endfunc psci_power_down_wfi diff --git a/lib/psci/psci_common.c b/lib/psci/psci_common.c index 93d71b814..4da7a9041 100644 --- a/lib/psci/psci_common.c +++ b/lib/psci/psci_common.c @@ -1019,8 +1019,12 @@ void psci_warmboot_entrypoint(void) */ if (psci_get_aff_info_state() == AFF_STATE_ON_PENDING) psci_cpu_on_finish(cpu_idx, &state_info); - else - psci_cpu_suspend_to_powerdown_finish(cpu_idx, &state_info); + else { + unsigned int max_off_lvl = psci_find_max_off_lvl(&state_info); + + assert(max_off_lvl != PSCI_INVALID_PWR_LVL); + psci_cpu_suspend_to_powerdown_finish(cpu_idx, max_off_lvl, &state_info); + } /* * Generic management: Now we just need to retrieve the @@ -1156,7 +1160,7 @@ int psci_secondaries_brought_up(void) * Initiate power down sequence, by calling power down operations registered for * this CPU. ******************************************************************************/ -void psci_pwrdown_cpu(unsigned int power_level) +void psci_pwrdown_cpu_start(unsigned int power_level) { #if ENABLE_RUNTIME_INSTRUMENTATION @@ -1196,6 +1200,60 @@ void psci_pwrdown_cpu(unsigned int power_level) #endif } +/******************************************************************************* + * Finish a terminal power down sequence, ending with a wfi. In case of wakeup + * will retry the sleep and panic if it persists. + ******************************************************************************/ +void __dead2 psci_pwrdown_cpu_end_terminal(void) +{ + /* + * Execute a wfi which, in most cases, will allow the power controller + * to physically power down this cpu. Under some circumstances that may + * be denied. Hopefully this is transient, retrying a few times should + * power down. + */ + for (int i = 0; i < 32; i++) + psci_power_down_wfi(); + + /* Wake up wasn't transient. System is probably in a bad state. */ + ERROR("Could not power off CPU.\n"); + panic(); +} + +/******************************************************************************* + * Finish a non-terminal power down sequence, ending with a wfi. In case of + * wakeup will unwind any CPU specific actions and return. + ******************************************************************************/ + +void psci_pwrdown_cpu_end_wakeup(unsigned int power_level) +{ + /* + * Usually, will be terminal. In some circumstances the powerdown will + * be denied and we'll need to unwind + */ + psci_power_down_wfi(); + + /* + * Waking up does not require hardware-assisted coherency, but that is + * the case for every core that can wake up. Untangling the cache + * coherency code from powerdown is a non-trivial effort which isn't + * needed for our purposes. + */ +#if !FEAT_PABANDON + ERROR("Systems without FEAT_PABANDON shouldn't wake up.\n"); + panic(); +#else /* FEAT_PABANDON */ + + /* + * Begin unwinding. Everything can be shared with CPU_ON and co later, + * except the CPU specific bit. Cores that have hardware-assisted + * coherency don't have much to do so just calling the hook again is + * the simplest way to achieve this + */ + prepare_cpu_pwr_dwn(power_level); +#endif /* FEAT_PABANDON */ +} + /******************************************************************************* * This function invokes the callback 'stop_func()' with the 'mpidr' of each * online PE. Caller can pass suitable method to stop a remote core. diff --git a/lib/psci/psci_off.c b/lib/psci/psci_off.c index d40ee3f6a..46b21140b 100644 --- a/lib/psci/psci_off.c +++ b/lib/psci/psci_off.c @@ -115,7 +115,7 @@ int psci_do_cpu_off(unsigned int end_pwrlvl) /* * Arch. management. Initiate power down sequence. */ - psci_pwrdown_cpu(psci_find_max_off_lvl(&state_info)); + psci_pwrdown_cpu_start(psci_find_max_off_lvl(&state_info)); /* * Plat. management: Perform platform specific actions to turn this @@ -153,7 +153,6 @@ exit: psci_inv_cpu_data(psci_svc_cpu_data.aff_info_state); #if ENABLE_RUNTIME_INSTRUMENTATION - /* * Update the timestamp with cache off. We assume this * timestamp can only be read from the current CPU and the @@ -164,17 +163,12 @@ exit: RT_INSTR_ENTER_HW_LOW_PWR, PMF_NO_CACHE_MAINT); #endif - if (psci_plat_pm_ops->pwr_domain_pwr_down_wfi != NULL) { - /* This function must not return */ + /* This function may not return */ psci_plat_pm_ops->pwr_domain_pwr_down_wfi(&state_info); - } else { - /* - * Enter a wfi loop which will allow the power - * controller to physically power down this cpu. - */ - psci_power_down_wfi(); } + + psci_pwrdown_cpu_end_terminal(); } return rc; diff --git a/lib/psci/psci_private.h b/lib/psci/psci_private.h index 6622755e1..49b19c991 100644 --- a/lib/psci/psci_private.h +++ b/lib/psci/psci_private.h @@ -349,7 +349,7 @@ int psci_cpu_suspend_start(unsigned int idx, psci_power_state_t *state_info, unsigned int is_power_down_state); -void psci_cpu_suspend_to_powerdown_finish(unsigned int cpu_idx, const psci_power_state_t *state_info); +void psci_cpu_suspend_to_powerdown_finish(unsigned int cpu_idx, unsigned int max_off_lvl, const psci_power_state_t *state_info); /* Private exported functions from psci_helpers.S */ void psci_do_pwrdown_cache_maintenance(unsigned int pwr_level); diff --git a/lib/psci/psci_suspend.c b/lib/psci/psci_suspend.c index 2aadbfd5c..aaf82a05c 100644 --- a/lib/psci/psci_suspend.c +++ b/lib/psci/psci_suspend.c @@ -25,8 +25,7 @@ * This function does generic and platform specific operations after a wake-up * from standby/retention states at multiple power levels. ******************************************************************************/ -static void psci_cpu_suspend_to_standby_finish(unsigned int cpu_idx, - unsigned int end_pwrlvl, +static void psci_cpu_suspend_to_standby_finish(unsigned int end_pwrlvl, psci_power_state_t *state_info) { /* @@ -44,11 +43,10 @@ static void psci_cpu_suspend_to_standby_finish(unsigned int cpu_idx, * operations. ******************************************************************************/ static void psci_suspend_to_pwrdown_start(unsigned int end_pwrlvl, + unsigned int max_off_lvl, const entry_point_info_t *ep, const psci_power_state_t *state_info) { - unsigned int max_off_lvl = psci_find_max_off_lvl(state_info); - PUBLISH_EVENT(psci_suspend_pwrdown_start); #if PSCI_OS_INIT_MODE @@ -94,10 +92,8 @@ static void psci_suspend_to_pwrdown_start(unsigned int end_pwrlvl, /* * Arch. management. Initiate power down sequence. - * TODO : Introduce a mechanism to query the cache level to flush - * and the cpu-ops power down to perform from the platform. */ - psci_pwrdown_cpu(max_off_lvl); + psci_pwrdown_cpu_start(max_off_lvl); } /******************************************************************************* @@ -127,6 +123,11 @@ int psci_cpu_suspend_start(unsigned int idx, int rc = PSCI_E_SUCCESS; bool skip_wfi = false; unsigned int parent_nodes[PLAT_MAX_PWR_LVL] = {0}; + unsigned int max_off_lvl = 0; +#if FEAT_PABANDON + cpu_context_t *ctx = cm_get_context(NON_SECURE); + cpu_context_t old_ctx; +#endif /* * This function must only be called on platforms where the @@ -196,8 +197,38 @@ int psci_cpu_suspend_start(unsigned int idx, psci_stats_update_pwr_down(idx, end_pwrlvl, state_info); #endif - if (is_power_down_state != 0U) - psci_suspend_to_pwrdown_start(end_pwrlvl, ep, state_info); + if (is_power_down_state != 0U) { + /* + * WHen CTX_INCLUDE_EL2_REGS is usnet, we're probably runnig + * with some SPD that assumes the core is going off so it + * doesn't bother saving NS's context. Do that here until we + * figure out a way to make this coherent. + */ +#if FEAT_PABANDON +#if !CTX_INCLUDE_EL2_REGS + cm_el1_sysregs_context_save(NON_SECURE); +#endif + /* + * when the core wakes it expects its context to already be in + * place so we must overwrite it before powerdown. But if + * powerdown never happens we want the old context. Save it in + * case we wake up. EL2/El1 will not be touched by PSCI so don't + * copy */ + memcpy(&ctx->gpregs_ctx, &old_ctx.gpregs_ctx, sizeof(gp_regs_t)); + memcpy(&ctx->el3state_ctx, &old_ctx.el3state_ctx, sizeof(el3_state_t)); +#if DYNAMIC_WORKAROUND_CVE_2018_3639 + memcpy(&ctx->cve_2018_3639_ctx, &old_ctx.cve_2018_3639_ctx, sizeof(cve_2018_3639_t)); +#endif +#if ERRATA_SPECULATIVE_AT + memcpy(&ctx->errata_speculative_at_ctx, &old_ctx.errata_speculative_at_ctx, sizeof(errata_speculative_at_t)); +#endif +#if CTX_INCLUDE_PAUTH_REGS + memcpy(&ctx->pauth_ctx, &old_ctx.pauth_ctx, sizeof(pauth_t)); +#endif +#endif + max_off_lvl = psci_find_max_off_lvl(state_info); + psci_suspend_to_pwrdown_start(end_pwrlvl, max_off_lvl, ep, state_info); + } /* * Plat. management: Allow the platform to perform the @@ -223,39 +254,33 @@ exit: return rc; } - if (is_power_down_state != 0U) { -#if ENABLE_RUNTIME_INSTRUMENTATION - - /* - * Update the timestamp with cache off. We assume this - * timestamp can only be read from the current CPU and the - * timestamp cache line will be flushed before return to - * normal world on wakeup. - */ - PMF_CAPTURE_TIMESTAMP(rt_instr_svc, - RT_INSTR_ENTER_HW_LOW_PWR, - PMF_NO_CACHE_MAINT); -#endif - - /* The function calls below must not return */ - if (psci_plat_pm_ops->pwr_domain_pwr_down_wfi != NULL) - psci_plat_pm_ops->pwr_domain_pwr_down_wfi(state_info); - else - psci_power_down_wfi(); - } - #if ENABLE_RUNTIME_INSTRUMENTATION + /* + * Update the timestamp with cache off. We assume this + * timestamp can only be read from the current CPU and the + * timestamp cache line will be flushed before return to + * normal world on wakeup. + */ PMF_CAPTURE_TIMESTAMP(rt_instr_svc, RT_INSTR_ENTER_HW_LOW_PWR, PMF_NO_CACHE_MAINT); #endif - /* - * We will reach here if only retention/standby states have been - * requested at multiple power levels. This means that the cpu - * context will be preserved. - */ - wfi(); + if (is_power_down_state != 0U) { + if (psci_plat_pm_ops->pwr_domain_pwr_down_wfi != NULL) { + /* This function may not return */ + psci_plat_pm_ops->pwr_domain_pwr_down_wfi(state_info); + } + + psci_pwrdown_cpu_end_wakeup(max_off_lvl); + } else { + /* + * We will reach here if only retention/standby states have been + * requested at multiple power levels. This means that the cpu + * context will be preserved. + */ + wfi(); + } #if ENABLE_RUNTIME_INSTRUMENTATION PMF_CAPTURE_TIMESTAMP(rt_instr_svc, @@ -277,10 +302,32 @@ exit: #endif /* - * After we wake up from context retaining suspend, call the - * context retaining suspend finisher. + * Waking up means we've retained all context. Call the finishers to put + * the system back to a usable state. */ - psci_cpu_suspend_to_standby_finish(idx, end_pwrlvl, state_info); + if (is_power_down_state != 0U) { +#if FEAT_PABANDON + psci_cpu_suspend_to_powerdown_finish(idx, max_off_lvl, state_info); + + /* we overwrote context ourselves, put it back */ + memcpy(&ctx->gpregs_ctx, &old_ctx.gpregs_ctx, sizeof(gp_regs_t)); + memcpy(&ctx->el3state_ctx, &old_ctx.el3state_ctx, sizeof(el3_state_t)); +#if DYNAMIC_WORKAROUND_CVE_2018_3639 + memcpy(&ctx->cve_2018_3639_ctx, &old_ctx.cve_2018_3639_ctx, sizeof(cve_2018_3639_t)); +#endif +#if ERRATA_SPECULATIVE_AT + memcpy(&ctx->errata_speculative_at_ctx, &old_ctx.errata_speculative_at_ctx, sizeof(errata_speculative_at_t)); +#endif +#if CTX_INCLUDE_PAUTH_REGS + memcpy(&ctx->pauth_ctx, &old_ctx.pauth_ctx, sizeof(pauth_t)); +#endif +#if !CTX_INCLUDE_EL2_REGS + cm_el1_sysregs_context_restore(NON_SECURE); +#endif +#endif + } else { + psci_cpu_suspend_to_standby_finish(end_pwrlvl, state_info); + } /* * Set the requested and target state of this CPU and all the higher @@ -298,10 +345,9 @@ exit: * are called by the common finisher routine in psci_common.c. The `state_info` * is the psci_power_state from which this CPU has woken up from. ******************************************************************************/ -void psci_cpu_suspend_to_powerdown_finish(unsigned int cpu_idx, const psci_power_state_t *state_info) +void psci_cpu_suspend_to_powerdown_finish(unsigned int cpu_idx, unsigned int max_off_lvl, const psci_power_state_t *state_info) { unsigned int counter_freq; - unsigned int max_off_lvl; /* Ensure we have been woken up from a suspended state */ assert((psci_get_aff_info_state() == AFF_STATE_ON) && @@ -338,8 +384,6 @@ void psci_cpu_suspend_to_powerdown_finish(unsigned int cpu_idx, const psci_power * error, it's expected to assert within */ if ((psci_spd_pm != NULL) && (psci_spd_pm->svc_suspend_finish != NULL)) { - max_off_lvl = psci_find_max_off_lvl(state_info); - assert(max_off_lvl != PSCI_INVALID_PWR_LVL); psci_spd_pm->svc_suspend_finish(max_off_lvl); } diff --git a/lib/psci/psci_system_off.c b/lib/psci/psci_system_off.c index 002392cad..b9418a393 100644 --- a/lib/psci/psci_system_off.c +++ b/lib/psci/psci_system_off.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014-2020, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2014-2024, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -30,7 +30,7 @@ void __dead2 psci_system_off(void) /* Call the platform specific hook */ psci_plat_pm_ops->system_off(); - /* This function does not return. We should never get here */ + psci_pwrdown_cpu_end_terminal(); } void __dead2 psci_system_reset(void) @@ -49,7 +49,7 @@ void __dead2 psci_system_reset(void) /* Call the platform specific hook */ psci_plat_pm_ops->system_reset(); - /* This function does not return. We should never get here */ + psci_pwrdown_cpu_end_terminal(); } u_register_t psci_system_reset2(uint32_t reset_type, u_register_t cookie) @@ -79,7 +79,10 @@ u_register_t psci_system_reset2(uint32_t reset_type, u_register_t cookie) } console_flush(); - return (u_register_t) - psci_plat_pm_ops->system_reset2((int) is_vendor, reset_type, - cookie); + u_register_t ret = + (u_register_t) psci_plat_pm_ops->system_reset2((int) is_vendor, reset_type, cookie); + if (ret != PSCI_E_SUCCESS) + return ret; + + psci_pwrdown_cpu_end_terminal(); } diff --git a/make_helpers/defaults.mk b/make_helpers/defaults.mk index 4985c0c5a..176b2ca95 100644 --- a/make_helpers/defaults.mk +++ b/make_helpers/defaults.mk @@ -1,5 +1,5 @@ # -# Copyright (c) 2016-2024, Arm Limited. All rights reserved. +# Copyright (c) 2016-2025, Arm Limited. All rights reserved. # # SPDX-License-Identifier: BSD-3-Clause # @@ -85,6 +85,9 @@ DYN_DISABLE_AUTH := 0 # Enable the Maximum Power Mitigation Mechanism on supporting cores. ENABLE_MPMM := 0 +# Enable support for powerdown abandons +FEAT_PABANDON := 0 + # Enable MPMM configuration via FCONF. ENABLE_MPMM_FCONF := 0 diff --git a/plat/arm/board/fvp/platform.mk b/plat/arm/board/fvp/platform.mk index 87938400a..70c713a3c 100644 --- a/plat/arm/board/fvp/platform.mk +++ b/plat/arm/board/fvp/platform.mk @@ -221,6 +221,7 @@ endif #Build AArch64-only CPUs with no FVP model yet. ifeq (${BUILD_CPUS_WITH_NO_FVP_MODEL},1) + FEAT_PABANDON := 1 FVP_CPU_LIBS += lib/cpus/aarch64/neoverse_n3.S \ lib/cpus/aarch64/cortex_gelas.S \ lib/cpus/aarch64/nevis.S \ diff --git a/plat/arm/board/tc/platform.mk b/plat/arm/board/tc/platform.mk index b2b32531c..d6f007967 100644 --- a/plat/arm/board/tc/platform.mk +++ b/plat/arm/board/tc/platform.mk @@ -1,4 +1,4 @@ -# Copyright (c) 2021-2024, Arm Limited. All rights reserved. +# Copyright (c) 2021-2025, Arm Limited. All rights reserved. # # SPDX-License-Identifier: BSD-3-Clause # @@ -149,6 +149,7 @@ endif # CPU libraries for TARGET_PLATFORM=4 ifeq (${TARGET_PLATFORM}, 4) +FEAT_PABANDON := 1 TC_CPU_SOURCES += lib/cpus/aarch64/cortex_gelas.S \ lib/cpus/aarch64/nevis.S \ lib/cpus/aarch64/travis.S diff --git a/plat/arm/css/common/css_pm.c b/plat/arm/css/common/css_pm.c index bfb690693..8ddd7a491 100644 --- a/plat/arm/css/common/css_pm.c +++ b/plat/arm/css/common/css_pm.c @@ -364,7 +364,7 @@ int css_reboot_interrupt_handler(uint32_t intr_raw, uint32_t flags, plat_arm_gic_cpuif_disable(); plat_arm_gic_redistif_off(); - psci_pwrdown_cpu(PLAT_MAX_PWR_LVL); + psci_pwrdown_cpu_start(PLAT_MAX_PWR_LVL); dmbsy(); From 45c7328c0b94d043745b4a44c2e14e1a77f5c347 Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Fri, 20 Sep 2024 13:37:51 +0100 Subject: [PATCH 5/9] fix(cpus): avoid SME related loss of context on powerdown Travis' and Gelas' TRMs tell us to disable SME (set PSTATE.{ZA, SM} to 0) when we're attempting to power down. What they don't tell us is that if this isn't done, the powerdown request will be rejected. On the CPU_OFF path that's not a problem - we can force SVCR to 0 and be certain the core will power off. On the suspend to powerdown path, however, we cannot do this. The TRM also tells us that the sequence could also be aborted on eg. GIC interrupts. If this were to happen when we have overwritten SVCR to 0, upon a return to the caller they would experience a loss of context. We know that at least Linux may call into PSCI with SVCR != 0. One option is to save the entire SME context which would be quite expensive just to work around. Another option is to downgrade the request to a normal suspend when SME was left on. This option is better as this is expected to happen rarely enough to ignore the wasted power and we don't want to burden the generic (correct) path with needless context management. Signed-off-by: Boyan Karatotev Change-Id: I698fa8490ebf51461f6aa8bba84f9827c5c46ad4 --- Makefile | 2 ++ docs/getting_started/build-options.rst | 6 ++++++ include/arch/aarch64/arch.h | 1 + include/arch/aarch64/arch_helpers.h | 1 + include/lib/cpus/aarch64/cortex_alto.h | 6 ------ include/lib/cpus/aarch64/cortex_gelas.h | 8 +------- include/lib/cpus/aarch64/travis.h | 8 +------- lib/cpus/aarch64/cortex_alto.S | 18 ++++-------------- lib/cpus/aarch64/cortex_gelas.S | 18 ++++-------------- lib/cpus/aarch64/travis.S | 18 ++++-------------- lib/psci/psci_common.c | 12 ++++++++++++ lib/psci/psci_main.c | 16 +++++++++++++++- lib/psci/psci_off.c | 2 +- make_helpers/defaults.mk | 3 +++ plat/arm/board/fvp/platform.mk | 2 ++ plat/arm/board/tc/platform.mk | 2 ++ 16 files changed, 59 insertions(+), 64 deletions(-) diff --git a/Makefile b/Makefile index cd763db3f..a677ea089 100644 --- a/Makefile +++ b/Makefile @@ -1241,6 +1241,7 @@ $(eval $(call assert_booleans,\ ENCRYPT_BL31 \ ENCRYPT_BL32 \ ERRATA_SPECULATIVE_AT \ + ERRATA_SME_POWER_DOWN \ RAS_TRAP_NS_ERR_REC_ACCESS \ COT_DESC_IN_DTB \ USE_SP804_TIMER \ @@ -1430,6 +1431,7 @@ $(eval $(call add_defines,\ BL2_INV_DCACHE \ USE_SPINLOCK_CAS \ ERRATA_SPECULATIVE_AT \ + ERRATA_SME_POWER_DOWN \ RAS_TRAP_NS_ERR_REC_ACCESS \ COT_DESC_IN_DTB \ USE_SP804_TIMER \ diff --git a/docs/getting_started/build-options.rst b/docs/getting_started/build-options.rst index e6d8a1d15..7c4a112d7 100644 --- a/docs/getting_started/build-options.rst +++ b/docs/getting_started/build-options.rst @@ -1207,6 +1207,12 @@ Common build options implement this workaround due to the behaviour of the errata mentioned in new SDEN document which will get published soon. +- ``ERRATA_SME_POWER_DOWN``: Boolean option to disable SME (PSTATE.{ZA,SM}=0) + before power down and downgrade a suspend to power down request to a normal + suspend request. This is necessary when software running at lower ELs requests + power down without first clearing these bits. On affected cores, the CME + connected to it will reject its power down request. The default value is 0. + - ``RAS_TRAP_NS_ERR_REC_ACCESS``: This flag enables/disables the SCR_EL3.TERR bit, to trap access to the RAS ERR and RAS ERX registers from lower ELs. This flag is disabled by default. diff --git a/include/arch/aarch64/arch.h b/include/arch/aarch64/arch.h index 4d26153dd..b911b0bb1 100644 --- a/include/arch/aarch64/arch.h +++ b/include/arch/aarch64/arch.h @@ -1139,6 +1139,7 @@ ******************************************************************************/ #define ID_AA64SMFR0_EL1 S3_0_C0_C4_5 #define SMCR_EL3 S3_6_C1_C2_6 +#define SVCR S3_3_C4_C2_2 /* ID_AA64SMFR0_EL1 definitions */ #define ID_AA64SMFR0_EL1_SME_FA64_SHIFT U(63) diff --git a/include/arch/aarch64/arch_helpers.h b/include/arch/aarch64/arch_helpers.h index 8b92f1919..4b0833740 100644 --- a/include/arch/aarch64/arch_helpers.h +++ b/include/arch/aarch64/arch_helpers.h @@ -572,6 +572,7 @@ DEFINE_RENAME_SYSREG_WRITE_FUNC(zcr_el2, ZCR_EL2) DEFINE_RENAME_IDREG_READ_FUNC(id_aa64smfr0_el1, ID_AA64SMFR0_EL1) DEFINE_RENAME_SYSREG_RW_FUNCS(smcr_el3, SMCR_EL3) +DEFINE_RENAME_SYSREG_RW_FUNCS(svcr, SVCR) DEFINE_RENAME_SYSREG_READ_FUNC(erridr_el1, ERRIDR_EL1) DEFINE_RENAME_SYSREG_WRITE_FUNC(errselr_el1, ERRSELR_EL1) diff --git a/include/lib/cpus/aarch64/cortex_alto.h b/include/lib/cpus/aarch64/cortex_alto.h index 1c8786a7b..9e2929f82 100644 --- a/include/lib/cpus/aarch64/cortex_alto.h +++ b/include/lib/cpus/aarch64/cortex_alto.h @@ -20,10 +20,4 @@ #define CORTEX_ALTO_IMP_CPUPWRCTLR_EL1 S3_0_C15_C2_7 #define CORTEX_ALTO_IMP_CPUPWRCTLR_EL1_CORE_PWRDN_EN_BIT U(1) -/******************************************************************************* - * SME Control registers - ******************************************************************************/ -#define CORTEX_ALTO_SVCRSM S0_3_C4_C2_3 -#define CORTEX_ALTO_SVCRZA S0_3_C4_C4_3 - #endif /* CORTEX_ALTO_H */ diff --git a/include/lib/cpus/aarch64/cortex_gelas.h b/include/lib/cpus/aarch64/cortex_gelas.h index 90bb78fee..a19572d02 100644 --- a/include/lib/cpus/aarch64/cortex_gelas.h +++ b/include/lib/cpus/aarch64/cortex_gelas.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Arm Limited. All rights reserved. + * Copyright (c) 2023-2024, Arm Limited. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -22,10 +22,4 @@ #define CORTEX_GELAS_CPUPWRCTLR_EL1 S3_0_C15_C2_7 #define CORTEX_GELAS_CPUPWRCTLR_EL1_CORE_PWRDN_BIT U(1) -/******************************************************************************* - * SME Control registers - ******************************************************************************/ -#define CORTEX_GELAS_SVCRSM S0_3_C4_C2_3 -#define CORTEX_GELAS_SVCRZA S0_3_C4_C4_3 - #endif /* CORTEX_GELAS_H */ diff --git a/include/lib/cpus/aarch64/travis.h b/include/lib/cpus/aarch64/travis.h index a8a255673..b622ea0d4 100644 --- a/include/lib/cpus/aarch64/travis.h +++ b/include/lib/cpus/aarch64/travis.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Arm Limited. All rights reserved. + * Copyright (c) 2023-2024, Arm Limited. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -20,10 +20,4 @@ #define TRAVIS_IMP_CPUPWRCTLR_EL1 S3_0_C15_C2_7 #define TRAVIS_IMP_CPUPWRCTLR_EL1_CORE_PWRDN_EN_BIT U(1) -/******************************************************************************* - * SME Control registers - ******************************************************************************/ -#define TRAVIS_SVCRSM S0_3_C4_C2_3 -#define TRAVIS_SVCRZA S0_3_C4_C4_3 - #endif /* TRAVIS_H */ diff --git a/lib/cpus/aarch64/cortex_alto.S b/lib/cpus/aarch64/cortex_alto.S index c0815f955..1422563ae 100644 --- a/lib/cpus/aarch64/cortex_alto.S +++ b/lib/cpus/aarch64/cortex_alto.S @@ -21,26 +21,16 @@ #error "Alto supports only AArch64. Compile with CTX_INCLUDE_AARCH32_REGS=0" #endif +#if ERRATA_SME_POWER_DOWN == 0 +#error "Travis needs ERRATA_SME_POWER_DOWN=1 to powerdown correctly" +#endif + cpu_reset_func_start cortex_alto /* Disable speculative loads */ msr SSBS, xzr cpu_reset_func_end cortex_alto func cortex_alto_core_pwr_dwn -#if ENABLE_SME_FOR_NS - /* --------------------------------------------------- - * Disable SME if enabled and supported - * --------------------------------------------------- - */ - mrs x0, ID_AA64PFR1_EL1 - ubfx x0, x0, #ID_AA64PFR1_EL1_SME_SHIFT, \ - #ID_AA64PFR1_EL1_SME_WIDTH - cmp x0, #SME_NOT_IMPLEMENTED - b.eq 1f - msr CORTEX_ALTO_SVCRSM, xzr - msr CORTEX_ALTO_SVCRZA, xzr -1: -#endif /* --------------------------------------------------- * Enable CPU power down bit in power control register * --------------------------------------------------- diff --git a/lib/cpus/aarch64/cortex_gelas.S b/lib/cpus/aarch64/cortex_gelas.S index df73a895e..e95820547 100644 --- a/lib/cpus/aarch64/cortex_gelas.S +++ b/lib/cpus/aarch64/cortex_gelas.S @@ -25,6 +25,10 @@ #error "Gelas must be compiled with FEAT_PABANDON enabled" #endif +#if ERRATA_SME_POWER_DOWN == 0 +#error "Gelas needs ERRATA_SME_POWER_DOWN=1 to powerdown correctly" +#endif + cpu_reset_func_start cortex_gelas /* ---------------------------------------------------- * Disable speculative loads @@ -38,20 +42,6 @@ cpu_reset_func_end cortex_gelas * ---------------------------------------------------- */ func cortex_gelas_core_pwr_dwn -#if ENABLE_SME_FOR_NS - /* --------------------------------------------------- - * Disable SME if enabled and supported - * --------------------------------------------------- - */ - mrs x0, ID_AA64PFR1_EL1 - ubfx x0, x0, #ID_AA64PFR1_EL1_SME_SHIFT, \ - #ID_AA64PFR1_EL1_SME_WIDTH - cmp x0, #SME_NOT_IMPLEMENTED - b.eq 1f - msr CORTEX_GELAS_SVCRSM, xzr - msr CORTEX_GELAS_SVCRZA, xzr -1: -#endif /* --------------------------------------------------- * Flip CPU power down bit in power control register. * It will be set on powerdown and cleared on wakeup diff --git a/lib/cpus/aarch64/travis.S b/lib/cpus/aarch64/travis.S index 3edd298b5..246159a10 100644 --- a/lib/cpus/aarch64/travis.S +++ b/lib/cpus/aarch64/travis.S @@ -25,6 +25,10 @@ #error "Travis must be compiled with FEAT_PABANDON enabled" #endif +#if ERRATA_SME_POWER_DOWN == 0 +#error "Travis needs ERRATA_SME_POWER_DOWN=1 to powerdown correctly" +#endif + cpu_reset_func_start travis /* ---------------------------------------------------- * Disable speculative loads @@ -34,20 +38,6 @@ cpu_reset_func_start travis cpu_reset_func_end travis func travis_core_pwr_dwn -#if ENABLE_SME_FOR_NS - /* --------------------------------------------------- - * Disable SME if enabled and supported - * --------------------------------------------------- - */ - mrs x0, ID_AA64PFR1_EL1 - ubfx x0, x0, #ID_AA64PFR1_EL1_SME_SHIFT, \ - #ID_AA64PFR1_EL1_SME_WIDTH - cmp x0, #SME_NOT_IMPLEMENTED - b.eq 1f - msr TRAVIS_SVCRSM, xzr - msr TRAVIS_SVCRZA, xzr -1: -#endif /* --------------------------------------------------- * Flip CPU power down bit in power control register. * It will be set on powerdown and cleared on wakeup diff --git a/lib/psci/psci_common.c b/lib/psci/psci_common.c index 4da7a9041..4bb23af32 100644 --- a/lib/psci/psci_common.c +++ b/lib/psci/psci_common.c @@ -1206,6 +1206,18 @@ void psci_pwrdown_cpu_start(unsigned int power_level) ******************************************************************************/ void __dead2 psci_pwrdown_cpu_end_terminal(void) { +#if ERRATA_SME_POWER_DOWN + /* + * force SME off to not get power down rejected. Getting here is + * terminal so we don't care if we lose context because of another + * wakeup + */ + if (is_feat_sme_supported()) { + write_svcr(0); + isb(); + } +#endif /* ERRATA_SME_POWER_DOWN */ + /* * Execute a wfi which, in most cases, will allow the power controller * to physically power down this cpu. Under some circumstances that may diff --git a/lib/psci/psci_main.c b/lib/psci/psci_main.c index 7ac0e0260..45be63a24 100644 --- a/lib/psci/psci_main.c +++ b/lib/psci/psci_main.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2022, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2013-2024, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -64,6 +65,19 @@ int psci_cpu_suspend(unsigned int power_state, plat_local_state_t prev[PLAT_MAX_PWR_LVL]; #endif +#if ERRATA_SME_POWER_DOWN + /* + * If SME isn't off, attempting a real power down will only end up being + * rejected. If we got called with SME on, fall back to a normal + * suspend. We can't force SME off as in the event the power down is + * rejected for another reason (eg GIC) we'd lose the SME context. + */ + if (is_feat_sme_supported() && read_svcr() != 0) { + power_state &= ~(PSTATE_TYPE_MASK << PSTATE_TYPE_SHIFT); + power_state &= ~(PSTATE_PWR_LVL_MASK << PSTATE_PWR_LVL_SHIFT); + } +#endif /* ERRATA_SME_POWER_DOWN */ + /* Validate the power_state parameter */ rc = psci_validate_power_state(power_state, &state_info); if (rc != PSCI_E_SUCCESS) { diff --git a/lib/psci/psci_off.c b/lib/psci/psci_off.c index 46b21140b..dbc646c96 100644 --- a/lib/psci/psci_off.c +++ b/lib/psci/psci_off.c @@ -93,7 +93,7 @@ int psci_do_cpu_off(unsigned int end_pwrlvl) */ if ((psci_spd_pm != NULL) && (psci_spd_pm->svc_off != NULL)) { rc = psci_spd_pm->svc_off(0); - if (rc != 0) + if (rc != PSCI_E_SUCCESS) goto exit; } diff --git a/make_helpers/defaults.mk b/make_helpers/defaults.mk index 176b2ca95..da58f0a44 100644 --- a/make_helpers/defaults.mk +++ b/make_helpers/defaults.mk @@ -347,6 +347,9 @@ SUPPORT_STACK_MEMTAG := no # Select workaround for AT speculative behaviour. ERRATA_SPECULATIVE_AT := 0 +# select workaround for SME aborting powerdown +ERRATA_SME_POWER_DOWN := 0 + # Trap RAS error record access from Non secure RAS_TRAP_NS_ERR_REC_ACCESS := 0 diff --git a/plat/arm/board/fvp/platform.mk b/plat/arm/board/fvp/platform.mk index 70c713a3c..95cf7eb9e 100644 --- a/plat/arm/board/fvp/platform.mk +++ b/plat/arm/board/fvp/platform.mk @@ -221,7 +221,9 @@ endif #Build AArch64-only CPUs with no FVP model yet. ifeq (${BUILD_CPUS_WITH_NO_FVP_MODEL},1) + # travis/gelas need these FEAT_PABANDON := 1 + ERRATA_SME_POWER_DOWN := 1 FVP_CPU_LIBS += lib/cpus/aarch64/neoverse_n3.S \ lib/cpus/aarch64/cortex_gelas.S \ lib/cpus/aarch64/nevis.S \ diff --git a/plat/arm/board/tc/platform.mk b/plat/arm/board/tc/platform.mk index d6f007967..601df43b2 100644 --- a/plat/arm/board/tc/platform.mk +++ b/plat/arm/board/tc/platform.mk @@ -150,6 +150,8 @@ endif # CPU libraries for TARGET_PLATFORM=4 ifeq (${TARGET_PLATFORM}, 4) FEAT_PABANDON := 1 +# prevent CME related wakups +ERRATA_SME_POWER_DOWN := 1 TC_CPU_SOURCES += lib/cpus/aarch64/cortex_gelas.S \ lib/cpus/aarch64/nevis.S \ lib/cpus/aarch64/travis.S From da305ec75dedca5e8e939790ab02fe7c0ba999d5 Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Thu, 26 Sep 2024 16:04:16 +0100 Subject: [PATCH 6/9] feat(arm): convert arm platforms to expect a wakeup Newer cores in upcoming platforms may refuse to power down. The PSCI library is already prepared for this so convert platform code to also allow this. This is simple - drop the `wfi` + panic and let common code deal with the fallout. The end result will be the same (sans the message) except the platform will have fewer responsibilities. The only exception is for cores being signalled to power off gracefully ahead of system reset. That path must also be terminal so replace the end with the same psci_pwrdown_cpu_end() to behave the same as the generic implementation. It will handle wakeups and panic, hoping that the system gets reset from under it. The dmb is upgraded to a dsb so no functional change. Change-Id: I381f96bec8532bda6ccdac65de57971aac42e7e8 Signed-off-by: Boyan Karatotev --- drivers/arm/css/scp/css_pm_scmi.c | 17 +++++------------ drivers/arm/css/scp/css_pm_scpi.c | 10 ++-------- include/drivers/arm/css/css_scp.h | 6 +++--- include/plat/arm/css/common/css_pm.h | 6 +++--- .../board/corstone1000/common/corstone1000_pm.c | 5 +---- plat/arm/board/fvp/fvp_pm.c | 10 ++-------- plat/arm/css/common/css_pm.c | 8 +++----- 7 files changed, 19 insertions(+), 43 deletions(-) diff --git a/drivers/arm/css/scp/css_pm_scmi.c b/drivers/arm/css/scp/css_pm_scmi.c index 84b8bd2be..b310ff42f 100644 --- a/drivers/arm/css/scp/css_pm_scmi.c +++ b/drivers/arm/css/scp/css_pm_scmi.c @@ -298,7 +298,7 @@ static void css_raise_pwr_down_interrupt(u_register_t mpidr) #endif } -void __dead2 css_scp_system_off(int state) +void css_scp_system_off(int state) { int ret; @@ -340,15 +340,12 @@ void __dead2 css_scp_system_off(int state) /* Powerdown of primary core */ psci_pwrdown_cpu_start(PLAT_MAX_PWR_LVL); - wfi(); - ERROR("CSS set power state: operation not handled.\n"); - panic(); } /* * Helper function to shutdown the system via SCMI. */ -void __dead2 css_scp_sys_shutdown(void) +void css_scp_sys_shutdown(void) { css_scp_system_off(SCMI_SYS_PWR_SHUTDOWN); } @@ -356,7 +353,7 @@ void __dead2 css_scp_sys_shutdown(void) /* * Helper function to reset the system via SCMI. */ -void __dead2 css_scp_sys_reboot(void) +void css_scp_sys_reboot(void) { css_scp_system_off(SCMI_SYS_PWR_COLD_RESET); } @@ -472,12 +469,8 @@ int css_system_reset2(int is_vendor, int reset_type, u_register_t cookie) return PSCI_E_INVALID_PARAMS; css_scp_system_off(SCMI_SYS_PWR_WARM_RESET); - /* - * css_scp_system_off cannot return (it is a __dead function), - * but css_system_reset2 has to return some value, even in - * this case. - */ - return 0; + /* return SUCCESS to finish the powerdown */ + return PSCI_E_SUCCESS; } #if PROGRAMMABLE_RESET_ADDRESS diff --git a/drivers/arm/css/scp/css_pm_scpi.c b/drivers/arm/css/scp/css_pm_scpi.c index b4019ce03..02be07019 100644 --- a/drivers/arm/css/scp/css_pm_scpi.c +++ b/drivers/arm/css/scp/css_pm_scpi.c @@ -117,7 +117,7 @@ int css_scp_get_power_state(u_register_t mpidr, unsigned int power_level) /* * Helper function to shutdown the system via SCPI. */ -void __dead2 css_scp_sys_shutdown(void) +void css_scp_sys_shutdown(void) { uint32_t response; @@ -134,15 +134,12 @@ void __dead2 css_scp_sys_shutdown(void) ERROR("CSS System Off: SCP error %u.\n", response); panic(); } - wfi(); - ERROR("CSS System Off: operation not handled.\n"); - panic(); } /* * Helper function to reset the system via SCPI. */ -void __dead2 css_scp_sys_reboot(void) +void css_scp_sys_reboot(void) { uint32_t response; @@ -159,7 +156,4 @@ void __dead2 css_scp_sys_reboot(void) ERROR("CSS System Reset: SCP error %u.\n", response); panic(); } - wfi(); - ERROR("CSS System Reset: operation not handled.\n"); - panic(); } diff --git a/include/drivers/arm/css/css_scp.h b/include/drivers/arm/css/css_scp.h index 2b506eaaf..539554625 100644 --- a/include/drivers/arm/css/css_scp.h +++ b/include/drivers/arm/css/css_scp.h @@ -22,9 +22,9 @@ void css_scp_suspend(const struct psci_power_state *target_state); void css_scp_off(const struct psci_power_state *target_state); void css_scp_on(u_register_t mpidr); int css_scp_get_power_state(u_register_t mpidr, unsigned int power_level); -void __dead2 css_scp_sys_shutdown(void); -void __dead2 css_scp_sys_reboot(void); -void __dead2 css_scp_system_off(int state); +void css_scp_sys_shutdown(void); +void css_scp_sys_reboot(void); +void css_scp_system_off(int state); /* API for SCP Boot Image transfer. Return 0 on success, -1 on error */ int css_scp_boot_image_xfer(void *image, unsigned int image_size); diff --git a/include/plat/arm/css/common/css_pm.h b/include/plat/arm/css/common/css_pm.h index 84e6b38de..d5a3c427d 100644 --- a/include/plat/arm/css/common/css_pm.h +++ b/include/plat/arm/css/common/css_pm.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015-2022, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2015-2024, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -35,8 +35,8 @@ void css_pwr_domain_off(const psci_power_state_t *target_state); void css_pwr_domain_suspend(const psci_power_state_t *target_state); void css_pwr_domain_suspend_finish( const psci_power_state_t *target_state); -void __dead2 css_system_off(void); -void __dead2 css_system_reset(void); +void css_system_off(void); +void css_system_reset(void); void css_cpu_standby(plat_local_state_t cpu_state); void css_get_sys_suspend_power_state(psci_power_state_t *req_state); int css_node_hw_state(u_register_t mpidr, unsigned int power_level); diff --git a/plat/arm/board/corstone1000/common/corstone1000_pm.c b/plat/arm/board/corstone1000/common/corstone1000_pm.c index bd3faec8e..526418700 100644 --- a/plat/arm/board/corstone1000/common/corstone1000_pm.c +++ b/plat/arm/board/corstone1000/common/corstone1000_pm.c @@ -14,7 +14,7 @@ * platform layer will take care of registering the handlers with PSCI. ******************************************************************************/ -static void __dead2 corstone1000_system_reset(void) +static void corstone1000_system_reset(void) { uint32_t volatile * const watchdog_ctrl_reg = (uint32_t *) SECURE_WATCHDOG_ADDR_CTRL_REG; @@ -31,9 +31,6 @@ static void __dead2 corstone1000_system_reset(void) *(watchdog_val_reg) = SECURE_WATCHDOG_COUNTDOWN_VAL; *watchdog_ctrl_reg = SECURE_WATCHDOG_MASK_ENABLE; - while (1) { - wfi(); - } } #if defined(CORSTONE1000_FVP_MULTICORE) diff --git a/plat/arm/board/fvp/fvp_pm.c b/plat/arm/board/fvp/fvp_pm.c index 80dfd2a90..2a0bb935f 100644 --- a/plat/arm/board/fvp/fvp_pm.c +++ b/plat/arm/board/fvp/fvp_pm.c @@ -295,28 +295,22 @@ static void fvp_pwr_domain_suspend_finish(const psci_power_state_t *target_state /******************************************************************************* * FVP handlers to shutdown/reboot the system ******************************************************************************/ -static void __dead2 fvp_system_off(void) +static void fvp_system_off(void) { /* Write the System Configuration Control Register */ mmio_write_32(V2M_SYSREGS_BASE + V2M_SYS_CFGCTRL, V2M_CFGCTRL_START | V2M_CFGCTRL_RW | V2M_CFGCTRL_FUNC(V2M_FUNC_SHUTDOWN)); - wfi(); - ERROR("FVP System Off: operation not handled.\n"); - panic(); } -static void __dead2 fvp_system_reset(void) +static void fvp_system_reset(void) { /* Write the System Configuration Control Register */ mmio_write_32(V2M_SYSREGS_BASE + V2M_SYS_CFGCTRL, V2M_CFGCTRL_START | V2M_CFGCTRL_RW | V2M_CFGCTRL_FUNC(V2M_FUNC_REBOOT)); - wfi(); - ERROR("FVP System Reset: operation not handled.\n"); - panic(); } static int fvp_node_hw_state(u_register_t target_cpu, diff --git a/plat/arm/css/common/css_pm.c b/plat/arm/css/common/css_pm.c index 8ddd7a491..f8bc54249 100644 --- a/plat/arm/css/common/css_pm.c +++ b/plat/arm/css/common/css_pm.c @@ -217,12 +217,12 @@ void css_pwr_domain_suspend_finish( /******************************************************************************* * Handlers to shutdown/reboot the system ******************************************************************************/ -void __dead2 css_system_off(void) +void css_system_off(void) { css_scp_sys_shutdown(); } -void __dead2 css_system_reset(void) +void css_system_reset(void) { css_scp_sys_reboot(); } @@ -366,9 +366,7 @@ int css_reboot_interrupt_handler(uint32_t intr_raw, uint32_t flags, psci_pwrdown_cpu_start(PLAT_MAX_PWR_LVL); - dmbsy(); - - wfi(); + psci_pwrdown_cpu_end_terminal(); return 0; } From dc0bf486694cd1f5045ae4090d27e27d5365eedf Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Tue, 8 Oct 2024 15:52:04 +0100 Subject: [PATCH 7/9] chore(psci): drop skip_wfi variable There is now a convent place at the end of the function to jump to when we shouldn't power down. This eliminates the need for skip_wfi. Change-Id: I8d1a88c77463e8ee6765b185adbe3d23d9fce101 Signed-off-by: Boyan Karatotev --- lib/psci/psci_suspend.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/psci/psci_suspend.c b/lib/psci/psci_suspend.c index aaf82a05c..baa3bd428 100644 --- a/lib/psci/psci_suspend.c +++ b/lib/psci/psci_suspend.c @@ -121,7 +121,6 @@ int psci_cpu_suspend_start(unsigned int idx, unsigned int is_power_down_state) { int rc = PSCI_E_SUCCESS; - bool skip_wfi = false; unsigned int parent_nodes[PLAT_MAX_PWR_LVL] = {0}; unsigned int max_off_lvl = 0; #if FEAT_PABANDON @@ -152,7 +151,6 @@ int psci_cpu_suspend_start(unsigned int idx, * detection that a wake-up interrupt has fired. */ if (read_isr_el1() != 0U) { - skip_wfi = true; goto exit; } @@ -164,7 +162,6 @@ int psci_cpu_suspend_start(unsigned int idx, */ rc = psci_validate_state_coordination(idx, end_pwrlvl, state_info); if (rc != PSCI_E_SUCCESS) { - skip_wfi = true; goto exit; } } else { @@ -183,7 +180,6 @@ int psci_cpu_suspend_start(unsigned int idx, if (psci_plat_pm_ops->pwr_domain_validate_suspend != NULL) { rc = psci_plat_pm_ops->pwr_domain_validate_suspend(state_info); if (rc != PSCI_E_SUCCESS) { - skip_wfi = true; goto exit; } } @@ -243,17 +239,12 @@ int psci_cpu_suspend_start(unsigned int idx, plat_psci_stat_accounting_start(state_info); #endif -exit: /* * Release the locks corresponding to each power level in the * reverse order to which they were acquired. */ psci_release_pwr_domain_locks(end_pwrlvl, parent_nodes); - if (skip_wfi) { - return rc; - } - #if ENABLE_RUNTIME_INSTRUMENTATION /* * Update the timestamp with cache off. We assume this @@ -335,6 +326,7 @@ exit: */ psci_set_pwr_domains_to_run(idx, end_pwrlvl); +exit: psci_release_pwr_domain_locks(end_pwrlvl, parent_nodes); return rc; From db5fe4f4934208ac8f8ae9283df2fbac6066e24e Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Tue, 8 Oct 2024 17:34:45 +0100 Subject: [PATCH 8/9] chore(docs): drop the "wfi" from `pwr_domain_pwr_down_wfi` To allow for generic handling of a wakeup, this hook is no longer expected to call wfi itself. Update the name everywhere to reflect this expectation so that future platform implementers don't get misled. Change-Id: Ic33f0b6da74592ad6778fd802c2f0b85223af614 Signed-off-by: Boyan Karatotev --- docs/porting-guide.rst | 8 ++++---- include/lib/psci/psci.h | 2 +- lib/psci/psci_off.c | 4 ++-- lib/psci/psci_suspend.c | 4 ++-- plat/amlogic/axg/axg_pm.c | 2 +- plat/amlogic/g12a/g12a_pm.c | 2 +- plat/amlogic/gxbb/gxbb_pm.c | 2 +- plat/amlogic/gxl/gxl_pm.c | 2 +- plat/imx/imx8m/imx8mm/imx8mm_psci.c | 2 +- plat/imx/imx8m/imx8mn/imx8mn_psci.c | 2 +- plat/imx/imx8m/imx8mp/imx8mp_psci.c | 2 +- plat/imx/imx8m/imx8mq/imx8mq_psci.c | 2 +- plat/imx/imx8ulp/imx8ulp_psci.c | 2 +- plat/marvell/armada/a8k/common/plat_pm.c | 2 +- plat/mediatek/lib/pm/mtk_pm.c | 4 ++-- plat/nuvoton/npcm845x/npcm845x_psci.c | 4 ++-- plat/nvidia/tegra/common/tegra_pm.c | 2 +- plat/nxp/common/psci/plat_psci.c | 2 +- plat/qemu/common/qemu_pm.c | 2 +- plat/qemu/qemu_sbsa/sbsa_pm.c | 2 +- plat/qti/common/src/qti_pm.c | 2 +- plat/renesas/common/plat_pm.c | 2 +- plat/rockchip/common/plat_pm.c | 2 +- plat/rpi/common/rpi3_pm.c | 2 +- plat/socionext/uniphier/uniphier_psci.c | 2 +- plat/st/stm32mp1/stm32mp1_pm.c | 2 +- plat/st/stm32mp2/stm32mp2_pm.c | 2 +- 27 files changed, 34 insertions(+), 34 deletions(-) diff --git a/docs/porting-guide.rst b/docs/porting-guide.rst index f0da2aa94..84d029d75 100644 --- a/docs/porting-guide.rst +++ b/docs/porting-guide.rst @@ -2897,7 +2897,7 @@ allocated in a special area if it cannot fit in the platform's global static data, for example in DRAM. The Distributor can then be powered down using an implementation-defined sequence. -plat_psci_ops.pwr_domain_pwr_down_wfi() +plat_psci_ops.pwr_domain_pwr_down() ....................................... This is an optional function and, if implemented, is expected to perform @@ -2968,7 +2968,7 @@ plat_psci_ops.system_off() This function is called by PSCI implementation in response to a ``SYSTEM_OFF`` call. It performs the platform-specific system poweroff sequence after notifying the Secure Payload Dispatcher. The caller will call ``wfi`` if this -function returns, similar to `plat_psci_ops.pwr_domain_pwr_down_wfi()`_. +function returns, similar to `plat_psci_ops.pwr_domain_pwr_down()`_. plat_psci_ops.system_reset() ............................ @@ -2976,7 +2976,7 @@ plat_psci_ops.system_reset() This function is called by PSCI implementation in response to a ``SYSTEM_RESET`` call. It performs the platform-specific system reset sequence after notifying the Secure Payload Dispatcher. The caller will call ``wfi`` if this -function returns, similar to `plat_psci_ops.pwr_domain_pwr_down_wfi()`_. +function returns, similar to `plat_psci_ops.pwr_domain_pwr_down()`_. plat_psci_ops.validate_power_state() .................................... @@ -3065,7 +3065,7 @@ function must return ``PSCI_E_NOT_SUPPORTED``. For architectural resets, all failures must return ``PSCI_E_INVALID_PARAMETERS`` and vendor reset can return other PSCI error codes as defined in `PSCI`_. If this function returns success, the caller will call -``wfi`` similar to `plat_psci_ops.pwr_domain_pwr_down_wfi()`_. +``wfi`` similar to `plat_psci_ops.pwr_domain_pwr_down()`_. plat_psci_ops.write_mem_protect() ................................. diff --git a/include/lib/psci/psci.h b/include/lib/psci/psci.h index 3a5bdbaba..68e721a03 100644 --- a/include/lib/psci/psci.h +++ b/include/lib/psci/psci.h @@ -331,7 +331,7 @@ typedef struct plat_psci_ops { const psci_power_state_t *target_state); void (*pwr_domain_suspend_finish)( const psci_power_state_t *target_state); - void (*pwr_domain_pwr_down_wfi)( + void (*pwr_domain_pwr_down)( const psci_power_state_t *target_state); void (*system_off)(void); void (*system_reset)(void); diff --git a/lib/psci/psci_off.c b/lib/psci/psci_off.c index dbc646c96..577fdd719 100644 --- a/lib/psci/psci_off.c +++ b/lib/psci/psci_off.c @@ -163,9 +163,9 @@ exit: RT_INSTR_ENTER_HW_LOW_PWR, PMF_NO_CACHE_MAINT); #endif - if (psci_plat_pm_ops->pwr_domain_pwr_down_wfi != NULL) { + if (psci_plat_pm_ops->pwr_domain_pwr_down != NULL) { /* This function may not return */ - psci_plat_pm_ops->pwr_domain_pwr_down_wfi(&state_info); + psci_plat_pm_ops->pwr_domain_pwr_down(&state_info); } psci_pwrdown_cpu_end_terminal(); diff --git a/lib/psci/psci_suspend.c b/lib/psci/psci_suspend.c index baa3bd428..05bbe39cb 100644 --- a/lib/psci/psci_suspend.c +++ b/lib/psci/psci_suspend.c @@ -258,9 +258,9 @@ int psci_cpu_suspend_start(unsigned int idx, #endif if (is_power_down_state != 0U) { - if (psci_plat_pm_ops->pwr_domain_pwr_down_wfi != NULL) { + if (psci_plat_pm_ops->pwr_domain_pwr_down != NULL) { /* This function may not return */ - psci_plat_pm_ops->pwr_domain_pwr_down_wfi(state_info); + psci_plat_pm_ops->pwr_domain_pwr_down(state_info); } psci_pwrdown_cpu_end_wakeup(max_off_lvl); diff --git a/plat/amlogic/axg/axg_pm.c b/plat/amlogic/axg/axg_pm.c index e67f263a4..8a1b5d227 100644 --- a/plat/amlogic/axg/axg_pm.c +++ b/plat/amlogic/axg/axg_pm.c @@ -152,7 +152,7 @@ static const plat_psci_ops_t axg_ops = { .pwr_domain_on = axg_pwr_domain_on, .pwr_domain_on_finish = axg_pwr_domain_on_finish, .pwr_domain_off = axg_pwr_domain_off, - .pwr_domain_pwr_down_wfi = axg_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = axg_pwr_domain_pwr_down_wfi, .system_off = axg_system_off, .system_reset = axg_system_reset }; diff --git a/plat/amlogic/g12a/g12a_pm.c b/plat/amlogic/g12a/g12a_pm.c index c9fe3e977..1203e3c80 100644 --- a/plat/amlogic/g12a/g12a_pm.c +++ b/plat/amlogic/g12a/g12a_pm.c @@ -200,7 +200,7 @@ static const plat_psci_ops_t g12a_ops = { .pwr_domain_on = g12a_pwr_domain_on, .pwr_domain_on_finish = g12a_pwr_domain_on_finish, .pwr_domain_off = g12a_pwr_domain_off, - .pwr_domain_pwr_down_wfi = g12a_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = g12a_pwr_domain_pwr_down_wfi, .system_off = g12a_system_off, .system_reset = g12a_system_reset }; diff --git a/plat/amlogic/gxbb/gxbb_pm.c b/plat/amlogic/gxbb/gxbb_pm.c index 48bff7ba5..eeebb41fd 100644 --- a/plat/amlogic/gxbb/gxbb_pm.c +++ b/plat/amlogic/gxbb/gxbb_pm.c @@ -176,7 +176,7 @@ static const plat_psci_ops_t gxbb_ops = { .pwr_domain_on = gxbb_pwr_domain_on, .pwr_domain_on_finish = gxbb_pwr_domain_on_finish, .pwr_domain_off = gxbb_pwr_domain_off, - .pwr_domain_pwr_down_wfi = gxbb_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = gxbb_pwr_domain_pwr_down_wfi, .system_off = gxbb_system_off, .system_reset = gxbb_system_reset, }; diff --git a/plat/amlogic/gxl/gxl_pm.c b/plat/amlogic/gxl/gxl_pm.c index 433140b77..5af4932d6 100644 --- a/plat/amlogic/gxl/gxl_pm.c +++ b/plat/amlogic/gxl/gxl_pm.c @@ -199,7 +199,7 @@ static const plat_psci_ops_t gxl_ops = { .pwr_domain_on = gxl_pwr_domain_on, .pwr_domain_on_finish = gxl_pwr_domain_on_finish, .pwr_domain_off = gxl_pwr_domain_off, - .pwr_domain_pwr_down_wfi = gxl_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = gxl_pwr_domain_pwr_down_wfi, .system_off = gxl_system_off, .system_reset = gxl_system_reset, }; diff --git a/plat/imx/imx8m/imx8mm/imx8mm_psci.c b/plat/imx/imx8m/imx8mm/imx8mm_psci.c index 815d3a2a5..766c7b3e4 100644 --- a/plat/imx/imx8m/imx8mm/imx8mm_psci.c +++ b/plat/imx/imx8m/imx8mm/imx8mm_psci.c @@ -25,7 +25,7 @@ static const plat_psci_ops_t imx_plat_psci_ops = { .cpu_standby = imx_cpu_standby, .pwr_domain_suspend = imx_domain_suspend, .pwr_domain_suspend_finish = imx_domain_suspend_finish, - .pwr_domain_pwr_down_wfi = imx_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = imx_pwr_domain_pwr_down_wfi, .get_sys_suspend_power_state = imx_get_sys_suspend_power_state, .system_reset = imx_system_reset, .system_reset2 = imx_system_reset2, diff --git a/plat/imx/imx8m/imx8mn/imx8mn_psci.c b/plat/imx/imx8m/imx8mn/imx8mn_psci.c index f541fc138..4947d6683 100644 --- a/plat/imx/imx8m/imx8mn/imx8mn_psci.c +++ b/plat/imx/imx8m/imx8mn/imx8mn_psci.c @@ -25,7 +25,7 @@ static const plat_psci_ops_t imx_plat_psci_ops = { .cpu_standby = imx_cpu_standby, .pwr_domain_suspend = imx_domain_suspend, .pwr_domain_suspend_finish = imx_domain_suspend_finish, - .pwr_domain_pwr_down_wfi = imx_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = imx_pwr_domain_pwr_down_wfi, .get_sys_suspend_power_state = imx_get_sys_suspend_power_state, .system_reset = imx_system_reset, .system_off = imx_system_off, diff --git a/plat/imx/imx8m/imx8mp/imx8mp_psci.c b/plat/imx/imx8m/imx8mp/imx8mp_psci.c index bc7b24679..0a8c351ec 100644 --- a/plat/imx/imx8m/imx8mp/imx8mp_psci.c +++ b/plat/imx/imx8m/imx8mp/imx8mp_psci.c @@ -25,7 +25,7 @@ static const plat_psci_ops_t imx_plat_psci_ops = { .cpu_standby = imx_cpu_standby, .pwr_domain_suspend = imx_domain_suspend, .pwr_domain_suspend_finish = imx_domain_suspend_finish, - .pwr_domain_pwr_down_wfi = imx_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = imx_pwr_domain_pwr_down_wfi, .get_sys_suspend_power_state = imx_get_sys_suspend_power_state, .system_reset = imx_system_reset, .system_off = imx_system_off, diff --git a/plat/imx/imx8m/imx8mq/imx8mq_psci.c b/plat/imx/imx8m/imx8mq/imx8mq_psci.c index 3375ce71b..c023b55dd 100644 --- a/plat/imx/imx8m/imx8mq/imx8mq_psci.c +++ b/plat/imx/imx8m/imx8mq/imx8mq_psci.c @@ -137,7 +137,7 @@ static const plat_psci_ops_t imx_plat_psci_ops = { .cpu_standby = imx_cpu_standby, .pwr_domain_suspend = imx_domain_suspend, .pwr_domain_suspend_finish = imx_domain_suspend_finish, - .pwr_domain_pwr_down_wfi = imx_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = imx_pwr_domain_pwr_down_wfi, .get_sys_suspend_power_state = imx_get_sys_suspend_power_state, .system_reset = imx_system_reset, .system_reset2 = imx_system_reset2, diff --git a/plat/imx/imx8ulp/imx8ulp_psci.c b/plat/imx/imx8ulp/imx8ulp_psci.c index 628aceabd..59af8be43 100644 --- a/plat/imx/imx8ulp/imx8ulp_psci.c +++ b/plat/imx/imx8ulp/imx8ulp_psci.c @@ -538,7 +538,7 @@ static const plat_psci_ops_t imx_plat_psci_ops = { .pwr_domain_suspend_finish = imx_domain_suspend_finish, .get_sys_suspend_power_state = imx_get_sys_suspend_power_state, .validate_power_state = imx_validate_power_state, - .pwr_domain_pwr_down_wfi = imx8ulp_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = imx8ulp_pwr_domain_pwr_down_wfi, }; int plat_setup_psci_ops(uintptr_t sec_entrypoint, diff --git a/plat/marvell/armada/a8k/common/plat_pm.c b/plat/marvell/armada/a8k/common/plat_pm.c index f08f08a35..ae3ee3770 100644 --- a/plat/marvell/armada/a8k/common/plat_pm.c +++ b/plat/marvell/armada/a8k/common/plat_pm.c @@ -845,7 +845,7 @@ const plat_psci_ops_t plat_arm_psci_pm_ops = { .pwr_domain_on_finish = a8k_pwr_domain_on_finish, .get_sys_suspend_power_state = a8k_get_sys_suspend_power_state, .pwr_domain_suspend_finish = a8k_pwr_domain_suspend_finish, - .pwr_domain_pwr_down_wfi = a8k_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = a8k_pwr_domain_pwr_down_wfi, .system_off = a8k_system_off, .system_reset = a8k_system_reset, .validate_power_state = a8k_validate_power_state, diff --git a/plat/mediatek/lib/pm/mtk_pm.c b/plat/mediatek/lib/pm/mtk_pm.c index 772c2d721..553fad29d 100644 --- a/plat/mediatek/lib/pm/mtk_pm.c +++ b/plat/mediatek/lib/pm/mtk_pm.c @@ -45,8 +45,8 @@ int plat_pm_ops_setup_pwr(struct plat_pm_pwr_ctrl *ops) mtk_pm_ops.get_sys_suspend_power_state = ops->get_sys_suspend_power_state; } - if (!mtk_pm_ops.pwr_domain_pwr_down_wfi) - mtk_pm_ops.pwr_domain_pwr_down_wfi = ops->pwr_domain_pwr_down_wfi; + if (!mtk_pm_ops.pwr_domain_pwr_down) + mtk_pm_ops.pwr_domain_pwr_down = ops->pwr_domain_pwr_down_wfi; mtk_pm_status |= MTK_PM_ST_PWR_READY; #endif diff --git a/plat/nuvoton/npcm845x/npcm845x_psci.c b/plat/nuvoton/npcm845x/npcm845x_psci.c index a954265dd..6e9523e36 100644 --- a/plat/nuvoton/npcm845x/npcm845x_psci.c +++ b/plat/nuvoton/npcm845x/npcm845x_psci.c @@ -384,7 +384,7 @@ static const plat_psci_ops_t npcm845x_plat_psci_ops = { /* For testing purposes only This PSCI states are not supported */ .pwr_domain_off = npcm845x_pwr_domain_off, - .pwr_domain_pwr_down_wfi = npcm845x_pwr_down_wfi, + .pwr_domain_pwr_down = npcm845x_pwr_down_wfi, }; /* For reference only @@ -400,7 +400,7 @@ static const plat_psci_ops_t npcm845x_plat_psci_ops = { * const psci_power_state_t *target_state); * void (*pwr_domain_suspend_finish)( * const psci_power_state_t *target_state); - * void __dead2 (*pwr_domain_pwr_down_wfi)( + * void __dead2 (*pwr_domain_pwr_down )( * const psci_power_state_t *target_state); * void __dead2 (*system_off)(void); * void __dead2 (*system_reset)(void); diff --git a/plat/nvidia/tegra/common/tegra_pm.c b/plat/nvidia/tegra/common/tegra_pm.c index 8edb0247e..234cb6b0e 100644 --- a/plat/nvidia/tegra/common/tegra_pm.c +++ b/plat/nvidia/tegra/common/tegra_pm.c @@ -284,7 +284,7 @@ static plat_psci_ops_t tegra_plat_psci_ops = { .pwr_domain_suspend = tegra_pwr_domain_suspend, .pwr_domain_on_finish = tegra_pwr_domain_on_finish, .pwr_domain_suspend_finish = tegra_pwr_domain_suspend_finish, - .pwr_domain_pwr_down_wfi = tegra_pwr_domain_power_down_wfi, + .pwr_domain_pwr_down = tegra_pwr_domain_power_down_wfi, .system_off = tegra_system_off, .system_reset = tegra_system_reset, .validate_power_state = tegra_validate_power_state, diff --git a/plat/nxp/common/psci/plat_psci.c b/plat/nxp/common/psci/plat_psci.c index f6dd7b30e..c154a7f6e 100644 --- a/plat/nxp/common/psci/plat_psci.c +++ b/plat/nxp/common/psci/plat_psci.c @@ -433,7 +433,7 @@ static plat_psci_ops_t _psci_pm_ops = { .pwr_domain_off = _pwr_domain_off, #endif #if (SOC_CORE_OFF || SOC_CORE_PWR_DWN) - .pwr_domain_pwr_down_wfi = _pwr_down_wfi, + .pwr_domain_pwr_down = _pwr_down_wfi, #endif #if (SOC_CORE_STANDBY || SOC_CORE_PWR_DWN) /* cpu_suspend */ diff --git a/plat/qemu/common/qemu_pm.c b/plat/qemu/common/qemu_pm.c index 5f64d70c5..7893c8164 100644 --- a/plat/qemu/common/qemu_pm.c +++ b/plat/qemu/common/qemu_pm.c @@ -218,7 +218,7 @@ static const plat_psci_ops_t plat_qemu_psci_pm_ops = { .cpu_standby = qemu_cpu_standby, .pwr_domain_on = qemu_pwr_domain_on, .pwr_domain_off = qemu_pwr_domain_off, - .pwr_domain_pwr_down_wfi = qemu_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = qemu_pwr_domain_pwr_down_wfi, .pwr_domain_suspend = qemu_pwr_domain_suspend, .pwr_domain_on_finish = qemu_pwr_domain_on_finish, .pwr_domain_suspend_finish = qemu_pwr_domain_suspend_finish, diff --git a/plat/qemu/qemu_sbsa/sbsa_pm.c b/plat/qemu/qemu_sbsa/sbsa_pm.c index 8d1e1d48c..7ce7beba9 100644 --- a/plat/qemu/qemu_sbsa/sbsa_pm.c +++ b/plat/qemu/qemu_sbsa/sbsa_pm.c @@ -215,7 +215,7 @@ static const plat_psci_ops_t plat_qemu_psci_pm_ops = { .cpu_standby = qemu_cpu_standby, .pwr_domain_on = qemu_pwr_domain_on, .pwr_domain_off = qemu_pwr_domain_off, - .pwr_domain_pwr_down_wfi = qemu_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = qemu_pwr_domain_pwr_down_wfi, .pwr_domain_suspend = qemu_pwr_domain_suspend, .pwr_domain_on_finish = qemu_pwr_domain_on_finish, .pwr_domain_suspend_finish = qemu_pwr_domain_suspend_finish, diff --git a/plat/qti/common/src/qti_pm.c b/plat/qti/common/src/qti_pm.c index 4400e40ea..242812642 100644 --- a/plat/qti/common/src/qti_pm.c +++ b/plat/qti/common/src/qti_pm.c @@ -278,7 +278,7 @@ const plat_psci_ops_t plat_qti_psci_pm_ops = { .pwr_domain_off = qti_node_power_off, .pwr_domain_suspend = qti_node_suspend, .pwr_domain_suspend_finish = qti_node_suspend_finish, - .pwr_domain_pwr_down_wfi = qti_domain_power_down_wfi, + .pwr_domain_pwr_down = qti_domain_power_down_wfi, .system_off = qti_system_off, .system_reset = qti_system_reset, .get_node_hw_state = NULL, diff --git a/plat/renesas/common/plat_pm.c b/plat/renesas/common/plat_pm.c index 9810596de..871dddce8 100644 --- a/plat/renesas/common/plat_pm.c +++ b/plat/renesas/common/plat_pm.c @@ -302,7 +302,7 @@ static const plat_psci_ops_t rcar_plat_psci_ops = { .system_off = rcar_system_off, .system_reset = rcar_system_reset, .validate_power_state = rcar_validate_power_state, - .pwr_domain_pwr_down_wfi = rcar_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = rcar_pwr_domain_pwr_down_wfi, #if RCAR_SYSTEM_SUSPEND .get_sys_suspend_power_state = rcar_get_sys_suspend_power_state, #endif diff --git a/plat/rockchip/common/plat_pm.c b/plat/rockchip/common/plat_pm.c index c3dc23474..df7403351 100644 --- a/plat/rockchip/common/plat_pm.c +++ b/plat/rockchip/common/plat_pm.c @@ -395,7 +395,7 @@ const plat_psci_ops_t plat_rockchip_psci_pm_ops = { .pwr_domain_suspend = rockchip_pwr_domain_suspend, .pwr_domain_on_finish = rockchip_pwr_domain_on_finish, .pwr_domain_suspend_finish = rockchip_pwr_domain_suspend_finish, - .pwr_domain_pwr_down_wfi = rockchip_pd_pwr_down_wfi, + .pwr_domain_pwr_down = rockchip_pd_pwr_down_wfi, .system_reset = rockchip_system_reset, .system_off = rockchip_system_poweroff, .validate_power_state = rockchip_validate_power_state, diff --git a/plat/rpi/common/rpi3_pm.c b/plat/rpi/common/rpi3_pm.c index 456e1603c..4dfe8f0df 100644 --- a/plat/rpi/common/rpi3_pm.c +++ b/plat/rpi/common/rpi3_pm.c @@ -272,7 +272,7 @@ static const plat_psci_ops_t plat_rpi3_psci_pm_ops = { .pwr_domain_off = rpi3_pwr_domain_off, .pwr_domain_on = rpi3_pwr_domain_on, .pwr_domain_on_finish = rpi3_pwr_domain_on_finish, - .pwr_domain_pwr_down_wfi = rpi3_pwr_down_wfi, + .pwr_domain_pwr_down = rpi3_pwr_down_wfi, .system_off = rpi3_system_off, .system_reset = rpi3_system_reset, .validate_power_state = rpi3_validate_power_state, diff --git a/plat/socionext/uniphier/uniphier_psci.c b/plat/socionext/uniphier/uniphier_psci.c index a371705b1..b009c5eb9 100644 --- a/plat/socionext/uniphier/uniphier_psci.c +++ b/plat/socionext/uniphier/uniphier_psci.c @@ -113,7 +113,7 @@ static const struct plat_psci_ops uniphier_psci_ops = { .pwr_domain_on = uniphier_psci_pwr_domain_on, .pwr_domain_off = uniphier_psci_pwr_domain_off, .pwr_domain_on_finish = uniphier_psci_pwr_domain_on_finish, - .pwr_domain_pwr_down_wfi = uniphier_psci_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = uniphier_psci_pwr_domain_pwr_down_wfi, .system_off = uniphier_psci_system_off, .system_reset = uniphier_psci_system_reset, }; diff --git a/plat/st/stm32mp1/stm32mp1_pm.c b/plat/st/stm32mp1/stm32mp1_pm.c index 97e1ac61b..784102243 100644 --- a/plat/st/stm32mp1/stm32mp1_pm.c +++ b/plat/st/stm32mp1/stm32mp1_pm.c @@ -215,7 +215,7 @@ static const plat_psci_ops_t stm32_psci_ops = { .pwr_domain_suspend = stm32_pwr_domain_suspend, .pwr_domain_on_finish = stm32_pwr_domain_on_finish, .pwr_domain_suspend_finish = stm32_pwr_domain_suspend_finish, - .pwr_domain_pwr_down_wfi = stm32_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = stm32_pwr_domain_pwr_down_wfi, .system_off = stm32_system_off, .system_reset = stm32_system_reset, .validate_power_state = stm32_validate_power_state, diff --git a/plat/st/stm32mp2/stm32mp2_pm.c b/plat/st/stm32mp2/stm32mp2_pm.c index 5bb381d0d..9be42c59c 100644 --- a/plat/st/stm32mp2/stm32mp2_pm.c +++ b/plat/st/stm32mp2/stm32mp2_pm.c @@ -103,7 +103,7 @@ static const plat_psci_ops_t stm32_psci_ops = { .pwr_domain_suspend = stm32_pwr_domain_suspend, .pwr_domain_on_finish = stm32_pwr_domain_on_finish, .pwr_domain_suspend_finish = stm32_pwr_domain_suspend_finish, - .pwr_domain_pwr_down_wfi = stm32_pwr_domain_pwr_down_wfi, + .pwr_domain_pwr_down = stm32_pwr_domain_pwr_down_wfi, .system_off = stm32_system_off, .system_reset = stm32_system_reset, .validate_power_state = stm32_validate_power_state, From c9f352c362a8d114a055bb9206c5b6391ec3b96a Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Wed, 16 Oct 2024 11:36:29 +0100 Subject: [PATCH 9/9] fix(cpus): clear CPUPWRCTLR_EL1.CORE_PWRDN_EN_BIT on reset The model has a bug where it will not clear CPUPWRCTLR_EL1 on reset, even though the actual cores do. The write of 1 to the bit itself triggers the powerdown sequnece, regardless of the value before the write. As such, the bug does not impact functionality but it does throw off software reading it. Clear the bit on Travis and Gelas as they are the only ones to require reading it back. Change-Id: I765a7fa055733d522480be30d412e3b417af2bd7 Signed-off-by: Boyan Karatotev --- lib/cpus/aarch64/cortex_gelas.S | 3 +++ lib/cpus/aarch64/travis.S | 3 +++ 2 files changed, 6 insertions(+) diff --git a/lib/cpus/aarch64/cortex_gelas.S b/lib/cpus/aarch64/cortex_gelas.S index e95820547..709bb129b 100644 --- a/lib/cpus/aarch64/cortex_gelas.S +++ b/lib/cpus/aarch64/cortex_gelas.S @@ -35,6 +35,9 @@ cpu_reset_func_start cortex_gelas * ---------------------------------------------------- */ msr SSBS, xzr + /* model bug: not cleared on reset */ + sysreg_bit_clear CORTEX_GELAS_CPUPWRCTLR_EL1, \ + CORTEX_GELAS_CPUPWRCTLR_EL1_CORE_PWRDN_BIT cpu_reset_func_end cortex_gelas /* ---------------------------------------------------- diff --git a/lib/cpus/aarch64/travis.S b/lib/cpus/aarch64/travis.S index 246159a10..2e41668f4 100644 --- a/lib/cpus/aarch64/travis.S +++ b/lib/cpus/aarch64/travis.S @@ -35,6 +35,9 @@ cpu_reset_func_start travis * ---------------------------------------------------- */ msr SSBS, xzr + /* model bug: not cleared on reset */ + sysreg_bit_clear TRAVIS_IMP_CPUPWRCTLR_EL1, \ + TRAVIS_IMP_CPUPWRCTLR_EL1_CORE_PWRDN_EN_BIT cpu_reset_func_end travis func travis_core_pwr_dwn