[PATCH 0/3] arm64: errata: remove BF16 HWCAP due to incorrect result on Cortex-A510

[PATCH 0/3] arm64: errata: remove BF16 HWCAP due to incorrect result on Cortex-A510

[James Morse]

Hello!

Cortex-A510 has an erratum where the BFMMLA or VMMLA instructions might
produce the incorrect result. This only happens if two Cortex-A510 CPUs
are configured by the SoC manafacturer in a particular way to share some
hardware, and are using it at the same time. It isn't possible for linux
to know how the CPUs were configured, so the only option is to disable
the BF16 feature for all Cortex-A510 CPUs.

This won't stop user-space from trying to use the instructions to see
if they work - but such software is already broken on big/little systems
with mismathed features.

Removing the BF16 feature involves removing both the HWCAP, as was done
by commit 44b3834b2eed "arm64: errata: Remove AES hwcap for COMPAT tasks",
and the emulated view of that register that user-space has.
(This wasn't previously needed as aarch32 ID registers aren't accessible
like this).

As there are now two of these things, this series tries to add a more
maintainable way to remove features, to avoid spilling workaround like
this into cpufeature.c.

Instead, cpu_errata.c modifies the user_mask that is used for emulation
of the id registers, and cpufeature.c builds the HWCAPs based on this.

I have patches to convert the AES workaround to do the same, but that
should wait until the aarch32 ID registers are generated by the sysreg
awk scripts (it needs some new masks defined).

[0] https://developer.arm.com/documentation/SDEN1873351/1400/?lang=en

Thanks,

James Morse (3):
  arm64: cpufeature: Force HWCAP to be based on the sysreg visible to
    user-space
  arm64: cpufeature: Expose get_arm64_ftr_reg() outside cpufeature.c
  arm64: errata: remove BF16 HWCAP due to incorrect result on
    Cortex-A510

 Documentation/arm64/silicon-errata.rst |  2 ++
 arch/arm64/Kconfig                     | 14 +++++++++
 arch/arm64/include/asm/cpufeature.h    |  2 ++
 arch/arm64/kernel/cpu_errata.c         | 27 ++++++++++++++++
 arch/arm64/kernel/cpufeature.c         | 43 ++++++++++++++++++++------
 arch/arm64/tools/cpucaps               |  1 +
 6 files changed, 79 insertions(+), 10 deletions(-)

-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] arm64: cpufeature: Force HWCAP to be based on the sysreg visible to user-space

[James Morse]

arm64 advertises hardware features to user-space via HWCAPs, and by
emulating access to the CPUs id registers. The cpufeature code has a
sanitised system-wide view of an id register, and a sanitised user-space
view of an id register, where some features use their 'safe' value
instead of the hardware value.

It is currently possible for a HWCAP to be advertised where the user-space
view of the id register does not show the feature as supported.
Erratum workaround need to remove both the HWCAP, and the feature from
the user-space view of the id register. This involves duplicating the
code, and spreading it over cpufeature.c and cpu_errata.c.

Make the HWCAP code use the user-space view of id registers. This ensures
the values never diverge, and allows erratum workaround to remove HWCAP
by modifying the user-space view of the id register.

Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/arm64/kernel/cpufeature.c | 41 ++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index af4de817d712..b6a4e97805a4 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1401,17 +1401,40 @@ feature_matches(u64 reg, const struct arm64_cpu_capabilities *entry)
 	return val >= entry->min_field_value;
 }
 
+static u64
+read_scoped_sysreg(const struct arm64_cpu_capabilities *entry, int scope)
+{
+	WARN_ON(scope == SCOPE_LOCAL_CPU && preemptible());
+	if (scope == SCOPE_SYSTEM)
+		return read_sanitised_ftr_reg(entry->sys_reg);
+	else
+		return __read_sysreg_by_encoding(entry->sys_reg);
+}
+
+static bool
+has_user_cpuid_feature(const struct arm64_cpu_capabilities *entry, int scope)
+{
+	int mask;
+	struct arm64_ftr_reg *regp;
+	u64 val = read_scoped_sysreg(entry, scope);
+
+	regp = get_arm64_ftr_reg(entry->sys_reg);
+	if (!regp)
+		return false;
+
+	mask = cpuid_feature_extract_unsigned_field_width(regp->user_mask,
+							  entry->field_pos,
+							  entry->field_width);
+	if (!mask)
+		return false;
+
+	return feature_matches(val, entry);
+}
+
 static bool
 has_cpuid_feature(const struct arm64_cpu_capabilities *entry, int scope)
 {
-	u64 val;
-
-	WARN_ON(scope == SCOPE_LOCAL_CPU && preemptible());
-	if (scope == SCOPE_SYSTEM)
-		val = read_sanitised_ftr_reg(entry->sys_reg);
-	else
-		val = __read_sysreg_by_encoding(entry->sys_reg);
-
+	u64 val = read_scoped_sysreg(entry, scope);
 	return feature_matches(val, entry);
 }
 
