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

From: Suzuki K Poulose
Date: Mon May 19 2025 - 05:53:48 EST

Next message: Ard Biesheuvel: "Re: [PATCH v4 1/6] x86/cpu: Use a new feature flag for 5 level paging"
Previous message: Mehdi Djait: "Re: [RFC PATCH v4] media: v4l2-common: Add a helper for obtaining the clock producer"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Catalin,

On 14/05/2025 13:05, Catalin Marinas wrote:

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.

Thanks, the change below does the trick. I am reasoning with the way
the scope has been defined (hacked ;-)).

SCOPE_LOCAL_CPU && SCOPE_SYSTEM

1. SCOPE_LOCAL_CPU : Because you need to run this on all the (early) CPUs.

2. SCOPE_SYSTEM: To check if the capability holds at the end of the
smp boot.

While, we really "detect" it on SCOPE_BOOT_CPU and only run the
cap checks, if that is available. But put another way, BOOT_CPU
is only used as an easy way to detect if this CPUs is the first
one to run the check vs at least one CPU has run and cleared the
cap.

i.e, as below.

> +
> + /*
> + * 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;
> +

I wonder if we could use some other flag to indicate the fact that,
a non-boot CPU is allowed to clear the capability explicitly, rather than implying it with SCOPE_SYSTEM && SCOPE_LOCAL_CPU. Or may be make
it explicit that the capability must be matched on ALL cpus and
finalized at the end ?

/*
* When paired with SCOPE_LOCAL_CPU, all CPUs must satisfy the
* condition. This is different from SCOPE_SYSTEM, where the check
* is performed only once at the end of SMP boot. But SCOPE_SYSTEM
* may not be sufficient in cases where the capability depends on
* properties that are not "sanitised" (e.g., MIDR_EL1) and must be
* satisfied by all the early SMP boot CPUs.
*/
#define ARM64_CPUCAP_MATCH_ALL_EARLY_CPUS ((u16)BIT(7))

statici inline bool cpucap_match_all_cpus(struct arm64_capability *cap)
{
return !!(cap->type & ARM64_CPUCAP_MATCH_ALL_EARLY_CPUS);
}

Also, we already go through the capablity list to report the ones
with "cpumask" separately, and we could use that to also report
the ones with MATCH_ALL_CPUs. Something like:

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9c4d6d552b25..14cbae51d802 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -3769,10 +3769,15 @@ static void __init setup_system_capabilities(void)
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;
+
+ 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));
+
+ /* Report capabilities that had to be matched on all CPUs */
+ if (capcpucap_match_all_cpus(caps) && cpus_have_cap(caps))
+ pr_info("detected: %s\n", caps->desc);
}

/*

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

#define ARM64_CPUCAP_MATCH_ALL_CPU_FEATURE ?

\

+ (ARM64_CPUCAP_SCOPE_LOCAL_CPU | ARM64_CPUCAP_SYSTEM_FEATURE)

(ARM64_CPUCAP_SCOPE_LOCAL_CPU | ARM64_CPUCAP_MATCH_ALL_EARLY_CPUS)

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

May be call it cpucap_match_all_cpus() ?

Thoughts ?

Suzuki

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

Next message: Ard Biesheuvel: "Re: [PATCH v4 1/6] x86/cpu: Use a new feature flag for 5 level paging"
Previous message: Mehdi Djait: "Re: [RFC PATCH v4] media: v4l2-common: Add a helper for obtaining the clock producer"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]