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

[Yang Shi <yang@os.amperecomputing.com>]

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@amperecomputing.com>, 
although I can't test on asymmetric system.

Yang


>