Re: [PATCH 1/9] arm64: Add logic to fully remove features from sanitised id registers

Marc Zyngier
2026-02-20T14:52:10Z

On Fri, 20 Feb 2026 11:06:03 +0000,
Fuad Tabba <tabba@google.com> wrote:
>
> > > > + switch (ftrp->visibility) {
> > > > + case FTR_VISIBLE:
> > > > + val = arm64_ftr_set_value(ftrp, val, ftr_new);
> > > > user_mask |= ftr_mask;
> > > > - else
> > > > + break;
> > > > + case FTR_ALL_HIDDEN:
> > > > + val = arm64_ftr_set_value(ftrp, val, ftrp->safe_val);
> > > > + reg->user_val = arm64_ftr_set_value(ftrp,
> > > > + reg->user_val,
> > > > + ftrp->safe_val);
> > >
> > > Should we also take the safe value in update_cpu_ftr_reg() for FTR_ALL_HIDDEN?
> >
> > I would expect arm64_ftr_safe_value() to do the right thing at that
> > stage, given that we have primed the boot CPU with the safe value, and
> > that we rely on that bootstrap to make the registers converge towards
> > something safe. This is also what happens for the command-line override.
> >
> > Or have you spotted a case where this go wrong?
>
> I think so... What if a future FTR_ALL_HIDDEN feature is defined as
> FTR_HIGHER_SAFE? Wouldn't that cause problems on secondary CPUs?
> init_cpu_ftr_reg() primes sys_val with safe_val on the boot CPU,
> update_cpu_ftr_reg() on secondary CPUs compares the hardware value
> (ftr_new) against safe_val (ftr_cur). For FTR_HIGHER_SAFE,
> arm64_ftr_safe_value() returns max(ftr_new, safe_val). Since the
> hardware value is higher, update_cpu_ftr_reg() overwrites sys_val with
> the hardware value, resurrecting the hidden feature globally.

Huh, that's an interesting observation.

SpecSEI is the only case we currently deal with that is
HIGHER_SAFE. But look at what this feature describes: bloody
speculative SErrors! Not taking this into account could be really
deadly, and the kernel really ought to know about it.

>
> The features in this patch are FTR_LOWER_SAFE or FTR_EXACT (which
> happen to sink to safe_val), which is why it's not a problem with
> these current features.

My conclusion is that it is simply not safe to make such feature
conditional in any way. Note that's also the case of for an override:
look at how we will refuse to downgrade a value in init_cpu_ftr_reg():

if ((ftr_mask & reg->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";
[...]

I think we must prevent this downgrade the same way, meaning that
ALL_HIDDEN and FTR_HIGHER are mutually exclusive.

How about that:

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index d58931e63a0b6..2cae00b4b0c5f 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1067,7 +1067,14 @@ static void init_cpu_ftr_reg(u32 sys_reg, u64 new)
user_mask |= ftr_mask;
break;
case FTR_ALL_HIDDEN:
- val = arm64_ftr_set_value(ftrp, val, ftrp->safe_val);
+ /*
+ * ALL_HIDDEN and HIGHER_SAFE are incompatible.
+ * Only hide from userspace, and log the oddity.
+ */
+ if (WARN_ON(ftrp->type == FTR_HIGHER_SAFE))
+ val = arm64_ftr_set_value(ftrp, val, ftr_new);
+ else
+ val = arm64_ftr_set_value(ftrp, val, ftrp->safe_val);
reg->user_val = arm64_ftr_set_value(ftrp,
reg->user_val,
ftrp->safe_val);

Thanks,

M.

--
Without deviation from the norm, progress is not possible.