linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36
@ 2025-04-15 15:47 D Scott Phillips
  2025-04-15 15:47 ` [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23 D Scott Phillips
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: D Scott Phillips @ 2025-04-15 15:47 UTC (permalink / raw)
  To: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Marc Zyngier, Mark Brown, Mark Rutland,
	Oliver Upton, Rob Herring (Arm), Shameer Kolothum, Shiqi Liu,
	Will Deacon, Yicong Yang, kvmarm, linux-arm-kernel, open list

AC03_CPU_36 can cause asynchronous exceptions to be routed to the wrong
exception level if an async exception coincides with an update to the
controls for the target exception level in HCR_EL2. On affected
machines, always do writes to HCR_EL2 with async exceptions blocked.

Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com>
---
 arch/arm64/Kconfig              | 17 +++++++++++++++++
 arch/arm64/include/asm/sysreg.h | 18 ++++++++++++++++--
 arch/arm64/kernel/cpu_errata.c  | 14 ++++++++++++++
 arch/arm64/tools/cpucaps        |  1 +
 4 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a182295e6f08b..e5fd87446a3b8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -445,6 +445,23 @@ menu "Kernel Features"
 
 menu "ARM errata workarounds via the alternatives framework"
 
+config AMPERE_ERRATUM_AC03_CPU_36
+        bool "AmpereOne: AC03_CPU_36: CPU can take an invalid exception, if an asynchronous exception to EL2 occurs while EL2 software is changing the EL2 exception controls."
+	default y
+	help
+	  This option adds an alternative code sequence to work around Ampere
+	  errata AC03_CPU_36 on AmpereOne.
+
+	  If an async exception happens at the same time as an update to the
+	  controls for the target EL for async exceptions, an exception can be
+	  delivered to the wrong EL. For example, an EL may be routed from EL2
+	  to EL1.
+
+	  The workaround masks all asynchronous exception types when writing
+	  to HCR_EL2.
+
+	  If unsure, say Y.
+
 config AMPERE_ERRATUM_AC03_CPU_38
         bool "AmpereOne: AC03_CPU_38: Certain bits in the Virtualization Translation Control Register and Translation Control Registers do not follow RES0 semantics"
 	default y
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 2639d3633073d..e7781f7e7f7a7 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -1136,14 +1136,28 @@
 	__val;							\
 })
 
+#define __sysreg_is_hcr_el2(r)					\
+	(__builtin_strcmp("hcr_el2", __stringify(r)) == 0)
+#define __hcr_el2_ac03_cpu_36(r)				\
+	(IS_ENABLED(CONFIG_AMPERE_ERRATUM_AC03_CPU_36) &&	\
+	 __sysreg_is_hcr_el2(r) &&				\
+	 alternative_has_cap_unlikely(ARM64_WORKAROUND_AMPERE_AC03_CPU_36))
+
 /*
  * The "Z" constraint normally means a zero immediate, but when combined with
  * the "%x0" template means XZR.
  */
 #define write_sysreg(v, r) do {					\
 	u64 __val = (u64)(v);					\
-	asm volatile("msr " __stringify(r) ", %x0"		\
-		     : : "rZ" (__val));				\
+	if (__hcr_el2_ac03_cpu_36(r)) {				\
+		u64 __daif;					\
+		asm volatile("mrs %0, daif; msr daifset, #0xf;"	\
+			     "msr hcr_el2, %x1; msr daif, %0"	\
+		: "=&r"(__daif) : "rZ" (__val));		\
+	} else {						\
+		asm volatile("msr " __stringify(r) ", %x0"	\
+			     : : "rZ" (__val));			\
+	}							\
 } while (0)
 
 /*
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index b55f5f7057502..89be85bf631fd 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -549,6 +549,13 @@ static const struct midr_range erratum_spec_ssbs_list[] = {
 };
 #endif
 
+#ifdef CONFIG_AMPERE_ERRATUM_AC03_CPU_36
+static const struct midr_range erratum_ac03_cpu_36_list[] = {
+	MIDR_ALL_VERSIONS(MIDR_AMPERE1),
+	{},
+};
+#endif
+
 #ifdef CONFIG_AMPERE_ERRATUM_AC03_CPU_38
 static const struct midr_range erratum_ac03_cpu_38_list[] = {
 	MIDR_ALL_VERSIONS(MIDR_AMPERE1),
@@ -869,6 +876,13 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		ERRATA_MIDR_RANGE_LIST(erratum_spec_unpriv_load_list),
 	},
 #endif
+#ifdef CONFIG_AMPERE_ERRATUM_AC03_CPU_36
+	{
+		.desc = "AmpereOne erratum AC03_CPU_36",
+		.capability = ARM64_WORKAROUND_AMPERE_AC03_CPU_36,
+		ERRATA_MIDR_RANGE_LIST(erratum_ac03_cpu_36_list),
+	},
+#endif
 #ifdef CONFIG_AMPERE_ERRATUM_AC03_CPU_38
 	{
 		.desc = "AmpereOne erratum AC03_CPU_38",
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 772c1b008e437..f430fd5900d15 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -93,6 +93,7 @@ WORKAROUND_2077057
 WORKAROUND_2457168
 WORKAROUND_2645198
 WORKAROUND_2658417
+WORKAROUND_AMPERE_AC03_CPU_36
 WORKAROUND_AMPERE_AC03_CPU_38
 WORKAROUND_TRBE_OVERWRITE_FILL_MODE
 WORKAROUND_TSB_FLUSH_FAILURE
-- 
2.48.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23
  2025-04-15 15:47 [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36 D Scott Phillips
@ 2025-04-15 15:47 ` D Scott Phillips
  2025-04-15 17:06   ` Oliver Upton
  2025-04-15 18:38   ` Marc Zyngier
  2025-04-15 17:12 ` [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36 Oliver Upton
  2025-04-16  7:19 ` Marc Zyngier
  2 siblings, 2 replies; 18+ messages in thread