@@ -2624,7 +2647,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 };
 
 #define HWCAP_CPUID_MATCH(reg, field, width, s, min_value)			\
-		.matches = has_cpuid_feature,					\
+		.matches = has_user_cpuid_feature,					\
 		.sys_reg = reg,							\
 		.field_pos = field,						\
 		.field_width = width,						\
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] arm64: cpufeature: Force HWCAP to be based on the sysreg visible to user-space

[Suzuki K Poulose]

On 09/09/2022 17:59, James Morse wrote:
> arm64 advertises hardware features to user-space via HWCAPs, and by
> emulating access to the CPUs id registers. The cpufeature code has a
> sanitised system-wide view of an id register, and a sanitised user-space
> view of an id register, where some features use their 'safe' value
> instead of the hardware value.
> 
> It is currently possible for a HWCAP to be advertised where the user-space
> view of the id register does not show the feature as supported.
> Erratum workaround need to remove both the HWCAP, and the feature from
> the user-space view of the id register. This involves duplicating the
> code, and spreading it over cpufeature.c and cpu_errata.c.
> 
> Make the HWCAP code use the user-space view of id registers. This ensures
> the values never diverge, and allows erratum workaround to remove HWCAP
> by modifying the user-space view of the id register.

That sounds good to me.

> 
> Signed-off-by: James Morse <james.morse@arm.com>

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] arm64: cpufeature: Expose get_arm64_ftr_reg() outside cpufeature.c

[James Morse]

get_arm64_ftr_reg() returns the properties of a system register based
on its instruction encoding.

This is needed by erratum workaround in cpu_errata.c to modify the
user-space visible view of id registers.

Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/arm64/include/asm/cpufeature.h | 2 ++
 arch/arm64/kernel/cpufeature.c      | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index fd7d75a275f6..5c3317bc06c7 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -907,6 +907,8 @@ static inline unsigned int get_vmid_bits(u64 mmfr1)
 	return 8;
 }
 
+struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id);
+
 extern struct arm64_ftr_override id_aa64mmfr1_override;
 extern struct arm64_ftr_override id_aa64pfr0_override;
 extern struct arm64_ftr_override id_aa64pfr1_override;
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index b6a4e97805a4..24f43ad02cd3 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -750,7 +750,7 @@ static struct arm64_ftr_reg *get_arm64_ftr_reg_nowarn(u32 sys_id)
  * returns - Upon success,  matching ftr_reg entry for id.
  *         - NULL on failure but with an WARN_ON().
  */
-static struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id)
+struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id)
 {
 	struct arm64_ftr_reg *reg;
 
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] arm64: cpufeature: Expose get_arm64_ftr_reg() outside cpufeature.c

[Suzuki K Poulose]

On 09/09/2022 17:59, James Morse wrote:
> get_arm64_ftr_reg() returns the properties of a system register based
> on its instruction encoding.
> 
> This is needed by erratum workaround in cpu_errata.c to modify the
> user-space visible view of id registers.
> 
> Signed-off-by: James Morse <james.morse@arm.com>

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] arm64: errata: remove BF16 HWCAP due to incorrect result on Cortex-A510

[James Morse]

Cortex-A510's erratum #2658417 causes two BF16 instructions to return the
wrong result in rare circumstances when a pair of A510 CPUs are using
shared neon hardware.

The two instructions affected are BFMMLA and VMMLA, support for these is
indicated by the BF16 HWCAP. Remove it on affected platforms.

Signed-off-by: James Morse <james.morse@arm.com>
---
 Documentation/arm64/silicon-errata.rst |  2 ++
 arch/arm64/Kconfig                     | 14 +++++++++++++
 arch/arm64/kernel/cpu_errata.c         | 27 ++++++++++++++++++++++++++
 arch/arm64/tools/cpucaps               |  1 +
 4 files changed, 44 insertions(+)

diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
index fda97b3fcf01..17d9fc5d14fb 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -110,6 +110,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A510     | #2441009        | ARM64_ERRATUM_2441009       |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-A510     | #2658417        | ARM64_ERRATUM_2658417       |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A710     | #2119858        | ARM64_ERRATUM_2119858       |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A710     | #2054223        | ARM64_ERRATUM_2054223       |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9fb9fff08c94..06eedcc8a2c6 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -733,6 +733,20 @@ config ARM64_ERRATUM_2077057
 
 	  If unsure, say Y.
 
+config ARM64_ERRATUM_2658417
+	bool "Cortex-A510: 2658417: remove BF16 support due to incorrect result"
+	default y
+	help
+	  This option adds the workaround for ARM Cortex-A510 erratum 2658417.
+	  Affected Cortex-A510 may produce the wrong result for BFMMLA or VMMLA
+	  instructions in rare circumstances when a pair of A510 CPUs are using
+	  shared neon hardware.
+	  As the sharing is not discoverable by the kernel, hide the BF16
+	  HWCAP to indicate that user-space should not be using these
+	  instructions.
+
+	  If unsure, say Y.
+
 config ARM64_ERRATUM_2119858
 	bool "Cortex-A710/X2: 2119858: workaround TRBE overwriting trace data in FILL mode"
 	default y
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 53b973b6059f..aef3245a8597 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -121,6 +121,22 @@ cpu_enable_cache_maint_trap(const struct arm64_cpu_capabilities *__unused)
 	sysreg_clear_set(sctlr_el1, SCTLR_EL1_UCI, 0);
 }
 
+static DEFINE_RAW_SPINLOCK(reg_user_mask_modification);
+static void __maybe_unused
+cpu_clear_bf16_from_user_emulation(const struct arm64_cpu_capabilities *__unused)
+{
+	struct arm64_ftr_reg *regp;
+
+	regp = get_arm64_ftr_reg(SYS_ID_AA64ISAR1_EL1);
+	if (!regp)
+		return;
+
+	raw_spin_lock(&reg_user_mask_modification);
+	if (regp->user_mask & ID_AA64ISAR1_EL1_BF16_MASK)
+		regp->user_mask &= ~ID_AA64ISAR1_EL1_BF16_MASK;
+	raw_spin_unlock(&reg_user_mask_modification);
+}
+
 #define CAP_MIDR_RANGE(model, v_min, r_min, v_max, r_max)	\
 	.matches = is_affected_midr_range,			\
 	.midr_range = MIDR_RANGE(model, v_min, r_min, v_max, r_max)
@@ -691,6 +707,17 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		CAP_MIDR_RANGE_LIST(broken_aarch32_aes),
 		.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
 	},
+#endif
+#ifdef CONFIG_ARM64_ERRATUM_2658417
+	{
+		.desc = "ARM erratum 2658417",
+		.capability = ARM64_WORKAROUND_2658417,
+		/* Cortex-A510 r0p0 - r1p1 */
+		ERRATA_MIDR_RANGE(MIDR_CORTEX_A510, 0, 0, 1, 1),
+		MIDR_FIXED(MIDR_CPU_VAR_REV(1,1), BIT(25)),
+		.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
+		.cpu_enable = cpu_clear_bf16_from_user_emulation,
+	},
 #endif
 	{
 	}
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 63b2484ce6c3..f553a7cb1b07 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -68,6 +68,7 @@ WORKAROUND_2038923
 WORKAROUND_2064142
 WORKAROUND_2077057
 WORKAROUND_2457168
+WORKAROUND_2658417
 WORKAROUND_TRBE_OVERWRITE_FILL_MODE
 WORKAROUND_TSB_FLUSH_FAILURE
 WORKAROUND_TRBE_WRITE_OUT_OF_RANGE
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: errata: remove BF16 HWCAP due to incorrect result on Cortex-A510

[Suzuki K Poulose]

On 09/09/2022 17:59, James Morse wrote:
> Cortex-A510's erratum #2658417 causes two BF16 instructions to return the
> wrong result in rare circumstances when a pair of A510 CPUs are using
> shared neon hardware.
> 
> The two instructions affected are BFMMLA and VMMLA, support for these is
> indicated by the BF16 HWCAP. Remove it on affected platforms.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
>   Documentation/arm64/silicon-errata.rst |  2 ++
>   arch/arm64/Kconfig                     | 14 +++++++++++++
>   arch/arm64/kernel/cpu_errata.c         | 27 ++++++++++++++++++++++++++
>   arch/arm64/tools/cpucaps               |  1 +
>   4 files changed, 44 insertions(+)
> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index fda97b3fcf01..17d9fc5d14fb 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -110,6 +110,8 @@ stable kernels.
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A510     | #2441009        | ARM64_ERRATUM_2441009       |
>   +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A510     | #2658417        | ARM64_ERRATUM_2658417       |
> ++----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A710     | #2119858        | ARM64_ERRATUM_2119858       |
>   +----------------+-----------------+-----------------+-----------------------------+
>   | ARM            | Cortex-A710     | #2054223        | ARM64_ERRATUM_2054223       |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9fb9fff08c94..06eedcc8a2c6 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -733,6 +733,20 @@ config ARM64_ERRATUM_2077057
>   
>   	  If unsure, say Y.
>   
> +config ARM64_ERRATUM_2658417
> +	bool "Cortex-A510: 2658417: remove BF16 support due to incorrect result"
> +	default y
> +	help
> +	  This option adds the workaround for ARM Cortex-A510 erratum 2658417.
> +	  Affected Cortex-A510 may produce the wrong result for BFMMLA or VMMLA
> +	  instructions in rare circumstances when a pair of A510 CPUs are using
> +	  shared neon hardware.
> +	  As the sharing is not discoverable by the kernel, hide the BF16
> +	  HWCAP to indicate that user-space should not be using these
> +	  instructions.
> +
> +	  If unsure, say Y.
> +
>   config ARM64_ERRATUM_2119858
>   	bool "Cortex-A710/X2: 2119858: workaround TRBE overwriting trace data in FILL mode"
>   	default y
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 53b973b6059f..aef3245a8597 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -121,6 +121,22 @@ cpu_enable_cache_maint_trap(const struct arm64_cpu_capabilities *__unused)
>   	sysreg_clear_set(sctlr_el1, SCTLR_EL1_UCI, 0);
>   }
>   
> +static DEFINE_RAW_SPINLOCK(reg_user_mask_modification);
> +static void __maybe_unused
> +cpu_clear_bf16_from_user_emulation(const struct arm64_cpu_capabilities *__unused)
> +{
> +	struct arm64_ftr_reg *regp;
> +
> +	regp = get_arm64_ftr_reg(SYS_ID_AA64ISAR1_EL1);
> +	if (!regp)
> +		return;
> +
> +	raw_spin_lock(&reg_user_mask_modification);
> +	if (regp->user_mask & ID_AA64ISAR1_EL1_BF16_MASK)
> +		regp->user_mask &= ~ID_AA64ISAR1_EL1_BF16_MASK;
> +	raw_spin_unlock(&reg_user_mask_modification);
> +}
> +
>   #define CAP_MIDR_RANGE(model, v_min, r_min, v_max, r_max)	\
>   	.matches = is_affected_midr_range,			\
>   	.midr_range = MIDR_RANGE(model, v_min, r_min, v_max, r_max)
> @@ -691,6 +707,17 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>   		CAP_MIDR_RANGE_LIST(broken_aarch32_aes),
>   		.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
>   	},
> +#endif
> +#ifdef CONFIG_ARM64_ERRATUM_2658417
> +	{
> +		.desc = "ARM erratum 2658417",
> +		.capability = ARM64_WORKAROUND_2658417,
> +		/* Cortex-A510 r0p0 - r1p1 */
> +		ERRATA_MIDR_RANGE(MIDR_CORTEX_A510, 0, 0, 1, 1),

> +		MIDR_FIXED(MIDR_CPU_VAR_REV(1,1), BIT(25)),

nit: Do we need to document this in the Kconfig help text ?

> +		.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,

nit: type is populated with ERRATA_MIDR_RANGE macro. So this may be
skipped.

> +		.cpu_enable = cpu_clear_bf16_from_user_emulation,
> +	},
>   #endif
>   	{
>   	}
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index 63b2484ce6c3..f553a7cb1b07 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -68,6 +68,7 @@ WORKAROUND_2038923
>   WORKAROUND_2064142
>   WORKAROUND_2077057
>   WORKAROUND_2457168
> +WORKAROUND_2658417
>   WORKAROUND_TRBE_OVERWRITE_FILL_MODE
>   WORKAROUND_TSB_FLUSH_FAILURE
>   WORKAROUND_TRBE_WRITE_OUT_OF_RANGE


Suzuki


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: errata: remove BF16 HWCAP due to incorrect result on Cortex-A510

