arm64: erratum: Workaround for Kryo reserved system register read
Subsystems (2)
PATCH 1/1 arm64: erratum: Workaround for Kryo reserved system register read
The ARMv8.0 architecture reserves several system register encodings for future use. These encodings should behave as read-only and always return zero on a read. The Kryo core errantly causes an instruction abort upon an AArch64 read attempt to the following system register encodings using the MRS instruction:
3, 0, C0, [C4-C7], [2-3, 6-7] 3, 0, C0, C3, [3-7] 3, 0, C0, [C4,C6,C7], [4-5] 3, 0, C0, C2, [6-7]
All system register encodings above use the following form Op0, Op1, CRn, CRm, Op2.
Note that some of the encodings listed above include the system register space reserved for the following identification registers which may appear in future revisions of the ARM architecture beyond ARMv8.0.
This space includes:
ID_AA64PFR[2-7]_EL1 ID_AA64DFR[2-3]_EL1 ID_AA64AFR[2-3]_EL1 ID_AA64ISAR[2-7]_EL1 ID_AA64MMFR[2-7]_EL1
Workaround:
Software must not rely on a zero value returned from any of the system register encodings listed above when attempting to determine support for certain architectural options. Software should instead infer architectural support when unaffected variant is in use by reading the MIDR_EL1.
System Impact
None. The required software workaround does not result in any reduction in functionality nor does it have any relevant impact on performance or power.
This change uses the ARMv8 implementer introduced in
https://www.mail-archive.com/kvm at vger.kernel.org/msg116473.html
--- Documentation/arm64/silicon-errata.txt | 29 +++++++++++++++-------------- arch/arm64/Kconfig | 29 +++++++++++++++++++++++++++++ arch/arm64/include/asm/cpufeature.h | 3 ++- arch/arm64/kernel/cpu_errata.c | 8 ++++++++ arch/arm64/kernel/cpuinfo.c | 10 +++++++--- 5 files changed, 61 insertions(+), 18 deletions(-)
Documentation/arm64/silicon-errata.txt +15 -14
@@ -42,17 +42,18 @@ file acts as a registry of software workarounds in the Linux Kernel and will be updated when new workarounds are committed and backported to stable kernels. -| Implementor | Component | Erratum ID | Kconfig | -+----------------+-----------------+-----------------+-------------------------+ -| ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 | -| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 | -| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 | -| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 | -| ARM | Cortex-A53 | #845719 | ARM64_ERRATUM_845719 | -| ARM | Cortex-A53 | #843419 | ARM64_ERRATUM_843419 | -| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 | -| ARM | Cortex-A57 | #852523 | N/A | -| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 | -| | | | | -| Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | -| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 | +| Implementor | Component | Erratum ID | Kconfig | ++------------------------+-----------------+-----------------+-------------------------+ +| ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 | +| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 | +| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 | +| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 | +| ARM | Cortex-A53 | #845719 | ARM64_ERRATUM_845719 | +| ARM | Cortex-A53 | #843419 | ARM64_ERRATUM_843419 | +| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 | +| ARM | Cortex-A57 | #852523 | N/A | +| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 | +| | | | | +| Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | +| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 | +| Qualcomm Technologies | Kryo | #94 | KRYO_ERRATUM_94 |
arch/arm64/Kconfig +29 -0
@@ -443,6 +443,35 @@ config CAVIUM_ERRATUM_23154 If unsure, say Y. +config KRYO_ERRATUM_94 + bool "ERRATUM to KRYO System Register Read" + help + The ARMv8.0 specification reserves several system registers + for future use. These registers should behave read-only and + return zero on a read. The Kryo core errantly causes + an instruction abort upon AArch64 read attempt of the following + system register encodings using the MRS instruction. + + 3, 0, C0, [C4-C7], [2-3, 6-7] + 3, 0, C0, C3, [3-7] + 3, 0, C0, [C4,C6,C7], [4-5] + 3, 0, C0, C2, [6-7] + + All system register encodings above use the form + + Op0, Op1, CRn, CRm, Op2. + + Note that some of the encodings listed above include + the system register space reserved for the following + identification registers which may appear in future revisions + of the ARM architecture beyond ARMv8.0. + This space includes: + ID_AA64PFR[2-7]_EL1 + ID_AA64DFR[2-3]_EL1 + ID_AA64AFR[2-3]_EL1 + ID_AA64ISAR[2-7]_EL1 + ID_AA64MMFR[2-7]_EL1 + endmenu
arch/arm64/include/asm/cpufeature.h +2 -1
@@ -30,8 +30,9 @@#define ARM64_HAS_LSE_ATOMICS 5#define ARM64_WORKAROUND_CAVIUM_23154 6#define ARM64_WORKAROUND_834220 7 +#define ARM64_WORKAROUND_KRYO_94 8 -#define ARM64_NCAPS 8 +#define ARM64_NCAPS 9#ifndef __ASSEMBLY__
arch/arm64/kernel/cpu_errata.c +8 -0
@@ -24,6 +24,7 @@#define MIDR_CORTEX_A53 MIDR_CPU_PART(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)#define MIDR_CORTEX_A57 MIDR_CPU_PART(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)#define MIDR_THUNDERX MIDR_CPU_PART(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX) +#define MIDR_QCOM_KRYO MIDR_CPU_PART(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO)#define CPU_MODEL_MASK (MIDR_IMPLEMENTOR_MASK | MIDR_PARTNUM_MASK | \ MIDR_ARCHITECTURE_MASK)
@@ -100,6 +101,13 @@ const struct arm64_cpu_capabilities arm64_errata[] = { MIDR_RANGE(MIDR_THUNDERX, 0x00, 0x01), },#endif +#ifdef CONFIG_KRYO_ERRATUM_94 + { + .desc = "Kryo Erratum 94", + .capability = ARM64_WORKAROUND_KRYO_94, + MIDR_RANGE(MIDR_QCOM_KRYO, 0x00, 0x01), + }, +#endif { } };
arch/arm64/kernel/cpuinfo.c +7 -3
@@ -204,13 +204,18 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info) info->reg_dczid =read_cpuid (SYS_DCZID_EL0 ); info->reg_midr =read_cpuid_id (); +check_local_cpu_errata (); + info->reg_id_aa64dfr0 =read_cpuid (SYS_ID_AA64DFR0_EL1 ); info->reg_id_aa64dfr1 =read_cpuid (SYS_ID_AA64DFR1_EL1 ); info->reg_id_aa64isar0 =read_cpuid (SYS_ID_AA64ISAR0_EL1 ); info->reg_id_aa64isar1 =read_cpuid (SYS_ID_AA64ISAR1_EL1 ); info->reg_id_aa64mmfr0 =read_cpuid (SYS_ID_AA64MMFR0_EL1 ); info->reg_id_aa64mmfr1 =read_cpuid (SYS_ID_AA64MMFR1_EL1 ); - info->reg_id_aa64mmfr2 =read_cpuid (SYS_ID_AA64MMFR2_EL1 ); +if (cpus_have_cap (ARM64_WORKAROUND_KRYO_94 )) + info->reg_id_aa64mmfr2 =0 ; +else + info->reg_id_aa64mmfr2 =read_cpuid (SYS_ID_AA64MMFR2_EL1 ); info->reg_id_aa64pfr0 =read_cpuid (SYS_ID_AA64PFR0_EL1 ); info->reg_id_aa64pfr1 =read_cpuid (SYS_ID_AA64PFR1_EL1 );
@@ -223,6 +228,7 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info) info->reg_id_isar5 =read_cpuid (SYS_ID_ISAR5_EL1 ); info->reg_id_mmfr0 =read_cpuid (SYS_ID_MMFR0_EL1 ); info->reg_id_mmfr1 =read_cpuid (SYS_ID_MMFR1_EL1 ); + info->reg_id_mmfr2 =read_cpuid (SYS_ID_MMFR2_EL1 ); info->reg_id_mmfr3 =read_cpuid (SYS_ID_MMFR3_EL1 ); info->reg_id_pfr0 =read_cpuid (SYS_ID_PFR0_EL1 );
@@ -233,8 +239,6 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info) info->reg_mvfr2 =read_cpuid (SYS_MVFR2_EL1 );cpuinfo_detect_icache_policy (info); - -check_local_cpu_errata (); }void cpuinfo_store_cpu (void )
Signature · 1 line
1.8.2.1
PATCH 1/1 arm64: erratum: Workaround for Kryo reserved system register read
HI Naveen,
Quoted 129 lines · Naveen Kaje
The ARMv8.0 architecture reserves several system register encodings for future use. These encodings should behave as read-only and always return zero on a read. The Kryo core errantly causes an instruction abort upon an AArch64 read attempt to the following system register encodings using the MRS instruction:
3, 0, C0, [C4-C7], [2-3, 6-7] 3, 0, C0, C3, [3-7] 3, 0, C0, [C4,C6,C7], [4-5] 3, 0, C0, C2, [6-7]All system register encodings above use the following form Op0, Op1, CRn, CRm, Op2.
Note that some of the encodings listed above include the system register space reserved for the following identification registers which may appear in future revisions of the ARM architecture beyond ARMv8.0.This space includes:
ID_AA64PFR[2-7]_EL1 ID_AA64DFR[2-3]_EL1 ID_AA64AFR[2-3]_EL1 ID_AA64ISAR[2-7]_EL1 ID_AA64MMFR[2-7]_EL1Workaround:
Software must not rely on a zero value returned from any of the system register encodings listed above when attempting to determine support for certain architectural options. Software should instead infer architectural support when unaffected variant is in use by reading theMIDR_EL1.System Impact
None. The required software workaround does not result in any reduction in functionality nor does it have any relevant impact on performance or power.This change uses the ARMv8 implementer introduced in
https://www.mail-archive.com/kvm at vger.kernel.org/msg116473.htmlChange-Id Id53e335b6c4524c730b95e5dd7279cf870bb82f6Signed-off-by Naveen Kaje <[email protected]>--- Documentation/arm64/silicon-errata.txt | 29 +++++++++++++++-------------- arch/arm64/Kconfig | 29 +++++++++++++++++++++++++++++ arch/arm64/include/asm/cpufeature.h | 3 ++- arch/arm64/kernel/cpu_errata.c | 8 ++++++++ arch/arm64/kernel/cpuinfo.c | 10 +++++++--- 5 files changed, 61 insertions(+), 18 deletions(-)Documentation/arm64/silicon-errata.txt +15
-14@@ -42,17 +42,18 @@ file acts as a registry of software workarounds in the Linux Kernel and will be updated when new workarounds are committed and backported to stable kernels. -| Implementor | Component | Erratum ID | Kconfig | -+----------------+-----------------+-----------------+-------------------------+ -| ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 | -| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 | -| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 | -| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 | -| ARM | Cortex-A53 | #845719 | ARM64_ERRATUM_845719 | -| ARM | Cortex-A53 | #843419 | ARM64_ERRATUM_843419 | -| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 | -| ARM | Cortex-A57 | #852523 | N/A | -| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 | -| | | | | -| Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | -| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 | +| Implementor | Component | Erratum ID | Kconfig | ++------------------------+-----------------+-----------------+-------------------------+ +| ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 | +| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 | +| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 | +| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 | +| ARM | Cortex-A53 | #845719 | ARM64_ERRATUM_845719 | +| ARM | Cortex-A53 | #843419 | ARM64_ERRATUM_843419 | +| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 | +| ARM | Cortex-A57 | #852523 | N/A | +| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 | +| | | | | +| Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | +| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 | +| Qualcomm Technologies | Kryo | #94 | KRYO_ERRATUM_94 |arch/arm64/Kconfig +29
-0@@ -443,6 +443,35 @@ config CAVIUM_ERRATUM_23154 If unsure, say Y. +config KRYO_ERRATUM_94 + bool "ERRATUM to KRYO System Register Read" + help + The ARMv8.0 specification reserves several system registers + for future use. These registers should behave read-only and + return zero on a read. The Kryo core errantly causes + an instruction abort upon AArch64 read attempt of the following + system register encodings using the MRS instruction. + + 3, 0, C0, [C4-C7], [2-3, 6-7] + 3, 0, C0, C3, [3-7] + 3, 0, C0, [C4,C6,C7], [4-5] + 3, 0, C0, C2, [6-7] + + All system register encodings above use the form + + Op0, Op1, CRn, CRm, Op2. + + Note that some of the encodings listed above include + the system register space reserved for the following + identification registers which may appear in future revisions + of the ARM architecture beyond ARMv8.0. + This space includes: + ID_AA64PFR[2-7]_EL1 + ID_AA64DFR[2-3]_EL1 + ID_AA64AFR[2-3]_EL1 + ID_AA64ISAR[2-7]_EL1 + ID_AA64MMFR[2-7]_EL1 + endmenuarch/arm64/include/asm/cpufeature.h +0
-0
arch/arm64/include/asm/cpufeature.h line 35
Quoted 7 lines
arch/arm64/include/asm/cpufeature.h +2
-1@@ -30,8 +30,9 @@#define ARM64_HAS_LSE_ATOMICS 5#define ARM64_WORKAROUND_CAVIUM_23154 6#define ARM64_WORKAROUND_834220 7 +#define ARM64_WORKAROUND_KRYO_94 8 -#define ARM64_NCAPS 8 +#define ARM64_NCAPS 9
Can you please make sure this applies on top of mainline? ARM64_NCAPS is already at 13, and counting...
Quoted 34 lines
arch/arm64/include/asm/cpufeature.h +0
-0#ifndef __ASSEMBLY__arch/arm64/kernel/cpu_errata.c +8
-0@@ -24,6 +24,7 @@#define MIDR_CORTEX_A53 MIDR_CPU_PART(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)#define MIDR_CORTEX_A57 MIDR_CPU_PART(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)#define MIDR_THUNDERX MIDR_CPU_PART(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX) +#define MIDR_QCOM_KRYO MIDR_CPU_PART(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO)#define CPU_MODEL_MASK (MIDR_IMPLEMENTOR_MASK | MIDR_PARTNUM_MASK | \ MIDR_ARCHITECTURE_MASK)@@ -100,6 +101,13 @@ const struct arm64_cpu_capabilities arm64_errata[] = { MIDR_RANGE(MIDR_THUNDERX, 0x00, 0x01), },#endif +#ifdef CONFIG_KRYO_ERRATUM_94 + { + .desc = "Kryo Erratum 94", + .capability = ARM64_WORKAROUND_KRYO_94, + MIDR_RANGE(MIDR_QCOM_KRYO, 0x00, 0x01), + }, +#endif { } };arch/arm64/kernel/cpuinfo.c +0
-0
arch/arm64/kernel/cpuinfo.c static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info) · line 208
Quoted 5 lines
arch/arm64/kernel/cpuinfo.c +2
-0@@ -204,13 +204,18 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info) info->reg_dczid =read_cpuid (SYS_DCZID_EL0 ); info->reg_midr =read_cpuid_id (); +check_local_cpu_errata (); +
What is the impact of moving this around? Suzuki, was there any particular reason why this check was done later rather than earlier?
Quoted 15 lines
arch/arm64/kernel/cpuinfo.c +4
-1info->reg_id_aa64dfr0 =read_cpuid (SYS_ID_AA64DFR0_EL1 ); info->reg_id_aa64dfr1 =read_cpuid (SYS_ID_AA64DFR1_EL1 ); info->reg_id_aa64isar0 =read_cpuid (SYS_ID_AA64ISAR0_EL1 ); info->reg_id_aa64isar1 =read_cpuid (SYS_ID_AA64ISAR1_EL1 ); info->reg_id_aa64mmfr0 =read_cpuid (SYS_ID_AA64MMFR0_EL1 ); info->reg_id_aa64mmfr1 =read_cpuid (SYS_ID_AA64MMFR1_EL1 ); - info->reg_id_aa64mmfr2 =read_cpuid (SYS_ID_AA64MMFR2_EL1 ); +if (cpus_have_cap (ARM64_WORKAROUND_KRYO_94 )) + info->reg_id_aa64mmfr2 =0 ; +else + info->reg_id_aa64mmfr2 =read_cpuid (SYS_ID_AA64MMFR2_EL1 ); info->reg_id_aa64pfr0 =read_cpuid (SYS_ID_AA64PFR0_EL1 ); info->reg_id_aa64pfr1 =read_cpuid (SYS_ID_AA64PFR1_EL1 );
arch/arm64/kernel/cpuinfo.c static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info) · line 231
Quoted 4 lines
arch/arm64/kernel/cpuinfo.c +1
-0@@ -223,6 +228,7 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info) info->reg_id_isar5 =read_cpuid (SYS_ID_ISAR5_EL1 ); info->reg_id_mmfr0 =read_cpuid (SYS_ID_MMFR0_EL1 ); info->reg_id_mmfr1 =read_cpuid (SYS_ID_MMFR1_EL1 ); +
Stray newline?
arch/arm64/kernel/cpuinfo.c static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info) · line 244
Quoted 13 lines
arch/arm64/kernel/cpuinfo.c +0
-2info->reg_id_mmfr2 =read_cpuid (SYS_ID_MMFR2_EL1 ); info->reg_id_mmfr3 =read_cpuid (SYS_ID_MMFR3_EL1 ); info->reg_id_pfr0 =read_cpuid (SYS_ID_PFR0_EL1 );@@ -233,8 +239,6 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info) info->reg_mvfr2 =read_cpuid (SYS_MVFR2_EL1 );cpuinfo_detect_icache_policy (info); - -check_local_cpu_errata (); }void cpuinfo_store_cpu (void )
So while this probably works for now, I'm a bit concerned that this doesn't cater for early code that would access system registers, nor does it cover the whole of the erratum (you only care about ID_AA64MMFR2_EL1 here).
Ideally, we'd have a trapping fallback for this.
Thanks,
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH 1/1 arm64: erratum: Workaround for Kryo reserved system register read
Quoted 6 lines · Marc Zyngier
Quoted 6 lines
arch/arm64/Kconfig +6
-0+ All system register encodings above use the form + + Op0, Op1, CRn, CRm, Op2. + + Note that some of the encodings listed above include + the system register space reserved for the following
Quoted 8 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_23154 · line 473
Quoted 8 lines
arch/arm64/Kconfig +8
-0+ identification registers which may appear in future revisions + of the ARM architecture beyond ARMv8.0. + This space includes: + ID_AA64PFR[2-7]_EL1 + ID_AA64DFR[2-3]_EL1 + ID_AA64AFR[2-3]_EL1 + ID_AA64ISAR[2-7]_EL1 + ID_AA64MMFR[2-7]_EL1
AFAIK, the id space is unassigned. So the naming above could cause confusion if the register is named something else.
Quoted 6 lines
arch/arm64/kernel/cpuinfo.c static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info) · line 208
Quoted 3 lines
arch/arm64/kernel/cpuinfo.c +3
-0+ +check_local_cpu_errata (); +What is the impact of moving this around? Suzuki, was there any particular reason why this check was done later rather than earlier?
All the existing errata look for MIDR to match, which is read separately using read_cpuid_id(). The moment we need to do something w.r.t an ID register, this will break. So at the moment moving this doesn't have much of an impact.
Thanks
Suzuki
PATCH 1/1 arm64: erratum: Workaround for Kryo reserved system register read
Quoted 12 lines · Suzuki K Poulose
Quoted 10 lines · Marc Zyngier
Quoted 10 lines
arch/arm64/Kconfig +10
-0+ All system register encodings above use the form + + Op0, Op1, CRn, CRm, Op2. + + Note that some of the encodings listed above include + the system register space reserved for the following + identification registers which may appear in future revisions + of the ARM architecture beyond ARMv8.0. + This space includes: + ID_AA64PFR[2-7]_EL1
Quoted 8 lines
Quoted 4 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_23154 · line 473
Quoted 4 lines
arch/arm64/Kconfig +4
-0+ ID_AA64DFR[2-3]_EL1 + ID_AA64AFR[2-3]_EL1 + ID_AA64ISAR[2-7]_EL1 + ID_AA64MMFR[2-7]_EL1AFAIK, the id space is unassigned. So the naming above could cause confusion if the register is named something else.
It is reserved *at the moment*, but already has a defined behaviour. My worry is that when some new architecture revision comes around, we start using these registers without thinking much about it (because we should be able to). At this point, your SoC will catch fire and nobody will have a clue about the problem because it is not apparent in the code.
I'd really like to see something a bit more forward looking that covers that space for good.
Thanks,
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH 1/1 arm64: erratum: Workaround for Kryo reserved system register read
Quoted 22 lines · Marc Zyngier
Quoted 12 lines · Suzuki K Poulose
Quoted 10 lines · Marc Zyngier
Quoted 10 lines
arch/arm64/Kconfig +10
-0+ All system register encodings above use the form + + Op0, Op1, CRn, CRm, Op2. + + Note that some of the encodings listed above include + the system register space reserved for the following + identification registers which may appear in future revisions + of the ARM architecture beyond ARMv8.0. + This space includes: + ID_AA64PFR[2-7]_EL1Quoted 8 lines
Quoted 4 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_23154 · line 473
Quoted 4 lines
arch/arm64/Kconfig +4
-0+ ID_AA64DFR[2-3]_EL1 + ID_AA64AFR[2-3]_EL1 + ID_AA64ISAR[2-7]_EL1 + ID_AA64MMFR[2-7]_EL1AFAIK, the id space is unassigned. So the naming above could cause confusion if the register is named something else.
Quoted 1 line
It is reserved *at the moment*, but already has a defined behaviour. My
Absolutely, they do need to be RAZ. My point was assigning names to the reserved space where the names are unassigned.
Quoted 7 lines
worry is that when some new architecture revision comes around, we start using these registers without thinking much about it (because we should be able to). At this point, your SoC will catch fire and nobody will have a clue about the problem because it is not apparent in the code.
I'd really like to see something a bit more forward looking that covers that space for good.
I agree, the patch definitely needs to take care of handling the entire space.
Cheers
Suzuki
PATCH 1/1 arm64: erratum: Workaround for Kryo reserved system register read
Quoted 23 lines · Suzuki K Poulose
Quoted 22 lines · Marc Zyngier
Quoted 20 lines · Suzuki K Poulose
Quoted 6 lines · Marc Zyngier
Quoted 6 lines
arch/arm64/Kconfig +6
-0+ All system register encodings above use the form + + Op0, Op1, CRn, CRm, Op2. + + Note that some of the encodings listed above include + the system register space reserved for the followingQuoted 8 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_23154 · line 473
Quoted 8 lines
arch/arm64/Kconfig +8
-0+ identification registers which may appear in future revisions + of the ARM architecture beyond ARMv8.0. + This space includes: + ID_AA64PFR[2-7]_EL1 + ID_AA64DFR[2-3]_EL1 + ID_AA64AFR[2-3]_EL1 + ID_AA64ISAR[2-7]_EL1 + ID_AA64MMFR[2-7]_EL1AFAIK, the id space is unassigned. So the naming above could cause confusion if the register is named something else.
Quoted 4 lines
Quoted 1 line
It is reserved *at the moment*, but already has a defined behaviour. My
Absolutely, they do need to be RAZ. My point was assigning names to the reserved space where the names are unassigned.
Sorry - I misread your statement. It makes a lot more sense now that the coffee has trickled in.
We're in violent agreement! ;-)
Thanks,
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH 1/1 arm64: erratum: Workaround for Kryo reserved system register read
In reply to Marc ZyngierQuoted 22 lines · Marc Zyngier
Quoted 12 lines · Suzuki K Poulose
Quoted 10 lines · Marc Zyngier
Quoted 10 lines
arch/arm64/Kconfig +10
-0+ All system register encodings above use the form + + Op0, Op1, CRn, CRm, Op2. + + Note that some of the encodings listed above include + the system register space reserved for the following + identification registers which may appear in future revisions + of the ARM architecture beyond ARMv8.0. + This space includes: + ID_AA64PFR[2-7]_EL1Quoted 8 lines
Quoted 4 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_23154 · line 473
Quoted 4 lines
arch/arm64/Kconfig +4
-0+ ID_AA64DFR[2-3]_EL1 + ID_AA64AFR[2-3]_EL1 + ID_AA64ISAR[2-7]_EL1 + ID_AA64MMFR[2-7]_EL1AFAIK, the id space is unassigned. So the naming above could cause confusion if the register is named something else.
Quoted 8 lines
It is reserved *at the moment*, but already has a defined behaviour. My worry is that when some new architecture revision comes around, we start using these registers without thinking much about it (because we should be able to). At this point, your SoC will catch fire and nobody will have a clue about the problem because it is not apparent in the code.
I'd really like to see something a bit more forward looking that covers that space for good.
At the risk of volunteering...
Registering these instructions with the undef hooks would be ideal, but they won't catch this instruction abort. I guess refactor them to be generic faulting instruction hooks, and have a list for the existing undef cases, and a new one for this instruction abort.
This won't cover early code in head.S, or KVM code that runs at EL2. Is this sufficient, or should any approach cover those too?
Thanks,
James
PATCH 1/1 arm64: erratum: Workaround for Kryo reserved system register read
Quoted 32 lines · James Morse
Quoted 22 lines · Marc Zyngier
Quoted 12 lines · Suzuki K Poulose
Quoted 10 lines · Marc Zyngier
Quoted 10 lines
arch/arm64/Kconfig +10
-0+ All system register encodings above use the form + + Op0, Op1, CRn, CRm, Op2. + + Note that some of the encodings listed above include + the system register space reserved for the following + identification registers which may appear in future revisions + of the ARM architecture beyond ARMv8.0. + This space includes: + ID_AA64PFR[2-7]_EL1Quoted 8 lines
Quoted 4 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_23154 · line 473
Quoted 4 lines
arch/arm64/Kconfig +4
-0+ ID_AA64DFR[2-3]_EL1 + ID_AA64AFR[2-3]_EL1 + ID_AA64ISAR[2-7]_EL1 + ID_AA64MMFR[2-7]_EL1AFAIK, the id space is unassigned. So the naming above could cause confusion if the register is named something else.
Quoted 8 lines
It is reserved *at the moment*, but already has a defined behaviour. My worry is that when some new architecture revision comes around, we start using these registers without thinking much about it (because we should be able to). At this point, your SoC will catch fire and nobody will have a clue about the problem because it is not apparent in the code.
I'd really like to see something a bit more forward looking that covers that space for good.
Quoted 8 lines
At the risk of volunteering...
Registering these instructions with the undef hooks would be ideal, but they won't catch this instruction abort. I guess refactor them to be generic faulting instruction hooks, and have a list for the existing undef cases, and a new one for this instruction abort.This won't cover early code in
head.S, or KVM code that runs at EL2. Is this sufficient, or should any approach cover those too?
I much prefer a trapping approach than trying to patch the instructions accessing the ID registers. The ID registers are used to figure out which alternatives need to be applied and having this circular dependency feels particularly fragile.
So, we need to figure out (a) what sort of exceptions we're likely to get and (b) what syndrome information is provided. In the worst case, we'll end up disassembling the instruction stream (or using an ugly out-of-line function to access system registers).
Will
Catalin Marinas
maintainerPATCH 1/1 arm64: erratum: Workaround for Kryo reserved system register read
In reply to Naveen KajeQuoted 10 lines · Naveen Kaje
The ARMv8.0 architecture reserves several system register encodings for future use. These encodings should behave as read-only and always return zero on a read. The Kryo core errantly causes an instruction abort upon an AArch64 read attempt to the following system register encodings using the MRS instruction:
3, 0, C0, [C4-C7], [2-3, 6-7] 3, 0, C0, C3, [3-7] 3, 0, C0, [C4,C6,C7], [4-5] 3, 0, C0, C2, [6-7]
Quoted 6 lines
All system register encodings above use the following form Op0, Op1, CRn, CRm, Op2.
Note that some of the encodings listed above include the system register space reserved for the following identification registers which may appear in future revisions of the ARM architecture beyond ARMv8.0.
Is this bug affecting test or production silicon? If the former (which I presume is the case since such chip wouldn't have passed the ARM validation suite), I won't merge the workaround into mainline. It is, however, fine by me to give it to your early testers. We have a similar approach with the ARM Ltd CPUs and don't upstream any CPU errata workaround unless a partner goes with the silicon into production.
Signature · 1 line
Catalin