Mailing List Archive

[xen staging] xen/arm: Sanitize cpuinfo ID registers fields
commit 0bf9efb9ee4cc6cdafbe0a0ed1529449bb9d6825
Author: Bertrand Marquis <bertrand.marquis@arm.com>
AuthorDate: Thu Sep 16 07:25:37 2021 +0100
Commit: Stefano Stabellini <stefano.stabellini@xilinx.com>
CommitDate: Thu Sep 16 14:15:44 2021 -0700

xen/arm: Sanitize cpuinfo ID registers fields

Define a sanitize_cpu function to be called on secondary cores to
sanitize the system cpuinfo structure.

The safest value is taken when possible and the system is marked tainted
if we encounter values which are incompatible with each other.

Call the update_system_features function on all secondary cores that are
kept running and taint the system if different midr are found between
cores but hmp-unsafe=true was passed on Xen command line.

This is only supported on arm64 so update_system_features is an empty
static inline on arm32.

The patch is adding a new TAINT_CPU_OUT_OF_SPEC to warn the user if
Xen is running on a system with features differences between cores which
are not supported.

The patch is disabling CTR_EL0, DCZID_EL0 and ZCRusing #if 0 with a TODO
as this patch is not handling sanitization of those registers.
CTR_EL0/DCZID will be handled in a future patch to properly handle
different cache attributes when possible.
ZCR should be sanitize once we add support for SVE in Xen.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
xen/arch/arm/arm64/Makefile | 1 +
xen/arch/arm/arm64/cpufeature.c | 117 +++++++++++++++++++++++++++++++++++++++
xen/arch/arm/smpboot.c | 34 +++++++++---
xen/common/kernel.c | 6 +-
xen/include/asm-arm/cpufeature.h | 9 +++
xen/include/xen/lib.h | 1 +
6 files changed, 158 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 40642ff574..701d66883d 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -1,6 +1,7 @@
obj-y += lib/

obj-y += cache.o
+obj-y += cpufeature.o
obj-$(CONFIG_HARDEN_BRANCH_PREDICTOR) += bpi.o
obj-$(CONFIG_EARLY_PRINTK) += debug.o
obj-y += domctl.o
diff --git a/xen/arch/arm/arm64/cpufeature.c b/xen/arch/arm/arm64/cpufeature.c
index 152086dc9d..58596495a8 100644
--- a/xen/arch/arm/arm64/cpufeature.c
+++ b/xen/arch/arm/arm64/cpufeature.c
@@ -275,6 +275,9 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {
ARM64_FTR_END,
};

+#if 0
+/* TODO: use this to sanitize the cache line size among cores */
+
static const struct arm64_ftr_bits ftr_ctr[] = {
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_EXACT, 31, 1, 1), /* RES1 */
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, CTR_DIC_SHIFT, 1, 1),
@@ -291,6 +294,7 @@ static const struct arm64_ftr_bits ftr_ctr[] = {
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, CTR_IMINLINE_SHIFT, 4, 0),
ARM64_FTR_END,
};
+#endif

static const struct arm64_ftr_bits ftr_id_mmfr0[] = {
S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_MMFR0_INNERSHR_SHIFT, 4, 0xf),
@@ -325,11 +329,14 @@ static const struct arm64_ftr_bits ftr_mvfr2[] = {
ARM64_FTR_END,
};

+#if 0
+/* TODO: handle this when sanitizing cache related registers */
static const struct arm64_ftr_bits ftr_dczid[] = {
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_EXACT, DCZID_DZP_SHIFT, 1, 1),
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, DCZID_BS_SHIFT, 4, 0),
ARM64_FTR_END,
};
+#endif

static const struct arm64_ftr_bits ftr_id_isar0[] = {
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_ISAR0_DIVIDE_SHIFT, 4, 0),
@@ -444,11 +451,15 @@ static const struct arm64_ftr_bits ftr_id_dfr1[] = {
ARM64_FTR_END,
};

+#if 0
+/* TODO: use this to sanitize SVE once we support it */
+
static const struct arm64_ftr_bits ftr_zcr[] = {
ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE,
ZCR_ELx_LEN_SHIFT, ZCR_ELx_LEN_SIZE, 0), /* LEN */
ARM64_FTR_END,
};
+#endif

/*
* Common ftr bits for a 32bit register with all hidden, strict
@@ -512,3 +523,109 @@ static s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp, s64 new,
* End of imported linux structures and code
*/

