From c2fb8ef66ccc8222c70ab802cdaf29f1592cbbb6 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 14 Nov 2022 10:39:48 +0000 Subject: [PATCH] feat(aarch64): make ID system register reads non-volatile Our system register access function wrappers are using "volatile" inline assembly instructions. On the first glance this is a good idea, since many system registers have side effects, and we don't want the compiler to optimise or reorder them (what "volatile" prevents). However this also naturally limits the compiler's freedom to optimise code better, and those volatile properties don't apply to every type of system register. One example are the CPU ID registers, which have constant values, are side-effect free and read-only. Introduce a new wrapper type that drops the volatile keyword, and use that for the wrappers instantiating ID register accessors. This allows the compiler to freely optimise those instructions away, if their result isn't actually used, which can trigger further optimisations. Change-Id: I3c64716ae4f4bf603f0ea57b652bd50bcc67bb0e Signed-off-by: Andre Przywara --- include/arch/aarch64/arch_helpers.h | 42 ++++++++++++++++++++--------- plat/qti/common/src/qti_pm.c | 3 +-- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/include/arch/aarch64/arch_helpers.h b/include/arch/aarch64/arch_helpers.h index fe9b5a563..5010ac5dd 100644 --- a/include/arch/aarch64/arch_helpers.h +++ b/include/arch/aarch64/arch_helpers.h @@ -27,6 +27,14 @@ static inline u_register_t read_ ## _name(void) \ return v; \ } +#define _DEFINE_SYSREG_READ_FUNC_NV(_name, _reg_name) \ +static inline u_register_t read_ ## _name(void) \ +{ \ + u_register_t v; \ + __asm__ ("mrs %0, " #_reg_name : "=r" (v)); \ + return v; \ +} + #define _DEFINE_SYSREG_WRITE_FUNC(_name, _reg_name) \ static inline void write_ ## _name(u_register_t v) \ { \ @@ -58,6 +66,14 @@ static inline void write_ ## _name(u_register_t v) \ #define DEFINE_RENAME_SYSREG_WRITE_FUNC(_name, _reg_name) \ _DEFINE_SYSREG_WRITE_FUNC(_name, _reg_name) +/* Define read function for ID register (w/o volatile qualifier) */ +#define DEFINE_IDREG_READ_FUNC(_name) \ + _DEFINE_SYSREG_READ_FUNC_NV(_name, _name) + +/* Define read function for renamed ID register (w/o volatile qualifier) */ +#define DEFINE_RENAME_IDREG_READ_FUNC(_name, _reg_name) \ + _DEFINE_SYSREG_READ_FUNC_NV(_name, _reg_name) + /********************************************************************** * Macros to create inline functions for system instructions *********************************************************************/ @@ -247,14 +263,14 @@ void disable_mpu_icache_el2(void); #define write_daifset(val) SYSREG_WRITE_CONST(daifset, val) DEFINE_SYSREG_RW_FUNCS(par_el1) -DEFINE_SYSREG_READ_FUNC(id_pfr1_el1) -DEFINE_SYSREG_READ_FUNC(id_aa64isar0_el1) -DEFINE_SYSREG_READ_FUNC(id_aa64isar1_el1) -DEFINE_RENAME_SYSREG_READ_FUNC(id_aa64isar2_el1, ID_AA64ISAR2_EL1) -DEFINE_SYSREG_READ_FUNC(id_aa64pfr0_el1) -DEFINE_SYSREG_READ_FUNC(id_aa64pfr1_el1) -DEFINE_SYSREG_READ_FUNC(id_aa64dfr0_el1) -DEFINE_SYSREG_READ_FUNC(id_afr0_el1) +DEFINE_IDREG_READ_FUNC(id_pfr1_el1) +DEFINE_IDREG_READ_FUNC(id_aa64isar0_el1) +DEFINE_IDREG_READ_FUNC(id_aa64isar1_el1) +DEFINE_RENAME_IDREG_READ_FUNC(id_aa64isar2_el1, ID_AA64ISAR2_EL1) +DEFINE_IDREG_READ_FUNC(id_aa64pfr0_el1) +DEFINE_IDREG_READ_FUNC(id_aa64pfr1_el1) +DEFINE_IDREG_READ_FUNC(id_aa64dfr0_el1) +DEFINE_IDREG_READ_FUNC(id_afr0_el1) DEFINE_SYSREG_READ_FUNC(CurrentEl) DEFINE_SYSREG_READ_FUNC(ctr_el0) DEFINE_SYSREG_RW_FUNCS(daif) @@ -367,10 +383,10 @@ void __dead2 smc(uint64_t x0, uint64_t x1, uint64_t x2, uint64_t x3, /******************************************************************************* * System register accessor prototypes ******************************************************************************/ -DEFINE_SYSREG_READ_FUNC(midr_el1) +DEFINE_IDREG_READ_FUNC(midr_el1) DEFINE_SYSREG_READ_FUNC(mpidr_el1) -DEFINE_SYSREG_READ_FUNC(id_aa64mmfr0_el1) -DEFINE_SYSREG_READ_FUNC(id_aa64mmfr1_el1) +DEFINE_IDREG_READ_FUNC(id_aa64mmfr0_el1) +DEFINE_IDREG_READ_FUNC(id_aa64mmfr1_el1) DEFINE_SYSREG_RW_FUNCS(scr_el3) DEFINE_SYSREG_RW_FUNCS(hcr_el2) @@ -516,7 +532,7 @@ DEFINE_RENAME_SYSREG_RW_FUNCS(pmblimitr_el1, PMBLIMITR_EL1) DEFINE_RENAME_SYSREG_WRITE_FUNC(zcr_el3, ZCR_EL3) DEFINE_RENAME_SYSREG_WRITE_FUNC(zcr_el2, ZCR_EL2) -DEFINE_RENAME_SYSREG_READ_FUNC(id_aa64smfr0_el1, ID_AA64SMFR0_EL1) +DEFINE_RENAME_IDREG_READ_FUNC(id_aa64smfr0_el1, ID_AA64SMFR0_EL1) DEFINE_RENAME_SYSREG_RW_FUNCS(smcr_el3, SMCR_EL3) DEFINE_RENAME_SYSREG_READ_FUNC(erridr_el1, ERRIDR_EL1) @@ -530,7 +546,7 @@ DEFINE_RENAME_SYSREG_READ_FUNC(erxmisc0_el1, ERXMISC0_EL1) DEFINE_RENAME_SYSREG_READ_FUNC(erxmisc1_el1, ERXMISC1_EL1) /* Armv8.2 Registers */ -DEFINE_RENAME_SYSREG_READ_FUNC(id_aa64mmfr2_el1, ID_AA64MMFR2_EL1) +DEFINE_RENAME_IDREG_READ_FUNC(id_aa64mmfr2_el1, ID_AA64MMFR2_EL1) /* Armv8.3 Pointer Authentication Registers */ DEFINE_RENAME_SYSREG_RW_FUNCS(apiakeyhi_el1, APIAKeyHi_EL1) diff --git a/plat/qti/common/src/qti_pm.c b/plat/qti/common/src/qti_pm.c index 5f1b7aa1d..ae361e973 100644 --- a/plat/qti/common/src/qti_pm.c +++ b/plat/qti/common/src/qti_pm.c @@ -47,8 +47,7 @@ #define QTI_CORE_PWRDN_EN_MASK 1 /* cpu power control happens to be same across all CPUs */ -_DEFINE_SYSREG_WRITE_FUNC(cpu_pwrctrl_val, S3_0_C15_C2_7) -_DEFINE_SYSREG_READ_FUNC(cpu_pwrctrl_val, S3_0_C15_C2_7) +DEFINE_RENAME_SYSREG_RW_FUNCS(cpu_pwrctrl_val, S3_0_C15_C2_7) const unsigned int qti_pm_idle_states[] = { qti_make_pwrstate_lvl0(QTI_LOCAL_STATE_OFF,