Re: [PATCH v2 1/2] arm64: Combine workarounds for speculative AT errata

[Steven Price]

On 13/11/2019 14:01, Suzuki K Poulose wrote:
> 
> On 13/11/2019 11:41, Steven Price wrote:
>> Cortex-A57/A72 have a similar errata to Cortex-A76 regarding speculation
>> of the AT instruction. Since the workaround for A57/A72 doesn't require
>> VHE, the restriction enforcing VHE for A76 can be removed by combining
>> the workaround flag for both errata.
>>
>> So combine WORKAROUND_1165522 and WORKAROUND_1319367 into
>> WORKAROUND_SPECULATIVE_AT. The majority of code is contained within VHE
>> or NVHE specific functions, for the cases where the code is shared extra
>> checks against has_vhe().
>>
>> This also paves the way for adding a similar erratum for Cortex-A55.
>>
>> Signed-off-by: Steven Price <steven.price@xxxxxxx>
> 
>> diff --git a/arch/arm64/kernel/cpu_errata.c
>> b/arch/arm64/kernel/cpu_errata.c
>> index 4f8187a4fc46..b801f8e832aa 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -744,6 +744,16 @@ static const struct midr_range
>> erratum_1418040_list[] = {
>> Â };
>> Â #endif
>> Â +#ifdef CONFIG_ARM64_WORKAROUND_SPECULATIVE_AT
>> +static const struct midr_range erratum_speculative_at_list[] = {
>> +#ifdef CONFIG_ARM64_ERRATUM_1165522
>> +ÂÂÂ /* Cortex A76 r0p0 to r2p0 */
>> +ÂÂÂ MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
>> +#endif
>> +ÂÂÂ {},
>> +};
>> +#endif
>> +
>> Â const struct arm64_cpu_capabilities arm64_errata[] = {
>> Â #ifdef CONFIG_ARM64_WORKAROUND_CLEAN_CACHE
>> ÂÂÂÂÂ {
>> @@ -868,12 +878,11 @@ const struct arm64_cpu_capabilities
>> arm64_errata[] = {
>> ÂÂÂÂÂÂÂÂÂ ERRATA_MIDR_RANGE_LIST(erratum_1418040_list),
>> ÂÂÂÂÂ },
>> Â #endif
>> -#ifdef CONFIG_ARM64_ERRATUM_1165522
>> +#ifdef CONFIG_ARM64_WORKAROUND_SPECULATIVE_AT
>> ÂÂÂÂÂ {
>> -ÂÂÂÂÂÂÂ /* Cortex-A76 r0p0 to r2p0 */
>> ÂÂÂÂÂÂÂÂÂ .desc = "ARM erratum 1165522",
>> -ÂÂÂÂÂÂÂ .capability = ARM64_WORKAROUND_1165522,
>> -ÂÂÂÂÂÂÂ ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
>> +ÂÂÂÂÂÂÂ .capability = ARM64_WORKAROUND_SPECULATIVE_AT,
>> +ÂÂÂÂÂÂÂ ERRATA_MIDR_RANGE_LIST(erratum_speculative_at_list),
>> ÂÂÂÂÂ },
>> Â #endif
>> Â #ifdef CONFIG_ARM64_ERRATUM_1463225
>> @@ -910,7 +919,7 @@ const struct arm64_cpu_capabilities arm64_errata[]
>> = {
>> Â #ifdef CONFIG_ARM64_ERRATUM_1319367
>> ÂÂÂÂÂ {
>> ÂÂÂÂÂÂÂÂÂ .desc = "ARM erratum 1319367",
>> -ÂÂÂÂÂÂÂ .capability = ARM64_WORKAROUND_1319367,
>> +ÂÂÂÂÂÂÂ .capability = ARM64_WORKAROUND_SPECULATIVE_AT,
>> ÂÂÂÂÂÂÂÂÂ ERRATA_MIDR_RANGE_LIST(ca57_a72),
>> ÂÂÂÂÂ },
>> Â #endif
> 
> Have you tested this patch with both the errata CONFIGs turned on ?
> Having multiple entries for the same capability should trigger a WARNING at
> boot with init_cpu_hwcaps_indirect_list_from_array().
> You could simply add the MIDRs to the midr_list and update the description
> to include all the Errata numbers.

Ha! You of course are right - I had 'tested' the combination but
apparently not looked carefully enough - there is indeed a WARNING in
the boot.

I might well be keeping the two entries anyway due to Marc's concerns
about the micro-architectural details of the 1319367 workaround.

Steve