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>

Makefile

@@ -1143,7 +1143,6 @@ $(eval $(call assert_booleans,\
         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 @@ $(eval $(call assert_numerics,\
         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 \

bl31/bl31.mk

@@ -120,7 +120,7 @@ ifneq (${ENABLE_BRBE_FOR_NS},0)
 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
 

bl32/sp_min/sp_min.mk

@@ -46,7 +46,7 @@ BL32_SOURCES		+=	services/std_svc/trng/trng_main.c	\
 				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
 

docs/getting_started/build-options.rst

@@ -1079,10 +1079,11 @@ Common build options
   ``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),

include/arch/aarch32/arch_features.h

@@ -43,6 +43,24 @@ static inline bool is_feat_trf_supported(void)
 	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 */

include/arch/aarch64/arch_features.h

@@ -320,6 +320,24 @@ static inline bool is_armv8_4_feat_dit_present(void)
 		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)
  ************************************************************************/

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 */

lib/el3_runtime/aarch32/context_mgmt.c

@@ -140,9 +140,9 @@ static void enable_extensions_nonsecure(bool el2_unused)
 	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();

lib/el3_runtime/aarch64/context_mgmt.c

@@ -510,9 +510,9 @@ static void manage_extensions_nonsecure(bool el2_unused, cpu_context_t *ctx)
 		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();

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);
 }

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);
 }

plat/arm/board/fvp/platform.mk

@@ -460,7 +460,7 @@ endif
 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