From: D Scott Phillips @ 2025-04-15 15:47 UTC (permalink / raw)
  To: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Marc Zyngier, Mark Brown, Mark Rutland,
	Oliver Upton, Rob Herring (Arm), Shameer Kolothum, Shiqi Liu,
	Will Deacon, Yicong Yang, kvmarm, linux-arm-kernel, open list

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));			\
 	} 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
-- 
2.48.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23
  2025-04-15 15:47 ` [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23 D Scott Phillips
@ 2025-04-15 17:06   ` Oliver Upton
  2025-04-15 22:13     ` D Scott Phillips
  2025-04-15 18:38   ` Marc Zyngier
  1 sibling, 1 reply; 18+ messages in thread
From: Oliver Upton @ 2025-04-15 17:06 UTC (permalink / raw)
  To: D Scott Phillips
  Cc: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Marc Zyngier, Mark Brown, Mark Rutland,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, open list

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36
  2025-04-15 15:47 [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36 D Scott Phillips
  2025-04-15 15:47 ` [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23 D Scott Phillips
@ 2025-04-15 17:12 ` Oliver Upton
  2025-04-15 17:30   ` D Scott Phillips
  2025-04-16  7:19 ` Marc Zyngier
  2 siblings, 1 reply; 18+ messages in thread
From: Oliver Upton @ 2025-04-15 17:12 UTC (permalink / raw)
  To: D Scott Phillips
  Cc: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Marc Zyngier, Mark Brown, Mark Rutland,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, open list

On Tue, Apr 15, 2025 at 08:47:10AM -0700, D Scott Phillips wrote:
> AC03_CPU_36 can cause asynchronous exceptions to be routed to the wrong
> exception level if an async exception coincides with an update to the
> controls for the target exception level in HCR_EL2. On affected
> machines, always do writes to HCR_EL2 with async exceptions blocked.
> 
> Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com>
> ---
>  arch/arm64/Kconfig              | 17 +++++++++++++++++
>  arch/arm64/include/asm/sysreg.h | 18 ++++++++++++++++--
>  arch/arm64/kernel/cpu_errata.c  | 14 ++++++++++++++
>  arch/arm64/tools/cpucaps        |  1 +
>  4 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index a182295e6f08b..e5fd87446a3b8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -445,6 +445,23 @@ menu "Kernel Features"
>  
>  menu "ARM errata workarounds via the alternatives framework"
>  
> +config AMPERE_ERRATUM_AC03_CPU_36
> +        bool "AmpereOne: AC03_CPU_36: CPU can take an invalid exception, if an asynchronous exception to EL2 occurs while EL2 software is changing the EL2 exception controls."
> +	default y
> +	help
> +	  This option adds an alternative code sequence to work around Ampere
> +	  errata AC03_CPU_36 on AmpereOne.
> +
> +	  If an async exception happens at the same time as an update to the
> +	  controls for the target EL for async exceptions, an exception can be
> +	  delivered to the wrong EL. For example, an EL may be routed from EL2
> +	  to EL1.
> +
> +	  The workaround masks all asynchronous exception types when writing
> +	  to HCR_EL2.
> +
> +	  If unsure, say Y.
> +
>  config AMPERE_ERRATUM_AC03_CPU_38
>          bool "AmpereOne: AC03_CPU_38: Certain bits in the Virtualization Translation Control Register and Translation Control Registers do not follow RES0 semantics"
>  	default y
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 2639d3633073d..e7781f7e7f7a7 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -1136,14 +1136,28 @@
>  	__val;							\
>  })
>  
> +#define __sysreg_is_hcr_el2(r)					\
> +	(__builtin_strcmp("hcr_el2", __stringify(r)) == 0)

This looks fragile. What about:

	write_sysreg(hcr, HCR_EL2);

or:

	write_sysreg_s(hcr, SYS_HCR_EL2);


