Message ID | 20220127122914.1585008-1-james.morse@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72 | expand |
On Thu, 27 Jan 2022 at 13:29, James Morse <james.morse@arm.com> wrote: > > Cortex-A57 and Cortex-A72 have an erratum where an interrupt that > occurs between a pair of AES instructions in aarch32 mode may corrupt > the ELR. The task will subsequently produce the wrong AES result. > > The AES instructions are part of the cryptographic extensions, which are > optional. User-space software will detect the support for these > instructions from the hwcaps. If the platform doesn't support these > instructions a software implementation should be used. > > Remove the hwcap bits on affected parts to indicate user-space should > not use the AES instructions. > > CC: Ard Biesheuvel <ardb@kernel.org> > CC: Suzuki K Poulose <suzuki.poulose@arm.com> > CC: <stable@vger.kernel.org> > Signed-off-by: James Morse <james.morse@arm.com> For this patch, Acked-by: Ard Biesheuvel <ardb@kernel.org> but I will note that - depending on the C library used, OpenSSL may use SIGILL trapping to decide whether these instructions are implemented or not, - the 32-bit kernel should ideally adopt the same approach, Fortunately, the only A72 that is known to be widely deployed with 32-bit kernels and/or user space is the Raspberry Pi 4, which does not implement the crypto extensions. > --- > SDEN: > A57: https://developer.arm.com/documentation/epm049219/2300 1742098 > A72: https://developer.arm.com/documentation/epm012079/11 1655431 > --- > Documentation/arm64/silicon-errata.rst | 4 ++++ > arch/arm64/Kconfig | 16 ++++++++++++++++ > arch/arm64/include/asm/cpufeature.h | 1 + > arch/arm64/kernel/cpu_errata.c | 17 +++++++++++++++++ > arch/arm64/kernel/cpufeature.c | 23 +++++++++++++++++++++++ > arch/arm64/tools/cpucaps | 1 + > 6 files changed, 62 insertions(+) > > diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst > index 5342e895fb60..0f255ab8c3e2 100644 > --- a/Documentation/arm64/silicon-errata.rst > +++ b/Documentation/arm64/silicon-errata.rst > @@ -76,10 +76,14 @@ stable kernels. > +----------------+-----------------+-----------------+-----------------------------+ > | ARM | Cortex-A57 | #1319537 | ARM64_ERRATUM_1319367 | > +----------------+-----------------+-----------------+-----------------------------+ > +| ARM | Cortex-A57 | #1742098 | ARM64_ERRATUM_1742098 | > ++----------------+-----------------+-----------------+-----------------------------+ > | ARM | Cortex-A72 | #853709 | N/A | > +----------------+-----------------+-----------------+-----------------------------+ > | ARM | Cortex-A72 | #1319367 | ARM64_ERRATUM_1319367 | > +----------------+-----------------+-----------------+-----------------------------+ > +| ARM | Cortex-A72 | #1655431 | ARM64_ERRATUM_1742098 | > ++----------------+-----------------+-----------------+-----------------------------+ > | ARM | Cortex-A73 | #858921 | ARM64_ERRATUM_858921 | > +----------------+-----------------+-----------------+-----------------------------+ > | ARM | Cortex-A76 | #1188873,1418040| ARM64_ERRATUM_1418040 | > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 6978140edfa4..0daf4fff0eaf 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -488,6 +488,22 @@ config ARM64_ERRATUM_834220 > > If unsure, say Y. > > +config ARM64_ERRATUM_1742098 > + bool "Cortex-A57/A72: 1742098: ELR recorded incorrectly on interrupt taken between cryptographic instructions in a sequence" > + depends on COMPAT > + default y > + help > + This option removes the AES hwcap for aarch32 user-space to > + workaround erratum 1742098 on Cortex-A57 and Cortex-A72. > + > + Affected parts may corrupt the AES state if an interrupt is > + taken between a pair of AES instructions. These instructions > + are only present if the cryptography extensions are present. > + All software should have a fallback implementation for CPUs > + that don't implement the cryptography extensions. > + > + If unsure, say Y. > + > config ARM64_ERRATUM_845719 > bool "Cortex-A53: 845719: a load might read incorrect data" > depends on COMPAT > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > index ef6be92b1921..355313d46c14 100644 > --- a/arch/arm64/include/asm/cpufeature.h > +++ b/arch/arm64/include/asm/cpufeature.h > @@ -857,6 +857,7 @@ extern struct arm64_ftr_override id_aa64isar1_override; > > u32 get_kvm_ipa_limit(void); > void dump_cpu_features(void); > +void arm64_remove_aes_compat_hwcap(const struct arm64_cpu_capabilities *cap); > > #endif /* __ASSEMBLY__ */ > > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c > index 9e1c1aef9ebd..b06fb054e055 100644 > --- a/arch/arm64/kernel/cpu_errata.c > +++ b/arch/arm64/kernel/cpu_errata.c > @@ -376,6 +376,14 @@ static struct midr_range trbe_write_out_of_range_cpus[] = { > }; > #endif /* CONFIG_ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE */ > > +#ifdef CONFIG_ARM64_ERRATUM_1742098 > +static struct midr_range broken_aarch32_aes[] = { > + MIDR_RANGE(MIDR_CORTEX_A57, 0, 1, 0xf, 0xf), > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A72), > + {}, > +}; > +#endif /* CONFIG_ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE */ > + > const struct arm64_cpu_capabilities arm64_errata[] = { > #ifdef CONFIG_ARM64_WORKAROUND_CLEAN_CACHE > { > @@ -597,6 +605,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = { > .type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE, > CAP_MIDR_RANGE_LIST(trbe_write_out_of_range_cpus), > }, > +#endif > +#ifdef CONFIG_ARM64_ERRATUM_1742098 > + { > + .desc = "ARM erratum 1742098", > + .capability = ARM64_WORKAROUND_1742098, > + CAP_MIDR_RANGE_LIST(broken_aarch32_aes), > + .type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM, > + .cpu_enable = arm64_remove_aes_compat_hwcap, > + }, > #endif > { > } > 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 */ > + > #ifdef CONFIG_KVM > static bool is_kvm_protected_mode(const struct arm64_cpu_capabilities *entry, int __unused) > { > diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps > index 870c39537dd0..6a3a5c116668 100644 > --- a/arch/arm64/tools/cpucaps > +++ b/arch/arm64/tools/cpucaps > @@ -55,6 +55,7 @@ WORKAROUND_1418040 > WORKAROUND_1463225 > WORKAROUND_1508412 > WORKAROUND_1542419 > +WORKAROUND_1742098 > WORKAROUND_TRBE_OVERWRITE_FILL_MODE > WORKAROUND_TSB_FLUSH_FAILURE > WORKAROUND_TRBE_WRITE_OUT_OF_RANGE > -- > 2.30.2 >
Hi Ard, On 27/01/2022 12:39, Ard Biesheuvel wrote: > On Thu, 27 Jan 2022 at 13:29, James Morse <james.morse@arm.com> wrote: >> >> Cortex-A57 and Cortex-A72 have an erratum where an interrupt that >> occurs between a pair of AES instructions in aarch32 mode may corrupt >> the ELR. The task will subsequently produce the wrong AES result. >> >> The AES instructions are part of the cryptographic extensions, which are >> optional. User-space software will detect the support for these >> instructions from the hwcaps. If the platform doesn't support these >> instructions a software implementation should be used. >> >> Remove the hwcap bits on affected parts to indicate user-space should >> not use the AES instructions. > For this patch, > > Acked-by: Ard Biesheuvel <ardb@kernel.org> Thanks! > but I will note that > - depending on the C library used, OpenSSL may use SIGILL trapping to > decide whether these instructions are implemented or not, I'd argue that such software is already broken on big/little system. If it tries on a CPU that has a feature, than migrates to one that doesn't, all bets are off. > - the 32-bit kernel should ideally adopt the same approach, I've had a stab at that, it will take me a little while to test it... > Fortunately, the only A72 that is known to be widely deployed with > 32-bit kernels and/or user space is the Raspberry Pi 4, which does not > implement the crypto extensions. Ah, I didn't know folk ran 32bit kernels on these. Thanks, James
On Thu, Jan 27, 2022 at 1:39 PM Ard Biesheuvel <ardb@kernel.org> wrote: > Cortex-A57 and Cortex-A72 have an erratum where an interrupt that > occurs between a pair of AES instructions in aarch32 mode may corrupt > the ELR. The task will subsequently produce the wrong AES result. > Fortunately, the only A72 that is known to be widely deployed with > 32-bit kernels and/or user space is the Raspberry Pi 4, which does not > implement the crypto extensions. I have previously used Amazon EC2 A1 instances (using Graviton 1, Cortex-A72) to run Debian armhf and armel user space in chroot environments. No idea how common that is for production environments, but there is a good chance that actual users might use e.g. 32-bit docker images for Alpine Linux on the same machines. There are also some early A57/A72 chips used in Android devices that may frequently run 32-bit applications, or even be limited to 32-bit kernels, see [1]. Not sure how likely it is for any of them to get (a backport of) this patch though. Arnd [1] https://www.androidpolice.com/2020/01/14/nvidias-shield-tv-dongle-can-only-run-32-bit-apps-but-you-probably-dont-need-to-panic/
Hi James, On Thu, Jan 27, 2022 at 12:29:14PM +0000, James Morse wrote: > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c > index 9e1c1aef9ebd..b06fb054e055 100644 > --- a/arch/arm64/kernel/cpu_errata.c > +++ b/arch/arm64/kernel/cpu_errata.c > @@ -376,6 +376,14 @@ static struct midr_range trbe_write_out_of_range_cpus[] = { > }; > #endif /* CONFIG_ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE */ > > +#ifdef CONFIG_ARM64_ERRATUM_1742098 > +static struct midr_range broken_aarch32_aes[] = { > + MIDR_RANGE(MIDR_CORTEX_A57, 0, 1, 0xf, 0xf), > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A72), > + {}, > +}; > +#endif /* CONFIG_ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE */ > + > const struct arm64_cpu_capabilities arm64_errata[] = { > #ifdef CONFIG_ARM64_WORKAROUND_CLEAN_CACHE > { > @@ -597,6 +605,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = { > .type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE, > CAP_MIDR_RANGE_LIST(trbe_write_out_of_range_cpus), > }, > +#endif > +#ifdef CONFIG_ARM64_ERRATUM_1742098 > + { > + .desc = "ARM erratum 1742098", > + .capability = ARM64_WORKAROUND_1742098, > + CAP_MIDR_RANGE_LIST(broken_aarch32_aes), > + .type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM, > + .cpu_enable = arm64_remove_aes_compat_hwcap, > + }, > #endif > { > } > 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. 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. Is this the first case where we need to change the ELF HWCAPs due to an erratum?
On 2022-01-27 14:52, Arnd Bergmann wrote: > On Thu, Jan 27, 2022 at 1:39 PM Ard Biesheuvel <ardb@kernel.org> wrote: >> Cortex-A57 and Cortex-A72 have an erratum where an interrupt that >> occurs between a pair of AES instructions in aarch32 mode may corrupt >> the ELR. The task will subsequently produce the wrong AES result. > >> Fortunately, the only A72 that is known to be widely deployed with >> 32-bit kernels and/or user space is the Raspberry Pi 4, which does not >> implement the crypto extensions. > > I have previously used Amazon EC2 A1 instances (using Graviton 1, Cortex-A72) > to run Debian armhf and armel user space in chroot environments. No idea how > common that is for production environments, but there is a good chance that > actual users might use e.g. 32-bit docker images for Alpine Linux on the same > machines. > > There are also some early A57/A72 chips used in Android devices that may > frequently run 32-bit applications, or even be limited to 32-bit > kernels, see [1]. > Not sure how likely it is for any of them to get (a backport of) this > patch though. Isn't ChromeOS still notoriously shipping AArch32 userspace? Between Mediatek and Rockchip, both A57 and A72 are definitely playing on that stage. I'd concur that 32-bit *kernel* on v8 is a bit of a specialist sport, especially these days. IIRC Exynos 5433 might have been one of the more common Android setups for that, but 2015 is now an eternity ago in phone years... Robin. > > Arnd > > [1] https://www.androidpolice.com/2020/01/14/nvidias-shield-tv-dongle-can-only-run-32-bit-apps-but-you-probably-dont-need-to-panic/ > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Jan 27, 2022 at 7:43 PM Robin Murphy <robin.murphy@arm.com> wrote: > On 2022-01-27 14:52, Arnd Bergmann wrote: > > On Thu, Jan 27, 2022 at 1:39 PM Ard Biesheuvel <ardb@kernel.org> wrote: > >> Cortex-A57 and Cortex-A72 have an erratum where an interrupt that > >> occurs between a pair of AES instructions in aarch32 mode may corrupt > >> the ELR. The task will subsequently produce the wrong AES result. > > > >> Fortunately, the only A72 that is known to be widely deployed with > >> 32-bit kernels and/or user space is the Raspberry Pi 4, which does not > >> implement the crypto extensions. > > > > I have previously used Amazon EC2 A1 instances (using Graviton 1, Cortex-A72) > > to run Debian armhf and armel user space in chroot environments. No idea how > > common that is for production environments, but there is a good chance that > > actual users might use e.g. 32-bit docker images for Alpine Linux on the same > > machines. > > > > There are also some early A57/A72 chips used in Android devices that may > > frequently run 32-bit applications, or even be limited to 32-bit > > kernels, see [1]. > > Not sure how likely it is for any of them to get (a backport of) this > > patch though. > > Isn't ChromeOS still notoriously shipping AArch32 userspace? Between > Mediatek and Rockchip, both A57 and A72 are definitely playing on that > stage. I wouldn't call it "notorious", but indeed that is an important user that I missed. For some reason I misremembered that both mt8173 and rk3399 are Cortex-A73 based, but they are A72 as you say. > I'd concur that 32-bit *kernel* on v8 is a bit of a specialist sport, > especially these days. IIRC Exynos 5433 might have been one of the more > common Android setups for that, but 2015 is now an eternity ago in phone > years.. Unfortunately, 32-bit kernels are still a thing on low-end Android phones, including some very recent Cortex-A55 based products with Android Go edition and 2GB of RAM or less, and also some highly popular (at the time) Cortex-A53 Snapdragon 410 phones started shipping before a 64-bit kernel port was available on Snapdragon. A57/A72 at least disappeared from phones a long time ago because of both cost and power consumption reasons, so it's only the low end devices now. I'm not aware of any Android devices actually shipping with a 64-bit kernel and 32-bit user space, maybe that was never an option in the SDK. Arnd
On 2022-01-27 19:55, Arnd Bergmann wrote: > On Thu, Jan 27, 2022 at 7:43 PM Robin Murphy <robin.murphy@arm.com> wrote: >> On 2022-01-27 14:52, Arnd Bergmann wrote: >>> On Thu, Jan 27, 2022 at 1:39 PM Ard Biesheuvel <ardb@kernel.org> wrote: >>>> Cortex-A57 and Cortex-A72 have an erratum where an interrupt that >>>> occurs between a pair of AES instructions in aarch32 mode may corrupt >>>> the ELR. The task will subsequently produce the wrong AES result. >>> >>>> Fortunately, the only A72 that is known to be widely deployed with >>>> 32-bit kernels and/or user space is the Raspberry Pi 4, which does not >>>> implement the crypto extensions. >>> >>> I have previously used Amazon EC2 A1 instances (using Graviton 1, Cortex-A72) >>> to run Debian armhf and armel user space in chroot environments. No idea how >>> common that is for production environments, but there is a good chance that >>> actual users might use e.g. 32-bit docker images for Alpine Linux on the same >>> machines. >>> >>> There are also some early A57/A72 chips used in Android devices that may >>> frequently run 32-bit applications, or even be limited to 32-bit >>> kernels, see [1]. >>> Not sure how likely it is for any of them to get (a backport of) this >>> patch though. >> >> Isn't ChromeOS still notoriously shipping AArch32 userspace? Between >> Mediatek and Rockchip, both A57 and A72 are definitely playing on that >> stage. > > I wouldn't call it "notorious", but indeed that is an important user > that I missed. For some reason I misremembered that both mt8173 and > rk3399 are Cortex-A73 based, but they are A72 as you say. Although it seems I misremembered about A57 - that must have been in a preproduction version of what became MT8173, or I'm imagining it completely. >> I'd concur that 32-bit *kernel* on v8 is a bit of a specialist sport, >> especially these days. IIRC Exynos 5433 might have been one of the more >> common Android setups for that, but 2015 is now an eternity ago in phone >> years.. > > Unfortunately, 32-bit kernels are still a thing on low-end Android phones, > including some very recent Cortex-A55 based products with Android Go > edition and 2GB of RAM or less, and also some highly popular (at the time) > Cortex-A53 Snapdragon 410 phones started shipping before a 64-bit kernel > port was available on Snapdragon. Apologies, I really picked the wrong wording there, my head was buried in the context of this patch and larger cores in general - by "on v8" I think I was trying to imply shorthand for A57 up to A75 (after which it becomes moot anyway), and bare-metal rather than VMs at that. At the lower end and into weird embedded stuff AArch32 is indeed very much alive and well - obviously Cortex-A32 is v8, for starters. Luckily it isn't relevant to this erratum, although there is of course the implication for workarounds in general. > A57/A72 at least disappeared from phones a long time ago because of > both cost and power consumption reasons, so it's only the low end devices > now. I'm not aware of any Android devices actually shipping with a 64-bit > kernel and 32-bit user space, maybe that was never an option in the SDK. FWIW I had the impression that that was pretty common for a while back before Google started actively pushing for 64-bit support, once new SoCs had 64-bit kernels from bringup but Android vendors still didn't like the storage/memory cost of having to carry both sets of runtime libraries around. But then my awareness of Android has always been more from the development end rather than what may have actually ended up going to market. Cheers, Robin.
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
diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst index 5342e895fb60..0f255ab8c3e2 100644 --- a/Documentation/arm64/silicon-errata.rst +++ b/Documentation/arm64/silicon-errata.rst @@ -76,10 +76,14 @@ stable kernels. +----------------+-----------------+-----------------+-----------------------------+ | ARM | Cortex-A57 | #1319537 | ARM64_ERRATUM_1319367 | +----------------+-----------------+-----------------+-----------------------------+ +| ARM | Cortex-A57 | #1742098 | ARM64_ERRATUM_1742098 | ++----------------+-----------------+-----------------+-----------------------------+ | ARM | Cortex-A72 | #853709 | N/A | +----------------+-----------------+-----------------+-----------------------------+ | ARM | Cortex-A72 | #1319367 | ARM64_ERRATUM_1319367 | +----------------+-----------------+-----------------+-----------------------------+ +| ARM | Cortex-A72 | #1655431 | ARM64_ERRATUM_1742098 | ++----------------+-----------------+-----------------+-----------------------------+ | ARM | Cortex-A73 | #858921 | ARM64_ERRATUM_858921 | +----------------+-----------------+-----------------+-----------------------------+ | ARM | Cortex-A76 | #1188873,1418040| ARM64_ERRATUM_1418040 | diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 6978140edfa4..0daf4fff0eaf 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -488,6 +488,22 @@ config ARM64_ERRATUM_834220 If unsure, say Y. +config ARM64_ERRATUM_1742098 + bool "Cortex-A57/A72: 1742098: ELR recorded incorrectly on interrupt taken between cryptographic instructions in a sequence" + depends on COMPAT + default y + help + This option removes the AES hwcap for aarch32 user-space to + workaround erratum 1742098 on Cortex-A57 and Cortex-A72. + + Affected parts may corrupt the AES state if an interrupt is + taken between a pair of AES instructions. These instructions + are only present if the cryptography extensions are present. + All software should have a fallback implementation for CPUs + that don't implement the cryptography extensions. + + If unsure, say Y. + config ARM64_ERRATUM_845719 bool "Cortex-A53: 845719: a load might read incorrect data" depends on COMPAT diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index ef6be92b1921..355313d46c14 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -857,6 +857,7 @@ extern struct arm64_ftr_override id_aa64isar1_override; u32 get_kvm_ipa_limit(void); void dump_cpu_features(void); +void arm64_remove_aes_compat_hwcap(const struct arm64_cpu_capabilities *cap); #endif /* __ASSEMBLY__ */ diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c index 9e1c1aef9ebd..b06fb054e055 100644 --- a/arch/arm64/kernel/cpu_errata.c +++ b/arch/arm64/kernel/cpu_errata.c @@ -376,6 +376,14 @@ static struct midr_range trbe_write_out_of_range_cpus[] = { }; #endif /* CONFIG_ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE */ +#ifdef CONFIG_ARM64_ERRATUM_1742098 +static struct midr_range broken_aarch32_aes[] = { + MIDR_RANGE(MIDR_CORTEX_A57, 0, 1, 0xf, 0xf), + MIDR_ALL_VERSIONS(MIDR_CORTEX_A72), + {}, +}; +#endif /* CONFIG_ARM64_WORKAROUND_TRBE_WRITE_OUT_OF_RANGE */ + const struct arm64_cpu_capabilities arm64_errata[] = { #ifdef CONFIG_ARM64_WORKAROUND_CLEAN_CACHE { @@ -597,6 +605,15 @@ const struct arm64_cpu_capabilities arm64_errata[] = { .type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE, CAP_MIDR_RANGE_LIST(trbe_write_out_of_range_cpus), }, +#endif +#ifdef CONFIG_ARM64_ERRATUM_1742098 + { + .desc = "ARM erratum 1742098", + .capability = ARM64_WORKAROUND_1742098, + CAP_MIDR_RANGE_LIST(broken_aarch32_aes), + .type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM, + .cpu_enable = arm64_remove_aes_compat_hwcap, + }, #endif { } 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 */ + #ifdef CONFIG_KVM static bool is_kvm_protected_mode(const struct arm64_cpu_capabilities *entry, int __unused) { diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps index 870c39537dd0..6a3a5c116668 100644 --- a/arch/arm64/tools/cpucaps +++ b/arch/arm64/tools/cpucaps @@ -55,6 +55,7 @@ WORKAROUND_1418040 WORKAROUND_1463225 WORKAROUND_1508412 WORKAROUND_1542419 +WORKAROUND_1742098 WORKAROUND_TRBE_OVERWRITE_FILL_MODE WORKAROUND_TSB_FLUSH_FAILURE WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
Cortex-A57 and Cortex-A72 have an erratum where an interrupt that occurs between a pair of AES instructions in aarch32 mode may corrupt the ELR. The task will subsequently produce the wrong AES result. The AES instructions are part of the cryptographic extensions, which are optional. User-space software will detect the support for these instructions from the hwcaps. If the platform doesn't support these instructions a software implementation should be used. Remove the hwcap bits on affected parts to indicate user-space should not use the AES instructions. CC: Ard Biesheuvel <ardb@kernel.org> CC: Suzuki K Poulose <suzuki.poulose@arm.com> CC: <stable@vger.kernel.org> Signed-off-by: James Morse <james.morse@arm.com> --- SDEN: A57: https://developer.arm.com/documentation/epm049219/2300 1742098 A72: https://developer.arm.com/documentation/epm012079/11 1655431 --- Documentation/arm64/silicon-errata.rst | 4 ++++ arch/arm64/Kconfig | 16 ++++++++++++++++ arch/arm64/include/asm/cpufeature.h | 1 + arch/arm64/kernel/cpu_errata.c | 17 +++++++++++++++++ arch/arm64/kernel/cpufeature.c | 23 +++++++++++++++++++++++ arch/arm64/tools/cpucaps | 1 + 6 files changed, 62 insertions(+)