Skip to contentSkip to conversation

arm64: Fully disable configured-out features

PATCH In review Marc Zyngier 4 participants 27 messages Mar 2 – Mar 31

Subsystems (2)
arm64 KVM arm64
lore
Share & tools
lore

Marc Zyngier

author maintainer
#0 lore raw

v2 cover arm64: Fully disable configured-out features

This is v2 of this series attempting at making the handling of features that are disabled at compile time a bit less awkward for non-userspace consumers of the sanitised feature bits.

A few things have changed, thanks to Fuad and Suzuki spending time on reviewing this, but the overall behaviour is the same.

* From v1 [1]:

- Mark ALL_HIDDEN and HIGHER_SAFE as incompatible

- Don't update a feature that is ALL_HIDDEN (or overridden) on the secondary boot path

- Reduce the size of struct arm64_ftr_bits

- Add a helper for setting a field to its safe value

[1] https://lore.kernel.org/r/[email protected]

Marc Zyngier (11):
arm64: Skip update of an idreg field affected by an override
arm64: Add a helper setting a feature field to its safe value
arm64: Add logic to fully remove features from sanitised id registers
arm64: Convert CONFIG_ARM64_PTR_AUTH to FTR_CONFIG()
arm64: Convert CONFIG_ARM64_SVE to FTR_CONFIG()
arm64: Convert CONFIG_ARM64_SME to FTR_CONFIG()
arm64: Convert CONFIG_ARM64_GCS to FTR_CONFIG()
arm64: Convert CONFIG_ARM64_MTE to FTR_CONFIG()
arm64: Convert CONFIG_ARM64_POE to FTR_CONFIG()
arm64: Convert CONFIG_ARM64_BTI to FTR_CONFIG()
arm64: Remove FTR_VISIBLE_IF_IS_ENABLED()

 arch/arm64/include/asm/cpufeature.h |  15 +--
 arch/arm64/kernel/cpufeature.c      | 141 +++++++++++++++++-----------
 2 files changed, 94 insertions(+), 62 deletions(-)
Signature · 1 line
2.47.3
author maintainer
#1 lore raw

PATCH v2 1/11 arm64: Skip update of an idreg field affected by an override

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 <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm64/kernel/cpufeature.c +7 -0
index c31f8e17732a3..28fc77443ccd3 100644
@@ -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;
+
 		if (ftr_cur == ftr_new)
 			continue;
 		/* Find a safe value */
Signature · 1 line
2.47.3
Reviewed-by on v2 1/11
#2 lore raw

Re: [PATCH v2 01/11] arm64: Skip update of an idreg field affected by an override

Hi Marc,

Quoted 2 lines · Marc Zyngier

When computing the new value od an idreg that contains a field

nit: od->of

Quoted 6 lines

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 <[email protected]>
Reviewed-by Fuad Tabba <[email protected]>

Cheers,
/fuad

Quoted 25 lines
---
 arch/arm64/kernel/cpufeature.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm64/kernel/cpufeature.c +7 -1
index c31f8e17732a3..28fc77443ccd3 100644
@@ -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;
+
                if (ftr_cur == ftr_new)
                        continue;
                /* Find a safe value */
--

2.47.3

#3 lore raw

Re: [PATCH v2 01/11] arm64: Skip update of an idreg field affected by an override

Quoted 15 lines · Fuad Tabba

Hi Marc,

Quoted 2 lines · Marc Zyngier

When computing the new value od an idreg that contains a field

nit: od->of

Quoted 6 lines

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 <[email protected]>

Another nit, should you add a fixes tag, since this flaw is tired to the original override facility? i.e.,
8f266a5d878a ("arm64: cpufeature: Add global feature override facility").

Quoted 1 line
Reviewed-by Fuad Tabba <[email protected]>

Thanks,
/fuad

Quoted 29 lines

Cheers,
/fuad

Quoted 25 lines
---
 arch/arm64/kernel/cpufeature.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm64/kernel/cpufeature.c +7 -1
index c31f8e17732a3..28fc77443ccd3 100644
@@ -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;
+
                if (ftr_cur == ftr_new)
                        continue;
                /* Find a safe value */
--

2.47.3

author maintainer
#4 lore raw

Re: [PATCH v2 01/11] arm64: Skip update of an idreg field affected by an override

Quoted 13 lines · Fuad Tabba
Quoted 11 lines · Fuad Tabba

Hi Marc,

Quoted 2 lines · Marc Zyngier

When computing the new value od an idreg that contains a field

nit: od->of

Quoted 2 lines

affected by an override, do not update that particular field.

Quoted 8 lines
Quoted 4 lines
Quoted 4 lines

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 <[email protected]>

Another nit, should you add a fixes tag, since this flaw is tired to the original override facility? i.e.,
8f266a5d878a ("arm64: cpufeature: Add global feature override facility").

Yup, good point.

Quoted 2 lines
Quoted 2 lines
Reviewed-by Fuad Tabba <[email protected]>

Thanks!

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
reviewer Reviewed-by on v2 1/11
#5 lore raw

Re: [PATCH v2 01/11] arm64: Skip update of an idreg field affected by an override

In reply to Marc Zyngier
Quoted 16 lines · Marc Zyngier

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 <[email protected]>
---
  arch/arm64/kernel/cpufeature.c | 7 +++++++
  1 file changed, 7 insertions(+)
arch/arm64/kernel/cpufeature.c +0 -0
index c31f8e17732a3..28fc77443ccd3 100644

arch/arm64/kernel/cpufeature.c static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new) · line 1233

Quoted 10 lines
arch/arm64/kernel/cpufeature.c +7 -0
@@ -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;
+
Reviewed-by Suzuki K Poulose <[email protected]>
Quoted 3 lines
arch/arm64/kernel/cpufeature.c +0 -0
  		if (ftr_cur == ftr_new)
  			continue;
  		/* Find a safe value */
maintainer
#6 lore raw

Re: [PATCH v2 01/11] arm64: Skip update of an idreg field affected by an override

In reply to Marc Zyngier
Quoted 16 lines · Marc Zyngier

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 <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm64/kernel/cpufeature.c +0 -0
index c31f8e17732a3..28fc77443ccd3 100644

