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

On Fri, 20 Feb 2026 11:06:03 +0000,
Fuad Tabba [off-list ref] wrote:

quoted

quoted

quoted

+               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.