[James Morse]

Hi Suzuki,

On 12/09/2022 16:30, Suzuki K Poulose wrote:
> On 09/09/2022 17:59, James Morse wrote:
>> Cortex-A510's erratum #2658417 causes two BF16 instructions to return the
>> wrong result in rare circumstances when a pair of A510 CPUs are using
>> shared neon hardware.
>>
>> The two instructions affected are BFMMLA and VMMLA, support for these is
>> indicated by the BF16 HWCAP. Remove it on affected platforms.

>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index 53b973b6059f..aef3245a8597 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -691,6 +707,17 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>           CAP_MIDR_RANGE_LIST(broken_aarch32_aes),
>>           .type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
>>       },
>> +#endif
>> +#ifdef CONFIG_ARM64_ERRATUM_2658417
>> +    {
>> +        .desc = "ARM erratum 2658417",
>> +        .capability = ARM64_WORKAROUND_2658417,
>> +        /* Cortex-A510 r0p0 - r1p1 */
>> +        ERRATA_MIDR_RANGE(MIDR_CORTEX_A510, 0, 0, 1, 1),
> 
>> +        MIDR_FIXED(MIDR_CPU_VAR_REV(1,1), BIT(25)),

> nit: Do we need to document this in the Kconfig help text ?

I agree its unusual, but isn't it a special case of "if your hardware isn't broken the
workaround isn't applied"?

Do you think a section on revisions affected in Kconfig really benefits anyone?
Can't they look at the errata document instead? (that is why the erratum number appears in
so many places)


>> +        .type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,

> nit: type is populated with ERRATA_MIDR_RANGE macro. So this may be
> skipped.

Those macro's are just a jungle!


Thanks,

James

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] arm64: errata: remove BF16 HWCAP due to incorrect result on Cortex-A510

[Robin Murphy]

On 2022-09-09 17:59, James Morse wrote:
> Hello!
> 
> Cortex-A510 has an erratum where the BFMMLA or VMMLA instructions might
> produce the incorrect result. This only happens if two Cortex-A510 CPUs
> are configured by the SoC manafacturer in a particular way to share some
> hardware, and are using it at the same time. It isn't possible for linux
> to know how the CPUs were configured, so the only option is to disable
> the BF16 feature for all Cortex-A510 CPUs.

Hmm, the TRM doesn't seem to describe any obvious restrictions on 
accessing IMP_CPUCFR_EL1 - is there some other nefarious secret at play 
there?

Robin.

> This won't stop user-space from trying to use the instructions to see
> if they work - but such software is already broken on big/little systems
> with mismathed features.
> 
> Removing the BF16 feature involves removing both the HWCAP, as was done
> by commit 44b3834b2eed "arm64: errata: Remove AES hwcap for COMPAT tasks",
> and the emulated view of that register that user-space has.
> (This wasn't previously needed as aarch32 ID registers aren't accessible
> like this).
> 
> As there are now two of these things, this series tries to add a more
> maintainable way to remove features, to avoid spilling workaround like
> this into cpufeature.c.
> 
> Instead, cpu_errata.c modifies the user_mask that is used for emulation
> of the id registers, and cpufeature.c builds the HWCAPs based on this.
> 
> I have patches to convert the AES workaround to do the same, but that
> should wait until the aarch32 ID registers are generated by the sysreg
> awk scripts (it needs some new masks defined).
> 
> [0] https://developer.arm.com/documentation/SDEN1873351/1400/?lang=en
> 
> Thanks,
> 
> James Morse (3):
>    arm64: cpufeature: Force HWCAP to be based on the sysreg visible to
>      user-space
>    arm64: cpufeature: Expose get_arm64_ftr_reg() outside cpufeature.c
>    arm64: errata: remove BF16 HWCAP due to incorrect result on
>      Cortex-A510
> 
>   Documentation/arm64/silicon-errata.rst |  2 ++
>   arch/arm64/Kconfig                     | 14 +++++++++
>   arch/arm64/include/asm/cpufeature.h    |  2 ++
>   arch/arm64/kernel/cpu_errata.c         | 27 ++++++++++++++++
>   arch/arm64/kernel/cpufeature.c         | 43 ++++++++++++++++++++------
>   arch/arm64/tools/cpucaps               |  1 +
>   6 files changed, 79 insertions(+), 10 deletions(-)
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] arm64: errata: remove BF16 HWCAP due to incorrect result on Cortex-A510

[James Morse]

