diff mbox series

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

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

Commit Message

James Morse Jan. 27, 2022, 12:29 p.m. UTC
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(+)

Comments

Ard Biesheuvel Jan. 27, 2022, 12:39 p.m. UTC | #1
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
>
James Morse Jan. 27, 2022, 2:45 p.m. UTC | #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
Arnd Bergmann Jan. 27, 2022, 2:52 p.m. UTC | #3
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/
Catalin Marinas Jan. 27, 2022, 3:25 p.m. UTC | #4
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?
Robin Murphy Jan. 27, 2022, 6:43 p.m. UTC | #5
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
Arnd Bergmann Jan. 27, 2022, 7:55 p.m. UTC | #6
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
Robin Murphy Jan. 27, 2022, 11:31 p.m. UTC | #7
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.
James Morse Feb. 15, 2022, 1:02 p.m. UTC | #8
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 mbox series

Patch

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