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

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

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

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.

-- 
Catalin