[PATCH v2 01/11] arm64: Skip update of an idreg field affected by an override
Suzuki K Poulose
suzuki.poulose at arm.com
Wed Mar 25 07:54:28 PDT 2026
On 19/03/2026 15:34, Catalin Marinas wrote:
> On Mon, Mar 02, 2026 at 11:56:42AM +0000, Marc Zyngier wrote:
>> When computing the new value od an idreg that contains a field
>> affected by an override, do not update that particular field.
>>
>> The value computed at init-time must be kept as-is, as that's
>> what the user has asked for, for better or worse.
>>
>> Signed-off-by: Marc Zyngier <maz at kernel.org>
>> ---
>> arch/arm64/kernel/cpufeature.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index c31f8e17732a3..28fc77443ccd3 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -1224,6 +1224,13 @@ static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new)
>> s64 ftr_cur = arm64_ftr_value(ftrp, reg->sys_val);
>> s64 ftr_new = arm64_ftr_value(ftrp, new);
>>
>> + /*
>> + * Don't alter the initial value that has been forced
>> + * by an override.
>> + */
>> + if ((reg->override->mask & arm64_ftr_mask(ftrp)) == arm64_ftr_mask(ftrp))
>> + continue;
>
> I got lost in the in the cpufeature framework, so I may be missing
> something.
>
> Let's say the primary CPU has a feature field with value 2 and we want
> to override it to value 1. For e.g. a LOWER_SAFE feature, boot_cpu_data
> will stored the overridden value of 1.
>
> A secondary CPU comes online with the same feature missing, so value 0.
> With the above change, we no longer update the system-wide feature
> value, leave it as 1. Later on, for a system feature we may turn it on
> even though the secondary CPU does not support it.
>
> In summary, this makes the overridden field sticky for secondary CPUs
> even if they don't support it.
That is true. I think we should let the secondary CPUs alter the values,
with initial CPU feature value with the override value set, the system
could then choose the safest among the override and the others.
>
> Unrelated to your patch, I think we can similarly fail to reject
> secondary CPUs in check_early_cpu_features() -> verify_local_cpu_caps()
> because of __read_sysreg_by_encoding() which uses the override value
> unconditionally. From this perspective, we are now consistent with your
> patch above.
This is true as well and the override takes the priority and with the
wrong level of override value the system could be made to think that
some features are available even when it is unsafe to do so.
We should sanitise the values read by __read_sysreg_by_encoding() with
the "overrides". I can cook something up.
>
> In all cases we taint the kernel for FTR_STRICT features but that may go
> unnoticed or if we had FTR_NONSTRICT (does it even matter in this
> case?).
>
> Maybe that's the intended use and blame the user for passing the wrong
> override. We are still slightly inconsistent depending on what the boot
This is correct. We should at least WARN for impossible overrides on the
secondaries. (We only do that for boot CPUs today). The other issue with
this is WEAK_LOCAL cpu features where we use the capability wherever
available. May be we could reduce the severity of the warning to
pr_warn_once().
Suzuki
> CPU supports where we still decide whether accept or reject an override.
> We don't do this for secondaries.
>
> Anyway, I'm not opposing to this patch if that's what's intended. I'm
> sure I'll forget everything about this framework in a couple of weeks.
>
More information about the linux-arm-kernel
mailing list