On 12/05/2025 17:33, Catalin Marinas wrote:
On Mon, May 12, 2025 at 02:35:01PM +0100, Ryan Roberts wrote:
On 12/05/2025 14:24, Suzuki K Poulose wrote:
On 12/05/2025 14:07, Ryan Roberts wrote:
On 09/05/2025 17:04, Catalin Marinas wrote:
On Fri, May 09, 2025 at 02:49:05PM +0100, Will Deacon wrote:
I wonder if we could treat it like an erratum in some way instead? That
is, invert things so that CPUs which _don't_ have BBML2_NOABORT are
considered to have a "BBM_CONFLICT_ABORT" erratum (which we obviously
wouldn't shout about). Then we should be able to say:
- If any of the early CPUs don't have BBML2_NOABORT, then the erratum
would be enabled and we wouln't elide BBM.
- If a late CPU doesn't have BBML2_NOABORT then it can't come online
if the erratum isn't already enabled.
Does that work? If not, then perhaps the cpufeature/cpuerrata code needs
some surgery for this.
Ah, I should have read this thread in order. I think we can treat this
as BBML2_NOABORT available as default based on ID regs and use the
allow/deny-list as an erratum.
Just to make sure I've understood all this, I think what you are both saying is
we can create a single capability called ARM64_HAS_NO_BBML2_NOABORT of type
ARM64_CPUCAP_LOCAL_CPU_ERRATUM. Each CPU will then check it has BBML2 and is in
the MIDR allow list; If any of those conditions are not met, the CPU is
considered to have ARM64_HAS_NO_BBML2_NOABORT.
I guess we need two caps.
1. SYSTEM cap -> ARM64_HAS_BBML2. Based on the ID registers
2. An erratum -> ARM64_BBML2_ABORTS. Based on BBLM2==1 && !in_midr_list()
I don't think we *need* two caps; I was suggesting to consider both of these
conditions for the single cap. You are suggesting to separate them. But I think
both approaches give the same result?
I'm easy either way, but keen to understand why 2 caps are preferred?
I guess it's easier to reason about than a single, negated property but
the result should be identical. With two properties we can easily
implement the idreg override like nobbml2 since this works on the
sanitised ID regs. But we could also implement this differently, no need
to rely on the ID regs.
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).
something like a SYSTEM_FEATURE which is the conjunction of all the
early CPUs. However, such system-level cap is only checked after all the
early CPUs booted _and_ only on the sanitised ID regs rather than MIDR.
We need a LOCAL_CPU feature behaviour to be called on each CPU but still
have the conjunction of early CPUs, more like the system one. It should
be permitted for late CPUs to have but not optional if already enabled.
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 if we can get the above, it would be the cleaner option than
trying to bend our minds around double negations like !NO_BBLM2_NOABORT.
Agree, every time I come back to the thread, I have to write down the
check and stare at it for a minute to agree with what it does. That said
it may be ideal solution for the short term. Or stick to what we do in
the patch currently, until we implement per-cpu capability proposal.
Cheers
Suzuki
