[PATCH 05/37] arm64: Add cpus_have_final_boot_cap()

[Suzuki K Poulose]

On 21/09/2023 17:36, Mark Rutland wrote:
> On Thu, Sep 21, 2023 at 10:13:31AM +0100, Suzuki K Poulose wrote:
>> Hi Mark
>>
>> On 19/09/2023 10:28, Mark Rutland wrote:
>>> The cpus_have_final_boot_cap() function can be used to test a cpucap
>>
>> nit: cpus_have_final_cap()
> 
> Thanks; fixed now.
> 
>>> while also verifying that we do not consume the cpucap until system
>>> capabilities have been finalized. It would be helpful if we could do
>>> likewise for boot cpucaps.
>>>
>>> This patch adds a new cpus_have_final_boot_cap() helper which can be
>>> used to test a cpucap while also verifying that boot capabilities have
>>> been finalized. Users will be added in subsequent patches.
>>>
>>> Signed-off-by: Mark Rutland <mark.rutland at arm.com>
>>> Cc: Catalin Marinas <catalin.marinas at arm.com>
>>> Cc: Mark Brown <broonie at kernel.org>
>>> Cc: Suzuki K Poulose <suzuki.poulose at arm.com>
>>> Cc: Will Deacon <will at kernel.org>
>>> ---
>>>    arch/arm64/include/asm/cpufeature.h | 27 +++++++++++++++++++++++++--
>>>    1 file changed, 25 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
>>> index 7d5317bc2429f..e832b86c6b57f 100644
>>> --- a/arch/arm64/include/asm/cpufeature.h
>>> +++ b/arch/arm64/include/asm/cpufeature.h
>>> @@ -438,6 +438,11 @@ unsigned long cpu_get_elf_hwcap2(void);
>>>    #define cpu_set_named_feature(name) cpu_set_feature(cpu_feature(name))
>>>    #define cpu_have_named_feature(name) cpu_have_feature(cpu_feature(name))
>>> +static __always_inline bool boot_capabilities_finalized(void)
>>> +{
>>> +	return alternative_has_cap_likely(ARM64_ALWAYS_BOOT);
>>> +}
>>> +
>>>    static __always_inline bool system_capabilities_finalized(void)
>>>    {
>>>    	return alternative_has_cap_likely(ARM64_ALWAYS_SYSTEM);
>>> @@ -473,8 +478,26 @@ static __always_inline bool __cpus_have_const_cap(int num)
>>>    /*
>>>     * Test for a capability without a runtime check.
>>>     *
>>> - * Before capabilities are finalized, this will BUG().
>>> - * After capabilities are finalized, this is patched to avoid a runtime check.
>>> + * Before boot capabilities are finalized, this will BUG().
>>> + * After boot capabilities are finalized, this is patched to avoid a runtime
>>> + * check.
>>> + *
>>> + * @num must be a compile-time constant.
>>> + */
>>> +static __always_inline bool cpus_have_final_boot_cap(int num)
>>> +{
>>> +	if (boot_capabilities_finalized())
>>
>> Does this need to make sure the cap is really a "BOOT" cap ? It is a bit of
>> an overkill, but prevents users from incorrectly assuming the cap is
>> finalised ?
> 
> Do you have an idea in mind for how to do that?
> 
> I had also wanted that, but we don't have the information available when
> compiling the callsites today since that's determined by the
> arm64_cpu_capabilities::type flags.
> 



> We could us an alternative callback for boot_capabilities_finalized() that
> goes and checks the arm64_cpu_capabilities::type flags, but that doesn't seem
> very nice.
> 
Thats what I had initially in mind, and is why I called it an
overkill.

But may be another option is to have a different alternative construct
for all capabilities, which defaults to BUG() and then patched to
"false" or "true" based on the real status ? This may be more
complicated.

> Otherwise, given this only has a few users, I could have those directly use:
> 
> 	BUG_ON(!boot_capabilities_finalized());
> 
> ... and remove cpus_have_final_boot_cap() for now?

I don't think that is necessary. We could keep your patch
as is, if we can't verify the boot capability easily.

Suzuki

> 
> Thanks,
> Mark.
> 
>>
>>
>> Suzuki
>>
>>> +		return __cpus_have_const_cap(num);
>>> +	else
>>> +		BUG();
>>> +}
>>> +
>>> +/*
>>> + * Test for a capability without a runtime check.
>>> + *
>>> + * Before system capabilities are finalized, this will BUG().
>>> + * After system capabilities are finalized, this is patched to avoid a runtime
>>> + * check.
>>>     *
>>>     * @num must be a compile-time constant.
>>>     */
>>