+static void sanitize_reg(u64 *cur_reg, u64 new_reg, const char *reg_name,
+ const struct arm64_ftr_bits *ftrp)
+{
+ int taint = 0;
+ u64 old_reg = *cur_reg;
+
+ for (;ftrp->width != 0;ftrp++)
+ {
+ s64 cur_field = arm64_ftr_value(ftrp, *cur_reg);
+ s64 new_field = arm64_ftr_value(ftrp, new_reg);
+
+ if (cur_field == new_field)
+ continue;
+
+ if (ftrp->strict)
+ taint = 1;
+
+ *cur_reg = arm64_ftr_set_value(ftrp, *cur_reg,
+ arm64_ftr_safe_value(ftrp, new_field, cur_field));
+ }
+
+ if (old_reg != new_reg)
+ printk(XENLOG_DEBUG "SANITY DIF: %s 0x%"PRIx64" -> 0x%"PRIx64"\n",
+ reg_name, old_reg, new_reg);
+ if (old_reg != *cur_reg)
+ printk(XENLOG_DEBUG "SANITY FIX: %s 0x%"PRIx64" -> 0x%"PRIx64"\n",
+ reg_name, old_reg, *cur_reg);
+
+ if (taint)
+ {
+ printk(XENLOG_WARNING "SANITY CHECK: Unexpected variation in %s.\n",
+ reg_name);
+ add_taint(TAINT_CPU_OUT_OF_SPEC);
+ }
+}
+
+
+/*
+ * This function should be called on secondary cores to sanitize the boot cpu
+ * cpuinfo.
+ */
+void update_system_features(const struct cpuinfo_arm *new)
+{
+
+#define SANITIZE_REG(field, num, reg) \
+ sanitize_reg(&system_cpuinfo.field.bits[num], new->field.bits[num], \
+ #reg, ftr_##reg)
+
+#define SANITIZE_ID_REG(field, num, reg) \
+ sanitize_reg(&system_cpuinfo.field.bits[num], new->field.bits[num], \
+ #reg, ftr_id_##reg)
+
+#define SANITIZE_RAZ_REG(field, num, reg) \
+ sanitize_reg(&system_cpuinfo.field.bits[num], new->field.bits[num], \
+ #reg, ftr_raz)
+
+#define SANITIZE_GENERIC_REG(field, num, reg) \
+ sanitize_reg(&system_cpuinfo.field.bits[num], new->field.bits[num], \
+ #reg, ftr_generic_32bits)
+
+ SANITIZE_ID_REG(pfr64, 0, aa64pfr0);
+ SANITIZE_ID_REG(pfr64, 1, aa64pfr1);
+
+ SANITIZE_ID_REG(dbg64, 0, aa64dfr0);
+ SANITIZE_RAZ_REG(dbg64, 1, aa64dfr1);
+
+ SANITIZE_ID_REG(mm64, 0, aa64mmfr0);
+ SANITIZE_ID_REG(mm64, 1, aa64mmfr1);
+ SANITIZE_ID_REG(mm64, 2, aa64mmfr2);
+
+ SANITIZE_ID_REG(isa64, 0, aa64isar0);
+ SANITIZE_ID_REG(isa64, 1, aa64isar1);
+
+ SANITIZE_ID_REG(zfr64, 0, aa64zfr0);
+
+ if ( cpu_feature64_has_el0_32(&system_cpuinfo) )
+ {
+ SANITIZE_ID_REG(pfr32, 0, pfr0);
+ SANITIZE_ID_REG(pfr32, 1, pfr1);
+ SANITIZE_ID_REG(pfr32, 2, pfr2);
+
+ SANITIZE_ID_REG(dbg32, 0, dfr0);
+ SANITIZE_ID_REG(dbg32, 1, dfr1);
+
+ SANITIZE_ID_REG(mm32, 0, mmfr0);
+ SANITIZE_GENERIC_REG(mm32, 1, mmfr1);
+ SANITIZE_GENERIC_REG(mm32, 2, mmfr2);
+ SANITIZE_GENERIC_REG(mm32, 3, mmfr3);
+ SANITIZE_ID_REG(mm32, 4, mmfr4);
+ SANITIZE_ID_REG(mm32, 5, mmfr5);
+
+ SANITIZE_ID_REG(isa32, 0, isar0);
+ SANITIZE_GENERIC_REG(isa32, 1, isar1);
+ SANITIZE_GENERIC_REG(isa32, 2, isar2);
+ SANITIZE_GENERIC_REG(isa32, 3, isar3);
+ SANITIZE_ID_REG(isa32, 4, isar4);
+ SANITIZE_ID_REG(isa32, 5, isar5);
+ SANITIZE_ID_REG(isa32, 6, isar6);
+
+ SANITIZE_GENERIC_REG(mvfr, 0, mvfr0);
+ SANITIZE_GENERIC_REG(mvfr, 1, mvfr1);
+#ifndef MVFR2_MAYBE_UNDEFINED
+ SANITIZE_REG(mvfr, 2, mvfr2);
+#endif
+ }
+}
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index c9f2827d56..60c0e82fc5 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -318,15 +318,26 @@ void start_secondary(void)
* is manually specified for all domains). Better to park them for
* now.
*/
- if ( !opt_hmp_unsafe &&
- current_cpu_data.midr.bits != system_cpuinfo.midr.bits )
+ if ( current_cpu_data.midr.bits != system_cpuinfo.midr.bits )
{
- printk(XENLOG_ERR
- "CPU%u MIDR (0x%"PRIregister") does not match boot CPU MIDR (0x%"PRIregister"),\n"
- XENLOG_ERR "disable cpu (see big.LITTLE.txt under docs/).\n",
- smp_processor_id(), current_cpu_data.midr.bits,
- system_cpuinfo.midr.bits);
- stop_cpu();
+ if ( !opt_hmp_unsafe )
+ {
+ printk(XENLOG_ERR
+ "CPU%u MIDR (0x%"PRIregister") does not match boot CPU MIDR (0x%"PRIregister"),\n"
+ XENLOG_ERR "disable cpu (see big.LITTLE.txt under docs/).\n",
+ smp_processor_id(), current_cpu_data.midr.bits,
+ system_cpuinfo.midr.bits);
+ stop_cpu();
+ }
+ else
+ {
+ printk(XENLOG_ERR
+ "CPU%u MIDR (0x%"PRIregister") does not match boot CPU MIDR (0x%"PRIregister"),\n"
+ XENLOG_ERR "hmp-unsafe turned on so tainting Xen and keep core on!!\n",
+ smp_processor_id(), current_cpu_data.midr.bits,
+ system_cpuinfo.midr.bits);
+ add_taint(TAINT_CPU_OUT_OF_SPEC);
+ }
}

if ( dcache_line_bytes != read_dcache_line_bytes() )
@@ -337,6 +348,13 @@ void start_secondary(void)
stop_cpu();
}

+ /*
+ * system features must be updated only if we do not stop the core or
+ * we might disable features due to a non used core (for example when
+ * booting on big cores on a big.LITTLE system with hmp_unsafe)
+ */
+ update_system_features(&current_cpu_data);
+
mmu_init_secondary_cpu();

gic_init_secondary_cpu();
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index d77756a81e..e119e5401f 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -327,6 +327,7 @@ unsigned int tainted;
* 'H' - HVM forced emulation prefix is permitted.
* 'M' - Machine had a machine check experience.
* 'U' - Platform is unsecure (usually due to an errata on the platform).
+ * 'S' - Out of spec CPU (One core has a feature incompatible with others).
*
* The string is overwritten by the next call to print_taint().
*/
@@ -334,12 +335,13 @@ char *print_tainted(char *str)
{
if ( tainted )
{
- snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c%c",
+ snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c%c%c",
tainted & TAINT_MACHINE_UNSECURE ? 'U' : ' ',
tainted & TAINT_MACHINE_CHECK ? 'M' : ' ',
tainted & TAINT_SYNC_CONSOLE ? 'C' : ' ',
tainted & TAINT_ERROR_INJECT ? 'E' : ' ',
- tainted & TAINT_HVM_FEP ? 'H' : ' ');
+ tainted & TAINT_HVM_FEP ? 'H' : ' ',
+ tainted & TAINT_CPU_OUT_OF_SPEC ? 'S' : ' ');
}
else
{
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 8f2b8e7830..52cb3133e0 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -330,6 +330,15 @@ extern struct cpuinfo_arm system_cpuinfo;

extern void identify_cpu(struct cpuinfo_arm *);

+#ifdef CONFIG_ARM_64
+extern void update_system_features(const struct cpuinfo_arm *);
+#else
+static inline void update_system_features(const struct cpuinfo_arm *cpuinfo)
+{
+ /* Not supported on arm32 */
+}
+#endif
+
extern struct cpuinfo_arm cpu_data[];
#define current_cpu_data cpu_data[smp_processor_id()]

diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 1198c7c0b2..c6987973bf 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -192,6 +192,7 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c);
#define TAINT_ERROR_INJECT (1u << 2)
#define TAINT_HVM_FEP (1u << 3)
#define TAINT_MACHINE_UNSECURE (1u << 4)
+#define TAINT_CPU_OUT_OF_SPEC (1u << 5)
extern unsigned int tainted;
#define TAINT_STRING_MAX_LEN 20
extern char *print_tainted(char *str);
--
generated by git-patchbot for /home/xen/git/xen.git#staging