fix(cm): disable SPE/TRBE correctly
SPE and TRBE are unusual features. They have multi-bit enables whose
function is not immediately apparent and disabling them is not
straightforward.
While attempting to figure this out, the disables were made a mess of.
Patch fc7dca72b began changing the owning security state of SPE and
TRBE. This was first used in patch 79c0c7fac0 with calls to
spe_disable() and trbe_disbale(). However, patch 13f4a2525 reverted the
security state ownership, making the spe_disable() and trbe_disable()
redundant and their comments incorrect - the DoS protection is achieved
by the psb/tsb barriers on context switch, introduces separately in
f80887337 and 73d98e375.
Those patches got the behaviour full circle to what it was in fc7dca72b
so the disables can be fully removed for clarity.
However, the original method for disabling these features is not fully
correct - letting the "disabled" state be all zeroes made the features
seem enabled for secure world but they would trap. That is not a problem
while secure world doesn't use them, but could lead to some confusing
debugging in the future. NS and Realm worlds were not affected. This
patch fully establishes the pattern for SPE and TRBE's enablement,
documents it, and implements it such.
The description comments in the features boil down to 2 rules. There is
a third rule possible:
3. To enable TRBE/SPE for world X with a dirty buffer:
* world X owns the buffer
* trapping enabled
This is not listed as it would not work correctly with
SMCCC_ARCH_FEATURE_AVAILABILITY which relies on trapping to be disabled
to report correctly. If that is ever implemented, the SMCCC
implementation should be considered too.
Change-Id: I5588a3d5fc074c2445470954d8c3b172bec77d43
Signed-off-by: Boyan Karatotev <boyan.karatotev@arm.com>
diff --git a/lib/extensions/spe/spe.c b/lib/extensions/spe/spe.c
index e499486..ddd8516 100644
--- a/lib/extensions/spe/spe.c
+++ b/lib/extensions/spe/spe.c
@@ -13,47 +13,78 @@
#include <plat/common/platform.h>
-void spe_enable(cpu_context_t *ctx)
+/*
+ * SPE is an unusual feature. Its enable is split into two:
+ * - (NSPBE, NSPB[0]) - the security state bits - determines which security
+ * state owns the profiling buffer.
+ * - NSPB[1] - the enable bit - determines if the security state that owns the
+ * buffer may access SPE registers.
+ *
+ * There is a secondary id register PMBIDR_EL1 that is more granular than
+ * ID_AA64DFR0_EL1. When a security state owns the buffer, PMBIDR_EL1.P will
+ * report that SPE programming is allowed. This means that the usual assumption
+ * that leaving all bits to a default of zero will disable the feature may not
+ * work correctly. To correctly disable SPE, the current security state must NOT
+ * own the buffer, irrespective of the enable bit. Then, to play nicely with
+ * SMCCC_ARCH_FEATURE_AVAILABILITY, the enable bit should correspond to the
+ * enable status. The feature is architected this way to allow for lazy context
+ * switching of the buffer - a world can be made owner of the buffer (with
+ * PMBIDR_EL1.P reporting full access) without giving it access to the registers
+ * (by trapping to EL3). Then context switching can be deferred until a world
+ * tries to use SPE at which point access can be given and the trapping
+ * instruction repeated.
+ *
+ * This can be simplified to the following rules:
+ * 1. To enable SPE for world X:
+ * * world X owns the buffer ((NSPBE, NSPB[0]) == SCR_EL3.{NSE, NS})
+ * * trapping disabled (NSPB[0] == 1)
+ * 2. To disable SPE for world X:
+ * * world X does not own the buffer ((NSPBE, NSPB[0]) != SCR_EL3.{NSE, NS})
+ * * trapping enabled (NSPB[0] == 0)
+ */
+
+/*
+ * MDCR_EL3.EnPMSN (ARM v8.7) and MDCR_EL3.EnPMS3: Do not trap access to
+ * PMSNEVFR_EL1 or PMSDSFR_EL1 register at NS-EL1 or NS-EL2 to EL3 if
+ * FEAT_SPEv1p2 or FEAT_SPE_FDS are implemented. Setting these bits to 1 doesn't
+ * have any effect on it when the features aren't implemented.
+ */
+void spe_enable_ns(cpu_context_t *ctx)
{
el3_state_t *state = get_el3state_ctx(ctx);
u_register_t mdcr_el3_val = read_ctx_reg(state, CTX_MDCR_EL3);
- /*
- * MDCR_EL3.NSPB (ARM v8.2): SPE enabled in Non-secure state
- * and disabled in secure state. Accesses to SPE registers at
- * S-EL1 generate trap exceptions to EL3.
- *
- * MDCR_EL3.NSPBE: Profiling Buffer uses Non-secure Virtual Addresses.
- * When FEAT_RME is not implemented, this field is RES0.
- *
- * MDCR_EL3.EnPMSN (ARM v8.7) and MDCR_EL3.EnPMS3: Do not trap access to
- * PMSNEVFR_EL1 or PMSDSFR_EL1 register at NS-EL1 or NS-EL2 to EL3 if FEAT_SPEv1p2
- * or FEAT_SPE_FDS are implemented. Setting these bits to 1 doesn't have any
- * effect on it when the features aren't implemented.
- */
- mdcr_el3_val |= MDCR_NSPB(MDCR_NSPB_EL1) | MDCR_EnPMSN_BIT | MDCR_EnPMS3_BIT;
+ mdcr_el3_val |= MDCR_NSPB_EN_BIT | MDCR_NSPB_SS_BIT | MDCR_EnPMSN_BIT | MDCR_EnPMS3_BIT;
mdcr_el3_val &= ~(MDCR_NSPBE_BIT);
+
write_ctx_reg(state, CTX_MDCR_EL3, mdcr_el3_val);
}
-void spe_disable(cpu_context_t *ctx)
+/*
+ * MDCR_EL3.EnPMSN (ARM v8.7) and MDCR_EL3.EnPMS3: Clear the bits to trap access
+ * of PMSNEVFR_EL1 and PMSDSFR_EL1 from EL2/EL1 to EL3.
+ */
+static void spe_disable_others(cpu_context_t *ctx)
{
el3_state_t *state = get_el3state_ctx(ctx);
u_register_t mdcr_el3_val = read_ctx_reg(state, CTX_MDCR_EL3);
- /*
- * MDCR_EL3.{NSPB,NSPBE} = 0b00, 0b0
- * Disable access of profiling buffer control registers from lower ELs
- * in any security state. Secure state owns the buffer.
- *
- * MDCR_EL3.EnPMSN (ARM v8.7) and MDCR_EL3.EnPMS3: Clear the bits to trap access
- * of PMSNEVFR_EL1 and PMSDSFR_EL1 from EL2/EL1 to EL3.
- */
- mdcr_el3_val &= ~(MDCR_NSPB(MDCR_NSPB_EL1) | MDCR_NSPBE_BIT | MDCR_EnPMSN_BIT |
+ mdcr_el3_val |= MDCR_NSPB_SS_BIT;
+ mdcr_el3_val &= ~(MDCR_NSPB_EN_BIT | MDCR_NSPBE_BIT | MDCR_EnPMSN_BIT |
MDCR_EnPMS3_BIT);
write_ctx_reg(state, CTX_MDCR_EL3, mdcr_el3_val);
}
+void spe_disable_secure(cpu_context_t *ctx)
+{
+ spe_disable_others(ctx);
+}
+
+void spe_disable_realm(cpu_context_t *ctx)
+{
+ spe_disable_others(ctx);
+}
+
void spe_init_el2_unused(void)
{
uint64_t v;