| 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: <aGevCYzaRPLMGzL0@J2N7QTR9R3>
Date: Fri, 4 Jul 2025 11:38:01 +0100
From: Mark Rutland <mark.rutland@....com>
To: Zaid Alali <zaidal@...amperecomputing.com>
Cc: catalin.marinas@....com, will@...nel.org, corbet@....net,
maz@...nel.org, mbenes@...e.cz, puranjay@...nel.org,
broonie@...nel.org, oliver.upton@...ux.dev, andre.przywara@....com,
kevin.brodsky@....com, scott@...amperecomputing.com,
james.clark@...aro.org, james.morse@....com,
anshuman.khandual@....com, shameerali.kolothum.thodi@...wei.com,
joey.gouly@....com, gshan@...hat.com, yangyicong@...ilicon.com,
linux-arm-kernel@...ts.infradead.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] arm64: errata: Add Ampere erratum AC03_CPU_50
workaround alternative
On Thu, Jul 03, 2025 at 02:46:57PM -0700, Zaid Alali wrote:
> Add an alternative code sequence to work around Ampere erratum
> AC03_CPU_50 on AmpereOne and AC04_CPU_19 on AmpereOne AC04.
>
> Due to AC03_CPU_50, when ICC_PMR_EL1 should have a value of 0xf0 a
> direct read of the register will return a value of 0xf8. An incorrect
> value from a direct read can only happen with the value 0xf0.
>
> This only occurs when the following conditions are met:
> SCR_EL3.FIQ=1 and,
> PE is NOT in EL3/Secure state and,
> ICC_PMR_EL1.Priority=0xf8 (Non-Secure Group 1)
>
> The returned interrupt filter priority is affected by this, and
> returns 0xf8 but should be 0xf0, as per ARM. This workaround fixes
> the issue here.
>
> Based on this Defect (AArch-21735), this does not apply to virtual
> interrupts. It also does not apply when SCR_EL3.FIQ=0, as no
> modification of ICC_PMR_EL1 is required.
Last time this was posted:
https://lore.kernel.org/linux-arm-kernel/20250127201829.209258-1-zaidal@os.amperecomputing.com/
... Marc Zyngier asked:
Are you saying that this is erratum is *strictly* AARCH-21735?
... can you please confirm?
> Note: Currently there are no checks against a value of 0xf0, and that
> save restore of 0xf8 -> 0xf0 is fine, so this is all future proofing.
This is a fair amount of work for no functional change.
AFAICT, this can only possibly matter when PMR is configured with
(GICV3_PRIO_UNMASKED | GICV3_PRIO_PSR_I_SET), and I hope to remove
GICV3_PRIO_PSR_I_SET entirely in the near future with a rework of the
way we manipulate DAIF and PMR.
If we did that, we'd only ever program PMR with:
* GICV3_PRIO_UNMASKED // 0xe0
* GICV3_PRIO_IRQ // 0xc0
* GICV3_PRIO_NMI // 0x80
... and IIUC that would avoid the problem entirely, no runtime patching
required.
Given there's no functional issue today, I wonder if we should just
leave this as-is for now, and mabye just add an N/A entry in
silicon-errata.txt.
Mark.
>
> V2:
> - Update commit message to clarify the conditions when the issue
> occurs.
> - Add entry in silicon errata documentation.
>
> Signed-off-by: Zaid Alali <zaidal@...amperecomputing.com>
> ---
> Documentation/arch/arm64/silicon-errata.rst | 4 ++++
> arch/arm64/Kconfig | 14 ++++++++++++++
> arch/arm64/include/asm/arch_gicv3.h | 2 +-
> arch/arm64/include/asm/daifflags.h | 5 ++---
> arch/arm64/include/asm/irqflags.h | 6 +++---
> arch/arm64/include/asm/sysreg.h | 9 +++++++++
> arch/arm64/kernel/cpu_errata.c | 15 +++++++++++++++
> arch/arm64/kernel/entry.S | 3 +++
> arch/arm64/tools/cpucaps | 1 +
> 9 files changed, 52 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
> index b18ef4064bc0..29e0bd8b07cd 100644
> --- a/Documentation/arch/arm64/silicon-errata.rst
> +++ b/Documentation/arch/arm64/silicon-errata.rst
> @@ -59,6 +59,10 @@ stable kernels.
> +----------------+-----------------+-----------------+-----------------------------+
> | Ampere | AmpereOne AC04 | AC04_CPU_23 | AMPERE_ERRATUM_AC04_CPU_23 |
> +----------------+-----------------+-----------------+-----------------------------+
> +| Ampere | AmpereOne | AC03_CPU_50 | AMPERE_ERRATUM_AC03_CPU_50 |
> ++----------------+-----------------+-----------------+-----------------------------+
> +| Ampere | AmpereOne AC04 | AC04_CPU_19 | AMPERE_ERRATUM_AC03_CPU_50 |
> ++----------------+-----------------+-----------------+-----------------------------+
> +----------------+-----------------+-----------------+-----------------------------+
> | ARM | Cortex-A510 | #2457168 | ARM64_ERRATUM_2457168 |
> +----------------+-----------------+-----------------+-----------------------------+
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 55fc331af337..1ca4c296deaa 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -479,6 +479,20 @@ config AMPERE_ERRATUM_AC04_CPU_23
> for corruption, and an ISB after is sufficient to prevent younger
> instructions from hitting the window for corruption.
>
> +config AMPERE_ERRATUM_AC03_CPU_50
> + bool "AmpereOne: AC03_CPU_50: Certain checks for ICC_PMR_EL1 that expects the value 0xf0 may read 0xf8 instead"
> + default y
> + help
> + This option adds an alternative code sequence to work around Ampere
> + erratum AC03_CPU_50 on AmperOne and AC04_CPU_19 on AmpereOne AC04.
> +
> + Due to AC03_CPU_50, when ICC_PMR_EL1 should have a value of 0xf0 a
> + direct read of the register will return a value of 0xf8. An incorrect
> + value from a direct read can only happen with the value 0xf0.
> +
> + The workaround for the erratum will do logical AND 0xf0 to the
> + value read from ICC_PMR_EL1 register before returning the value.
> +
> If unsure, say Y.
>
> config ARM64_WORKAROUND_CLEAN_CACHE
> diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
> index 9e96f024b2f1..299d7e17abdf 100644
> --- a/arch/arm64/include/asm/arch_gicv3.h
> +++ b/arch/arm64/include/asm/arch_gicv3.h
> @@ -127,7 +127,7 @@ static inline void gic_write_bpr1(u32 val)
>
> static inline u32 gic_read_pmr(void)
> {
> - return read_sysreg_s(SYS_ICC_PMR_EL1);
> + return read_sysreg_pmr();
> }
>
> static __always_inline void gic_write_pmr(u32 val)
> diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h
> index fbb5c99eb2f9..022a3640d584 100644
> --- a/arch/arm64/include/asm/daifflags.h
> +++ b/arch/arm64/include/asm/daifflags.h
> @@ -22,8 +22,7 @@
> static inline void local_daif_mask(void)
> {
> WARN_ON(system_has_prio_mask_debugging() &&
> - (read_sysreg_s(SYS_ICC_PMR_EL1) == (GIC_PRIO_IRQOFF |
> - GIC_PRIO_PSR_I_SET)));
> + (read_sysreg_pmr() == (GIC_PRIO_IRQOFF | GIC_PRIO_PSR_I_SET)));
>
> asm volatile(
> "msr daifset, #0xf // local_daif_mask\n"
> @@ -46,7 +45,7 @@ static inline unsigned long local_daif_save_flags(void)
>
> if (system_uses_irq_prio_masking()) {
> /* If IRQs are masked with PMR, reflect it in the flags */
> - if (read_sysreg_s(SYS_ICC_PMR_EL1) != GIC_PRIO_IRQON)
> + if (read_sysreg_pmr() != GIC_PRIO_IRQON)
> flags |= PSR_I_BIT | PSR_F_BIT;
> }
>
> diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
> index d4d7451c2c12..757e7e837992 100644
> --- a/arch/arm64/include/asm/irqflags.h
> +++ b/arch/arm64/include/asm/irqflags.h
> @@ -30,7 +30,7 @@ static __always_inline void __daif_local_irq_enable(void)
> static __always_inline void __pmr_local_irq_enable(void)
> {
> if (IS_ENABLED(CONFIG_ARM64_DEBUG_PRIORITY_MASKING)) {
> - u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
> + u32 pmr = read_sysreg_pmr();
> WARN_ON_ONCE(pmr != GIC_PRIO_IRQON && pmr != GIC_PRIO_IRQOFF);
> }
>
> @@ -59,7 +59,7 @@ static __always_inline void __daif_local_irq_disable(void)
> static __always_inline void __pmr_local_irq_disable(void)
> {
> if (IS_ENABLED(CONFIG_ARM64_DEBUG_PRIORITY_MASKING)) {
> - u32 pmr = read_sysreg_s(SYS_ICC_PMR_EL1);
> + u32 pmr = read_sysreg_pmr();
> WARN_ON_ONCE(pmr != GIC_PRIO_IRQON && pmr != GIC_PRIO_IRQOFF);
> }
>
> @@ -84,7 +84,7 @@ static __always_inline unsigned long __daif_local_save_flags(void)
>
> static __always_inline unsigned long __pmr_local_save_flags(void)
> {
> - return read_sysreg_s(SYS_ICC_PMR_EL1);
> + return read_sysreg_pmr();
> }
>
> /*
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index f1bb0d10c39a..9033110a4589 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -1223,6 +1223,15 @@
> par; \
> })
>
> +#define read_sysreg_pmr() ({ \
> + u64 pmr = read_sysreg_s(SYS_ICC_PMR_EL1); \
> + asm(ALTERNATIVE("nop", "and %0, %0, #0xf0", \
> + ARM64_WORKAROUND_AMPERE_AC03_CPU_50) \
> + : "+r" (pmr) \
> + ); \
> + pmr; \
> +})
> +
> #define SYS_FIELD_VALUE(reg, field, val) reg##_##field##_##val
>
> #define SYS_FIELD_GET(reg, field, val) \
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 59d723c9ab8f..9eec9977ee21 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -564,6 +564,14 @@ static const struct midr_range erratum_ac04_cpu_23_list[] = {
> };
> #endif
>
> +#ifdef CONFIG_AMPERE_ERRATUM_AC03_CPU_50
> +static const struct midr_range erratum_ac03_cpu_50_list[] = {
> + MIDR_ALL_VERSIONS(MIDR_AMPERE1),
> + MIDR_ALL_VERSIONS(MIDR_AMPERE1A),
> + {},
> +};
> +#endif
> +
> const struct arm64_cpu_capabilities arm64_errata[] = {
> #ifdef CONFIG_ARM64_WORKAROUND_CLEAN_CACHE
> {
> @@ -905,6 +913,13 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
> .matches = has_impdef_pmuv3,
> .cpu_enable = cpu_enable_impdef_pmuv3_traps,
> },
> +#ifdef CONFIG_AMPERE_ERRATUM_AC03_CPU_50
> + {
> + .desc = "AmpereOne erratum AC03_CPU_50",
> + .capability = ARM64_WORKAROUND_AMPERE_AC03_CPU_50,
> + ERRATA_MIDR_RANGE_LIST(erratum_ac03_cpu_50_list),
> + },
> +#endif
> {
> }
> };
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 5ae2a34b50bd..6d76f79335a0 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -317,6 +317,9 @@ alternative_if_not ARM64_HAS_GIC_PRIO_MASKING
> alternative_else_nop_endif
>
> mrs_s x20, SYS_ICC_PMR_EL1
> +alternative_if ARM64_WORKAROUND_AMPERE_AC03_CPU_50
> + and x20, x20, #0xf0
> +alternative_else_nop_endif
> str w20, [sp, #S_PMR]
> mov x20, #GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET
> msr_s SYS_ICC_PMR_EL1, x20
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index 10effd4cff6b..de34e36c79ee 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -96,6 +96,7 @@ WORKAROUND_2645198
> WORKAROUND_2658417
> WORKAROUND_AMPERE_AC03_CPU_38
> WORKAROUND_AMPERE_AC04_CPU_23
> +WORKAROUND_AMPERE_AC03_CPU_50
> WORKAROUND_TRBE_OVERWRITE_FILL_MODE
> WORKAROUND_TSB_FLUSH_FAILURE
> WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
> --
> 2.43.0
>
Powered by blists - more mailing lists