Thanks,
Oliver


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36
  2025-04-15 17:12 ` [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36 Oliver Upton
@ 2025-04-15 17:30   ` D Scott Phillips
  2025-04-15 18:12     ` Oliver Upton
  0 siblings, 1 reply; 18+ messages in thread
From: D Scott Phillips @ 2025-04-15 17:30 UTC (permalink / raw)
  To: Oliver Upton
  Cc: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Marc Zyngier, Mark Brown, Mark Rutland,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, linux-kernel

Oliver Upton <oliver.upton@linux.dev> writes:

> On Tue, Apr 15, 2025 at 08:47:10AM -0700, D Scott Phillips wrote:
>> AC03_CPU_36 can cause asynchronous exceptions to be routed to the wrong
>> exception level if an async exception coincides with an update to the
>> controls for the target exception level in HCR_EL2. On affected
>> machines, always do writes to HCR_EL2 with async exceptions blocked.
>> 
>> Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com>
>> ---
>>  arch/arm64/Kconfig              | 17 +++++++++++++++++
>>  arch/arm64/include/asm/sysreg.h | 18 ++++++++++++++++--
>>  arch/arm64/kernel/cpu_errata.c  | 14 ++++++++++++++
>>  arch/arm64/tools/cpucaps        |  1 +
>>  4 files changed, 48 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index a182295e6f08b..e5fd87446a3b8 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -445,6 +445,23 @@ menu "Kernel Features"
>>  
>>  menu "ARM errata workarounds via the alternatives framework"
>>  
>> +config AMPERE_ERRATUM_AC03_CPU_36
>> +        bool "AmpereOne: AC03_CPU_36: CPU can take an invalid exception, if an asynchronous exception to EL2 occurs while EL2 software is changing the EL2 exception controls."
>> +	default y
>> +	help
>> +	  This option adds an alternative code sequence to work around Ampere
>> +	  errata AC03_CPU_36 on AmpereOne.
>> +
>> +	  If an async exception happens at the same time as an update to the
>> +	  controls for the target EL for async exceptions, an exception can be
>> +	  delivered to the wrong EL. For example, an EL may be routed from EL2
>> +	  to EL1.
>> +
>> +	  The workaround masks all asynchronous exception types when writing
>> +	  to HCR_EL2.
>> +
>> +	  If unsure, say Y.
>> +
>>  config AMPERE_ERRATUM_AC03_CPU_38
>>          bool "AmpereOne: AC03_CPU_38: Certain bits in the Virtualization Translation Control Register and Translation Control Registers do not follow RES0 semantics"
>>  	default y
>> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
>> index 2639d3633073d..e7781f7e7f7a7 100644
>> --- a/arch/arm64/include/asm/sysreg.h
>> +++ b/arch/arm64/include/asm/sysreg.h
>> @@ -1136,14 +1136,28 @@
>>  	__val;							\
>>  })
>>  
>> +#define __sysreg_is_hcr_el2(r)					\
>> +	(__builtin_strcmp("hcr_el2", __stringify(r)) == 0)
>
> This looks fragile. What about:
>
> 	write_sysreg(hcr, HCR_EL2);
>
> or:
>
> 	write_sysreg_s(hcr, SYS_HCR_EL2);

I had also thought about changing the users of write_sysreg(..hcr_el2)
to some new function write_hcr_el2() or something, but I guess that
would have the same fragility. Any suggestions on a better way? Trying
harder with the string stuff, or do something totally else?


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36
  2025-04-15 17:30   ` D Scott Phillips
@ 2025-04-15 18:12     ` Oliver Upton
  2025-04-15 18:17       ` D Scott Phillips
  0 siblings, 1 reply; 18+ messages in thread
From: Oliver Upton @ 2025-04-15 18:12 UTC (permalink / raw)
  To: D Scott Phillips
  Cc: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Marc Zyngier, Mark Brown, Mark Rutland,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, linux-kernel