arch/arm64/kernel/cpufeature.c static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new) · line 1232

Quoted 9 lines
arch/arm64/kernel/cpufeature.c +6 -0
@@ -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.

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.

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

Signature · 1 line
Catalin
reviewer
#7 lore raw

Re: [PATCH v2 01/11] arm64: Skip update of an idreg field affected by an override

Quoted 34 lines · Catalin Marinas
Quoted 16 lines · Marc Zyngier

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 <[email protected]>
---
  arch/arm64/kernel/cpufeature.c | 7 +++++++
  1 file changed, 7 insertions(+)
arch/arm64/kernel/cpufeature.c +0 -0
index c31f8e17732a3..28fc77443ccd3 100644

arch/arm64/kernel/cpufeature.c static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new) · line 1232

Quoted 9 lines
arch/arm64/kernel/cpufeature.c +6 -0
@@ -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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 7 lines

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

Quoted 6 lines

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.

maintainer
#8 lore raw

Re: [PATCH v2 01/11] arm64: Skip update of an idreg field affected by an override

Quoted 39 lines · Suzuki K Poulose
Quoted 38 lines · Catalin Marinas
Quoted 16 lines · Marc Zyngier

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 <[email protected]>
---
  arch/arm64/kernel/cpufeature.c | 7 +++++++
  1 file changed, 7 insertions(+)
arch/arm64/kernel/cpufeature.c +0 -0
index c31f8e17732a3..28fc77443ccd3 100644

arch/arm64/kernel/cpufeature.c static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new) · line 1231

Quoted 8 lines
arch/arm64/kernel/cpufeature.c +6 -0
@@ -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.

Quoted 6 lines
Quoted 2 lines

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.

It works for me. We should add a comment somewhere that the override is not expected to work for features where we allow differences (some FTR_NONSTRICT).

Quoted 11 lines
Quoted 5 lines

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.

Or remove this check if we expect the override to only work on the resulting sanitised value, not individual checks.

Signature · 1 line
Catalin
reviewer
#9 lore raw

Re: [PATCH v2 01/11] arm64: Skip update of an idreg field affected by an override

Quoted 57 lines · Catalin Marinas
Quoted 39 lines · Suzuki K Poulose
Quoted 38 lines · Catalin Marinas
Quoted 16 lines · Marc Zyngier

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 <[email protected]>
---
   arch/arm64/kernel/cpufeature.c | 7 +++++++
   1 file changed, 7 insertions(+)
arch/arm64/kernel/cpufeature.c +0 -0
index c31f8e17732a3..28fc77443ccd3 100644

arch/arm64/kernel/cpufeature.c static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new) · line 1231

Quoted 8 lines
arch/arm64/kernel/cpufeature.c +6 -0
@@ -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.

Quoted 6 lines
Quoted 2 lines

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.

It works for me. We should add a comment somewhere that the override is not expected to work for features where we allow differences (some FTR_NONSTRICT).

Quoted 6 lines
Quoted 5 lines

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.

Quoted 8 lines
Quoted 5 lines

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.

Or remove this check if we expect the override to only work on the resulting sanitised value, not individual checks.

True, but if some capabilities are PERCPU local features, then there is no way to override them with the controls. I have the following patch, that could do the trick :

--8>--

arm64: Apply overrides to CPU local capabilities

If an override has been applied, make sure we apply that for the secondary CPUs too, to limit the features.

Signed-off-by Suzuki K Poulose <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 40 +++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 8 deletions(-)
arch/arm64/kernel/cpufeature.c +32 -8
index 2e1e4de9a2cd..2b494302b767 100644
@@ -1217,10 +1217,41 @@ void __init init_cpu_features(struct cpuinfo_arm64 *info)
 		init_cpu_ftr_reg(SYS_GMID_EL1, info->reg_gmid);
 }
 
+/*
+ * Sanitise the register fields to clamp the values to the overrides that
+ * has been applied.
+ */
+static u64 override_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 val)
+{
+	const struct arm64_ftr_bits *ftrp;
+
+	if (!reg || !reg->override->mask)
+		return val;
+
+	for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
+		u64 ftr_mask = arm64_ftr_mask(ftrp);
+		s64 ftr_val, ftr_ovr, ftr_safe;
+
+		/* Skip the fields not overridden */
+		if ((ftr_mask & reg->override->mask) != ftr_mask)
+			continue;
+
+		ftr_val = arm64_ftr_value(ftrp, val);
+		ftr_ovr = arm64_ftr_value(ftrp, reg->override->val);
+		ftr_safe = arm64_ftr_safe_value(ftrp, ftr_ovr, ftr_val);
+
+		if (ftr_safe != ftr_val)
+			val = arm64_ftr_set_value(ftrp, val, ftr_safe);
+	}
+	return val;
+}
+
 static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new)
 {
 	const struct arm64_ftr_bits *ftrp;
 
+	/* Apply the overrides */
+	new = override_cpu_ftr_reg(reg, new);
 	for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
 		s64 ftr_cur = arm64_ftr_value(ftrp, reg->sys_val);
 		s64 ftr_new = arm64_ftr_value(ftrp, new);

@@ -1524,7 +1555,6 @@ EXPORT_SYMBOL_GPL(read_sanitised_ftr_reg);
  */
 u64 __read_sysreg_by_encoding(u32 sys_id)
 {
-	struct arm64_ftr_reg *regp;
 	u64 val;
 
 	switch (sys_id) {

@@ -1577,13 +1607,7 @@ u64 __read_sysreg_by_encoding(u32 sys_id)
 		return 0;
 	}
 
-	regp  = get_arm64_ftr_reg(sys_id);
-	if (regp) {
-		val &= ~regp->override->mask;
-		val |= (regp->override->val & regp->override->mask);
-	}
-
-	return val;
+	return override_cpu_ftr_reg(get_arm64_ftr_reg(sys_id), val);
 }
 
 #include <linux/irqchip/arm-gic-v3.h>
Signature · 1 line
2.43.0
Quoted 1 line
author maintainer
#10 lore raw

PATCH v2 2/11 arm64: Add a helper setting a feature field to its safe value

In reply to Marc Zyngier

In order to make the code more readable, add a simple helper setting a given field to its safe value, and update the only user so far. More will be added later.

Suggested-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
arch/arm64/kernel/cpufeature.c +7 -3
index 28fc77443ccd3..102c5bac4d502 100644
@@ -926,6 +926,11 @@ static u64 arm64_ftr_set_value(const struct arm64_ftr_bits *ftrp, s64 reg,
 	return reg;
 }
 
+static u64 arm64_ftr_set_safe_value(const struct arm64_ftr_bits *ftrp, s64 reg)
+{
+	return arm64_ftr_set_value(ftrp, reg, ftrp->safe_val);
+}
+
 s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp, s64 new,
 				s64 cur)
 {

@@ -1066,9 +1071,8 @@ static void init_cpu_ftr_reg(u32 sys_reg, u64 new)
 		if (ftrp->visible)
 			user_mask |= ftr_mask;
 		else
-			reg->user_val = arm64_ftr_set_value(ftrp,
-							    reg->user_val,
-							    ftrp->safe_val);
+			reg->user_val = arm64_ftr_set_safe_value(ftrp,
+								 reg->user_val);
 	}
 
 	val &= valid_mask;
Signature · 1 line
2.47.3
reviewer Reviewed-by on v2 2/11
#11 lore raw

Re: [PATCH v2 02/11] arm64: Add a helper setting a feature field to its safe value

Quoted 6 lines · Marc Zyngier

In order to make the code more readable, add a simple helper setting a given field to its safe value, and update the only user so far. More will be added later.

Suggested-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
Reviewed-by Suzuki K Poulose <[email protected]>
Quoted 32 lines
---
  arch/arm64/kernel/cpufeature.c | 10 +++++++---
  1 file changed, 7 insertions(+), 3 deletions(-)
arch/arm64/kernel/cpufeature.c +7 -3
index 28fc77443ccd3..102c5bac4d502 100644
@@ -926,6 +926,11 @@ static u64 arm64_ftr_set_value(const struct arm64_ftr_bits *ftrp, s64 reg,
  	return reg;
  }
 
+static u64 arm64_ftr_set_safe_value(const struct arm64_ftr_bits *ftrp, s64 reg)
+{
+	return arm64_ftr_set_value(ftrp, reg, ftrp->safe_val);
+}
+
  s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp, s64 new,
  				s64 cur)
  {

@@ -1066,9 +1071,8 @@ static void init_cpu_ftr_reg(u32 sys_reg, u64 new)
  		if (ftrp->visible)
  			user_mask |= ftr_mask;
  		else
-			reg->user_val = arm64_ftr_set_value(ftrp,
-							    reg->user_val,
-							    ftrp->safe_val);
+			reg->user_val = arm64_ftr_set_safe_value(ftrp,
+								 reg->user_val);
  	}
 
  	val &= valid_mask;
Reviewed-by on v2 2/11
#12 lore raw

Re: [PATCH v2 02/11] arm64: Add a helper setting a feature field to its safe value

In reply to Marc Zyngier

Hi Marc.

Quoted 7 lines · Marc Zyngier

In order to make the code more readable, add a simple helper setting a given field to its safe value, and update the only user so far. More will be added later.

Suggested-by Suzuki K Poulose <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
Reviewed-by Fuad Tabba <[email protected]>

Cheers,
/fuad

Quoted 35 lines
---
 arch/arm64/kernel/cpufeature.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
arch/arm64/kernel/cpufeature.c +7 -4
index 28fc77443ccd3..102c5bac4d502 100644
@@ -926,6 +926,11 @@ static u64 arm64_ftr_set_value(const struct arm64_ftr_bits *ftrp, s64 reg,
        return reg;
 }
 
+static u64 arm64_ftr_set_safe_value(const struct arm64_ftr_bits *ftrp, s64 reg)
+{
+       return arm64_ftr_set_value(ftrp, reg, ftrp->safe_val);
+}
+
 s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp, s64 new,
                                s64 cur)
 {

@@ -1066,9 +1071,8 @@ static void init_cpu_ftr_reg(u32 sys_reg, u64 new)
                if (ftrp->visible)
                        user_mask |= ftr_mask;
                else
-                       reg->user_val = arm64_ftr_set_value(ftrp,
-                                                           reg->user_val,
-                                                           ftrp->safe_val);
+                       reg->user_val = arm64_ftr_set_safe_value(ftrp,
+                                                                reg->user_val);
        }
 
        val &= valid_mask;
--

2.47.3

author maintainer
#13 lore raw

PATCH v2 3/11 arm64: Add logic to fully remove features from sanitised id registers

In reply to Marc Zyngier

We currently make support for some features such as Pointer Auth, SVE or S1POE a compile time decision.

However, while we hide that feature from userspace when such support is disabled, we still leave the value provided by the HW visible to the rest of the kernel, including KVM.

This has the potential to result in ugly state leakage, as half of the kernel knows about the feature, and the other doesn't.

Short of completely banning such compilation options and restore universal knowledge, introduce the possibility to fully remove such knowledge from the sanitised id registers.

This has more or less the same effect as the idreg override that a user can pass on the command-line, only defined at build-time.

For that purpose, we provide a new macro (FTR_CONFIG()) that defines the behaviour of a feature, both when enabled and disabled.

At this stage, nothing is making use of this anti-feature.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/cpufeature.h | 17 +++++++++------
 arch/arm64/kernel/cpufeature.c      | 32 ++++++++++++++++++++++-------
 2 files changed, 36 insertions(+), 13 deletions(-)
arch/arm64/include/asm/cpufeature.h +11 -6
index 4de51f8d92cba..e853a0ac7db38 100644
@@ -53,17 +53,22 @@ enum ftr_type {
 #define FTR_SIGNED	true	/* Value should be treated as signed */
 #define FTR_UNSIGNED	false	/* Value should be treated as unsigned */
 
-#define FTR_VISIBLE	true	/* Feature visible to the user space */
-#define FTR_HIDDEN	false	/* Feature is hidden from the user */
+enum ftr_visibility {
+	FTR_HIDDEN,		/* Feature hidden from the user */
+	FTR_ALL_HIDDEN,		/* Feature hidden from kernel, user and KVM */
+	FTR_VISIBLE,		/* Feature visible to all observers */
+};
+
+#define FTR_CONFIG(c, e, d)				\
+	(IS_ENABLED(c) ? FTR_ ## e : FTR_ ## d)
 
-#define FTR_VISIBLE_IF_IS_ENABLED(config)		\
-	(IS_ENABLED(config) ? FTR_VISIBLE : FTR_HIDDEN)
+#define FTR_VISIBLE_IF_IS_ENABLED(c)	FTR_CONFIG(c, VISIBLE, HIDDEN)
 
 struct arm64_ftr_bits {
 	bool		sign;	/* Value is signed ? */
-	bool		visible;
+	enum ftr_visibility visibility:8;
 	bool		strict;	/* CPU Sanity check: strict matching required ? */
-	enum ftr_type	type;
+	enum ftr_type	type:8;
 	u8		shift;
 	u8		width;
 	s64		safe_val; /* safe value for FTR_EXACT features */
arch/arm64/kernel/cpufeature.c +25 -7
index 102c5bac4d502..965dd2acf0640 100644
@@ -192,7 +192,7 @@ void dump_cpu_features(void)
 #define __ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
 	{						\
 		.sign = SIGNED,				\
-		.visible = VISIBLE,			\
+		.visibility = VISIBLE,			\
 		.strict = STRICT,			\
 		.type = TYPE,				\
 		.shift = SHIFT,				\

@@ -1063,16 +1063,33 @@ static void init_cpu_ftr_reg(u32 sys_reg, u64 new)
 				ftrp->shift);
 		}
 
-		val = arm64_ftr_set_value(ftrp, val, ftr_new);
-
 		valid_mask |= ftr_mask;
 		if (!ftrp->strict)
 			strict_mask &= ~ftr_mask;
-		if (ftrp->visible)
+
+		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:
+			/*
+			 * 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_safe_value(ftrp, val);
 			reg->user_val = arm64_ftr_set_safe_value(ftrp,
 								 reg->user_val);
+			break;
+		case FTR_HIDDEN:
+			val = arm64_ftr_set_value(ftrp, val, ftr_new);
+			reg->user_val = arm64_ftr_set_safe_value(ftrp,
+								 reg->user_val);
+			break;
+		}
 	}
 
 	val &= valid_mask;

@@ -1230,9 +1247,10 @@ static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new)
 
 		/*
 		 * Don't alter the initial value that has been forced
-		 * by an override.
+		 * by an override or a disabled feature.
 		 */
-		if ((reg->override->mask & arm64_ftr_mask(ftrp)) == arm64_ftr_mask(ftrp))
+		if (ftrp->visibility == FTR_ALL_HIDDEN ||
+		    (reg->override->mask & arm64_ftr_mask(ftrp)) == arm64_ftr_mask(ftrp))
 			continue;
 
 		if (ftr_cur == ftr_new)
Signature · 1 line
2.47.3
reviewer Reviewed-by on v2 3/11
#14 lore raw

Re: [PATCH v2 03/11] arm64: Add logic to fully remove features from sanitised id registers

Quoted 115 lines · Marc Zyngier

We currently make support for some features such as Pointer Auth, SVE or S1POE a compile time decision.

However, while we hide that feature from userspace when such support is disabled, we still leave the value provided by the HW visible to the rest of the kernel, including KVM.

This has the potential to result in ugly state leakage, as half of the kernel knows about the feature, and the other doesn't.

Short of completely banning such compilation options and restore universal knowledge, introduce the possibility to fully remove such knowledge from the sanitised id registers.

This has more or less the same effect as the idreg override that a user can pass on the command-line, only defined at build-time.

For that purpose, we provide a new macro (FTR_CONFIG()) that defines the behaviour of a feature, both when enabled and disabled.

At this stage, nothing is making use of this anti-feature.

Signed-off-by Marc Zyngier <[email protected]>
---
  arch/arm64/include/asm/cpufeature.h | 17 +++++++++------
  arch/arm64/kernel/cpufeature.c      | 32 ++++++++++++++++++++++-------
  2 files changed, 36 insertions(+), 13 deletions(-)
arch/arm64/include/asm/cpufeature.h +11 -6
index 4de51f8d92cba..e853a0ac7db38 100644
@@ -53,17 +53,22 @@ enum ftr_type {
  #define FTR_SIGNED	true	/* Value should be treated as signed */
  #define FTR_UNSIGNED	false	/* Value should be treated as unsigned */
 
-#define FTR_VISIBLE	true	/* Feature visible to the user space */
-#define FTR_HIDDEN	false	/* Feature is hidden from the user */
+enum ftr_visibility {
+	FTR_HIDDEN,		/* Feature hidden from the user */
+	FTR_ALL_HIDDEN,		/* Feature hidden from kernel, user and KVM */
+	FTR_VISIBLE,		/* Feature visible to all observers */
+};
+
+#define FTR_CONFIG(c, e, d)				\
+	(IS_ENABLED(c) ? FTR_ ## e : FTR_ ## d)
 
-#define FTR_VISIBLE_IF_IS_ENABLED(config)		\
-	(IS_ENABLED(config) ? FTR_VISIBLE : FTR_HIDDEN)
+#define FTR_VISIBLE_IF_IS_ENABLED(c)	FTR_CONFIG(c, VISIBLE, HIDDEN)
 
  struct arm64_ftr_bits {
  	bool		sign;	/* Value is signed ? */
-	bool		visible;
+	enum ftr_visibility visibility:8;
  	bool		strict;	/* CPU Sanity check: strict matching required ? */
-	enum ftr_type	type;
+	enum ftr_type	type:8;
  	u8		shift;
  	u8		width;
  	s64		safe_val; /* safe value for FTR_EXACT features */
arch/arm64/kernel/cpufeature.c +22 -5
index 102c5bac4d502..965dd2acf0640 100644
@@ -192,7 +192,7 @@ void dump_cpu_features(void)
  #define __ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
  	{						\
  		.sign = SIGNED,				\
-		.visible = VISIBLE,			\
+		.visibility = VISIBLE,			\
  		.strict = STRICT,			\
  		.type = TYPE,				\
  		.shift = SHIFT,				\

@@ -1063,16 +1063,33 @@ static void init_cpu_ftr_reg(u32 sys_reg, u64 new)
  				ftrp->shift);
  		}
 
-		val = arm64_ftr_set_value(ftrp, val, ftr_new);
-
  		valid_mask |= ftr_mask;
  		if (!ftrp->strict)
  			strict_mask &= ~ftr_mask;
-		if (ftrp->visible)
+
+		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:
+			/*
+			 * 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_safe_value(ftrp, val);
  			reg->user_val = arm64_ftr_set_safe_value(ftrp,
  								 reg->user_val);
+			break;
+		case FTR_HIDDEN:
+			val = arm64_ftr_set_value(ftrp, val, ftr_new);
+			reg->user_val = arm64_ftr_set_safe_value(ftrp,
+								 reg->user_val);
+			break;
+		}
  	}
 
  	val &= valid_mask;

@@ -1230,9 +1247,10 @@ static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new)
 
  		/*

arch/arm64/kernel/cpufeature.c static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new) · line 1256

Quoted 10 lines
arch/arm64/kernel/cpufeature.c +3 -2
  		 * Don't alter the initial value that has been forced
-		 * by an override.
+		 * by an override or a disabled feature.
  		 */
-		if ((reg->override->mask & arm64_ftr_mask(ftrp)) == arm64_ftr_mask(ftrp))
+		if (ftrp->visibility == FTR_ALL_HIDDEN ||
+		    (reg->override->mask & arm64_ftr_mask(ftrp)) == arm64_ftr_mask(ftrp))
  			continue;
 
  		if (ftr_cur == ftr_new)

Looks good to me.

Reviewed-by Suzuki K Poulose <[email protected]>
Reviewed-by on v2 3/11
#15 lore raw

Re: [PATCH v2 03/11] arm64: Add logic to fully remove features from sanitised id registers

In reply to Marc Zyngier

Hi Marc,

Quoted 88 lines · Marc Zyngier

We currently make support for some features such as Pointer Auth, SVE or S1POE a compile time decision.

However, while we hide that feature from userspace when such support is disabled, we still leave the value provided by the HW visible to the rest of the kernel, including KVM.

This has the potential to result in ugly state leakage, as half of the kernel knows about the feature, and the other doesn't.

Short of completely banning such compilation options and restore universal knowledge, introduce the possibility to fully remove such knowledge from the sanitised id registers.

This has more or less the same effect as the idreg override that a user can pass on the command-line, only defined at build-time.

For that purpose, we provide a new macro (FTR_CONFIG()) that defines the behaviour of a feature, both when enabled and disabled.

At this stage, nothing is making use of this anti-feature.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/cpufeature.h | 17 +++++++++------
 arch/arm64/kernel/cpufeature.c      | 32 ++++++++++++++++++++++-------
 2 files changed, 36 insertions(+), 13 deletions(-)
arch/arm64/include/asm/cpufeature.h +11 -6
index 4de51f8d92cba..e853a0ac7db38 100644
@@ -53,17 +53,22 @@ enum ftr_type {
 #define FTR_SIGNED     true    /* Value should be treated as signed */
 #define FTR_UNSIGNED   false   /* Value should be treated as unsigned */
 
-#define FTR_VISIBLE    true    /* Feature visible to the user space */
-#define FTR_HIDDEN     false   /* Feature is hidden from the user */
+enum ftr_visibility {
+       FTR_HIDDEN,             /* Feature hidden from the user */
+       FTR_ALL_HIDDEN,         /* Feature hidden from kernel, user and KVM */
+       FTR_VISIBLE,            /* Feature visible to all observers */
+};
+
+#define FTR_CONFIG(c, e, d)                            \
+       (IS_ENABLED(c) ? FTR_ ## e : FTR_ ## d)
 
-#define FTR_VISIBLE_IF_IS_ENABLED(config)              \
-       (IS_ENABLED(config) ? FTR_VISIBLE : FTR_HIDDEN)
+#define FTR_VISIBLE_IF_IS_ENABLED(c)   FTR_CONFIG(c, VISIBLE, HIDDEN)
 
 struct arm64_ftr_bits {
        bool            sign;   /* Value is signed ? */
-       bool            visible;
+       enum ftr_visibility visibility:8;
        bool            strict; /* CPU Sanity check: strict matching required ? */
-       enum ftr_type   type;
+       enum ftr_type   type:8;
        u8              shift;
        u8              width;
        s64             safe_val; /* safe value for FTR_EXACT features */
arch/arm64/kernel/cpufeature.c +4 -4
index 102c5bac4d502..965dd2acf0640 100644
@@ -192,7 +192,7 @@ void dump_cpu_features(void)
 #define __ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
        {                                               \
                .sign = SIGNED,                         \
-               .visible = VISIBLE,                     \
+               .visibility = VISIBLE,                  \
                .strict = STRICT,                       \
                .type = TYPE,                           \
                .shift = SHIFT,                         \

@@ -1063,16 +1063,33 @@ static void init_cpu_ftr_reg(u32 sys_reg, u64 new)
                                ftrp->shift);
                }
 
-               val = arm64_ftr_set_value(ftrp, val, ftr_new);
-
                valid_mask |= ftr_mask;
                if (!ftrp->strict)
                        strict_mask &= ~ftr_mask;
-               if (ftrp->visible)
+
+               switch (ftrp->visibility) {
+               case FTR_VISIBLE:

arch/arm64/kernel/cpufeature.c static void init_cpu_ftr_reg(u32 sys_reg, u64 new) · line 1080

Quoted 10 lines
arch/arm64/kernel/cpufeature.c +8 -1
+                       val = arm64_ftr_set_value(ftrp, val, ftr_new);
                        user_mask |= ftr_mask;
-               else
+                       break;
+               case FTR_ALL_HIDDEN:
+                       /*
+                        * ALL_HIDDEN and HIGHER_SAFE are incompatible.
+                        * Only hide from userspace, and log the oddity.
+                        */
+                       if (WARN_ON(ftrp->type == FTR_HIGHER_SAFE))

What about FTR_HIGHER_OR_ZERO_SAFE? It's not actually being used for any feature id register, only for CTR_EL0. Even there, it's a (theoretical at this point) performance issue, not a correctness one, as 0 assumes the largest possible size. I still think that it's safer to include it here.

That said:

Reviewed-by Fuad Tabba <[email protected]>

Cheers,
/fuad

Quoted 31 lines
arch/arm64/kernel/cpufeature.c +13 -3
+                               val = arm64_ftr_set_value(ftrp, val, ftr_new);
+                       else
+                               val = arm64_ftr_set_safe_value(ftrp, val);
                        reg->user_val = arm64_ftr_set_safe_value(ftrp,
                                                                 reg->user_val);
+                       break;
+               case FTR_HIDDEN:
+                       val = arm64_ftr_set_value(ftrp, val, ftr_new);
+                       reg->user_val = arm64_ftr_set_safe_value(ftrp,
+                                                                reg->user_val);
+                       break;
+               }
        }
 
        val &= valid_mask;

@@ -1230,9 +1247,10 @@ static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new)
 
                /*
                 * Don't alter the initial value that has been forced
-                * by an override.
+                * by an override or a disabled feature.
                 */
-               if ((reg->override->mask & arm64_ftr_mask(ftrp)) == arm64_ftr_mask(ftrp))
+               if (ftrp->visibility == FTR_ALL_HIDDEN ||
+                   (reg->override->mask & arm64_ftr_mask(ftrp)) == arm64_ftr_mask(ftrp))
                        continue;
 
                if (ftr_cur == ftr_new)
--

2.47.3

maintainer
#16 lore raw

Re: [PATCH v2 03/11] arm64: Add logic to fully remove features from sanitised id registers

In reply to Marc Zyngier
Quoted 7 lines · Marc Zyngier

We currently make support for some features such as Pointer Auth, SVE or S1POE a compile time decision.

However, while we hide that feature from userspace when such support is disabled, we still leave the value provided by the HW visible to the rest of the kernel, including KVM.

Quoted 6 lines

This has the potential to result in ugly state leakage, as half of the kernel knows about the feature, and the other doesn't.

Short of completely banning such compilation options and restore universal knowledge, introduce the possibility to fully remove such knowledge from the sanitised id registers.

I wouldn't oppose to it really. If there are features affecting the kernel compilation (e.g. kasan/mte), we need configs but for most features I don't think we should bother, especially if they don't take significantly more code/data memory when not present. It makes it easier for us to reason about.

Of course, I'd keep the command-line overriding, it helps with debugging.

Quoted 31 lines
arch/arm64/kernel/cpufeature.c +7 -5
index 102c5bac4d502..965dd2acf0640 100644
@@ -192,7 +192,7 @@ void dump_cpu_features(void)
 #define __ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
 	{						\
 		.sign = SIGNED,				\
-		.visible = VISIBLE,			\
+		.visibility = VISIBLE,			\
 		.strict = STRICT,			\
 		.type = TYPE,				\
 		.shift = SHIFT,				\

@@ -1063,16 +1063,33 @@ static void init_cpu_ftr_reg(u32 sys_reg, u64 new)
 				ftrp->shift);
 		}
 
-		val = arm64_ftr_set_value(ftrp, val, ftr_new);
-
 		valid_mask |= ftr_mask;
 		if (!ftrp->strict)
 			strict_mask &= ~ftr_mask;
-		if (ftrp->visible)
+
+		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:

arch/arm64/kernel/cpufeature.c static void init_cpu_ftr_reg(u32 sys_reg, u64 new) · line 1085

Quoted 10 lines
arch/arm64/kernel/cpufeature.c +8 -0
+			/*
+			 * 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_safe_value(ftrp, val);
 			reg->user_val = arm64_ftr_set_safe_value(ftrp,
 								 reg->user_val);

IIUC, if a feature is now disabled in .config and marked as FTR_ALL_HIDDEN, we end up with a 0 field in the sanitised sysreg (or whatever the safe value is). We now have a discrepancy between VHE and nVHE in finalise_el2_state. The check_override_idreg macro uses the sanitised sysregs for nVHE and the actual hw ones with VHE. Maybe not an issue in the cases you are targeting but it's something that may bite us in the future.

One ugly workaround is to add #ifdefs or .ifs to el2_setup.h. A slightly better one (I think) would be to force the above visibility into the override masks/values rather than adding FTR_ALL_HIDDEN. Let the override checks end up with a safe value.

And, of course, my preferred way would be to drop this config-based visibility altogether ;).

Signature · 1 line
Catalin
author maintainer
#17 lore raw

PATCH v2 4/11 arm64: Convert CONFIG_ARM64_PTR_AUTH to FTR_CONFIG()

In reply to Marc Zyngier

While CONFIG_ARM64_PTR_AUTH=n prevents userspace from using PAC, the sanitised ID registers still advertise the feature.

Make it clear that nothing in the kernel should rely on this by marking the feature as hidden for all when CONFIG_ARM64_PTR_AUTH=n.

This is functionnaly equivalent to using arm64.nopauth on the kernel command-line.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
arch/arm64/kernel/cpufeature.c +6 -6
index 965dd2acf0640..0726c2a186028 100644
@@ -248,16 +248,16 @@ static const struct arm64_ftr_bits ftr_id_aa64isar1[] = {
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_EL1_SPECRES_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_EL1_SB_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_EL1_FRINTTS_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_PTR_AUTH, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_EL1_GPI_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_PTR_AUTH, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_EL1_GPA_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_EL1_LRCPC_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_EL1_FCMA_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_EL1_JSCVT_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_PTR_AUTH, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64ISAR1_EL1_API_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_PTR_AUTH, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64ISAR1_EL1_APA_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_EL1_DPB_SHIFT, 4, 0),
 	ARM64_FTR_END,

@@ -270,9 +270,9 @@ static const struct arm64_ftr_bits ftr_id_aa64isar2[] = {
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR2_EL1_CLRBHB_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR2_EL1_BC_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR2_EL1_MOPS_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_PTR_AUTH, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64ISAR2_EL1_APA3_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_PTR_AUTH, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR2_EL1_GPA3_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64ISAR2_EL1_RPRES_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64ISAR2_EL1_WFxT_SHIFT, 4, 0),
Signature · 1 line
2.47.3
author maintainer
#18 lore raw

PATCH v2 5/11 arm64: Convert CONFIG_ARM64_SVE to FTR_CONFIG()

In reply to Marc Zyngier

While CONFIG_ARM64_SVE=n prevents userspace from using SVE, the sanitised ID registers still advertise the feature.

Make it clear that nothing in the kernel should rely on this by marking the feature as hidden for all when CONFIG_ARM64_SVE=n.

This is functionnaly equivalent to using arm64.nosve on the kernel command-line.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)
arch/arm64/kernel/cpufeature.c +13 -13
index 0726c2a186028..b5b19f21709f1 100644
@@ -293,7 +293,7 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = {
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_AMU_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_MPAM_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_SEL2_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 				   FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_SVE_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_RAS_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_GIC_SHIFT, 4, 0),

@@ -331,29 +331,29 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr2[] = {
 };
 
 static const struct arm64_ftr_bits ftr_id_aa64zfr0[] = {
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ZFR0_EL1_F64MM_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ZFR0_EL1_F32MM_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ZFR0_EL1_F16MM_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ZFR0_EL1_I8MM_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ZFR0_EL1_SM4_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ZFR0_EL1_SHA3_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ZFR0_EL1_B16B16_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ZFR0_EL1_BF16_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ZFR0_EL1_BitPerm_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ZFR0_EL1_EltPerm_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ZFR0_EL1_AES_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SVE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SVE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ZFR0_EL1_SVEver_SHIFT, 4, 0),
 	ARM64_FTR_END,
 };
Signature · 1 line
2.47.3
author maintainer
#19 lore raw

PATCH v2 6/11 arm64: Convert CONFIG_ARM64_SME to FTR_CONFIG()

In reply to Marc Zyngier

While CONFIG_ARM64_SME=n prevents userspace from using SME, the sanitised ID registers still advertise the feature.

Make it clear that nothing in the kernel should rely on this by marking the feature as hidden for all when CONFIG_ARM64_SME=n.

This is functionnaly equivalent to using arm64.nosme on the kernel command-line.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 48 +++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 24 deletions(-)
arch/arm64/kernel/cpufeature.c +24 -24
index b5b19f21709f1..cf1e53aa8e475 100644
@@ -311,7 +311,7 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr1[] = {
 	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_GCS),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_GCS_SHIFT, 4, 0),
 	S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_MTE_frac_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_SME_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_MPAM_frac_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_RAS_frac_SHIFT, 4, 0),

@@ -359,51 +359,51 @@ static const struct arm64_ftr_bits ftr_id_aa64zfr0[] = {
 };
 
 static const struct arm64_ftr_bits ftr_id_aa64smfr0[] = {
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_FA64_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_LUTv2_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_SMEver_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_I16I64_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_F64F64_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_I16I32_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_B16B16_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_F16F16_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_F8F16_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_F8F32_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_I8I32_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_F16F32_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_B16F32_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_BI32I32_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_F32F32_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_SF8FMA_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_SF8DP4_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_SF8DP2_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_SBitPerm_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_AES_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_SFEXPA_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_STMOP_SHIFT, 1, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_SME),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_EXACT, ID_AA64SMFR0_EL1_SMOP4_SHIFT, 1, 0),
 	ARM64_FTR_END,
 };
Signature · 1 line
2.47.3
author maintainer
#20 lore raw

PATCH v2 7/11 arm64: Convert CONFIG_ARM64_GCS to FTR_CONFIG()

In reply to Marc Zyngier

While CONFIG_ARM64_GCS=n prevents userspace from using GCS, the sanitised ID registers still advertise the feature.

Make it clear that nothing in the kernel should rely on this by marking the feature as hidden for all when CONFIG_ARM64_GCS=n.

This is functionnaly equivalent to using arm64.nogcs on the kernel command-line.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpufeature.c +1 -1
index cf1e53aa8e475..ab0a7d72608d4 100644
@@ -308,7 +308,7 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = {
 
 static const struct arm64_ftr_bits ftr_id_aa64pfr1[] = {
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_DF2_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_GCS),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_GCS, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_GCS_SHIFT, 4, 0),
 	S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_MTE_frac_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_SME, VISIBLE, ALL_HIDDEN),
Signature · 1 line
2.47.3
author maintainer
#21 lore raw

PATCH v2 8/11 arm64: Convert CONFIG_ARM64_MTE to FTR_CONFIG()

In reply to Marc Zyngier

While CONFIG_ARM64_MTE=n prevents userspace from using MTE, the sanitised ID registers still advertise the feature.

Make it clear that nothing in the kernel should rely on this by marking the feature as hidden for all when CONFIG_ARM64_MTE=n.

This is functionnaly equivalent to using arm64.nomte on the kernel command-line.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpufeature.c +1 -1
index ab0a7d72608d4..a56d242fe1489 100644
@@ -315,7 +315,7 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr1[] = {
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_SME_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_MPAM_frac_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_RAS_frac_SHIFT, 4, 0),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_MTE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_MTE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_MTE_SHIFT, 4, ID_AA64PFR1_EL1_MTE_NI),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_SSBS_SHIFT, 4, ID_AA64PFR1_EL1_SSBS_NI),
 	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_BTI),
Signature · 1 line
2.47.3
#22 lore raw

Re: [PATCH v2 08/11] arm64: Convert CONFIG_ARM64_MTE to FTR_CONFIG()

Hi Marc,

Quoted 20 lines · Marc Zyngier

While CONFIG_ARM64_MTE=n prevents userspace from using MTE, the sanitised ID registers still advertise the feature.

Make it clear that nothing in the kernel should rely on this by marking the feature as hidden for all when CONFIG_ARM64_MTE=n.

This is functionnaly equivalent to using arm64.nomte on the kernel command-line.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpufeature.c +0 -0
index ab0a7d72608d4..a56d242fe1489 100644

arch/arm64/kernel/cpufeature.c static const struct arm64_ftr_bits ftr_id_aa64pfr1[] = { · line 321

Quoted 8 lines
arch/arm64/kernel/cpufeature.c +1 -1
@@ -315,7 +315,7 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr1[] = {
                       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_SME_SHIFT, 4, 0),
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_MPAM_frac_SHIFT, 4, 0),
        ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_RAS_frac_SHIFT, 4, 0),
-       ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_MTE),
+       ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_MTE, VISIBLE, ALL_HIDDEN),
                       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_MTE_SHIFT, 4, ID_AA64PFR1_EL1_MTE_NI),
        ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_SSBS_SHIFT, 4, ID_AA64PFR1_EL1_SSBS_NI),
        ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_BTI),

Although it wouldn't really be for the scope of this patch, but what about gating the other MTE fields? e.g., MTE_frac here, ID_AA64PFR2_EL1.MTEFAR and MTESTOREONLY?

Cheers,
/fuad

Quoted 4 lines

--
2.47.3

author maintainer
#23 lore raw

PATCH v2 9/11 arm64: Convert CONFIG_ARM64_POE to FTR_CONFIG()

In reply to Marc Zyngier

While CONFIG_ARM64_POE=n prevents userspace from using S1POE, the sanitised ID registers still advertise the feature.

Make it clear that nothing in the kernel should rely on this by marking the feature as hidden for all when CONFIG_ARM64_POE=n.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpufeature.c +1 -1
index a56d242fe1489..1af5f5b0c48a7 100644
@@ -504,7 +504,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {
 };
 
 static const struct arm64_ftr_bits ftr_id_aa64mmfr3[] = {
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_POE),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_POE, VISIBLE, ALL_HIDDEN),
 		       FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64MMFR3_EL1_S1POE_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64MMFR3_EL1_S1PIE_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR3_EL1_SCTLRX_SHIFT, 4, 0),
Signature · 1 line
2.47.3
author maintainer
#24 lore raw

PATCH v2 10/11 arm64: Convert CONFIG_ARM64_BTI to FTR_CONFIG()

In reply to Marc Zyngier

Even if the kernel doesn't use BTI and doesn't expose it to userspace, it is still OK to expose the feature to the rest of the kernel including KVM, as there is no additional state attached to this feature.

The only purpose of this change is to kill the last user of the FTR_VISIBLE_IF_IS_ENABLED() macro.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kernel/cpufeature.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/cpufeature.c +2 -2
index 1af5f5b0c48a7..b3e94ad3f08a4 100644
@@ -318,8 +318,8 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr1[] = {
 	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_MTE, VISIBLE, ALL_HIDDEN),
 		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_MTE_SHIFT, 4, ID_AA64PFR1_EL1_MTE_NI),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_SSBS_SHIFT, 4, ID_AA64PFR1_EL1_SSBS_NI),
-	ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_BTI),
-				    FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_BT_SHIFT, 4, 0),
+	ARM64_FTR_BITS(FTR_CONFIG(CONFIG_ARM64_BTI, VISIBLE, HIDDEN),
+		       FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_EL1_BT_SHIFT, 4, 0),
 	ARM64_FTR_END,
 };
Signature · 1 line
2.47.3
author maintainer
#25 lore raw

PATCH v2 11/11 arm64: Remove FTR_VISIBLE_IF_IS_ENABLED()

In reply to Marc Zyngier

Now that FTR_VISIBLE_IF_IS_ENABLED() is completely unused, remove it.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/cpufeature.h | 2 --
 1 file changed, 2 deletions(-)
arch/arm64/include/asm/cpufeature.h +0 -2
index e853a0ac7db38..d8accc9c94fab 100644
@@ -62,8 +62,6 @@ enum ftr_visibility {
 #define FTR_CONFIG(c, e, d)				\
 	(IS_ENABLED(c) ? FTR_ ## e : FTR_ ## d)
 
-#define FTR_VISIBLE_IF_IS_ENABLED(c)	FTR_CONFIG(c, VISIBLE, HIDDEN)
-
 struct arm64_ftr_bits {
 	bool		sign;	/* Value is signed ? */
 	enum ftr_visibility visibility:8;
Signature · 1 line
2.47.3
Reviewed-by Tested-by on the v2 series
#26 lore raw
In reply to Marc Zyngier

Hi Marc,

Quoted 12 lines · Marc Zyngier

This is v2 of this series attempting at making the handling of features that are disabled at compile time a bit less awkward for non-userspace consumers of the sanitised feature bits.

A few things have changed, thanks to Fuad and Suzuki spending time on reviewing this, but the overall behaviour is the same.

* From v1 [1]:

- Mark ALL_HIDDEN and HIGHER_SAFE as incompatible

Quoted 8 lines

- Don't update a feature that is ALL_HIDDEN (or overridden) on the secondary boot path

- Reduce the size of struct arm64_ftr_bits

- Add a helper for setting a field to its safe value

[1] https://lore.kernel.org/r/[email protected]

For the series:

Reviewed-by Fuad Tabba <[email protected]>
Tested-by Fuad Tabba <[email protected]>

Cheers,
/fuad

Quoted 21 lines

Marc Zyngier (11):
arm64: Skip update of an idreg field affected by an override
arm64: Add a helper setting a feature field to its safe value
arm64: Add logic to fully remove features from sanitised id registers
arm64: Convert CONFIG_ARM64_PTR_AUTH to FTR_CONFIG()
arm64: Convert CONFIG_ARM64_SVE to FTR_CONFIG()
arm64: Convert CONFIG_ARM64_SME to FTR_CONFIG()
arm64: Convert CONFIG_ARM64_GCS to FTR_CONFIG()
arm64: Convert CONFIG_ARM64_MTE to FTR_CONFIG()
arm64: Convert CONFIG_ARM64_POE to FTR_CONFIG()
arm64: Convert CONFIG_ARM64_BTI to FTR_CONFIG()
arm64: Remove FTR_VISIBLE_IF_IS_ENABLED()

 arch/arm64/include/asm/cpufeature.h |  15 +--
 arch/arm64/kernel/cpufeature.c      | 141 +++++++++++++++++-----------
 2 files changed, 94 insertions(+), 62 deletions(-)

--
2.47.3