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

[Fuad Tabba]

Hi Marc,

On Fri, 20 Feb 2026 at 14:52, Marc Zyngier <maz at kernel.org> wrote:
>
> On Fri, 20 Feb 2026 11:06:03 +0000,
> Fuad Tabba <tabba at 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.

I didn't think that much about it, but I guess features designated as
FTR_HIGHER_SAFE (like SpecSEI) usually represent hardware errata or
critical mitigations. So we should not be hiding them at all.

> >
> > 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);
>

Yes, I think WARN_ON() here is the right call.

That said, I still think you should explicitly short-circuit
update_cpu_ftr_reg() for FTR_ALL_HIDDEN features, in addition to the
WARN_ON(). Relying on arm64_ftr_safe_value() to naturally preserve the
safe_val during secondary CPU boot seems mathematically fragile.

Take MTE_frac as an example. It uses S_ARM64_FTR_BITS and
FTR_LOWER_SAFE with a safe_val of 0. If it were marked FTR_ALL_HIDDEN,
init_cpu_ftr_reg() would prime sys_val with 0. But if a secondary CPU
boots and reports -1 (NI), arm64_ftr_safe_value() will execute min(-1,
0) and return -1. update_cpu_ftr_reg() will then overwrite the primed
safe_val (0) with -1. The "hidden" state established by the boot CPU
is gone, and the feature's hardware state is now exposed globally.

Note that MTE is currently ALL_HIDDEN when configured out, so it's not
totally inconceivable that someone decides to make MTE_frac ALL_HIDDEN
as well. Explicitly short-circuiting for FTR_ALL_HIDDEN features in
update_cpu_ftr_reg() seems to be the safer bet here.

Cheers,
/fuad

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