On Tue, Apr 15, 2025 at 10:30:36AM -0700, D Scott Phillips wrote:
> Oliver Upton <oliver.upton@linux.dev> writes:
> > On Tue, Apr 15, 2025 at 08:47:10AM -0700, D Scott Phillips wrote:
> >> AC03_CPU_36 can cause asynchronous exceptions to be routed to the wrong
> >> exception level if an async exception coincides with an update to the
> >> controls for the target exception level in HCR_EL2. On affected
> >> machines, always do writes to HCR_EL2 with async exceptions blocked.
> >> 
> >> Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com>
> >> ---
> >>  arch/arm64/Kconfig              | 17 +++++++++++++++++
> >>  arch/arm64/include/asm/sysreg.h | 18 ++++++++++++++++--
> >>  arch/arm64/kernel/cpu_errata.c  | 14 ++++++++++++++
> >>  arch/arm64/tools/cpucaps        |  1 +
> >>  4 files changed, 48 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> >> index a182295e6f08b..e5fd87446a3b8 100644
> >> --- a/arch/arm64/Kconfig
> >> +++ b/arch/arm64/Kconfig
> >> @@ -445,6 +445,23 @@ menu "Kernel Features"
> >>  
> >>  menu "ARM errata workarounds via the alternatives framework"
> >>  
> >> +config AMPERE_ERRATUM_AC03_CPU_36
> >> +        bool "AmpereOne: AC03_CPU_36: CPU can take an invalid exception, if an asynchronous exception to EL2 occurs while EL2 software is changing the EL2 exception controls."
> >> +	default y
> >> +	help
> >> +	  This option adds an alternative code sequence to work around Ampere
> >> +	  errata AC03_CPU_36 on AmpereOne.
> >> +
> >> +	  If an async exception happens at the same time as an update to the
> >> +	  controls for the target EL for async exceptions, an exception can be
> >> +	  delivered to the wrong EL. For example, an EL may be routed from EL2
> >> +	  to EL1.
> >> +
> >> +	  The workaround masks all asynchronous exception types when writing
> >> +	  to HCR_EL2.
> >> +
> >> +	  If unsure, say Y.
> >> +
> >>  config AMPERE_ERRATUM_AC03_CPU_38
> >>          bool "AmpereOne: AC03_CPU_38: Certain bits in the Virtualization Translation Control Register and Translation Control Registers do not follow RES0 semantics"
> >>  	default y
> >> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> >> index 2639d3633073d..e7781f7e7f7a7 100644
> >> --- a/arch/arm64/include/asm/sysreg.h
> >> +++ b/arch/arm64/include/asm/sysreg.h
> >> @@ -1136,14 +1136,28 @@
> >>  	__val;							\
> >>  })
> >>  
> >> +#define __sysreg_is_hcr_el2(r)					\
> >> +	(__builtin_strcmp("hcr_el2", __stringify(r)) == 0)
> >
> > This looks fragile. What about:
> >
> > 	write_sysreg(hcr, HCR_EL2);
> >
> > or:
> >
> > 	write_sysreg_s(hcr, SYS_HCR_EL2);
> 
> I had also thought about changing the users of write_sysreg(..hcr_el2)
> to some new function write_hcr_el2() or something, but I guess that
> would have the same fragility. Any suggestions on a better way? Trying
> harder with the string stuff, or do something totally else?

I think the least bad approach would be to convert to HCR-specific
accessors. It's the most likely to encourage folks to respect the errata
mitigation + keeps the ugliness out of unrelated common helpers.

Thanks,
Oliver


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36
  2025-04-15 18:12     ` Oliver Upton
@ 2025-04-15 18:17       ` D Scott Phillips
  0 siblings, 0 replies; 18+ messages in thread
From: D Scott Phillips @ 2025-04-15 18:17 UTC (permalink / raw)
  To: Oliver Upton
  Cc: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Marc Zyngier, Mark Brown, Mark Rutland,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, linux-kernel

Oliver Upton <oliver.upton@linux.dev> writes:

> On Tue, Apr 15, 2025 at 10:30:36AM -0700, D Scott Phillips wrote:
>> Oliver Upton <oliver.upton@linux.dev> writes:
>> > On Tue, Apr 15, 2025 at 08:47:10AM -0700, D Scott Phillips wrote:
>> >> AC03_CPU_36 can cause asynchronous exceptions to be routed to the wrong
>> >> exception level if an async exception coincides with an update to the
>> >> controls for the target exception level in HCR_EL2. On affected
>> >> machines, always do writes to HCR_EL2 with async exceptions blocked.
>> >> 
>> >> Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com>
>> >> ---
>> >>  arch/arm64/Kconfig              | 17 +++++++++++++++++
>> >>  arch/arm64/include/asm/sysreg.h | 18 ++++++++++++++++--
>> >>  arch/arm64/kernel/cpu_errata.c  | 14 ++++++++++++++
>> >>  arch/arm64/tools/cpucaps        |  1 +
>> >>  4 files changed, 48 insertions(+), 2 deletions(-)
>> >> 
>> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> >> index a182295e6f08b..e5fd87446a3b8 100644
>> >> --- a/arch/arm64/Kconfig
>> >> +++ b/arch/arm64/Kconfig
>> >> @@ -445,6 +445,23 @@ menu "Kernel Features"
>> >>  
>> >>  menu "ARM errata workarounds via the alternatives framework"
>> >>  
>> >> +config AMPERE_ERRATUM_AC03_CPU_36
>> >> +        bool "AmpereOne: AC03_CPU_36: CPU can take an invalid exception, if an asynchronous exception to EL2 occurs while EL2 software is changing the EL2 exception controls."
>> >> +	default y
>> >> +	help
>> >> +	  This option adds an alternative code sequence to work around Ampere
>> >> +	  errata AC03_CPU_36 on AmpereOne.
>> >> +
>> >> +	  If an async exception happens at the same time as an update to the
>> >> +	  controls for the target EL for async exceptions, an exception can be
>> >> +	  delivered to the wrong EL. For example, an EL may be routed from EL2
>> >> +	  to EL1.
>> >> +
>> >> +	  The workaround masks all asynchronous exception types when writing
>> >> +	  to HCR_EL2.
>> >> +
>> >> +	  If unsure, say Y.
>> >> +
>> >>  config AMPERE_ERRATUM_AC03_CPU_38
>> >>          bool "AmpereOne: AC03_CPU_38: Certain bits in the Virtualization Translation Control Register and Translation Control Registers do not follow RES0 semantics"
>> >>  	default y
>> >> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
>> >> index 2639d3633073d..e7781f7e7f7a7 100644
>> >> --- a/arch/arm64/include/asm/sysreg.h
>> >> +++ b/arch/arm64/include/asm/sysreg.h
>> >> @@ -1136,14 +1136,28 @@
>> >>  	__val;							\
>> >>  })
>> >>  
>> >> +#define __sysreg_is_hcr_el2(r)					\
>> >> +	(__builtin_strcmp("hcr_el2", __stringify(r)) == 0)
>> >
>> > This looks fragile. What about:
>> >
>> > 	write_sysreg(hcr, HCR_EL2);
>> >
>> > or:
>> >
>> > 	write_sysreg_s(hcr, SYS_HCR_EL2);
>> 
>> I had also thought about changing the users of write_sysreg(..hcr_el2)
>> to some new function write_hcr_el2() or something, but I guess that
>> would have the same fragility. Any suggestions on a better way? Trying
>> harder with the string stuff, or do something totally else?
>
> I think the least bad approach would be to convert to HCR-specific
> accessors. It's the most likely to encourage folks to respect the errata
> mitigation + keeps the ugliness out of unrelated common helpers.

