| Message ID | 20210118094533.2874082-10-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Early CPU feature override, and applications to VHE, BTI and PAuth | expand |
On Mon, Jan 18, 2021 at 09:45:21AM +0000, Marc Zyngier wrote: > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > index 9a555809b89c..465d2cb63bfc 100644 > --- a/arch/arm64/include/asm/cpufeature.h > +++ b/arch/arm64/include/asm/cpufeature.h > @@ -75,6 +75,8 @@ struct arm64_ftr_reg { > u64 sys_val; > u64 user_val; > const struct arm64_ftr_bits *ftr_bits; > + u64 *override_val; > + u64 *override_mask; > }; > > extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0; > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index e99eddec0a46..aaa075c6f029 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -544,13 +544,17 @@ static const struct arm64_ftr_bits ftr_raz[] = { > ARM64_FTR_END, > }; > > -#define ARM64_FTR_REG(id, table) { \ > - .sys_id = id, \ > - .reg = &(struct arm64_ftr_reg){ \ > - .name = #id, \ > - .ftr_bits = &((table)[0]), \ > +#define ARM64_FTR_REG_OVERRIDE(id, table, v, m) { \ > + .sys_id = id, \ > + .reg = &(struct arm64_ftr_reg){ \ > + .name = #id, \ > + .ftr_bits = &((table)[0]), \ > + .override_val = v, \ > + .override_mask = m, \ > }} > > +#define ARM64_FTR_REG(id, table) ARM64_FTR_REG_OVERRIDE(id, table, NULL, NULL) > + > static const struct __ftr_reg_entry { > u32 sys_id; > struct arm64_ftr_reg *reg; > @@ -760,6 +764,7 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new) > u64 strict_mask = ~0x0ULL; > u64 user_mask = 0; > u64 valid_mask = 0; > + u64 override_val = 0, override_mask = 0; > > const struct arm64_ftr_bits *ftrp; > struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg); > @@ -767,9 +772,38 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new) > if (!reg) > return; > > + if (reg->override_mask && reg->override_val) { > + override_mask = *reg->override_mask; > + override_val = *reg->override_val; > + } > + > for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) { > u64 ftr_mask = arm64_ftr_mask(ftrp); > s64 ftr_new = arm64_ftr_value(ftrp, new); > + s64 ftr_ovr = arm64_ftr_value(ftrp, override_val); > + > + if ((ftr_mask & override_mask) == ftr_mask) { > + s64 tmp = arm64_ftr_safe_value(ftrp, ftr_ovr, ftr_new); > + char *str = NULL; > + > + if (ftr_ovr != tmp) { > + /* Unsafe, remove the override */ > + *reg->override_mask &= ~ftr_mask; > + *reg->override_val &= ~ftr_mask; Do we need such clearing here? I don't think that's ever called again for this feature/reg. > + tmp = ftr_ovr; > + str = "ignoring override"; > + } else if (ftr_new != tmp) { > + /* Override was valid */ > + ftr_new = tmp; > + str = "forced"; > + } > + > + if (str) > + pr_warn("%s[%d:%d]: %s to %llx\n", > + reg->name, > + ftrp->shift + ftrp->width - 1, > + ftrp->shift, str, tmp); > + } > > val = arm64_ftr_set_value(ftrp, val, ftr_new); I wonder whether we could call, after init_cpu_ftr_reg(), a new function similar to update_cpu_ftr_reg() that takes a mask and value and leave init_cpu_ftr_reg() unchanged. The only advantage would be if we can get rid of the reg->override* fields. Anyway, I need to read the rest of the series to see whether it's possible. Otherwise this patch looks fine.
On 1/18/21 9:45 AM, Marc Zyngier wrote: > Add a facility to globally override a feature, no matter what > the HW says. Yes, this sounds dangerous, but we do respect the > "safe" value for a given feature. This doesn't mean the user > doesn't need to know what they are doing. > > Nothing uses this yet, so we are pretty safe. For now. > > Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 9a555809b89c..465d2cb63bfc 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -75,6 +75,8 @@ struct arm64_ftr_reg { u64 sys_val; u64 user_val; const struct arm64_ftr_bits *ftr_bits; + u64 *override_val; + u64 *override_mask; }; extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0; diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index e99eddec0a46..aaa075c6f029 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -544,13 +544,17 @@ static const struct arm64_ftr_bits ftr_raz[] = { ARM64_FTR_END, }; -#define ARM64_FTR_REG(id, table) { \ - .sys_id = id, \ - .reg = &(struct arm64_ftr_reg){ \ - .name = #id, \ - .ftr_bits = &((table)[0]), \ +#define ARM64_FTR_REG_OVERRIDE(id, table, v, m) { \ + .sys_id = id, \ + .reg = &(struct arm64_ftr_reg){ \ + .name = #id, \ + .ftr_bits = &((table)[0]), \ + .override_val = v, \ + .override_mask = m, \ }} +#define ARM64_FTR_REG(id, table) ARM64_FTR_REG_OVERRIDE(id, table, NULL, NULL) + static const struct __ftr_reg_entry { u32 sys_id; struct arm64_ftr_reg *reg; @@ -760,6 +764,7 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new) u64 strict_mask = ~0x0ULL; u64 user_mask = 0; u64 valid_mask = 0; + u64 override_val = 0, override_mask = 0; const struct arm64_ftr_bits *ftrp; struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg); @@ -767,9 +772,38 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new) if (!reg) return; + if (reg->override_mask && reg->override_val) { + override_mask = *reg->override_mask; + override_val = *reg->override_val; + } + for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) { u64 ftr_mask = arm64_ftr_mask(ftrp); s64 ftr_new = arm64_ftr_value(ftrp, new); + s64 ftr_ovr = arm64_ftr_value(ftrp, override_val); + + if ((ftr_mask & override_mask) == ftr_mask) { + s64 tmp = arm64_ftr_safe_value(ftrp, ftr_ovr, ftr_new); + char *str = NULL; + + if (ftr_ovr != tmp) { + /* Unsafe, remove the override */ + *reg->override_mask &= ~ftr_mask; + *reg->override_val &= ~ftr_mask; + tmp = ftr_ovr; + str = "ignoring override"; + } else if (ftr_new != tmp) { + /* Override was valid */ + ftr_new = tmp; + str = "forced"; + } + + if (str) + pr_warn("%s[%d:%d]: %s to %llx\n", + reg->name, + ftrp->shift + ftrp->width - 1, + ftrp->shift, str, tmp); + } val = arm64_ftr_set_value(ftrp, val, ftr_new);
Add a facility to globally override a feature, no matter what the HW says. Yes, this sounds dangerous, but we do respect the "safe" value for a given feature. This doesn't mean the user doesn't need to know what they are doing. Nothing uses this yet, so we are pretty safe. For now. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/include/asm/cpufeature.h | 2 ++ arch/arm64/kernel/cpufeature.c | 44 +++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 5 deletions(-)