Re: [PATCH v2 1/4] arm64: Add BBM Level 2 cpu feature

[Yang Shi]

On 3/3/25 1:40 AM, Mikołaj Lenczewski wrote:
> On Fri, Feb 28, 2025 at 06:45:38PM -0800, Yang Shi wrote:
> > On 2/28/25 5:29 PM, Yang Shi wrote:
> > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > > index 940343beb3d4..baae6d458996 100644
> > > > --- a/arch/arm64/Kconfig
> > > > +++ b/arch/arm64/Kconfig
> > > > @@ -2057,6 +2057,17 @@ config ARM64_TLB_RANGE
> > > >          The feature introduces new assembly instructions, and they were
> > > >          support when binutils >= 2.30.
> > > >    +config ARM64_ENABLE_BBML2_NOABORT
> > > > +    bool "Enable support for Break-Before-Make Level 2 detection
> > > > and usage"
> > > > +    default y
> > > > +    help
> > > > +      FEAT_BBM provides detection of support levels for
> > > > break-before-make
> > > > +      sequences. If BBM level 2 is supported, some TLB maintenance
> > > > requirements
> > > > +      can be relaxed to improve performance. We additonally require the
> > > > +      property that the implementation cannot ever raise TLB
> > > > Conflict Aborts.
> > > > +      Selecting N causes the kernel to fallback to BBM level 0
> > > > behaviour
> > > > +      even if the system supports BBM level 2.
> > > > +
> > > >    endmenu # "ARMv8.4 architectural features"
> > > >      menu "ARMv8.5 architectural features"
> > > > diff --git a/arch/arm64/include/asm/cpucaps.h
> > > > b/arch/arm64/include/asm/cpucaps.h
> > > > index 0b5ca6e0eb09..2d6db33d4e45 100644
> > > > --- a/arch/arm64/include/asm/cpucaps.h
> > > > +++ b/arch/arm64/include/asm/cpucaps.h
> > > > @@ -23,6 +23,8 @@ cpucap_is_possible(const unsigned int cap)
> > > >            return IS_ENABLED(CONFIG_ARM64_PAN);
> > > >        case ARM64_HAS_EPAN:
> > > >            return IS_ENABLED(CONFIG_ARM64_EPAN);
> > > > +    case ARM64_HAS_BBML2_NOABORT:
> > > > +        return IS_ENABLED(CONFIG_ARM64_BBML2_NOABORT);
> > > >        case ARM64_SVE:
> > > >            return IS_ENABLED(CONFIG_ARM64_SVE);
> > > >        case ARM64_SME:
> > > > diff --git a/arch/arm64/include/asm/cpufeature.h
> > > > b/arch/arm64/include/asm/cpufeature.h
> > > > index e0e4478f5fb5..108ef3fbbc00 100644
> > > > --- a/arch/arm64/include/asm/cpufeature.h
> > > > +++ b/arch/arm64/include/asm/cpufeature.h
> > > > @@ -866,6 +866,11 @@ static __always_inline bool
> > > > system_supports_mpam_hcr(void)
> > > >        return alternative_has_cap_unlikely(ARM64_MPAM_HCR);
> > > >    }
> > > >    +static inline bool system_supports_bbml2_noabort(void)
> > > > +{
> > > > +    return alternative_has_cap_unlikely(ARM64_HAS_BBML2_NOABORT);
> > > > +}
> > > Hi Miko,
> > >
> > > I added AmpereOne mdir on top of this patch. I can see BBML2 feature is
> > > detected via dmesg. But system_supports_bbml2_noabort() returns false.
> > > The warning in the below debug patch is triggered:
> > >
> > > diff --git a/arch/arm64/kernel/cpufeature.c
> > > b/arch/arm64/kernel/cpufeature.c
> > > index faa9094d97dd..a70829ae2bd0 100644
> > > --- a/arch/arm64/kernel/cpufeature.c
> > > +++ b/arch/arm64/kernel/cpufeature.c
> > > @@ -3814,6 +3814,9 @@ void __init setup_system_features(void)
> > >   {
> > >          setup_system_capabilities();
> > >
> > > +       if (!system_supports_bbml2_noabort())
> > > +               WARN_ON_ONCE(1);
> > > +
> > >          kpti_install_ng_mappings();
> > >
> > >          sve_setup();
> > >
> > > I thought it may be too early. But it seems other system features work
> > > well, for example, MPAM. I didn't figure out why. It is weird.
> > I just figured out the problem It is because the wrong kconfig name is used
> > in cpucaps.h. The code is:
> >
> > +    case ARM64_HAS_BBML2_NOABORT:
> > +        return IS_ENABLED(CONFIG_ARM64_BBML2_NOABORT);
> >
> > But the kconfig name actually is:
> >
> > +config ARM64_ENABLE_BBML2_NOABORT
> >
> > IMHO, the "ENABLE" in kconfig name sounds unnecessary.
> >
> > Thanks,
> > Yang
> Hi Yang,
>
> Thank you for the review, and apologies for the slight delay.
>
> Thanks again for the spot, I agree that `ENABLE` is probably redundant
> (and clearly, caused an issue here). Will remove this. Please let me
> know if there are any other issues with rebasing your patches on top of
> mine.

Thank you. I didn't spot any other problem on our machines. With this 
addressed, you can have Tested-by: Yang Shi <yang@xxxxxxxxxxxxxxxxxxx>, 
although I can't test on asymmetric system.

Yang


>