Re: [PATCH 08/16] arm64: capabilities: Group handling of features and errata

[Suzuki K Poulose <Suzuki.Poulose@arm.com>]

On 29/01/18 17:14, Dave Martin wrote:
> On Fri, Jan 26, 2018 at 12:31:18PM +0000, Suzuki K Poulose wrote:
>> On 26/01/18 11:47, Dave Martin wrote:
>>> On Tue, Jan 23, 2018 at 12:28:01PM +0000, Suzuki K Poulose wrote:
>>>> So far we have had separate routes for triggering errata and feature
>>>
>>> "triggering errata" ? ;)
>>>
>>
>> :-). Should have been "triggering errata and feature capability *checks*.
>>
>>> Maybe "[...] for determining whether to activate errata workarounds and
>>> whether to enable feature capabilities."
>>>
>>
>>
>>>> capabilities. Also, we never allowed "features" based on local CPU
>>>> and "errata" based on System wide safe registers. This patch
>>>> groups the handling of errata and features and also allows them
>>>> to have all the possible scopes.
>>>>
>>>> So, we now run through the arm64_features and arm64_errata:
>>>
>>> when?
>>
>> with this patch.
> 
> I mean, when at runtime?

Sorry, I thought that was evident from the comment below :

> 
>>> What about late cpus?
>>>
>>
>> We don't detect any new capabilities on them. They continue to get
>> verified against the enabled capabilities.
>>
>>>>   1) with SCOPE_LOCAL_CPU filter on each boot time enabeld CPUs,
>>>>      via update_cpu_local_capabilities().

Here ^^.  Earlier we ran only through the errata list. But now, we
run through errata and the features, using a type filter of SCOPE_LOCAL_CPU.

It also said :

   2) with SCOPE_SYSTEM filter only once after all boot time enabled
     CPUs are active.

(2) happens from setup_cpu_features(), just like it was done earlier, but
with a filter of SCOPE_SYSTEM.

>>>
>>> "each [...] enabeld CPUs" -> "each [...] enabled CPU"
>>>
>>> Also, changing "boot time" -> "boot-time" helps avoid this being misread
>>> as "on each boot", which could be taken to mean "each time a CPU comes
>>> online".  I'm guessing that's not the intended meaning here.
>>

>>>
>>>    }
>>>>   DEFINE_STATIC_KEY_FALSE(arm64_const_caps_ready);
>>>> @@ -1422,9 +1435,7 @@ void __init setup_cpu_features(void)
>>>>   	u32 cwg;
>>>>   	int cls;
>>>> -	/* Set the CPU feature capabilies */
>>>> -	setup_feature_capabilities();
>>>> -	enable_errata_workarounds();
>>>> +	setup_system_capabilities();
>>>>   	mark_const_caps_ready();
>>>>   	setup_elf_hwcaps(arm64_elf_hwcaps);
>>>
>>> I wonder whether we could unify the elf hwcaps handling too.
>>
>> I was thinking about it today. The only catch is how do we know
>> if we have "the capability", as it is spread across multiple bitmasks.
>> (HWCAP, COMPAT_HWCAP, COMPAT_HWCAP2).
> 
> An easy-ish solution might be to maintain our own bitmap in the style
> of cpu_hwcaps, and set bits in parallel with the elf_hwcap etc. bits.
> Or, add a method that knows how to set/query the appropriate bit.
> 
> I guess we could do this later.  It's certainly not urgent.

Yes, I understand.

Cheers
Suzuki