Hi Robin,

On 09/09/2022 19:29, Robin Murphy wrote:
> On 2022-09-09 17:59, James Morse wrote:
>> Cortex-A510 has an erratum where the BFMMLA or VMMLA instructions might
>> produce the incorrect result. This only happens if two Cortex-A510 CPUs
>> are configured by the SoC manafacturer in a particular way to share some
>> hardware, and are using it at the same time. It isn't possible for linux
>> to know how the CPUs were configured, so the only option is to disable
>> the BF16 feature for all Cortex-A510 CPUs.

> Hmm, the TRM doesn't seem to describe any obvious restrictions on accessing IMP_CPUCFR_EL1
> - is there some other nefarious secret at play there?

HCR_EL2.TIDCP.

KVM traps all IMP_* registers from EL1, and has no ability to emulate them.
For an erratum workaround at EL1, touching an imp-def register is the wrong thing to do.

We could add some discovery API with firmware to determine if the part is affected, but I
think that is overkill for this. I'm pretty sure affected Cortex-A510 silicon is only in
mobile phones, which can have the Kconfig option disabled if the target platform isn't
affected. (Even with GKI, I assume things don't run one kernel image in the same way
distros do)


Thanks,

James

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] arm64: errata: remove BF16 HWCAP due to incorrect result on Cortex-A510

[Robin Murphy]

On 2022-09-15 14:39, James Morse wrote:
> Hi Robin,
> 
> On 09/09/2022 19:29, Robin Murphy wrote:
>> On 2022-09-09 17:59, James Morse wrote:
>>> Cortex-A510 has an erratum where the BFMMLA or VMMLA instructions might
>>> produce the incorrect result. This only happens if two Cortex-A510 CPUs
>>> are configured by the SoC manafacturer in a particular way to share some
>>> hardware, and are using it at the same time. It isn't possible for linux
>>> to know how the CPUs were configured, so the only option is to disable
>>> the BF16 feature for all Cortex-A510 CPUs.
> 
>> Hmm, the TRM doesn't seem to describe any obvious restrictions on accessing IMP_CPUCFR_EL1
>> - is there some other nefarious secret at play there?
> 
> HCR_EL2.TIDCP.

Aha, indeed I should have looked more closely at the pseudocode.

> KVM traps all IMP_* registers from EL1, and has no ability to emulate them.
> For an erratum workaround at EL1, touching an imp-def register is the wrong thing to do.

Well, there's no real reason we couldn't make KVM handle this case, but 
that doesn't help if we're booted at EL1 under some other hypervisor, so 
in general I concur.

> We could add some discovery API with firmware to determine if the part is affected, but I
> think that is overkill for this. I'm pretty sure affected Cortex-A510 silicon is only in
> mobile phones, which can have the Kconfig option disabled if the target platform isn't
> affected. (Even with GKI, I assume things don't run one kernel image in the same way
> distros do)

I suppose it wouldn't take *too* much to check this in init_el2 and 
stash a "no actually it's fine" flag for later, but whether even that's 
worth it depends on how big the impact of losing BF16 on unaffected 
configurations actually is (which I have no idea about). If someone does 
care then that can always be added later, so that's good enough for me!

Cheers,
Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] arm64: errata: remove BF16 HWCAP due to incorrect result on Cortex-A510

[Catalin Marinas]

On Fri, 9 Sep 2022 17:59:35 +0100, James Morse wrote:
> Cortex-A510 has an erratum where the BFMMLA or VMMLA instructions might
> produce the incorrect result. This only happens if two Cortex-A510 CPUs
> are configured by the SoC manafacturer in a particular way to share some
> hardware, and are using it at the same time. It isn't possible for linux
> to know how the CPUs were configured, so the only option is to disable
> the BF16 feature for all Cortex-A510 CPUs.
> 
> [...]

Applied to arm64 (for-next/a510-erratum-2658417), thanks!

(I updated the last patch following Suzuki's comments)

[1/3] arm64: cpufeature: Force HWCAP to be based on the sysreg visible to user-space
      https://git.kernel.org/arm64/c/237405ebef58
[2/3] arm64: cpufeature: Expose get_arm64_ftr_reg() outside cpufeature.c
      https://git.kernel.org/arm64/c/445c953e4a84
[3/3] arm64: errata: remove BF16 HWCAP due to incorrect result on Cortex-A510
      https://git.kernel.org/arm64/c/1bdb0fbb2e27

-- 
Catalin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel