From 45c7328c0b94d043745b4a44c2e14e1a77f5c347 Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Fri, 20 Sep 2024 13:37:51 +0100 Subject: [PATCH] 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