refactor(cm): set MDCR_EL3/CPTR_EL3 bits in respective feat_init_el3() only
These bits (MDCR_EL3.{NSTB, NSTBE, TTRF, TPM}, CPTR_EL3.TTA) only affect
EL2 (and lower) execution. Each feat_init_el3() is called long before
any lower EL has had a chance to execute, so setting the bits at reset
is redundant. Removing them from reset code also improves readability of
the immutable EL3 state.
Preserve the original intention for the TTA bit of "enabled for NS and
disabled everywhere else" (inferred from commit messages d4582d3088 and
2031d6166a and the comment). This is because CPTR_EL3 will be contexted
and so everyone will eventually get whatever NS has anyway.
Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
Change-Id: I3d24b45d3ea80882c8e450b2d9db9d5531facec1
diff --git a/lib/el3_runtime/aarch64/context_mgmt.c b/lib/el3_runtime/aarch64/context_mgmt.c
index 8fd9f75..9d717bb 100644
--- a/lib/el3_runtime/aarch64/context_mgmt.c
+++ b/lib/el3_runtime/aarch64/context_mgmt.c
@@ -662,6 +662,11 @@
sme_disable(ctx);
}
}
+
+ /* NS can access this but Secure shouldn't */
+ if (is_feat_sys_reg_trace_supported()) {
+ sys_reg_trace_disable(ctx);
+ }
#endif /* IMAGE_BL31 */
}
diff --git a/lib/extensions/pmuv3/aarch64/pmuv3.c b/lib/extensions/pmuv3/aarch64/pmuv3.c
index fda71aa..61fc47d 100644
--- a/lib/extensions/pmuv3/aarch64/pmuv3.c
+++ b/lib/extensions/pmuv3/aarch64/pmuv3.c
@@ -82,9 +82,12 @@
* 0 | 1 | enabled | disabled
* 1 | 1 | enabled | disabled only for counters 0 to
* MDCR_EL2.HPMN - 1. Enabled for the rest
+ *
+ * MDCR_EL3.TPM: Set to zero so that EL0, EL1, and EL2 System register
+ * accesses to all Performance Monitors registers do not trap to EL3.
*/
mdcr_el3 = (mdcr_el3 | MDCR_SCCD_BIT | MDCR_MCCD_BIT) &
- ~(MDCR_MPMX_BIT | MDCR_SPME_BIT);
+ ~(MDCR_MPMX_BIT | MDCR_SPME_BIT | MDCR_TPM_BIT);
mdcr_el3 = mtpmu_disable_el3(mdcr_el3);
write_mdcr_el3(mdcr_el3);
diff --git a/lib/extensions/sys_reg_trace/aarch64/sys_reg_trace.c b/lib/extensions/sys_reg_trace/aarch64/sys_reg_trace.c
index 4b57f67..1349566 100644
--- a/lib/extensions/sys_reg_trace/aarch64/sys_reg_trace.c
+++ b/lib/extensions/sys_reg_trace/aarch64/sys_reg_trace.c
@@ -12,16 +12,26 @@
void sys_reg_trace_enable(cpu_context_t *ctx)
{
- uint64_t val;
+ /*
+ * CPTR_EL3.TTA: Set to zero so that System register accesses to the
+ * trace registers do not trap to EL3.
+ */
+ uint64_t val = read_ctx_reg(get_el3state_ctx(ctx), CTX_CPTR_EL3);
+
+ val &= ~(TTA_BIT);
+ write_ctx_reg(get_el3state_ctx(ctx), CTX_CPTR_EL3, val);
+}
- /* Retrieve CPTR_EL3 value from the given context 'ctx',
- * and update CPTR_EL3.TTA bit to 0.
- * This function is called while switching context to NS to
- * allow system trace register access to NS-EL2 and NS-EL1
- * when NS-EL2 is implemented but not used.
+void sys_reg_trace_disable(cpu_context_t *ctx)
+{
+ /*
+ * CPTR_EL3.TTA: Set to one so that System register accesses to the
+ * trace registers trap to EL3, unless it is trapped by CPACR.TRCDIS,
+ * CPACR_EL1.TTA, or CPTR_EL2.TTA
*/
- val = read_ctx_reg(get_el3state_ctx(ctx), CTX_CPTR_EL3);
- val &= ~TTA_BIT;
+ uint64_t val = read_ctx_reg(get_el3state_ctx(ctx), CTX_CPTR_EL3);
+
+ val |= TTA_BIT;
write_ctx_reg(get_el3state_ctx(ctx), CTX_CPTR_EL3, val);
}
diff --git a/lib/extensions/trbe/trbe.c b/lib/extensions/trbe/trbe.c
index 461ea73..d4fbdfb 100644
--- a/lib/extensions/trbe/trbe.c
+++ b/lib/extensions/trbe/trbe.c
@@ -24,13 +24,18 @@
u_register_t val;
/*
+ * MDCR_EL3.NSTBE = 0b0
+ * Trace Buffer owning Security state is Non-secure state. If FEAT_RME
+ * is not implemented, this field is RES0.
+ *
* MDCR_EL3.NSTB = 0b11
- * Allow access of trace buffer control registers from NS-EL1
- * and NS-EL2, tracing is prohibited in Secure and Realm state
- * (if implemented).
+ * Allow access of trace buffer control registers from NS-EL1 and
+ * NS-EL2, tracing is prohibited in Secure and Realm state (if
+ * implemented).
*/
val = read_mdcr_el3();
val |= MDCR_NSTB(MDCR_NSTB_EL1);
+ val &= ~(MDCR_NSTBE_BIT);
write_mdcr_el3(val);
}