| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <865xilir33.wl-maz@kernel.org>
Date: Wed, 30 Apr 2025 16:53:04 +0100
From: Marc Zyngier <maz@...nel.org>
To: D Scott Phillips <scott@...amperecomputing.com>
Cc: Catalin Marinas <catalin.marinas@....com>,
James Clark <james.clark@...aro.org>,
James Morse <james.morse@....com>,
Joey Gouly <joey.gouly@....com>,
Kevin Brodsky <kevin.brodsky@....com>,
Mark Brown <broonie@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Oliver Upton <oliver.upton@...ux.dev>,
"Rob Herring (Arm)" <robh@...nel.org>,
Shameer Kolothum <shameerali.kolothum.thodi@...wei.com>,
Shiqi Liu <shiqiliu@...t.edu.cn>,
Will Deacon <will@...nel.org>,
Yicong Yang <yangyicong@...ilicon.com>,
kvmarm@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23
On Fri, 25 Apr 2025 03:47:41 +0100,
D Scott Phillips <scott@...amperecomputing.com> wrote:
>
> Updates to HCR_EL2 can rarely corrupt simultaneous translations for data
> addresses initiated by load/store instructions. Only instruction
> initiated translations are vulnerable, not translations from prefetches
> for example. A DSB before the store to HCR_EL2 is sufficient to prevent older
> instructions from hitting the window for corruption, and an ISB after
> is sufficient to prevent younger instructions from hitting the window
> for corruption.
>
> Signed-off-by: D Scott Phillips <scott@...amperecomputing.com>
> ---
> v1: https://lore.kernel.org/kvmarm/20250415154711.1698544-2-scott@os.amperecomputing.com/
> Changes since v1:
> - Add a write_sysreg_hcr() helper (Oliver)
> - Add more specific wording in the errata description (Oliver & Marc)
>
> arch/arm64/Kconfig | 17 +++++++++++++++++
> arch/arm64/include/asm/hardirq.h | 4 ++--
> arch/arm64/include/asm/sysreg.h | 10 ++++++++++
> arch/arm64/kernel/cpu_errata.c | 14 ++++++++++++++
> arch/arm64/kvm/at.c | 8 ++++----
> arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
> arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +-
> arch/arm64/kvm/hyp/nvhe/switch.c | 2 +-
> arch/arm64/kvm/hyp/vhe/switch.c | 2 +-
> arch/arm64/kvm/hyp/vhe/tlb.c | 4 ++--
> arch/arm64/tools/cpucaps | 1 +
> 11 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index a182295e6f08b..3ae4e80e3002b 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -464,6 +464,23 @@ config AMPERE_ERRATUM_AC03_CPU_38
>
> If unsure, say Y.
>
> +config AMPERE_ERRATUM_AC04_CPU_23
> + bool "AmpereOne: AC04_CPU_23: Failure to synchronize writes to HCR_EL2 may corrupt address translations."
> + default y
> + help
> + This option adds an alternative code sequence to work around Ampere
> + errata AC04_CPU_23 on AmpereOne.
> +
> + Updates to HCR_EL2 can rarely corrupt simultaneous translations for
> + data addresses initiated by load/store instructions. Only
> + instruction initiated translations are vulnerable, not translations
> + from prefetches for example. A DSB before the store to HCR_EL2 is
> + sufficient to prevent older instructions from hitting the window
> + for corruption, and an ISB after is sufficient to prevent younger
> + instructions from hitting the window for corruption.
> +
> + If unsure, say Y.
> +
> config ARM64_WORKAROUND_CLEAN_CACHE
> bool
>
> diff --git a/arch/arm64/include/asm/hardirq.h b/arch/arm64/include/asm/hardirq.h
> index cbfa7b6f2e098..77d6b8c63d4e6 100644
> --- a/arch/arm64/include/asm/hardirq.h
> +++ b/arch/arm64/include/asm/hardirq.h
> @@ -41,7 +41,7 @@ do { \
> \
> ___hcr = read_sysreg(hcr_el2); \
> if (!(___hcr & HCR_TGE)) { \
> - write_sysreg(___hcr | HCR_TGE, hcr_el2); \
> + write_sysreg_hcr(___hcr | HCR_TGE); \
> isb(); \
> } \
> /* \
> @@ -82,7 +82,7 @@ do { \
> */ \
> barrier(); \
> if (!___ctx->cnt && !(___hcr & HCR_TGE)) \
> - write_sysreg(___hcr, hcr_el2); \
> + write_sysreg_hcr(___hcr); \
> } while (0)
>
> static inline void ack_bad_irq(unsigned int irq)
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 2639d3633073d..d41eeba7f8201 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -1185,6 +1185,16 @@
> write_sysreg_s(__scs_new, sysreg); \
> } while (0)
>
> +#define write_sysreg_hcr(__val) do { \
> + if(IS_ENABLED(CONFIG_AMPERE_ERRATUM_AC04_CPU_23) && \
> + alternative_has_cap_unlikely(ARM64_WORKAROUND_AMPERE_AC04_CPU_23)) \
> + asm volatile("dsb nsh; msr hcr_el2, %x0; isb" \
> + : : "rZ" (__val)); \
> + else \
> + asm volatile("msr hcr_el2, %x0" \
> + : : "rZ" (__val)); \
> +} while (0)
> +
I'm worried that some of these accesses may occur before we compute
the capabilities. I'd be more confident if the default was to have the
workaround, and only to relax it once we know we're not on a broken
system.
But that leaves the question of the early stuff that runs before we
get to C code. Are you sure this isn't affected by this issue (for
example, the code in head.S, hyp-stub.S, and el2-setup.h)?
> #define read_sysreg_par() ({ \
> u64 par; \
> asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412)); \
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index b55f5f7057502..60f1b70fc0845 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -557,6 +557,13 @@ static const struct midr_range erratum_ac03_cpu_38_list[] = {
> };
> #endif
>
> +#ifdef CONFIG_AMPERE_ERRATUM_AC04_CPU_23
> +static const struct midr_range erratum_ac04_cpu_23_list[] = {
> + MIDR_ALL_VERSIONS(MIDR_AMPERE1A),
> + {},
> +};
> +#endif
> +
> const struct arm64_cpu_capabilities arm64_errata[] = {
> #ifdef CONFIG_ARM64_WORKAROUND_CLEAN_CACHE
> {
> @@ -875,6 +882,13 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
> .capability = ARM64_WORKAROUND_AMPERE_AC03_CPU_38,
> ERRATA_MIDR_RANGE_LIST(erratum_ac03_cpu_38_list),
> },
> +#endif
> +#ifdef CONFIG_AMPERE_ERRATUM_AC04_CPU_23
> + {
> + .desc = "AmpereOne erratum AC04_CPU_23",
> + .capability = ARM64_WORKAROUND_AMPERE_AC04_CPU_23,
> + ERRATA_MIDR_RANGE_LIST(erratum_ac04_cpu_23_list),
> + },
> #endif
This needs to be captured in Documentation/arch/arm64/silicon-errata.rst.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists