Skip to content

Commit

Permalink
xen/arm: Sanitize cpuinfo ID registers fields
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
bertrand-marquis authored and Stefano Stabellini committed Sep 16, 2021
1 parent 3b78e27 commit 0bf9efb
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 10 deletions.
1 change: 1 addition & 0 deletions xen/arch/arm/arm64/Makefile
Original file line number Diff line number Diff line change
@@ -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
Expand Down
117 changes: 117 additions & 0 deletions xen/arch/arm/arm64/cpufeature.c
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
34 changes: 26 additions & 8 deletions xen/arch/arm/smpboot.c
Original file line number Diff line number Diff line change
Expand Up @@ -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() )
Expand All @@ -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();
Expand Down
6 changes: 4 additions & 2 deletions xen/common/kernel.c
Original file line number Diff line number Diff line change
Expand Up @@ -327,19 +327,21 @@ 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().
*/
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
{
Expand Down
9 changes: 9 additions & 0 deletions xen/include/asm-arm/cpufeature.h
Original file line number Diff line number Diff line change
Expand Up @@ -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()]

Expand Down
1 change: 1 addition & 0 deletions xen/include/xen/lib.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 0bf9efb

Please sign in to comment.