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@xxxxxxxxxxxxxxxxxxxxxx/ > > ... 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@xxxxxxxxxxxxxxxxxxxxxx> > > --- > > 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 > >
