Re: [RESEND PATCH v6 1/3] arm64: Add BBM Level 2 cpu feature

[Catalin Marinas <catalin.marinas@arm.com>]

On Tue, May 13, 2025 at 10:15:49AM +0100, Suzuki K Poulose wrote:
> On 12/05/2025 17:33, Catalin Marinas wrote:
> > Stepping back a bit, we know that the MIDR allow-list implies
> > BBML2_NOABORT (and at least BBML2 as in the ID regs). In theory, we need
> 
> Please be aware that BBML2_NOABORT midr list may not always imply BBLM2 in
> ID registers (e.g., AmpereOne. But the plan is to fixup the per cpu
> ID register - struct cpuinfo_arm64 - for such cores at early boot,
> individually, before it is used for sanitisation of the system wide
> copy).

Ah, good point. We can then ignore BBML2 ID regs and only rely on MIDR
(and some future BBML3).

> > So how about we introduce a WEAK_BOOT_CPU_FEATURE which gets enabled by
> > the boot CPU if it has it _but_ cleared by any secondary early CPU if it
> > doesn't (and never enabled by secondary CPUs). When the features are
> > finalised, we know if all early CPUs had it. In combination with
> > PERMITTED_FOR_LATE_CPU, we'd reject late CPUs that don't have it.
> 
> That could work, but it introduces this "clearing" a capability, which
> we don't do at the moment.
> 
> We had an offline discussion about this some time ago, with Mark
> Rutland. The best way to deal with this is to change the way we compute
> capabilities. i.e.,
> 
> 
> 1. Each boot CPU run through all the capabilities and maintain a per-cpu
>    copy of the state.
> 2. System wide capabilities can then be constructed from the all early
>    boot CPU capability state (e.g., ANDing all the state from all CPUs
>    for SCOPE_SYSTEM or ORing for LOCAL_CPU).
> 
> But this requires a drastic change to the infrastructure.

I think it's a lot simpler to achieve the ANDing - set the (system)
capability if detected on the boot CPU, only clear it if missing on
subsequent CPUs. See below on an attempt to introduce this. For lack of
inspiration, I called it ARM64_CPUCAP_GLOBAL_CPU_FEATURE which has both
SCOPE_LOCAL and SCOPE_SYSTEM. It's permitted for late CPUs but not
optional if already enabled. The advantage of having both local&system
is that the match function will be called for both scopes. I added a
mask in to cpucap_default_scope() when calling matches() since so far
no cap had both.

---------------------8<-----------------------------------------
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index c4326f1cb917..0b0b26a6f27b 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -331,6 +331,15 @@ extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0;
 #define ARM64_CPUCAP_BOOT_CPU_FEATURE                  \
 	(ARM64_CPUCAP_SCOPE_BOOT_CPU | ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU)
 
+/*
+ * CPU feature detected at boot time based on all CPUs. It is safe for a late
+ * CPU to have this feature even though the system hasn't enabled it, although
+ * the feature will not be used by Linux in this case. If the system has
+ * enabled this feature already, then every late CPU must have it.
+ */
+#define ARM64_CPUCAP_GLOBAL_CPU_FEATURE			\
+	 (ARM64_CPUCAP_SCOPE_LOCAL_CPU | ARM64_CPUCAP_SYSTEM_FEATURE)
+
 struct arm64_cpu_capabilities {
 	const char *desc;
 	u16 capability;
@@ -391,6 +400,11 @@ static inline int cpucap_default_scope(const struct arm64_cpu_capabilities *cap)
 	return cap->type & ARM64_CPUCAP_SCOPE_MASK;
 }
 
