| Message ID | 20250127201829.209258-1-zaidal@os.amperecomputing.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: errata: Add Ampere erratum AC04_CPU_50 workaround alternative | expand |
On Mon, 27 Jan 2025 20:18:29 +0000, Zaid Alali <zaidal@os.amperecomputing.com> wrote: > > Add an alternative code sequence to work around Ampere erratum > AC03_CPU_50 on AmpereOne and Ampere1A. > > 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. Under which precise conditions? Does it equally apply to virtual interrupts or SCR_EL3.FIQ==0, for which there is no non-secure shift (which I can only assume is the source of the erratum)? Does it equally affect G0 and G1 interrupts? > > 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. > > Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com> > --- > arch/arm64/Kconfig | 16 ++++++++++++++++ > arch/arm64/include/asm/arch_gicv3.h | 2 +- > arch/arm64/include/asm/daifflags.h | 4 ++-- > 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 | 4 ++++ > arch/arm64/tools/cpucaps | 1 + Please add an entry to Documentation/arch/arm64/silicon-errata.txt. > 8 files changed, 51 insertions(+), 6 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index fcdd0ed3eca8..8d6e263d66c7 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -461,6 +461,22 @@ config AMPERE_ERRATUM_AC03_CPU_38 > > If unsure, say Y. > > +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 AmpereOne and Ampere1A. > + > + 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. > + An alternative for this would simply to prevent the enabling of pNMI on this platform. M.
On Tue, Jan 28, 2025 at 08:34:47AM +0000, Marc Zyngier wrote: > > +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 AmpereOne and Ampere1A. > > + > > + 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. > > + > > An alternative for this would simply to prevent the enabling of pNMI > on this platform. There's also AC03_CPU_36, where the CPU goes into the weeds if you take an asynchronous exception while fiddling with HCR_EL2. We don't have a mitigation for it, and it can be pretty easily reproduced by using pNMIs while running VMs. So I agree, disabling pNMIs might be the easier way out. [*] https://amperecomputing.com/assets/AmpereOne_Developer_ER_v0_80_20240823_28945022f4.pdf
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index fcdd0ed3eca8..8d6e263d66c7 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -461,6 +461,22 @@ config AMPERE_ERRATUM_AC03_CPU_38 If unsure, say Y. +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 AmpereOne and Ampere1A. + + 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 bool 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..2abea378ebd8 100644 --- a/arch/arm64/include/asm/daifflags.h +++ b/arch/arm64/include/asm/daifflags.h @@ -22,7 +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 | + (read_sysreg_pmr() == (GIC_PRIO_IRQOFF | GIC_PRIO_PSR_I_SET))); asm volatile( @@ -46,7 +46,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 b8303a83c0bf..190409fff3b3 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -1226,6 +1226,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 a78f247029ae..469f778228c8 100644 --- a/arch/arm64/kernel/cpu_errata.c +++ b/arch/arm64/kernel/cpu_errata.c @@ -467,6 +467,14 @@ static const struct midr_range erratum_ac03_cpu_38_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 { @@ -785,6 +793,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_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..aed01144a351 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -318,6 +318,10 @@ alternative_else_nop_endif mrs_s x20, SYS_ICC_PMR_EL1 str w20, [sp, #S_PMR] +alternative_if ARM64_WORKAROUND_AMPERE_AC03_CPU_50 + and x20, x20, #0xf0 +alternative_else_nop_endif + str x20, [sp, #S_PMR_SAVE] 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 eb17f59e543c..9e0776bb8c3b 100644 --- a/arch/arm64/tools/cpucaps +++ b/arch/arm64/tools/cpucaps @@ -93,6 +93,7 @@ WORKAROUND_2457168 WORKAROUND_2645198 WORKAROUND_2658417 WORKAROUND_AMPERE_AC03_CPU_38 +WORKAROUND_AMPERE_AC03_CPU_50 WORKAROUND_TRBE_OVERWRITE_FILL_MODE WORKAROUND_TSB_FLUSH_FAILURE WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
Add an alternative code sequence to work around Ampere erratum AC03_CPU_50 on AmpereOne and Ampere1A. 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. 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. Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com> --- arch/arm64/Kconfig | 16 ++++++++++++++++ arch/arm64/include/asm/arch_gicv3.h | 2 +- arch/arm64/include/asm/daifflags.h | 4 ++-- 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 | 4 ++++ arch/arm64/tools/cpucaps | 1 + 8 files changed, 51 insertions(+), 6 deletions(-)