| 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: <aG7zzpFgWEQuCp4w@zaid>
Date: Wed, 9 Jul 2025 15:57:18 -0700
From: Zaid Alali <zaidal@...amperecomputing.com>
To: Mark Rutland <mark.rutland@....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 Fri, Jul 04, 2025 at 11:38:01AM +0100, Mark Rutland wrote:
> 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?
Yes, this is strictly AARCH-21735. Apologies, I meant to include that
in the commit message.
>
> > 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.
I agree removing GICV3_PRIO_PSR_I_SET should resolve the issue. I would
leave it as is for now, not sure if adding N/A entry would be useful
since we are not adding a workaround for it.
>
> >
> > 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