+static inline bool cpucap_global_scope(const struct arm64_cpu_capabilities *cap)
+{
+	return (cap->type & SCOPE_LOCAL_CPU) && (cap->type & SCOPE_SYSTEM);
+}
+
 /*
  * Generic helper for handling capabilities with multiple (match,enable) pairs
  * of call backs, sharing the same capability bit.
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 5ba149c0c2ac..1a5a51090c0e 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -3359,13 +3381,47 @@ static void update_cpu_capabilities(u16 scope_mask)
 
 	scope_mask &= ARM64_CPUCAP_SCOPE_MASK;
 	for (i = 0; i < ARM64_NCAPS; i++) {
+		bool global_cap = false;
+
 		caps = cpucap_ptrs[i];
-		if (!caps || !(caps->type & scope_mask) ||
-		    cpus_have_cap(caps->capability) ||
-		    !caps->matches(caps, cpucap_default_scope(caps)))
+		if (!caps || !(caps->type & scope_mask))
 			continue;
 
-		if (caps->desc && !caps->cpus)
+		global_cap = cpucap_global_scope(caps);
+
+		/*
+		 * If it's not a global CPU capability, avoid probing if
+		 * already detected.
+		 */
+		if (!global_cap && cpus_have_cap(caps->capability))
+			continue;
+
+		/*
+		 * Pass the actual scope we are probing to the match function.
+		 * This is important for the global CPU capabilities that are
+		 * checked both as a local CPU feature and as a system one.
+		 */
+		if (!caps->matches(caps,
+				   cpucap_default_scope(caps) & scope_mask)) {
+			/* All CPUs must have the global capability */
+			if (global_cap)
+				__clear_bit(caps->capability, system_cpucaps);
+			continue;
+		}
+
+		/*
+		 * A global capability is only set when probing the boot CPU.
+		 * It may be cleared subsequently if not detected on secondary
+		 * ones.
+		 */
+		if (global_cap && !(scope_mask & SCOPE_BOOT_CPU))
+			continue;
+
+		/*
+		 * Global CPU capabilities are logged later when the system
+		 * capabilities are finalised.
+		 */
+		if (!global_cap && caps->desc && !caps->cpus)
 			pr_info("detected: %s\n", caps->desc);
 
 		__set_bit(caps->capability, system_cpucaps);
@@ -3771,17 +3827,24 @@ static void __init setup_system_capabilities(void)
 	enable_cpu_capabilities(SCOPE_ALL & ~SCOPE_BOOT_CPU);
 	apply_alternatives_all();
 
-	/*
-	 * Log any cpucaps with a cpumask as these aren't logged by
-	 * update_cpu_capabilities().
-	 */
 	for (int i = 0; i < ARM64_NCAPS; i++) {
 		const struct arm64_cpu_capabilities *caps = cpucap_ptrs[i];
 
-		if (caps && caps->cpus && caps->desc &&
-			cpumask_any(caps->cpus) < nr_cpu_ids)
+		if (!caps || !caps->desc)
+			continue;
+
+		/*
+		 * Log any cpucaps with a cpumask as these aren't logged by
+		 * update_cpu_capabilities().
+		 */
+		if (caps->cpus && cpumask_any(caps->cpus) < nr_cpu_ids)
 			pr_info("detected: %s on CPU%*pbl\n",
 				caps->desc, cpumask_pr_args(caps->cpus));
+
+		/* Log global CPU capabilities */
+		if (cpucap_global_scope(caps) &&
+		    cpus_have_cap(caps->capability))
+			pr_info("detected: %s\n", caps->desc);
 	}
 
 	/*
---------------------8<-----------------------------------------

And an dummy test:

---------------------8<-----------------------------------------
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 5ba149c0c2ac..1a5a51090c0e 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2480,6 +2480,21 @@ test_has_mpam_hcr(const struct arm64_cpu_capabilities *entry, int scope)
 	return idr & MPAMIDR_EL1_HAS_HCR;
 }
 
+static void
+cpu_enable_dummy_global(const struct arm64_cpu_capabilities *entry)
+{
+	pr_info("%s: %s: smp_processor_id() = %d", __func__, entry->desc, smp_processor_id());
+}
+
+static bool
+has_dummy_global(const struct arm64_cpu_capabilities *entry, int scope)
+{
+	pr_info("%s: %s: scope = %x smp_processor_id() = %d", __func__, entry->desc, scope, smp_processor_id());
+	if (smp_processor_id() < 4)
+		return true;
+	return false;
+}
+
 static const struct arm64_cpu_capabilities arm64_features[] = {
 	{
 		.capability = ARM64_ALWAYS_BOOT,
@@ -3050,6 +3065,13 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.matches = has_pmuv3,
 	},
 #endif
+	{
+		.desc = "Dummy test for global CPU feature",
+		.capability = ARM64_HAS_GLOBAL_CPU_TEST,
+		.type = ARM64_CPUCAP_GLOBAL_CPU_FEATURE,
+		.cpu_enable = cpu_enable_dummy_global,
+		.matches = has_dummy_global,
+	},
 	{},
 };
 
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 772c1b008e43..dbc5a3eb5b3d 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -37,6 +37,7 @@ HAS_GENERIC_AUTH_IMP_DEF
 HAS_GIC_CPUIF_SYSREGS
 HAS_GIC_PRIO_MASKING
 HAS_GIC_PRIO_RELAXED_SYNC
+HAS_GLOBAL_CPU_TEST
 HAS_HCR_NV1
 HAS_HCX
 HAS_LDAPR
---------------------8<-----------------------------------------

-- 
Catalin