refactor(cpufeat): enable SYS_REG_TRACE for FEAT_STATE_CHECKED
At the moment we only support access to the trace unit by system
registers (SYS_REG_TRACE) to be either unconditionally compiled in, or
to be not supported at all.
Add support for runtime detection (ENABLE_SYS_REG_TRACE_FOR_NS=2), by
adding is_feat_sys_reg_trace_supported(). That function considers both
build time settings and runtime information (if needed), and is used
before we access SYS_REG_TRACE related registers.
The FVP platform decided to compile in support unconditionally (=1),
even though this is an optional feature, so it is not available with the
FVP model's default command line.
Change that to the now supported dynamic option (=2), so the right
decision can be made by the code at runtime.
Change-Id: I450a574a4f6bd9fc269887037049c94c906f54b2
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
diff --git a/Makefile b/Makefile
index 797b2cf..8e789de 100644
--- a/Makefile
+++ b/Makefile
@@ -1143,7 +1143,6 @@
COT_DESC_IN_DTB \
USE_SP804_TIMER \
PSA_FWU_SUPPORT \
- ENABLE_SYS_REG_TRACE_FOR_NS \
ENABLE_MPMM \
ENABLE_MPMM_FCONF \
SIMICS_BUILD \
@@ -1182,6 +1181,7 @@
ENABLE_MPAM_FOR_LOWER_ELS \
ENABLE_RME \
ENABLE_SPE_FOR_NS \
+ ENABLE_SYS_REG_TRACE_FOR_NS \
ENABLE_TRF_FOR_NS \
FW_ENC_STATUS \
NR_OF_FW_BANKS \
diff --git a/bl31/bl31.mk b/bl31/bl31.mk
index 91406cf..006843e 100644
--- a/bl31/bl31.mk
+++ b/bl31/bl31.mk
@@ -120,7 +120,7 @@
BL31_SOURCES += lib/extensions/brbe/brbe.c
endif
-ifeq (${ENABLE_SYS_REG_TRACE_FOR_NS},1)
+ifneq (${ENABLE_SYS_REG_TRACE_FOR_NS},0)
BL31_SOURCES += lib/extensions/sys_reg_trace/aarch64/sys_reg_trace.c
endif
diff --git a/bl32/sp_min/sp_min.mk b/bl32/sp_min/sp_min.mk
index 2a6612a..e85e273 100644
--- a/bl32/sp_min/sp_min.mk
+++ b/bl32/sp_min/sp_min.mk
@@ -46,7 +46,7 @@
services/std_svc/trng/trng_entropy_pool.c
endif
-ifeq (${ENABLE_SYS_REG_TRACE_FOR_NS},1)
+ifneq (${ENABLE_SYS_REG_TRACE_FOR_NS},0)
BL32_SOURCES += lib/extensions/sys_reg_trace/aarch32/sys_reg_trace.c
endif
diff --git a/docs/getting_started/build-options.rst b/docs/getting_started/build-options.rst
index 9415871..94c12de 100644
--- a/docs/getting_started/build-options.rst
+++ b/docs/getting_started/build-options.rst
@@ -1079,10 +1079,11 @@
``FEATURE_DETECTION`` mechanism. The default is 0 and it is automatically
disabled when the target architecture is AArch32.
-- ``ENABLE_SYS_REG_TRACE_FOR_NS``: Boolean option to enable trace system
+- ``ENABLE_SYS_REG_TRACE_FOR_NS``: Numeric value to enable trace system
registers access from NS ELs, NS-EL2 or NS-EL1 (when NS-EL2 is implemented
but unused). This feature is available if trace unit such as ETMv4.x, and
- ETE(extending ETM feature) is implemented. This flag is disabled by default.
+ ETE(extending ETM feature) is implemented. This flag can take the values
+ 0 to 2, to align with the ``FEATURE_DETECTION`` mechanism. The default is 0.
- ``ENABLE_TRF_FOR_NS``: Numeric value to enable trace filter control registers
access from NS ELs, NS-EL2 or NS-EL1 (when NS-EL2 is implemented but unused),
diff --git a/include/arch/aarch32/arch_features.h b/include/arch/aarch32/arch_features.h
index 12df6da..252b407 100644
--- a/include/arch/aarch32/arch_features.h
+++ b/include/arch/aarch32/arch_features.h
@@ -43,6 +43,24 @@
return read_feat_trf_id_field() != 0U;
}
+static inline unsigned int read_feat_coptrc_id_field(void)
+{
+ return ISOLATE_FIELD(read_id_dfr0(), ID_DFR0_COPTRC);
+}
+
+static inline bool is_feat_sys_reg_trace_supported(void)
+{
+ if (ENABLE_SYS_REG_TRACE_FOR_NS == FEAT_STATE_DISABLED) {
+ return false;
+ }
+
+ if (ENABLE_SYS_REG_TRACE_FOR_NS == FEAT_STATE_ALWAYS) {
+ return true;
+ }
+
+ return read_feat_coptrc_id_field() != 0U;
+}
+
static inline bool is_feat_spe_supported(void)
{
/* FEAT_SPE is AArch64 only */
diff --git a/include/arch/aarch64/arch_features.h b/include/arch/aarch64/arch_features.h
index f1a13d2..ad938ae 100644
--- a/include/arch/aarch64/arch_features.h
+++ b/include/arch/aarch64/arch_features.h
@@ -320,6 +320,24 @@
ID_AA64PFR0_DIT_MASK) == ID_AA64PFR0_DIT_SUPPORTED);
}
+static inline unsigned int read_feat_tracever_id_field(void)
+{
+ return ISOLATE_FIELD(read_id_aa64dfr0_el1(), ID_AA64DFR0_TRACEVER);
+}
+
+static inline bool is_feat_sys_reg_trace_supported(void)
+{
+ if (ENABLE_SYS_REG_TRACE_FOR_NS == FEAT_STATE_DISABLED) {
+ return false;
+ }
+
+ if (ENABLE_SYS_REG_TRACE_FOR_NS == FEAT_STATE_ALWAYS) {
+ return true;
+ }
+
+ return read_feat_tracever_id_field() != 0U;
+}
+
/*************************************************************************
* Function to identify the presence of FEAT_TRF (TraceLift)
************************************************************************/
diff --git a/include/lib/extensions/sys_reg_trace.h b/include/lib/extensions/sys_reg_trace.h
index 74470fe..5915c55 100644
--- a/include/lib/extensions/sys_reg_trace.h
+++ b/include/lib/extensions/sys_reg_trace.h
@@ -9,10 +9,24 @@
#include <context.h>
+#if ENABLE_SYS_REG_TRACE_FOR_NS
#if __aarch64__
void sys_reg_trace_enable(cpu_context_t *context);
#else
void sys_reg_trace_enable(void);
#endif /* __aarch64__ */
+#else /* !ENABLE_SYS_REG_TRACE_FOR_NS */
+
+#if __aarch64__
+static inline void sys_reg_trace_enable(cpu_context_t *context)
+{
+}
+#else
+static inline void sys_reg_trace_enable(void)
+{
+}
+#endif /* __aarch64__ */
+#endif /* ENABLE_SYS_REG_TRACE_FOR_NS */
+
#endif /* SYS_REG_TRACE_H */
diff --git a/lib/el3_runtime/aarch32/context_mgmt.c b/lib/el3_runtime/aarch32/context_mgmt.c
index e494a86..e7a0e58 100644
--- a/lib/el3_runtime/aarch32/context_mgmt.c
+++ b/lib/el3_runtime/aarch32/context_mgmt.c
@@ -140,9 +140,9 @@
amu_enable(el2_unused);
#endif
-#if ENABLE_SYS_REG_TRACE_FOR_NS
- sys_reg_trace_enable();
-#endif /* ENABLE_SYS_REG_TRACE_FOR_NS */
+ if (is_feat_sys_reg_trace_supported()) {
+ sys_reg_trace_enable();
+ }
if (is_feat_trf_supported()) {
trf_enable();
diff --git a/lib/el3_runtime/aarch64/context_mgmt.c b/lib/el3_runtime/aarch64/context_mgmt.c
index 20eb5f6..c91070e 100644
--- a/lib/el3_runtime/aarch64/context_mgmt.c
+++ b/lib/el3_runtime/aarch64/context_mgmt.c
@@ -510,9 +510,9 @@
brbe_enable();
}
-#if ENABLE_SYS_REG_TRACE_FOR_NS
- sys_reg_trace_enable(ctx);
-#endif /* ENABLE_SYS_REG_TRACE_FOR_NS */
+ if (is_feat_sys_reg_trace_supported()) {
+ sys_reg_trace_enable(ctx);
+ }
if (is_feat_trf_supported()) {
trf_enable();
diff --git a/lib/extensions/sys_reg_trace/aarch32/sys_reg_trace.c b/lib/extensions/sys_reg_trace/aarch32/sys_reg_trace.c
index 89b8029..b3f44b7 100644
--- a/lib/extensions/sys_reg_trace/aarch32/sys_reg_trace.c
+++ b/lib/extensions/sys_reg_trace/aarch32/sys_reg_trace.c
@@ -10,27 +10,16 @@
#include <arch_helpers.h>
#include <lib/extensions/sys_reg_trace.h>
-static bool sys_reg_trace_supported(void)
-{
- uint32_t features;
-
- features = read_id_dfr0() >> ID_DFR0_COPTRC_SHIFT;
- return ((features & ID_DFR0_COPTRC_MASK) ==
- ID_DFR0_COPTRC_SUPPORTED);
-}
-
void sys_reg_trace_enable(void)
{
uint32_t val;
- if (sys_reg_trace_supported()) {
- /*
- * NSACR.NSTRCDIS = b0
- * enable NS system register access to implemented trace
- * registers.
- */
- val = read_nsacr();
- val &= ~NSTRCDIS_BIT;
- write_nsacr(val);
- }
+ /*
+ * NSACR.NSTRCDIS = b0
+ * enable NS system register access to implemented trace
+ * registers.
+ */
+ val = read_nsacr();
+ val &= ~NSTRCDIS_BIT;
+ write_nsacr(val);
}
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 960d698..e61cb90 100644
--- a/lib/extensions/sys_reg_trace/aarch64/sys_reg_trace.c
+++ b/lib/extensions/sys_reg_trace/aarch64/sys_reg_trace.c
@@ -10,28 +10,17 @@
#include <arch_helpers.h>
#include <lib/extensions/sys_reg_trace.h>
-static bool sys_reg_trace_supported(void)
-{
- uint64_t features;
-
- features = read_id_aa64dfr0_el1() >> ID_AA64DFR0_TRACEVER_SHIFT;
- return ((features & ID_AA64DFR0_TRACEVER_MASK) ==
- ID_AA64DFR0_TRACEVER_SUPPORTED);
-}
-
void sys_reg_trace_enable(cpu_context_t *ctx)
{
uint64_t val;
- if (sys_reg_trace_supported()) {
- /* 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.
- */
- 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.
+ */
+ 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/plat/arm/board/fvp/platform.mk b/plat/arm/board/fvp/platform.mk
index 6acc879..d5e59cd 100644
--- a/plat/arm/board/fvp/platform.mk
+++ b/plat/arm/board/fvp/platform.mk
@@ -460,7 +460,7 @@
endif
# enable trace system registers access to NS by default
-ENABLE_SYS_REG_TRACE_FOR_NS := 1
+ENABLE_SYS_REG_TRACE_FOR_NS := 2
# enable trace filter control registers access to NS by default
ENABLE_TRF_FOR_NS := 2