From 1d6d6802dd547c8b378a9a47572ee72e68cceb3b Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Tue, 6 Dec 2022 09:03:42 +0000 Subject: [PATCH] fix(pmu): unconditionally save PMCR_EL0 Reading back a RES0 bit does not necessarily mean it will be read as 0. The Arm ARM explicitly warns against doing this. The PMU initialisation code tries to set such bits to 1 (in MDCR_EL3) regardless of whether they are in use or are RES0, checking their value could be wrong and PMCR_EL0 might not end up being saved. Save PMCR_EL0 unconditionally to prevent this. Remove the security state change as the outgoing state is not relevant to what the root world context should look like. Signed-off-by: Boyan Karatotev Change-Id: Id43667d37b0e2da3ded0beaf23fa0d4f9013f470 --- bl1/aarch64/bl1_exceptions.S | 4 +- bl31/aarch64/ea_delegate.S | 8 +--- bl31/aarch64/runtime_exceptions.S | 12 ++---- lib/el3_runtime/aarch64/context.S | 61 +------------------------------ 4 files changed, 7 insertions(+), 78 deletions(-) diff --git a/bl1/aarch64/bl1_exceptions.S b/bl1/aarch64/bl1_exceptions.S index c54219fc1..eaaf59a22 100644 --- a/bl1/aarch64/bl1_exceptions.S +++ b/bl1/aarch64/bl1_exceptions.S @@ -218,9 +218,7 @@ unexpected_sync_exception: smc_handler: /* ----------------------------------------------------- * Save x0-x29 and ARMv8.3-PAuth (if enabled) registers. - * If Secure Cycle Counter is not disabled in MDCR_EL3 - * when ARMv8.5-PMU is implemented, save PMCR_EL0 and - * disable Cycle Counter. + * Save PMCR_EL0 and disable Cycle Counter. * TODO: Revisit to store only SMCCC specified registers. * ----------------------------------------------------- */ diff --git a/bl31/aarch64/ea_delegate.S b/bl31/aarch64/ea_delegate.S index 9419476ce..92df653b1 100644 --- a/bl31/aarch64/ea_delegate.S +++ b/bl31/aarch64/ea_delegate.S @@ -81,9 +81,7 @@ func handle_lower_el_sync_ea 1: /* * Save general purpose and ARMv8.3-PAuth registers (if enabled). - * If Secure Cycle Counter is not disabled in MDCR_EL3 when - * ARMv8.5-PMU is implemented, save PMCR_EL0 and disable Cycle Counter. - * Also set the PSTATE to a known state. + * Also save PMCR_EL0 and set the PSTATE to a known state. */ bl prepare_el3_entry @@ -123,9 +121,7 @@ func handle_lower_el_async_ea /* * Save general purpose and ARMv8.3-PAuth registers (if enabled). - * If Secure Cycle Counter is not disabled in MDCR_EL3 when - * ARMv8.5-PMU is implemented, save PMCR_EL0 and disable Cycle Counter. - * Also set the PSTATE to a known state. + * Also save PMCR_EL0 and set the PSTATE to a known state. */ bl prepare_el3_entry diff --git a/bl31/aarch64/runtime_exceptions.S b/bl31/aarch64/runtime_exceptions.S index 2fa9f06c5..b02e8bb21 100644 --- a/bl31/aarch64/runtime_exceptions.S +++ b/bl31/aarch64/runtime_exceptions.S @@ -72,9 +72,7 @@ /* * Save general purpose and ARMv8.3-PAuth registers (if enabled). - * If Secure Cycle Counter is not disabled in MDCR_EL3 when - * ARMv8.5-PMU is implemented, save PMCR_EL0 and disable Cycle Counter. - * Also set the PSTATE to a known state. + * Also save PMCR_EL0 and set the PSTATE to a known state. */ bl prepare_el3_entry @@ -164,9 +162,7 @@ /* * Save general purpose and ARMv8.3-PAuth registers (if enabled). - * If Secure Cycle Counter is not disabled in MDCR_EL3 when - * ARMv8.5-PMU is implemented, save PMCR_EL0 and disable Cycle Counter. - * Also set the PSTATE to a known state. + * Also save PMCR_EL0 and set the PSTATE to a known state. */ bl prepare_el3_entry @@ -440,9 +436,7 @@ sync_handler64: /* * Save general purpose and ARMv8.3-PAuth registers (if enabled). - * If Secure Cycle Counter is not disabled in MDCR_EL3 when - * ARMv8.5-PMU is implemented, save PMCR_EL0 and disable Cycle Counter. - * Also set the PSTATE to a known state. + * Also save PMCR_EL0 and set the PSTATE to a known state. */ bl prepare_el3_entry diff --git a/lib/el3_runtime/aarch64/context.S b/lib/el3_runtime/aarch64/context.S index 769117163..456ed5113 100644 --- a/lib/el3_runtime/aarch64/context.S +++ b/lib/el3_runtime/aarch64/context.S @@ -596,48 +596,12 @@ endfunc fpregs_context_restore stp x28, x29, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X28] mrs x18, sp_el0 str x18, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_SP_EL0] - - /* ---------------------------------------------------------- - * Check if earlier initialization of MDCR_EL3.SCCD/MCCD to 1 - * has failed. - * - * MDCR_EL3: - * MCCD bit set, Prohibits the Cycle Counter PMCCNTR_EL0 from - * counting at EL3. - * SCCD bit set, Secure Cycle Counter Disable. Prohibits PMCCNTR_EL0 - * from counting in Secure state. - * If these bits are not set, meaning that FEAT_PMUv3p5/7 is - * not implemented and PMCR_EL0 should be saved in non-secure - * context. - * ---------------------------------------------------------- - */ - mov_imm x10, (MDCR_SCCD_BIT | MDCR_MCCD_BIT) - mrs x9, mdcr_el3 - tst x9, x10 - bne 1f - - /* ---------------------------------------------------------- - * If control reaches here, it ensures the Secure Cycle - * Counter (PMCCNTR_EL0) is not prohibited from counting at - * EL3 and in secure states. - * Henceforth, PMCR_EL0 to be saved before world switch. - * ---------------------------------------------------------- - */ mrs x9, pmcr_el0 - - /* Check caller's security state */ - mrs x10, scr_el3 - tst x10, #SCR_NS_BIT - beq 2f - - /* Save PMCR_EL0 if called from Non-secure state */ str x9, [sp, #CTX_EL3STATE_OFFSET + CTX_PMCR_EL0] - /* Disable cycle counter when event counting is prohibited */ -2: orr x9, x9, #PMCR_EL0_DP_BIT + orr x9, x9, #PMCR_EL0_DP_BIT msr pmcr_el0, x9 isb -1: #if CTX_INCLUDE_PAUTH_REGS /* ---------------------------------------------------------- * Save the ARMv8.3-PAuth keys as they are not banked @@ -715,31 +679,8 @@ func restore_gp_pmcr_pauth_regs msr APGAKeyLo_EL1, x8 msr APGAKeyHi_EL1, x9 #endif /* CTX_INCLUDE_PAUTH_REGS */ - - /* ---------------------------------------------------------- - * Restore PMCR_EL0 when returning to Non-secure state if - * Secure Cycle Counter is not disabled in MDCR_EL3 when - * ARMv8.5-PMU is implemented. - * ---------------------------------------------------------- - */ - mrs x0, scr_el3 - tst x0, #SCR_NS_BIT - beq 2f - - /* ---------------------------------------------------------- - * Back to Non-secure state. - * Check if earlier initialization MDCR_EL3.SCCD/MCCD to 1 - * failed, meaning that FEAT_PMUv3p5/7 is not implemented and - * PMCR_EL0 should be restored from non-secure context. - * ---------------------------------------------------------- - */ - mov_imm x1, (MDCR_SCCD_BIT | MDCR_MCCD_BIT) - mrs x0, mdcr_el3 - tst x0, x1 - bne 2f ldr x0, [sp, #CTX_EL3STATE_OFFSET + CTX_PMCR_EL0] msr pmcr_el0, x0 -2: ldp x0, x1, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X0] ldp x2, x3, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X2] ldp x4, x5, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X4] -- 2.39.5