Hi Oliver,
On Thu, Jan 30, 2025 at 12:46:15PM -0800, Oliver Upton wrote:
There are a few places where the idreg overrides are read w/ the MMU off, for example the VHE and hVHE checks in __finalise_el2. And while the infrastructure gets this _mostly_ right (i.e. does the appropriate cache maintenance), the placement of the data itself is problematic and could share a cache line with something else.
Depending on how unforgiving an implementation's handling of mismatched attributes is, this could lead to data corruption. In one observed case, the system_cpucaps shared a line with arm64_sw_feature_override and the cpucaps got nuked after entering the hyp stub...
This doesn't sound right. Non-cacheable/Device reads should not lead to corruption of a cached copy regardless of whether that cached copy is clean or dirty.
The corruption suggests that either we're performing a *write* with mismatched attributes (in which case the use of .mmuoff.data.read below isn't quite right), or we have a plan invalidate somewhere without a clean (and e.g. something else might need to be moved into .mmuoff.data.write).
Seconding Ard's point, I think we need to understand this scenario better.
To be clear, I think moving all the overrides into .mmuoff.data.read makes sense, but it doesn't explain the problem above, and it seems like there must be a latent issue (which this might only mask rather than solve).
Mark.
Even though only a few overrides are read without the MMU on, just throw the whole lot into the mmuoff section and be done with it.
Cc: stable@vger.kernel.org # v5.15+ Tested-by: Moritz Fischer moritzf@google.com Tested-by: Pedro Martelletto martelletto@google.com Reported-by: Jon Masters jonmasters@google.com Signed-off-by: Oliver Upton oliver.upton@linux.dev
arch/arm64/kernel/cpufeature.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index d41128e37701..92506d9f90db 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -755,17 +755,20 @@ static const struct arm64_ftr_bits ftr_raz[] = { #define ARM64_FTR_REG(id, table) \ __ARM64_FTR_REG_OVERRIDE(#id, id, table, &no_override) -struct arm64_ftr_override id_aa64mmfr0_override; -struct arm64_ftr_override id_aa64mmfr1_override; -struct arm64_ftr_override id_aa64mmfr2_override; -struct arm64_ftr_override id_aa64pfr0_override; -struct arm64_ftr_override id_aa64pfr1_override; -struct arm64_ftr_override id_aa64zfr0_override; -struct arm64_ftr_override id_aa64smfr0_override; -struct arm64_ftr_override id_aa64isar1_override; -struct arm64_ftr_override id_aa64isar2_override;
-struct arm64_ftr_override arm64_sw_feature_override; +#define DEFINE_FTR_OVERRIDE(name) \
- struct arm64_ftr_override __section(".mmuoff.data.read") name
+DEFINE_FTR_OVERRIDE(id_aa64mmfr0_override); +DEFINE_FTR_OVERRIDE(id_aa64mmfr1_override); +DEFINE_FTR_OVERRIDE(id_aa64mmfr2_override); +DEFINE_FTR_OVERRIDE(id_aa64pfr0_override); +DEFINE_FTR_OVERRIDE(id_aa64pfr1_override); +DEFINE_FTR_OVERRIDE(id_aa64zfr0_override); +DEFINE_FTR_OVERRIDE(id_aa64smfr0_override); +DEFINE_FTR_OVERRIDE(id_aa64isar1_override); +DEFINE_FTR_OVERRIDE(id_aa64isar2_override);
+DEFINE_FTR_OVERRIDE(arm64_sw_feature_override); static const struct __ftr_reg_entry { u32 sys_id;
base-commit: 1dd3393696efba1598aa7692939bba99d0cffae3
2.39.5