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 797b2cfeb21b0daa32c2afbae6f55b6d0bb7cb7c..8e789def0c3388afacc03dff03e421152dd1805c 100644 (file)
--- a/Makefile
+++ b/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 \

diff --git a/bl31/bl31.mk b/bl31/bl31.mk
index 91406cfc574e71a6acfaf33cb8f8ff283ef0db19..006843efd45994bee2fd5cedccb49478a4748177 100644 (file)
--- a/bl31/bl31.mk
+++ b/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
 

diff --git a/bl32/sp_min/sp_min.mk b/bl32/sp_min/sp_min.mk
index 2a6612ad93c9044ef36b7c780cb34d01daf1690a..e85e2738c587f7c4e578993438ea3b5c42f0af5f 100644 (file)
--- a/bl32/sp_min/sp_min.mk
+++ b/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
 

diff --git a/docs/getting_started/build-options.rst b/docs/getting_started/build-options.rst
index 9415871df09c216ddf084932ee2d935a68ffc2df..94c12defeebd19a9a80c3d22e8be2eb595a8b44f 100644 (file)
--- a/docs/getting_started/build-options.rst
+++ b/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),

diff --git a/include/arch/aarch32/arch_features.h b/include/arch/aarch32/arch_features.h
index 12df6da4b2c53403c2075494c8d16f5b0d0d86d7..252b40734d672b753b80105637cc026c8d420d7d 100644 (file)
--- a/include/arch/aarch32/arch_features.h
+++ b/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 */

diff --git a/include/arch/aarch64/arch_features.h b/include/arch/aarch64/arch_features.h
index f1a13d290aef742ed30ea7daed30240f832f7bcd..ad938aea3062ad3a279e191f2363b5aa6b180e48 100644 (file)
--- a/include/arch/aarch64/arch_features.h
+++ b/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)
  ************************************************************************/

diff --git a/include/lib/extensions/sys_reg_trace.h b/include/lib/extensions/sys_reg_trace.h
index 74470fee22bfcd672e324591010dcb96e197330f..5915c5547a9788cfc55f596a3eb1d5abb5599c76 100644 (file)
--- 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 e494a86cf2ef90582280a1c6c7a079b8ba4259d7..e7a0e5871c8b3d001ca3d8dde9696732004589dd 100644 (file)
--- a/lib/el3_runtime/aarch32/context_mgmt.c
+++ b/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();

diff --git a/lib/el3_runtime/aarch64/context_mgmt.c b/lib/el3_runtime/aarch64/context_mgmt.c
index 20eb5f67d9f0b369f545f247236f799b90f09737..c91070ece129d531b012a79e1ed2cd9129a32ce9 100644 (file)
--- a/lib/el3_runtime/aarch64/context_mgmt.c
+++ b/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();

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 89b8029ca58640340917083cf782354cba1bd7a6..b3f44b7c1ee36d05ac20d0671240226f344697e6 100644 (file)
--- 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 960d69842a8c3bdf84b6ad7d891e7edc084b04e0..e61cb90375799f3eb6af8899f5823afcdae27676 100644 (file)
--- 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 6acc879e14e9b8cb80c550c42331ecd17ed4427d..d5e59cd2508737178a2b5ff058e60263e1bc2d63 100644 (file)
--- a/plat/arm/board/fvp/platform.mk
+++ b/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