From 13f4a25251cc6ce0230e999f39a4668cff25dcd0 Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Fri, 10 Jan 2025 12:04:50 +0000 Subject: [PATCH] fix(cm): change back owning security state when a feature is disabled Patch fc7dca72ba656e5f147487b20f9f0fb6eb39e116 changed the owning security states of the TRBE and SPE buffers to NS. The thinking was that this would assist SMCCC feature availability to more easily determine if the feature is enabled or disabled. However, that only changed bit 0 while the SMCCC feature only looks at bit 1 so this change is redundant. It was also meant to tighten security but that was done by 73d98e37593f4a4044dd28f52127cdc890911c0c instead. Annoyingly, FEAT_TRBE has TRBIDR_EL1 which reports that programming is allowed when the current security state owns the buffer even when the MDCR_EL3 setting disallows this in practice. So revert the functional aspect of the patch as it causes linux panics with ERRATA_A520_2938996. Keep the defines as they are used elsewhere. Change-Id: I39463d585df89aee44d1996137616da85d678f41 Signed-off-by: Boyan Karatotev --- lib/extensions/spe/spe.c | 13 ++++--------- lib/extensions/trbe/trbe.c | 5 ++--- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/lib/extensions/spe/spe.c b/lib/extensions/spe/spe.c index 8edba00498..d7df2674d2 100644 --- a/lib/extensions/spe/spe.c +++ b/lib/extensions/spe/spe.c @@ -42,19 +42,14 @@ void spe_disable(cpu_context_t *ctx) u_register_t mdcr_el3_val = read_ctx_reg(state, CTX_MDCR_EL3); /* - * MDCR_EL3.NSPB: set to 0x2. After, Non-Secure state owns - * the Profiling Buffer and accesses to Statistical Profiling and Profiling - * Buffer control registers at EL2 and EL1 generate Trap exceptions to EL3. - * Profiling is disabled in Secure and Realm states. - * - * MDCR_EL3.NSPBE: Don't care as it was cleared during spe_enable and setting - * this to 1 does not make sense as NSPBE{1} and NSPB{0b0x} is RESERVED. + * 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): Clear the bit to trap access of PMSNEVFR_EL1 * from EL2/EL1 to EL3. */ - mdcr_el3_val &= ~(MDCR_NSPB(MDCR_NSPB_EL1) | MDCR_EnPMSN_BIT); - mdcr_el3_val |= MDCR_NSPB(MDCR_NSPB_EL3); + mdcr_el3_val &= ~(MDCR_NSPB(MDCR_NSPB_EL1) | MDCR_NSPBE_BIT | MDCR_EnPMSN_BIT); write_ctx_reg(state, CTX_MDCR_EL3, mdcr_el3_val); } diff --git a/lib/extensions/trbe/trbe.c b/lib/extensions/trbe/trbe.c index d8eb4c29b6..24d88aee59 100644 --- a/lib/extensions/trbe/trbe.c +++ b/lib/extensions/trbe/trbe.c @@ -35,13 +35,12 @@ void trbe_disable(cpu_context_t *ctx) u_register_t mdcr_el3_val = read_ctx_reg(state, CTX_MDCR_EL3); /* - * MDCR_EL3.{NSTBE,NSTB} = 0b0, 0b10 + * MDCR_EL3.{NSTBE,NSTB} = 0b0, 0b00 * Disable access of trace buffer control registers from lower ELs in - * any security state. Non-secure owns the buffer. + * any security state. Secure state owns the buffer. */ mdcr_el3_val &= ~(MDCR_NSTB(MDCR_NSTB_EL1)); mdcr_el3_val &= ~(MDCR_NSTBE_BIT); - mdcr_el3_val |= MDCR_NSTB(MDCR_NSTB_EL3); write_ctx_reg(state, CTX_MDCR_EL3, mdcr_el3_val); }