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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/05/2025 14:35, 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:
On Tue, May 06, 2025 at 03:52:59PM +0100, Ryan Roberts wrote:
On 06/05/2025 15:25, Will Deacon wrote:
This penalises large homogeneous systems and it feels unnecessary given
that we have the ability to check this per-CPU. Can you use
ARM64_CPUCAP_BOOT_CPU_FEATURE instead of ARM64_CPUCAP_SYSTEM_FEATURE
to solve this?

We are trying to solve for the case where the boot CPU has BBML2 but a
secondary
CPU doesn't. (e.g. hetrogeneous system where boot CPU is big and secondary is
little and does not advertise the feature. I can't remember if we proved there
are real systems with this config - I have vague recollection that we did
but my
memory is poor...).

My understanding is that for ARM64_CPUCAP_BOOT_CPU_FEATURE, "If the boot CPU
has enabled this feature already, then every late CPU must have it". So that
would exclude any secondary CPUs without BBML2 from coming online?

Damn, yes, you're right. However, it still feels horribly hacky to iterate
over the online CPUs in has_bbml2_noabort() -- the cpufeature framework
has the ability to query features locally and we should be able to use
that. We're going to want that should the architecture eventually decide
on something like BBML3 for this.

What we have with BBML2_NOABORT seems similar to an hwcap in that we only
support the capability if all CPUs have it (rejecting late CPUs without it
in that case) but we can live without it if not all of the early CPUs
have it. Unlikely hwcaps, though, we shouldn't be advertising this to
userspace and we can't derive the capability solely from the sanitised
system registers.

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?

Ah, my bad. I think a single cap should work as long as it makes sure
we "no BBML2" doesn't est the erratum.

Just to confirm, you are proposing:

ARM64_HAS_NO_BBML2_NOABORT (or in other words, ARM64_NO_SAFE_BBML2)
as CPU_LOCAL_ERRATUM (remove the "description" field from the cap, so
that we don't report it, when we detect it).

matches() =>  !in_midr_list() ||  !ID_AA64MMFR*.BBML2

I think that should work with the inverted check.
I am wondering how we can plug in the AmpereOne missing the ID register
case. May be special case it in the matches() above for the same cap.

I think that should work.

Suzuki



I'm easy either way, but keen to understand why 2 caps are preferred?

Perhaps for my version it would be better to refer to it as
ARM64_CPUCAP_BOOT_RESTRICTED_CPU_LOCAL_FEATURE instead of
ARM64_CPUCAP_LOCAL_CPU_ERRATUM (they both have the exact same semantics under
the hood AFAICT).

Thanks,
Ryan



And then:



Then we have this helper:

static inline bool system_supports_bbml2_noabort(void)
{
     return system_capabilities_finalized() &&
         alternative_has_cap_unlikely(ARM64_HAS_BBML2) &&
         !alternative_has_cap_unlikely(!ARM64_HAS_BBML2_ABORTS)

Without (1), we may enable BBML2 on a (system with) CPU that doesn't
have BBML2 feature.

And (1) can prevent any non-BBML2 capable CPUs from booting or (2) can prevent
anything that aborts with BBML2.


Suzuki


            !alternative_has_cap_unlikely(ARM64_HAS_NO_BBML2_NOABORT);


}

system_capabilities_finalized() is there to ensure an early call to this helper
returns false (i.e. the safe value before we have evaluated on all CPUs).
Because ARM64_HAS_NO_BBML2_NOABORT is inverted it would otherwise return true
prior to finalization.

I don't believe we need any second (SYSTEM or BOOT) feature. This is sufficient
on its own?

Thanks,
Ryan








[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux