[PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72

[James Morse]

Hi Catalin,

On 27/01/2022 15:25, Catalin Marinas wrote:
> On Thu, Jan 27, 2022 at 12:29:14PM +0000, James Morse wrote:
>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index a46ab3b1c4d5..06605e267ab0 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -1900,6 +1900,29 @@ static void cpu_enable_mte(struct arm64_cpu_capabilities const *cap)
>>  }
>>  #endif /* CONFIG_ARM64_MTE */
>>  
>> +#ifdef CONFIG_ARM64_ERRATUM_1742098
>> +/*
>> + * compat_elf_hwcap{,2} are built from the sanitised id registers after the
>> + * enable calls have run.  See the order of the setup_system_capabilities()
>> + * and setup_elf_hwcaps() calls in setup_cpu_features(). Removing the AES
>> + * field prevents the AES hwcap from being advertised.
>> + */
>> +void arm64_remove_aes_compat_hwcap(const struct arm64_cpu_capabilities *cap)
>> +{
>> +	struct arm64_ftr_reg *aa32isar5 = get_arm64_ftr_reg(SYS_ID_ISAR5_EL1);
>> +	u64 aes_mask = GENMASK_ULL(ID_ISAR5_AES_SHIFT + 3, ID_ISAR5_AES_SHIFT);
>> +
>> +	/*
>> +	 * On affected platforms this call is made via stop_machine() on all
>> +	 * online CPUs. Only clear the register from the boot CPU.
>> +	 */
>> +	if (smp_processor_id())
>> +		return;
>> +
>> +	aa32isar5->sys_val &= ~aes_mask;
>> +}
>> +#endif /* CONFIG_ARM64_ERRATUM_1742098 */
> 
> I wonder whether this would look better if we use the
> ARM64_FTR_REG_OVERRIDE approach with an id_isar5_override defined in
> cpu_errata.c and populated there. I haven't checked the order in which
> they would be called though.

The order shouldn't be a problem, the feature override stuff hacks
__read_sysreg_by_encoding(), which is what the hwcap ->matches() ends up calling.

I didn't try it this way as it would need re-writing (to this!) if you want to backport it
before v5.12. (That said, I've not actually tried to backport it yet).


> Alternatively, we could make the ftr_id_isar5 reg global and patch it
> directly in cpu_errata.c (arm64_ftr_reg_ctrel0 is another case where we
> made it global). Yet another option would be to add some function in
> cpufeature.[hc] to patch a feature directly and we'd call it from
> cpu_errata.c. My preference would be to keep the cpu_enable function for
> the workaround in cpu_errata.c.

Yes, that is the ugly bit of this. Keeping this all in cpu_errata.c would require a
'broken features' mask which we use to mask off the hwcaps once we detect them. I don't
think the file matters - cpufeature and cpu_errata are part of the same piece of
infrastructure.


> Is this the first case where we need to change the ELF HWCAPs due to an
> erratum?

It is. Over lunch today I realised as it is, this patch will break KVM guest migration on
affected parts, because the sanitised ID registers are exposed to Qemu, and it isn't
allowed to change them.

I'll come up with a new way, (and tackle 32bit too). I suspect a HWCAP_CAP_HOOKED() that
lets the .matches call do something else will be cleanest, and leave the sanitised ID
registers unchanged.


Thanks,

James