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(-)
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(-)
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
--
2.34.1On 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. -- Without deviation from the norm, progress is not possible.
On Tue, Jan 28, 2025 at 08:34:47AM +0000, Marc Zyngier wrote: > 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? > Hi Marc, This only occurs when: When SCR_EL3.FIQ==1 and PE is NOT in EL3/Secure State, and ICC_PMR_EL1.Priority==LowestPriorityImplemented==0xf8 (highest priority is 0x00). Does it equally apply to virtual interrupts or SCR_EL3.FIQ==0? Based on this Defect (AArch-21735) and our implementation, it only affected ICC_PMR_EL1, therefore 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. Automatically sends the unshifted version. For which there is no non-secure shift, does it equally affect G0 and G1 interrupts? When SCR_EL3.FIQ==1 and PE is NOT in EL3/Secure State, and ICC_PMR_EL1.Priority==LowestPriorityImplemented==0xf8 (highest priority is 0x00): The ICC_PMR_EL1.Priority read returns an unshifted version (0xf8). It should have returned 0xf0. HW shifts the priority and inserts a 1b1 ensure the priority is a Grp1 priority. Should only be visible in HW. When SCR_EL3.FIQ==1 and PE is NOT in EL3/Secure State, and ICC_PMR_EL1.Priority==Grp0: This issue would not occur. Returned value would be the highest priority, 0x00. The returned value is correct as per ARM. When SCR_EL3.FIQ==1 and PE is NOT in EL3/Secure State, and ICC_PMR_EL1.Priority==Grp1 (but not the lowest priority): This issue would not occur. Returned value would be the shifted version. The returned value is correct as per ARM. > > > > 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. > > -- > Without deviation from the norm, progress is not possible.
On Tue, 04 Feb 2025 21:46:06 +0000, Zaid Alali <zaidal@os.amperecomputing.com> wrote: > > On Tue, Jan 28, 2025 at 08:34:47AM +0000, Marc Zyngier wrote: > > 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? > > > > Hi Marc, > > This only occurs when: > When SCR_EL3.FIQ==1 and PE is NOT in EL3/Secure State, > and ICC_PMR_EL1.Priority==LowestPriorityImplemented==0xf8 (highest priority is 0x00). > > Does it equally apply to virtual interrupts or SCR_EL3.FIQ==0? > > Based on this Defect (AArch-21735) and our implementation, it only > affected ICC_PMR_EL1, therefore does not apply to virtual interrupts. Are you saying that this is erratum is *strictly* AARCH-21735? M. -- Without deviation from the norm, progress is not possible.
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 -- Thanks, Oliver
On Tue, Jan 28, 2025 at 12:54:26PM -0800, Oliver Upton wrote: > 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 > We prefer to keep pNMIs enabled if possible since it is useful, and we will send another patch for AC03_CPU_36 to fix that. If this is not feasible we can send another patch to disable it in both AC03_CPU_36 and AC03_CPU_50. > -- > Thanks, > Oliver
© 2016 - 2026 Red Hat, Inc.