All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72
@ 2022-01-27 12:29 James Morse
  2022-01-27 12:39 ` Ard Biesheuvel
  2022-01-27 15:25 ` Catalin Marinas
  0 siblings, 2 replies; 9+ messages in thread
From: James Morse @ 2022-01-27 12:29 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Suzuki K Poulose, Ard Biesheuvel,
	james.morse

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(+)

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72
  2022-01-27 12:29 [PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72 James Morse
@ 2022-01-27 12:39 ` Ard Biesheuvel
  2022-01-27 14:45   ` James Morse
  2022-01-27 14:52   ` Arnd Bergmann
  2022-01-27 15:25 ` Catalin Marinas
  1 sibling, 2 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2022-01-27 12:39 UTC (permalink / raw)
  To: James Morse; +Cc: Linux ARM, Catalin Marinas, Will Deacon, Suzuki K Poulose

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
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72
  2022-01-27 12:39 ` Ard Biesheuvel
@ 2022-01-27 14:45   ` James Morse
  2022-01-27 14:52   ` Arnd Bergmann
  1 sibling, 0 replies; 9+ messages in thread
From: James Morse @ 2022-01-27 14:45 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: Linux ARM, Catalin Marinas, Will Deacon, Suzuki K Poulose

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72
  2022-01-27 12:39 ` Ard Biesheuvel
  2022-01-27 14:45   ` James Morse
@ 2022-01-27 14:52   ` Arnd Bergmann
  2022-01-27 18:43     ` Robin Murphy
  1 sibling, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2022-01-27 14:52 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: James Morse, Linux ARM, Catalin Marinas, Will Deacon, Suzuki K Poulose

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/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72
  2022-01-27 12:29 [PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72 James Morse
  2022-01-27 12:39 ` Ard Biesheuvel
@ 2022-01-27 15:25 ` Catalin Marinas
  2022-02-15 13:02   ` James Morse
  1 sibling, 1 reply; 9+ messages in thread
From: Catalin Marinas @ 2022-01-27 15:25 UTC (permalink / raw)
  To: James Morse
  Cc: linux-arm-kernel, Will Deacon, Suzuki K Poulose, Ard Biesheuvel

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?

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72
  2022-01-27 14:52   ` Arnd Bergmann
@ 2022-01-27 18:43     ` Robin Murphy
  2022-01-27 19:55       ` Arnd Bergmann
  0 siblings, 1 reply; 9+ messages in thread
From: Robin Murphy @ 2022-01-27 18:43 UTC (permalink / raw)
  To: Arnd Bergmann, Ard Biesheuvel
  Cc: James Morse, Linux ARM, Catalin Marinas, Will Deacon, Suzuki K Poulose

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72
  2022-01-27 18:43     ` Robin Murphy
@ 2022-01-27 19:55       ` Arnd Bergmann
  2022-01-27 23:31         ` Robin Murphy
  0 siblings, 1 reply; 9+ messages in thread
From: Arnd Bergmann @ 2022-01-27 19:55 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Arnd Bergmann, Ard Biesheuvel, James Morse, Linux ARM,
	Catalin Marinas, Will Deacon, Suzuki K Poulose

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72
  2022-01-27 19:55       ` Arnd Bergmann
@ 2022-01-27 23:31         ` Robin Murphy
  0 siblings, 0 replies; 9+ messages in thread
From: Robin Murphy @ 2022-01-27 23:31 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Ard Biesheuvel, James Morse, Linux ARM, Catalin Marinas,
	Will Deacon, Suzuki K Poulose

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72
  2022-01-27 15:25 ` Catalin Marinas
@ 2022-02-15 13:02   ` James Morse
  0 siblings, 0 replies; 9+ messages in thread
From: James Morse @ 2022-02-15 13:02 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-arm-kernel, Will Deacon, Suzuki K Poulose, Ard Biesheuvel

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-02-15 13:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-27 12:29 [PATCH] arm64: errata: Remove AES hwcap for COMPAT tasks on A57 and A72 James Morse
2022-01-27 12:39 ` Ard Biesheuvel
2022-01-27 14:45   ` James Morse
2022-01-27 14:52   ` Arnd Bergmann
2022-01-27 18:43     ` Robin Murphy
2022-01-27 19:55       ` Arnd Bergmann
2022-01-27 23:31         ` Robin Murphy
2022-01-27 15:25 ` Catalin Marinas
2022-02-15 13:02   ` James Morse

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.