OK, will do


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23
  2025-04-15 15:47 ` [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23 D Scott Phillips
  2025-04-15 17:06   ` Oliver Upton
@ 2025-04-15 18:38   ` Marc Zyngier
  1 sibling, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2025-04-15 18:38 UTC (permalink / raw)
  To: D Scott Phillips
  Cc: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Mark Brown, Mark Rutland, Oliver Upton,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, open list

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.

-- 
Without deviation from the norm, progress is not possible.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23
  2025-04-15 17:06   ` Oliver Upton
@ 2025-04-15 22:13     ` D Scott Phillips
  2025-04-16  0:29       ` Oliver Upton
  2025-04-16  7:11       ` Marc Zyngier
  0 siblings, 2 replies; 18+ messages in thread
From: D Scott Phillips @ 2025-04-15 22:13 UTC (permalink / raw)
  To: Oliver Upton, Marc Zyngier
  Cc: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Mark Brown, Mark Rutland, Rob Herring (Arm),
	Shameer Kolothum, Shiqi Liu, Will Deacon, Yicong Yang, kvmarm,
	linux-arm-kernel, open list

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.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23
  2025-04-15 22:13     ` D Scott Phillips
@ 2025-04-16  0:29       ` Oliver Upton
  2025-04-16 23:05         ` D Scott Phillips
  2025-04-16  7:11       ` Marc Zyngier
  1 sibling, 1 reply; 18+ messages in thread
From: Oliver Upton @ 2025-04-16  0:29 UTC (permalink / raw)
  To: D Scott Phillips
  Cc: Marc Zyngier, Catalin Marinas, James Clark, James Morse,
	Joey Gouly, Kevin Brodsky, Mark Brown, Mark Rutland,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, open list

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23
  2025-04-15 22:13     ` D Scott Phillips
  2025-04-16  0:29       ` Oliver Upton
@ 2025-04-16  7:11       ` Marc Zyngier
  2025-04-16 23:06         ` D Scott Phillips
  1 sibling, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2025-04-16  7:11 UTC (permalink / raw)
  To: D Scott Phillips
  Cc: Oliver Upton, Catalin Marinas, James Clark, James Morse,
	Joey Gouly, Kevin Brodsky, Mark Brown, Mark Rutland,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, open list

On Tue, 15 Apr 2025 23:13:43 +0100,
D Scott Phillips <scott@os.amperecomputing.com> wrote:
> 
> 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.

Are you guaranteed that a younger data access can't be hoisted up and
affect things similarly? I don't see anything that would prevent this.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36
  2025-04-15 15:47 [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36 D Scott Phillips
  2025-04-15 15:47 ` [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23 D Scott Phillips
  2025-04-15 17:12 ` [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36 Oliver Upton
@ 2025-04-16  7:19 ` Marc Zyngier
  2025-04-16 23:14   ` D Scott Phillips
  2025-04-25  2:02   ` D Scott Phillips
  2 siblings, 2 replies; 18+ messages in thread
From: Marc Zyngier @ 2025-04-16  7:19 UTC (permalink / raw)
  To: D Scott Phillips
  Cc: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Mark Brown, Mark Rutland, Oliver Upton,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, open list

On Tue, 15 Apr 2025 16:47:10 +0100,
D Scott Phillips <scott@os.amperecomputing.com> wrote:
> 
> AC03_CPU_36 can cause asynchronous exceptions to be routed to the wrong
> exception level if an async exception coincides with an update to the
> controls for the target exception level in HCR_EL2. On affected
> machines, always do writes to HCR_EL2 with async exceptions blocked.

From the actual errata document [1]:

<quote>
If an Asynchronous Exception to EL2 occurs, while EL2 software is
changing the EL2 exception control bits from a configuration where
asynchronous exceptions are routed to EL2 to a configuration where
asynchronous exceptions are routed to EL1, the processor may exhibit
the incorrect exception behavior of routing an interrupt taken at EL2
to EL1.  The affected system register is HCR_EL2, which contains
control bits for routing and enabling of EL2 exceptions.
</quote>

My reading is that things can go wrong when clearing the xMO bits.

I don't think we need to touch the xMO bits at all when running
VHE. So my preference would be to:

- simply leave the xMO bits set at all times (nothing bad can happen
  from that, can it?)

- prevent these systems from using anything but VHE (and fail KVM init
  otherwise)

This would save a lot of maintenance hassle and the extreme ugliness
of this patch.

Thanks,

	M.

[1] https://amperecomputing.com/assets/AmpereOne_Developer_ER_v0_80_20240823_28945022f4.pdf

-- 
Without deviation from the norm, progress is not possible.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23
  2025-04-16  0:29       ` Oliver Upton
@ 2025-04-16 23:05         ` D Scott Phillips
  0 siblings, 0 replies; 18+ messages in thread
From: D Scott Phillips @ 2025-04-16 23:05 UTC (permalink / raw)
  To: Oliver Upton
  Cc: Marc Zyngier, Catalin Marinas, James Clark, James Morse,
	Joey Gouly, Kevin Brodsky, Mark Brown, Mark Rutland,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, open list

Oliver Upton <oliver.upton@linux.dev> writes:

> 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.

Yes, that's right, it's just translations from load/store instructions,
and the DSB before closes the window where earlier instructions can get
corrupted, and then the ISB after prevents the window for corruption
from later instructions from overlapping with the write to HCR.

> 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.

No worries, and sorry to not have the description more clear
originally. I'll include a more complete description in the next
version.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23
  2025-04-16  7:11       ` Marc Zyngier
@ 2025-04-16 23:06         ` D Scott Phillips
  0 siblings, 0 replies; 18+ messages in thread
From: D Scott Phillips @ 2025-04-16 23:06 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Oliver Upton, Catalin Marinas, James Clark, James Morse,
	Joey Gouly, Kevin Brodsky, Mark Brown, Mark Rutland,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, open list

Marc Zyngier <maz@kernel.org> writes:

> On Tue, 15 Apr 2025 23:13:43 +0100,
> D Scott Phillips <scott@os.amperecomputing.com> wrote:
>> 
>> 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.
>
> Are you guaranteed that a younger data access can't be hoisted up and
> affect things similarly? I don't see anything that would prevent this.

Yes that's my understanding, that the younger instructions
(mis-speculated or not) can't have their window for corruption of a
translation line up with the store to HCR due to the ISB.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36
  2025-04-16  7:19 ` Marc Zyngier
@ 2025-04-16 23:14   ` D Scott Phillips
  2025-04-25  2:02   ` D Scott Phillips
  1 sibling, 0 replies; 18+ messages in thread
From: D Scott Phillips @ 2025-04-16 23:14 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Mark Brown, Mark Rutland, Oliver Upton,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, open list

Marc Zyngier <maz@kernel.org> writes:

> On Tue, 15 Apr 2025 16:47:10 +0100,
> D Scott Phillips <scott@os.amperecomputing.com> wrote:
>> 
>> AC03_CPU_36 can cause asynchronous exceptions to be routed to the wrong
>> exception level if an async exception coincides with an update to the
>> controls for the target exception level in HCR_EL2. On affected
>> machines, always do writes to HCR_EL2 with async exceptions blocked.
>
> From the actual errata document [1]:
>
> <quote>
> If an Asynchronous Exception to EL2 occurs, while EL2 software is
> changing the EL2 exception control bits from a configuration where
> asynchronous exceptions are routed to EL2 to a configuration where
> asynchronous exceptions are routed to EL1, the processor may exhibit
> the incorrect exception behavior of routing an interrupt taken at EL2
> to EL1.  The affected system register is HCR_EL2, which contains
> control bits for routing and enabling of EL2 exceptions.
> </quote>
>
> My reading is that things can go wrong when clearing the xMO bits.
>
> I don't think we need to touch the xMO bits at all when running
> VHE. So my preference would be to:
>
> - simply leave the xMO bits set at all times (nothing bad can happen
>   from that, can it?)
>
> - prevent these systems from using anything but VHE (and fail KVM init
>   otherwise)
>
> This would save a lot of maintenance hassle and the extreme ugliness
> of this patch.

Yes that also matches my understanding, that having the physical IRQ
routing permanently set to EL2 would avoid the erratum case. I'll take
the approach of restricting to VHE mode in the next version, thanks very
much Marc.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36
  2025-04-16  7:19 ` Marc Zyngier
  2025-04-16 23:14   ` D Scott Phillips
@ 2025-04-25  2:02   ` D Scott Phillips
  2025-04-27 12:21     ` Marc Zyngier
  1 sibling, 1 reply; 18+ messages in thread
From: D Scott Phillips @ 2025-04-25  2:02 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Mark Brown, Mark Rutland, Oliver Upton,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, open list

Marc Zyngier <maz@kernel.org> writes:

> On Tue, 15 Apr 2025 16:47:10 +0100,
> D Scott Phillips <scott@os.amperecomputing.com> wrote:
>> 
>> AC03_CPU_36 can cause asynchronous exceptions to be routed to the wrong
>> exception level if an async exception coincides with an update to the
>> controls for the target exception level in HCR_EL2. On affected
>> machines, always do writes to HCR_EL2 with async exceptions blocked.
>
> From the actual errata document [1]:
>
> <quote>
> If an Asynchronous Exception to EL2 occurs, while EL2 software is
> changing the EL2 exception control bits from a configuration where
> asynchronous exceptions are routed to EL2 to a configuration where
> asynchronous exceptions are routed to EL1, the processor may exhibit
> the incorrect exception behavior of routing an interrupt taken at EL2
> to EL1.  The affected system register is HCR_EL2, which contains
> control bits for routing and enabling of EL2 exceptions.
> </quote>
>
> My reading is that things can go wrong when clearing the xMO bits.
>
> I don't think we need to touch the xMO bits at all when running
> VHE. So my preference would be to:
>
> - simply leave the xMO bits set at all times (nothing bad can happen
>   from that, can it?)
>
> - prevent these systems from using anything but VHE (and fail KVM init
>   otherwise)

Hi Marc, I started writing up this patch and then realized that the
issue can also not happen in nvhe mode. While xMO bits are modified
there, async exceptions are always masked and so the "simultaneously
take an async exception" part of the erratum can't happen.

Does that sound right to you, or are there cases that I'm missing. If
it's right the nvhe is also can't hit the erratum case, then what do you
think is the right thing for me to do here?

Thanks,

Scott


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36
  2025-04-25  2:02   ` D Scott Phillips
@ 2025-04-27 12:21     ` Marc Zyngier
  2025-04-28 16:35       ` D Scott Phillips
  0 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2025-04-27 12:21 UTC (permalink / raw)
  To: D Scott Phillips
  Cc: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Mark Brown, Mark Rutland, Oliver Upton,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, open list

On Fri, 25 Apr 2025 03:02:29 +0100,
D Scott Phillips <scott@os.amperecomputing.com> wrote:
> 
> Marc Zyngier <maz@kernel.org> writes:
> 
> > On Tue, 15 Apr 2025 16:47:10 +0100,
> > D Scott Phillips <scott@os.amperecomputing.com> wrote:
> >> 
> >> AC03_CPU_36 can cause asynchronous exceptions to be routed to the wrong
> >> exception level if an async exception coincides with an update to the
> >> controls for the target exception level in HCR_EL2. On affected
> >> machines, always do writes to HCR_EL2 with async exceptions blocked.
> >
> > From the actual errata document [1]:
> >
> > <quote>
> > If an Asynchronous Exception to EL2 occurs, while EL2 software is
> > changing the EL2 exception control bits from a configuration where
> > asynchronous exceptions are routed to EL2 to a configuration where
> > asynchronous exceptions are routed to EL1, the processor may exhibit
> > the incorrect exception behavior of routing an interrupt taken at EL2
> > to EL1.  The affected system register is HCR_EL2, which contains
> > control bits for routing and enabling of EL2 exceptions.
> > </quote>
> >
> > My reading is that things can go wrong when clearing the xMO bits.
> >
> > I don't think we need to touch the xMO bits at all when running
> > VHE. So my preference would be to:
> >
> > - simply leave the xMO bits set at all times (nothing bad can happen
> >   from that, can it?)
> >
> > - prevent these systems from using anything but VHE (and fail KVM init
> >   otherwise)
> 
> Hi Marc, I started writing up this patch and then realized that the
> issue can also not happen in nvhe mode. While xMO bits are modified
> there, async exceptions are always masked and so the "simultaneously
> take an async exception" part of the erratum can't happen.
> 
> Does that sound right to you, or are there cases that I'm missing. If
> it's right the nvhe is also can't hit the erratum case, then what do you
> think is the right thing for me to do here?

That's an interesting point. We always run the nVHE/hVHE hypervisor
code with interrupts disabled by virtue of taking an HVC exception
into EL2, so that particular case seems OK as it literally implements
the proposed workaround.

However, there's at least one catch: the SError handling code in
hyp/entry.S relies on clearing PSTATE.A to take a pending abort (the
so-called VAXorcism). I take that this CPU implements FEAT_RAS, and
that we don't need to worry about this code path either, and that the
erratum cannot trigger on speculatively executed paths?

If we're OK with that, then I don't think there is much to do, other
than always setting the xMO bits at all times, for which I already
have a patch in review (v2 coming shortly).

	M.

-- 
Without deviation from the norm, progress is not possible.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36
  2025-04-27 12:21     ` Marc Zyngier
@ 2025-04-28 16:35       ` D Scott Phillips
  0 siblings, 0 replies; 18+ messages in thread
From: D Scott Phillips @ 2025-04-28 16:35 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Catalin Marinas, James Clark, James Morse, Joey Gouly,
	Kevin Brodsky, Mark Brown, Mark Rutland, Oliver Upton,
	Rob Herring (Arm), Shameer Kolothum, Shiqi Liu, Will Deacon,
	Yicong Yang, kvmarm, linux-arm-kernel, open list

Marc Zyngier <maz@kernel.org> writes:

> On Fri, 25 Apr 2025 03:02:29 +0100,
> D Scott Phillips <scott@os.amperecomputing.com> wrote:
>> 
>> Marc Zyngier <maz@kernel.org> writes:
>> 
>> > On Tue, 15 Apr 2025 16:47:10 +0100,
>> > D Scott Phillips <scott@os.amperecomputing.com> wrote:
>> >> 
>> >> AC03_CPU_36 can cause asynchronous exceptions to be routed to the wrong
>> >> exception level if an async exception coincides with an update to the
>> >> controls for the target exception level in HCR_EL2. On affected
>> >> machines, always do writes to HCR_EL2 with async exceptions blocked.
>> >
>> > From the actual errata document [1]:
>> >
>> > <quote>
>> > If an Asynchronous Exception to EL2 occurs, while EL2 software is
>> > changing the EL2 exception control bits from a configuration where
>> > asynchronous exceptions are routed to EL2 to a configuration where
>> > asynchronous exceptions are routed to EL1, the processor may exhibit
>> > the incorrect exception behavior of routing an interrupt taken at EL2
>> > to EL1.  The affected system register is HCR_EL2, which contains
>> > control bits for routing and enabling of EL2 exceptions.
>> > </quote>
>> >
>> > My reading is that things can go wrong when clearing the xMO bits.
>> >
>> > I don't think we need to touch the xMO bits at all when running
>> > VHE. So my preference would be to:
>> >
>> > - simply leave the xMO bits set at all times (nothing bad can happen
>> >   from that, can it?)
>> >
>> > - prevent these systems from using anything but VHE (and fail KVM init
>> >   otherwise)
>> 
>> Hi Marc, I started writing up this patch and then realized that the
>> issue can also not happen in nvhe mode. While xMO bits are modified
>> there, async exceptions are always masked and so the "simultaneously
>> take an async exception" part of the erratum can't happen.
>> 
>> Does that sound right to you, or are there cases that I'm missing. If
>> it's right the nvhe is also can't hit the erratum case, then what do you
>> think is the right thing for me to do here?
>
> That's an interesting point. We always run the nVHE/hVHE hypervisor
> code with interrupts disabled by virtue of taking an HVC exception
> into EL2, so that particular case seems OK as it literally implements
> the proposed workaround.
>
> However, there's at least one catch: the SError handling code in
> hyp/entry.S relies on clearing PSTATE.A to take a pending abort (the
> so-called VAXorcism). I take that this CPU implements FEAT_RAS, and
> that we don't need to worry about this code path either, and that the
> erratum cannot trigger on speculatively executed paths?

Yep, right on both counts, the cpu supports FEAT_RAS, and the erratum
case doesn't happen speculatively.

> If we're OK with that, then I don't think there is much to do, other
> than always setting the xMO bits at all times, for which I already
> have a patch in review (v2 coming shortly).

OK, sounds good to me.


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2025-04-28 16:40 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-15 15:47 [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36 D Scott Phillips
2025-04-15 15:47 ` [PATCH 2/2] arm64: errata: Work around AmpereOne's erratum AC04_CPU_23 D Scott Phillips
2025-04-15 17:06   ` Oliver Upton
2025-04-15 22:13     ` D Scott Phillips
2025-04-16  0:29       ` Oliver Upton
2025-04-16 23:05         ` D Scott Phillips
2025-04-16  7:11       ` Marc Zyngier
2025-04-16 23:06         ` D Scott Phillips
2025-04-15 18:38   ` Marc Zyngier
2025-04-15 17:12 ` [PATCH 1/2] arm64: errata: Work around AmpereOne's erratum AC03_CPU_36 Oliver Upton
2025-04-15 17:30   ` D Scott Phillips
2025-04-15 18:12     ` Oliver Upton
2025-04-15 18:17       ` D Scott Phillips
2025-04-16  7:19 ` Marc Zyngier
2025-04-16 23:14   ` D Scott Phillips
2025-04-25  2:02   ` D Scott Phillips
2025-04-27 12:21     ` Marc Zyngier
2025-04-28 16:35       ` D Scott Phillips

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).