| Message ID | 20250415154711.1698544-2-scott@os.amperecomputing.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36 | expand |
Hi, On Tue, Apr 15, 2025 at 08:47:11AM -0700, D Scott Phillips wrote: > Updates to HCR_EL2 can rarely corrupt simultaneous translations from > either earlier translations (back to the previous dsb) or later > translations (up to the next isb). Put a dsb before and isb after writes > to HCR_EL2. > > Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com> > --- > arch/arm64/Kconfig | 13 +++++++++++++ > arch/arm64/include/asm/sysreg.h | 7 +++++++ > arch/arm64/kernel/cpu_errata.c | 14 ++++++++++++++ > arch/arm64/tools/cpucaps | 1 + > 4 files changed, 35 insertions(+) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index e5fd87446a3b8..2a2e1c8de6a16 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -481,6 +481,19 @@ config AMPERE_ERRATUM_AC03_CPU_38 > > If unsure, say Y. > > +config AMPERE_ERRATUM_AC04_CPU_23 > + bool "AmpereOne: AC04_CPU_23: Failure to synchronize writes to HCR_EL2 may corrupt address translations." > + default y > + help > + This option adds an alternative code sequence to work around Ampere > + errata AC04_CPU_23 on AmpereOne. > + > + Updates to HCR_EL2 can rarely corrupt simultaneous translations from > + either earlier translations (back to the previous dsb) or later > + translations (up to the next isb). > + > + If unsure, say Y. > + > config ARM64_WORKAROUND_CLEAN_CACHE > bool > > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h > index e7781f7e7f7a7..253de5bc68834 100644 > --- a/arch/arm64/include/asm/sysreg.h > +++ b/arch/arm64/include/asm/sysreg.h > @@ -1142,6 +1142,10 @@ > (IS_ENABLED(CONFIG_AMPERE_ERRATUM_AC03_CPU_36) && \ > __sysreg_is_hcr_el2(r) && \ > alternative_has_cap_unlikely(ARM64_WORKAROUND_AMPERE_AC03_CPU_36)) > +#define __hcr_el2_ac04_cpu_23(r) \ > + (IS_ENABLED(CONFIG_AMPERE_ERRATUM_AC04_CPU_23) && \ > + __sysreg_is_hcr_el2(r) && \ > + alternative_has_cap_unlikely(ARM64_WORKAROUND_AMPERE_AC04_CPU_23)) > > /* > * The "Z" constraint normally means a zero immediate, but when combined with > @@ -1154,6 +1158,9 @@ > asm volatile("mrs %0, daif; msr daifset, #0xf;" \ > "msr hcr_el2, %x1; msr daif, %0" \ > : "=&r"(__daif) : "rZ" (__val)); \ > + } else if (__hcr_el2_ac04_cpu_23(r)) { \ > + asm volatile("dsb nsh; msr hcr_el2, %x0; isb" \ > + : : "rZ" (__val)); \ At least from your erratum description it isn't clear to me that this eliminates the problem and only narrows the window of opportunity. Couldn't the implementation speculatively fetch translations with an unsynchronized HCR up to the ISB? Do we know what translation regimes are affected by the erratum? Thanks, Oliver
On Tue, 15 Apr 2025 16:47:11 +0100, D Scott Phillips <scott@os.amperecomputing.com> wrote: > > Updates to HCR_EL2 can rarely corrupt simultaneous translations from > either earlier translations (back to the previous dsb) or later > translations (up to the next isb). Put a dsb before and isb after writes > to HCR_EL2. If the write to HCR_EL2 can corrupt translations, what guarantees that such write placed on a page boundary (therefore requiring another TLB lookup to continue in sequence) will be able to get to the ISB? M.
Oliver Upton <oliver.upton@linux.dev> writes: > Hi, > > On Tue, Apr 15, 2025 at 08:47:11AM -0700, D Scott Phillips wrote: >> Updates to HCR_EL2 can rarely corrupt simultaneous translations from >> either earlier translations (back to the previous dsb) or later >> translations (up to the next isb). Put a dsb before and isb after writes >> to HCR_EL2. >> >> Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com> >> --- >> arch/arm64/Kconfig | 13 +++++++++++++ >> arch/arm64/include/asm/sysreg.h | 7 +++++++ >> arch/arm64/kernel/cpu_errata.c | 14 ++++++++++++++ >> arch/arm64/tools/cpucaps | 1 + >> 4 files changed, 35 insertions(+) >> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index e5fd87446a3b8..2a2e1c8de6a16 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -481,6 +481,19 @@ config AMPERE_ERRATUM_AC03_CPU_38 >> >> If unsure, say Y. >> >> +config AMPERE_ERRATUM_AC04_CPU_23 >> + bool "AmpereOne: AC04_CPU_23: Failure to synchronize writes to HCR_EL2 may corrupt address translations." >> + default y >> + help >> + This option adds an alternative code sequence to work around Ampere >> + errata AC04_CPU_23 on AmpereOne. >> + >> + Updates to HCR_EL2 can rarely corrupt simultaneous translations from >> + either earlier translations (back to the previous dsb) or later >> + translations (up to the next isb). >> + >> + If unsure, say Y. >> + >> config ARM64_WORKAROUND_CLEAN_CACHE >> bool >> >> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h >> index e7781f7e7f7a7..253de5bc68834 100644 >> --- a/arch/arm64/include/asm/sysreg.h >> +++ b/arch/arm64/include/asm/sysreg.h >> @@ -1142,6 +1142,10 @@ >> (IS_ENABLED(CONFIG_AMPERE_ERRATUM_AC03_CPU_36) && \ >> __sysreg_is_hcr_el2(r) && \ >> alternative_has_cap_unlikely(ARM64_WORKAROUND_AMPERE_AC03_CPU_36)) >> +#define __hcr_el2_ac04_cpu_23(r) \ >> + (IS_ENABLED(CONFIG_AMPERE_ERRATUM_AC04_CPU_23) && \ >> + __sysreg_is_hcr_el2(r) && \ >> + alternative_has_cap_unlikely(ARM64_WORKAROUND_AMPERE_AC04_CPU_23)) >> >> /* >> * The "Z" constraint normally means a zero immediate, but when combined with >> @@ -1154,6 +1158,9 @@ >> asm volatile("mrs %0, daif; msr daifset, #0xf;" \ >> "msr hcr_el2, %x1; msr daif, %0" \ >> : "=&r"(__daif) : "rZ" (__val)); \ >> + } else if (__hcr_el2_ac04_cpu_23(r)) { \ >> + asm volatile("dsb nsh; msr hcr_el2, %x0; isb" \ >> + : : "rZ" (__val)); \ > > At least from your erratum description it isn't clear to me that this > eliminates the problem and only narrows the window of opportunity. > Couldn't the implementation speculatively fetch translations with an > unsynchronized HCR up to the ISB? Do we know what translation regimes > are affected by the erratum? Hi Oliver, I got some clarification from the core folks. The issue affects the data side of the core only, not the instruction side. I'll update my description to include that. Speculation after the `msr hcr_el2` couldn't launch a problem translation when the `msr` is followed by an `isb` like this. Marc Zyngier <maz@kernel.org> writes: > On Tue, 15 Apr 2025 16:47:11 +0100, > If the write to HCR_EL2 can corrupt translations, what guarantees that > such write placed on a page boundary (therefore requiring another TLB > lookup to continue in sequence) will be able to get to the ISB? Hi Marc, I understand now from the core team that an ISB on another page will be ok as the problem is on the data side only.
On Tue, Apr 15, 2025 at 03:13:43PM -0700, D Scott Phillips wrote: > > At least from your erratum description it isn't clear to me that this > > eliminates the problem and only narrows the window of opportunity. > > Couldn't the implementation speculatively fetch translations with an > > unsynchronized HCR up to the ISB? Do we know what translation regimes > > are affected by the erratum? > > Hi Oliver, I got some clarification from the core folks. The issue > affects the data side of the core only, not the instruction side. I'll > update my description to include that. > > Speculation after the `msr hcr_el2` couldn't launch a problem > translation when the `msr` is followed by an `isb` like this. Thanks, agree that the subsequent ISB protects against speculative behavior relating to the instruction stream. To be absolutely certain, there's no risk of, say, a TLB prefetcher pulling in a problematic translation in this window? IOW, there's no behavior that meets the ARM ARM definition of a Speculative operation that can lead to a corrupted translation. Sorry to hassle about these issues but they're helpful for maintaining the workaround in the future. If you can fold all the extra details into the patch for v2 that'd be great. > Marc Zyngier <maz@kernel.org> writes: > > > On Tue, 15 Apr 2025 16:47:11 +0100, > > If the write to HCR_EL2 can corrupt translations, what guarantees that > > such write placed on a page boundary (therefore requiring another TLB > > lookup to continue in sequence) will be able to get to the ISB? > > Hi Marc, I understand now from the core team that an ISB on another page > will be ok as the problem is on the data side only. Thanks, Oliver
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index e5fd87446a3b8..2a2e1c8de6a16 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -481,6 +481,19 @@ config AMPERE_ERRATUM_AC03_CPU_38 If unsure, say Y. +config AMPERE_ERRATUM_AC04_CPU_23 + bool "AmpereOne: AC04_CPU_23: Failure to synchronize writes to HCR_EL2 may corrupt address translations." + default y + help + This option adds an alternative code sequence to work around Ampere + errata AC04_CPU_23 on AmpereOne. + + Updates to HCR_EL2 can rarely corrupt simultaneous translations from + either earlier translations (back to the previous dsb) or later + translations (up to the next isb). + + If unsure, say Y. + config ARM64_WORKAROUND_CLEAN_CACHE bool diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index e7781f7e7f7a7..253de5bc68834 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -1142,6 +1142,10 @@ (IS_ENABLED(CONFIG_AMPERE_ERRATUM_AC03_CPU_36) && \ __sysreg_is_hcr_el2(r) && \ alternative_has_cap_unlikely(ARM64_WORKAROUND_AMPERE_AC03_CPU_36)) +#define __hcr_el2_ac04_cpu_23(r) \ + (IS_ENABLED(CONFIG_AMPERE_ERRATUM_AC04_CPU_23) && \ + __sysreg_is_hcr_el2(r) && \ + alternative_has_cap_unlikely(ARM64_WORKAROUND_AMPERE_AC04_CPU_23)) /* * The "Z" constraint normally means a zero immediate, but when combined with @@ -1154,6 +1158,9 @@ asm volatile("mrs %0, daif; msr daifset, #0xf;" \ "msr hcr_el2, %x1; msr daif, %0" \ : "=&r"(__daif) : "rZ" (__val)); \ + } else if (__hcr_el2_ac04_cpu_23(r)) { \ + asm volatile("dsb nsh; msr hcr_el2, %x0; isb" \ + : : "rZ" (__val)); \ } else { \ asm volatile("msr " __stringify(r) ", %x0" \ : : "rZ" (__val)); \ diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c index 89be85bf631fd..bdb92872791f3 100644 --- a/arch/arm64/kernel/cpu_errata.c +++ b/arch/arm64/kernel/cpu_errata.c @@ -564,6 +564,13 @@ static const struct midr_range erratum_ac03_cpu_38_list[] = { }; #endif +#ifdef CONFIG_AMPERE_ERRATUM_AC04_CPU_23 +static const struct midr_range erratum_ac04_cpu_23_list[] = { + MIDR_ALL_VERSIONS(MIDR_AMPERE1A), + {}, +}; +#endif + const struct arm64_cpu_capabilities arm64_errata[] = { #ifdef CONFIG_ARM64_WORKAROUND_CLEAN_CACHE { @@ -889,6 +896,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_AC04_CPU_23 + { + .desc = "AmpereOne erratum AC04_CPU_23", + .capability = ARM64_WORKAROUND_AMPERE_AC04_CPU_23, + ERRATA_MIDR_RANGE_LIST(erratum_ac04_cpu_23_list), + }, #endif { .desc = "Broken CNTVOFF_EL2", diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps index f430fd5900d15..2b3afe4421af9 100644 --- a/arch/arm64/tools/cpucaps +++ b/arch/arm64/tools/cpucaps @@ -95,6 +95,7 @@ WORKAROUND_2645198 WORKAROUND_2658417 WORKAROUND_AMPERE_AC03_CPU_36 WORKAROUND_AMPERE_AC03_CPU_38 +WORKAROUND_AMPERE_AC04_CPU_23 WORKAROUND_TRBE_OVERWRITE_FILL_MODE WORKAROUND_TSB_FLUSH_FAILURE WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
Updates to HCR_EL2 can rarely corrupt simultaneous translations from either earlier translations (back to the previous dsb) or later translations (up to the next isb). Put a dsb before and isb after writes to HCR_EL2. Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com> --- arch/arm64/Kconfig | 13 +++++++++++++ arch/arm64/include/asm/sysreg.h | 7 +++++++ arch/arm64/kernel/cpu_errata.c | 14 ++++++++++++++ arch/arm64/tools/cpucaps | 1 + 4 files changed, 35 insertions(+)