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

Suzuki K Poulose
2026-02-23T18:18:29Z

On 23/02/2026 09:48, Marc Zyngier wrote:
> Hi Fuad,
>
> On Fri, 20 Feb 2026 15:36:37 +0000,
> Fuad Tabba <tabba@google.com> wrote:
>>
>>> 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.
>
> Right, the signed feature is a pretty compelling argument. And we
> should do the same thing for overrides, probably as a preliminary
> patch.
>

Suggestions look good to me and I was thinking on similar lines with the
FTR_CONFIG() (Was away last week, now back from holidays).

One minor nit: Given we now have more uses
of arm64_ftr_set_value(ftrp, x, ftrp->safe_value) , could we wrap it
into something like :

static inline s64 arm64_ftr_set_safe_value(... *ftrp, s64 val);
{
return arm64_ftr_set_value(ftrp, val, ftrp->safe_val);
}

To me that makes it way easier to comprehend what we are doing.

Suzuki

Thanks,
>
> M.
>