linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] arm64: errata: Rework Spectre BHB mitigations to not assume "safe"
@ 2024-12-19 20:53 Douglas Anderson
  2024-12-19 20:53 ` [PATCH v3 1/3] arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre BHB Douglas Anderson
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Douglas Anderson @ 2024-12-19 20:53 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland
  Cc: Roxana Bradescu, Julius Werner, bjorn.andersson, linux-arm-kernel,
	linux-arm-msm, Jeffrey Hugo, Trilok Soni, Douglas Anderson,
	Anshuman Khandual, Besar Wicaksono, D Scott Phillips,
	Easwar Hariharan, James Morse, Kent Overstreet, Oliver Upton,
	Suren Baghdasaryan, linux-kernel


Recently I realized that a device with some Qualcomm Kryo 4xx cores
reported in `lscpu` that it was _not_ vulnerable to Spectre BHB. This
seemed unlikely to me.

I wrote up a patch series to attempt (with a lot of guesswork) to add
Qualcomm cores to the tables governing how the Spectre BHB mitigation
worked.

In response to that patch, Will suggested that I flip the mitigation
on its head and assume things are vulnerable until we find that
they're not [1]. This patch series _attempts_ to accomplish that.

In case it's not obvious, v2 of this patch series was pretty different
than v1 because it flips the logic on its head. Some of the patches
carried over, though.

v3 is yet more different, avoiding the guesses (and thus dropping
some patches) and also incorporating feedback from Julius in response
to v2.

As a last caveat, I'll note that I am certainly no expert on
Spectre. Mostly I ended up here running `lscpu` on a device and
noticing that it thought that it wasn't affected by Spectre v2 when I
thought it was.

Link to prev versions:
v1: https://lore.kernel.org/r/20241209174430.2904353-1-dianders@chromium.org/
v2: https://lore.kernel.org/r/20241214005248.198803-1-dianders@chromium.org

[1] https://lore.kernel.org/r/20241211213410.GB17486@willie-the-truck

Changes in v3:
- Don't guess the mitigation; just report unknown cores as vulnerable.
- Restructure the code since is_spectre_bhb_affected() defaults to true
- arm64: cputype: Add MIDR_CORTEX_A76AE
- arm64: errata: Add newer ARM cores to the spectre_bhb_loop_affected() lists

Changes in v2:
- arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre BHB

Douglas Anderson (3):
  arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre
    BHB
  arm64: cputype: Add MIDR_CORTEX_A76AE
  arm64: errata: Add newer ARM cores to the spectre_bhb_loop_affected()
    lists

 arch/arm64/include/asm/cputype.h |   2 +
 arch/arm64/include/asm/spectre.h |   1 -
 arch/arm64/kernel/proton-pack.c  | 157 ++++++++++++++++++-------------
 3 files changed, 92 insertions(+), 68 deletions(-)

-- 
2.47.1.613.gc27f4b7a9f-goog



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

* [PATCH v3 1/3] arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre BHB
  2024-12-19 20:53 [PATCH v3 0/3] arm64: errata: Rework Spectre BHB mitigations to not assume "safe" Douglas Anderson
@ 2024-12-19 20:53 ` Douglas Anderson
  2024-12-20 16:33   ` Julius Werner
                     ` (2 more replies)
  2024-12-19 20:53 ` [PATCH v3 2/3] arm64: cputype: Add MIDR_CORTEX_A76AE Douglas Anderson
  2024-12-19 20:53 ` [PATCH v3 3/3] arm64: errata: Add newer ARM cores to the spectre_bhb_loop_affected() lists Douglas Anderson
  2 siblings, 3 replies; 7+ messages in thread
From: Douglas Anderson @ 2024-12-19 20:53 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland
  Cc: Roxana Bradescu, Julius Werner, bjorn.andersson, linux-arm-kernel,
	linux-arm-msm, Jeffrey Hugo, Trilok Soni, Douglas Anderson,
	stable, James Morse, Kent Overstreet, Suren Baghdasaryan,
	linux-kernel

The code for detecting CPUs that are vulnerable to Spectre BHB was
based on a hardcoded list of CPU IDs that were known to be affected.
Unfortunately, the list mostly only contained the IDs of standard ARM
cores. The IDs for many cores that are minor variants of the standard
ARM cores (like many Qualcomm Kyro CPUs) weren't listed. This led the
code to assume that those variants were not affected.

Flip the code on its head and instead assume that a core is vulnerable
if it doesn't have CSV2_3 but is unrecognized as being safe. This
involves creating a "Spectre BHB safe" list.

As of right now, the only CPU IDs added to the "Spectre BHB safe" list
are ARM Cortex A35, A53, A55, A510, and A520. This list was created by
looking for cores that weren't listed in ARM's list [1] as per review
feedback on v2 of this patch [2].

NOTE: this patch will not actually _mitigate_ anyone, it will simply
cause them to report themselves as vulnerable. If any cores in the
system are reported as vulnerable but not mitigated then the whole
system will be reported as vulnerable though the system will attempt
to mitigate with the information it has about the known cores.

[1] https://developer.arm.com/Arm%20Security%20Center/Spectre-BHB
[2] https://lore.kernel.org/r/20241219175128.GA25477@willie-the-truck


Fixes: 558c303c9734 ("arm64: Mitigate spectre style branch history side channels")
Cc: stable@vger.kernel.org
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v3:
- Don't guess the mitigation; just report unknown cores as vulnerable.
- Restructure the code since is_spectre_bhb_affected() defaults to true

Changes in v2:
- New

 arch/arm64/include/asm/spectre.h |   1 -
 arch/arm64/kernel/proton-pack.c  | 144 +++++++++++++++++--------------
 2 files changed, 77 insertions(+), 68 deletions(-)

diff --git a/arch/arm64/include/asm/spectre.h b/arch/arm64/include/asm/spectre.h
index 0c4d9045c31f..f1524cdeacf1 100644
--- a/arch/arm64/include/asm/spectre.h
+++ b/arch/arm64/include/asm/spectre.h
@@ -97,7 +97,6 @@ enum mitigation_state arm64_get_meltdown_state(void);
 
 enum mitigation_state arm64_get_spectre_bhb_state(void);
 bool is_spectre_bhb_affected(const struct arm64_cpu_capabilities *entry, int scope);
-u8 spectre_bhb_loop_affected(int scope);
 void spectre_bhb_enable_mitigation(const struct arm64_cpu_capabilities *__unused);
 bool try_emulate_el1_ssbs(struct pt_regs *regs, u32 instr);
 
diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c
index da53722f95d4..06e04c9e6480 100644
--- a/arch/arm64/kernel/proton-pack.c
+++ b/arch/arm64/kernel/proton-pack.c
@@ -845,52 +845,68 @@ static unsigned long system_bhb_mitigations;
  * This must be called with SCOPE_LOCAL_CPU for each type of CPU, before any
  * SCOPE_SYSTEM call will give the right answer.
  */
-u8 spectre_bhb_loop_affected(int scope)
+static bool is_spectre_bhb_safe(int scope)
+{
+	static const struct midr_range spectre_bhb_safe_list[] = {
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A35),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A53),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A55),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A510),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A520),
+		{},
+	};
+	static bool all_safe = true;
+
+	if (scope != SCOPE_LOCAL_CPU)
+		return all_safe;
+
+	if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_safe_list))
+		return true;
+
+	all_safe = false;
+
+	return false;
+}
+
+static u8 spectre_bhb_loop_affected(void)
 {
 	u8 k = 0;
-	static u8 max_bhb_k;
-
-	if (scope == SCOPE_LOCAL_CPU) {
-		static const struct midr_range spectre_bhb_k32_list[] = {
-			MIDR_ALL_VERSIONS(MIDR_CORTEX_A78),
-			MIDR_ALL_VERSIONS(MIDR_CORTEX_A78AE),
-			MIDR_ALL_VERSIONS(MIDR_CORTEX_A78C),
-			MIDR_ALL_VERSIONS(MIDR_CORTEX_X1),
-			MIDR_ALL_VERSIONS(MIDR_CORTEX_A710),
-			MIDR_ALL_VERSIONS(MIDR_CORTEX_X2),
-			MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N2),
-			MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V1),
-			{},
-		};
-		static const struct midr_range spectre_bhb_k24_list[] = {
-			MIDR_ALL_VERSIONS(MIDR_CORTEX_A76),
-			MIDR_ALL_VERSIONS(MIDR_CORTEX_A77),
-			MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
-			{},
-		};
-		static const struct midr_range spectre_bhb_k11_list[] = {
-			MIDR_ALL_VERSIONS(MIDR_AMPERE1),
-			{},
-		};
-		static const struct midr_range spectre_bhb_k8_list[] = {
-			MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
-			MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
-			{},
-		};
-
-		if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k32_list))
-			k = 32;
-		else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k24_list))
-			k = 24;
-		else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k11_list))
-			k = 11;
-		else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k8_list))
-			k =  8;
-
-		max_bhb_k = max(max_bhb_k, k);
-	} else {
-		k = max_bhb_k;
-	}
+
+	static const struct midr_range spectre_bhb_k32_list[] = {
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A78),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A78AE),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A78C),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_X1),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A710),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_X2),
+		MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N2),
+		MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V1),
+		{},
+	};
+	static const struct midr_range spectre_bhb_k24_list[] = {
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A76),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A77),
+		MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
+		{},
+	};
+	static const struct midr_range spectre_bhb_k11_list[] = {
+		MIDR_ALL_VERSIONS(MIDR_AMPERE1),
+		{},
+	};
+	static const struct midr_range spectre_bhb_k8_list[] = {
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
+		{},
+	};
+
+	if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k32_list))
+		k = 32;
+	else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k24_list))
+		k = 24;
+	else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k11_list))
+		k = 11;
+	else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k8_list))
+		k =  8;
 
 	return k;
 }
@@ -916,9 +932,8 @@ static enum mitigation_state spectre_bhb_get_cpu_fw_mitigation_state(void)
 	}
 }
 
-static bool is_spectre_bhb_fw_affected(int scope)
+static bool is_spectre_bhb_fw_affected(void)
 {
-	static bool system_affected;
 	enum mitigation_state fw_state;
 	bool has_smccc = arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_NONE;
 	static const struct midr_range spectre_bhb_firmware_mitigated_list[] = {
@@ -929,16 +944,8 @@ static bool is_spectre_bhb_fw_affected(int scope)
 	bool cpu_in_list = is_midr_in_range_list(read_cpuid_id(),
 					 spectre_bhb_firmware_mitigated_list);
 
-	if (scope != SCOPE_LOCAL_CPU)
-		return system_affected;
-
 	fw_state = spectre_bhb_get_cpu_fw_mitigation_state();
-	if (cpu_in_list || (has_smccc && fw_state == SPECTRE_MITIGATED)) {
-		system_affected = true;
-		return true;
-	}
-
-	return false;
+	return cpu_in_list || (has_smccc && fw_state == SPECTRE_MITIGATED);
 }
 
 static bool supports_ecbhb(int scope)
@@ -954,6 +961,8 @@ static bool supports_ecbhb(int scope)
 						    ID_AA64MMFR1_EL1_ECBHB_SHIFT);
 }
 
+static u8 max_bhb_k;
+
 bool is_spectre_bhb_affected(const struct arm64_cpu_capabilities *entry,
 			     int scope)
 {
@@ -962,16 +971,18 @@ bool is_spectre_bhb_affected(const struct arm64_cpu_capabilities *entry,
 	if (supports_csv2p3(scope))
 		return false;
 
-	if (supports_clearbhb(scope))
-		return true;
-
-	if (spectre_bhb_loop_affected(scope))
-		return true;
+	if (is_spectre_bhb_safe(scope))
+		return false;
 
-	if (is_spectre_bhb_fw_affected(scope))
-		return true;
+	/*
+	 * At this point the core isn't known to be "safe" so we're going to
+	 * assume it's vulnerable. We still need to update `max_bhb_k` though,
+	 * but only if we aren't mitigating with clearbhb though.
+	 */
+	if (scope == SCOPE_LOCAL_CPU && !supports_clearbhb(SCOPE_LOCAL_CPU))
+		max_bhb_k = max(max_bhb_k, spectre_bhb_loop_affected());
 
-	return false;
+	return true;
 }
 
 static void this_cpu_set_vectors(enum arm64_bp_harden_el1_vectors slot)
@@ -1028,7 +1039,7 @@ void spectre_bhb_enable_mitigation(const struct arm64_cpu_capabilities *entry)
 		this_cpu_set_vectors(EL1_VECTOR_BHB_CLEAR_INSN);
 		state = SPECTRE_MITIGATED;
 		set_bit(BHB_INSN, &system_bhb_mitigations);
-	} else if (spectre_bhb_loop_affected(SCOPE_LOCAL_CPU)) {
+	} else if (spectre_bhb_loop_affected()) {
 		/*
 		 * Ensure KVM uses the indirect vector which will have the
 		 * branchy-loop added. A57/A72-r0 will already have selected
@@ -1041,7 +1052,7 @@ void spectre_bhb_enable_mitigation(const struct arm64_cpu_capabilities *entry)
 		this_cpu_set_vectors(EL1_VECTOR_BHB_LOOP);
 		state = SPECTRE_MITIGATED;
 		set_bit(BHB_LOOP, &system_bhb_mitigations);
-	} else if (is_spectre_bhb_fw_affected(SCOPE_LOCAL_CPU)) {
+	} else if (is_spectre_bhb_fw_affected()) {
 		fw_state = spectre_bhb_get_cpu_fw_mitigation_state();
 		if (fw_state == SPECTRE_MITIGATED) {
 			/*
@@ -1100,7 +1111,6 @@ void noinstr spectre_bhb_patch_loop_iter(struct alt_instr *alt,
 {
 	u8 rd;
 	u32 insn;
-	u16 loop_count = spectre_bhb_loop_affected(SCOPE_SYSTEM);
 
 	BUG_ON(nr_inst != 1); /* MOV -> MOV */
 
@@ -1109,7 +1119,7 @@ void noinstr spectre_bhb_patch_loop_iter(struct alt_instr *alt,
 
 	insn = le32_to_cpu(*origptr);
 	rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, insn);
-	insn = aarch64_insn_gen_movewide(rd, loop_count, 0,
+	insn = aarch64_insn_gen_movewide(rd, max_bhb_k, 0,
 					 AARCH64_INSN_VARIANT_64BIT,
 					 AARCH64_INSN_MOVEWIDE_ZERO);
 	*updptr++ = cpu_to_le32(insn);
-- 
2.47.1.613.gc27f4b7a9f-goog



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

* [PATCH v3 2/3] arm64: cputype: Add MIDR_CORTEX_A76AE
  2024-12-19 20:53 [PATCH v3 0/3] arm64: errata: Rework Spectre BHB mitigations to not assume "safe" Douglas Anderson
  2024-12-19 20:53 ` [PATCH v3 1/3] arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre BHB Douglas Anderson
@ 2024-12-19 20:53 ` Douglas Anderson
  2024-12-19 20:53 ` [PATCH v3 3/3] arm64: errata: Add newer ARM cores to the spectre_bhb_loop_affected() lists Douglas Anderson
  2 siblings, 0 replies; 7+ messages in thread
From: Douglas Anderson @ 2024-12-19 20:53 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland
  Cc: Roxana Bradescu, Julius Werner, bjorn.andersson, linux-arm-kernel,
	linux-arm-msm, Jeffrey Hugo, Trilok Soni, Douglas Anderson,
	stable, Anshuman Khandual, Besar Wicaksono, D Scott Phillips,
	Easwar Hariharan, Oliver Upton, linux-kernel

From the TRM, MIDR_CORTEX_A76AE has a partnum of 0xDOE and an
implementor of 0x41 (ARM). Add the values.


Cc: stable@vger.kernel.org
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v3:
- New

 arch/arm64/include/asm/cputype.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 488f8e751349..a345628fce51 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -75,6 +75,7 @@
 #define ARM_CPU_PART_CORTEX_A76		0xD0B
 #define ARM_CPU_PART_NEOVERSE_N1	0xD0C
 #define ARM_CPU_PART_CORTEX_A77		0xD0D
+#define ARM_CPU_PART_CORTEX_A76AE	0xD0E
 #define ARM_CPU_PART_NEOVERSE_V1	0xD40
 #define ARM_CPU_PART_CORTEX_A78		0xD41
 #define ARM_CPU_PART_CORTEX_A78AE	0xD42
@@ -158,6 +159,7 @@
 #define MIDR_CORTEX_A76	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A76)
 #define MIDR_NEOVERSE_N1 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N1)
 #define MIDR_CORTEX_A77	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A77)
+#define MIDR_CORTEX_A76AE	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A76AE)
 #define MIDR_NEOVERSE_V1	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_V1)
 #define MIDR_CORTEX_A78	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A78)
 #define MIDR_CORTEX_A78AE	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A78AE)
-- 
2.47.1.613.gc27f4b7a9f-goog



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

* [PATCH v3 3/3] arm64: errata: Add newer ARM cores to the spectre_bhb_loop_affected() lists
  2024-12-19 20:53 [PATCH v3 0/3] arm64: errata: Rework Spectre BHB mitigations to not assume "safe" Douglas Anderson
  2024-12-19 20:53 ` [PATCH v3 1/3] arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre BHB Douglas Anderson
  2024-12-19 20:53 ` [PATCH v3 2/3] arm64: cputype: Add MIDR_CORTEX_A76AE Douglas Anderson
@ 2024-12-19 20:53 ` Douglas Anderson
  2 siblings, 0 replies; 7+ messages in thread
From: Douglas Anderson @ 2024-12-19 20:53 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland
  Cc: Roxana Bradescu, Julius Werner, bjorn.andersson, linux-arm-kernel,
	linux-arm-msm, Jeffrey Hugo, Trilok Soni, Douglas Anderson,
	stable, James Morse, linux-kernel

When comparing to the ARM list [1], it appears that several ARM cores
were missing from the lists in spectre_bhb_loop_affected(). Add them.

NOTE: for some of these cores it may not matter since other ways of
clearing the BHB may be used (like the CLRBHB instruction or ECBHB),
but it still seems good to have all the info from ARM's whitepaper
included.

[1] https://developer.arm.com/Arm%20Security%20Center/Spectre-BHB


Fixes: 558c303c9734 ("arm64: Mitigate spectre style branch history side channels")
Cc: stable@vger.kernel.org
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v3:
- New

 arch/arm64/kernel/proton-pack.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c
index 06e04c9e6480..86d67f5a5a72 100644
--- a/arch/arm64/kernel/proton-pack.c
+++ b/arch/arm64/kernel/proton-pack.c
@@ -872,6 +872,14 @@ static u8 spectre_bhb_loop_affected(void)
 {
 	u8 k = 0;
 
+	static const struct midr_range spectre_bhb_k132_list[] = {
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_X3),
+		MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V2),
+	};
+	static const struct midr_range spectre_bhb_k38_list[] = {
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A715),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A720),
+	};
 	static const struct midr_range spectre_bhb_k32_list[] = {
 		MIDR_ALL_VERSIONS(MIDR_CORTEX_A78),
 		MIDR_ALL_VERSIONS(MIDR_CORTEX_A78AE),
@@ -885,6 +893,7 @@ static u8 spectre_bhb_loop_affected(void)
 	};
 	static const struct midr_range spectre_bhb_k24_list[] = {
 		MIDR_ALL_VERSIONS(MIDR_CORTEX_A76),
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A76AE),
 		MIDR_ALL_VERSIONS(MIDR_CORTEX_A77),
 		MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
 		{},
@@ -899,7 +908,11 @@ static u8 spectre_bhb_loop_affected(void)
 		{},
 	};
 
-	if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k32_list))
+	if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k132_list))
+		k = 132;
+	else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k38_list))
+		k = 38;
+	else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k32_list))
 		k = 32;
 	else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k24_list))
 		k = 24;
-- 
2.47.1.613.gc27f4b7a9f-goog



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

* Re: [PATCH v3 1/3] arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre BHB
  2024-12-19 20:53 ` [PATCH v3 1/3] arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre BHB Douglas Anderson
@ 2024-12-20 16:33   ` Julius Werner
  2025-01-06 22:32   ` Doug Anderson
  2025-01-06 22:48   ` Florian Fainelli
  2 siblings, 0 replies; 7+ messages in thread
From: Julius Werner @ 2024-12-20 16:33 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Catalin Marinas, Will Deacon, Mark Rutland, Roxana Bradescu,
	Julius Werner, bjorn.andersson, linux-arm-kernel, linux-arm-msm,
	Jeffrey Hugo, Trilok Soni, stable, James Morse, Kent Overstreet,
	Suren Baghdasaryan, linux-kernel

Reviewed-by: Julius Werner <jwerner@chromium.org>

On Thu, Dec 19, 2024 at 9:54 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> The code for detecting CPUs that are vulnerable to Spectre BHB was
> based on a hardcoded list of CPU IDs that were known to be affected.
> Unfortunately, the list mostly only contained the IDs of standard ARM
> cores. The IDs for many cores that are minor variants of the standard
> ARM cores (like many Qualcomm Kyro CPUs) weren't listed. This led the
> code to assume that those variants were not affected.
>
> Flip the code on its head and instead assume that a core is vulnerable
> if it doesn't have CSV2_3 but is unrecognized as being safe. This
> involves creating a "Spectre BHB safe" list.
>
> As of right now, the only CPU IDs added to the "Spectre BHB safe" list
> are ARM Cortex A35, A53, A55, A510, and A520. This list was created by
> looking for cores that weren't listed in ARM's list [1] as per review
> feedback on v2 of this patch [2].
>
> NOTE: this patch will not actually _mitigate_ anyone, it will simply
> cause them to report themselves as vulnerable. If any cores in the
> system are reported as vulnerable but not mitigated then the whole
> system will be reported as vulnerable though the system will attempt
> to mitigate with the information it has about the known cores.
>
> [1] https://developer.arm.com/Arm%20Security%20Center/Spectre-BHB
> [2] https://lore.kernel.org/r/20241219175128.GA25477@willie-the-truck
>
>
> Fixes: 558c303c9734 ("arm64: Mitigate spectre style branch history side channels")
> Cc: stable@vger.kernel.org
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v3:
> - Don't guess the mitigation; just report unknown cores as vulnerable.
> - Restructure the code since is_spectre_bhb_affected() defaults to true
>
> Changes in v2:
> - New
>
>  arch/arm64/include/asm/spectre.h |   1 -
>  arch/arm64/kernel/proton-pack.c  | 144 +++++++++++++++++--------------
>  2 files changed, 77 insertions(+), 68 deletions(-)
>
> diff --git a/arch/arm64/include/asm/spectre.h b/arch/arm64/include/asm/spectre.h
> index 0c4d9045c31f..f1524cdeacf1 100644
> --- a/arch/arm64/include/asm/spectre.h
> +++ b/arch/arm64/include/asm/spectre.h
> @@ -97,7 +97,6 @@ enum mitigation_state arm64_get_meltdown_state(void);
>
>  enum mitigation_state arm64_get_spectre_bhb_state(void);
>  bool is_spectre_bhb_affected(const struct arm64_cpu_capabilities *entry, int scope);
> -u8 spectre_bhb_loop_affected(int scope);
>  void spectre_bhb_enable_mitigation(const struct arm64_cpu_capabilities *__unused);
>  bool try_emulate_el1_ssbs(struct pt_regs *regs, u32 instr);
>
> diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c
> index da53722f95d4..06e04c9e6480 100644
> --- a/arch/arm64/kernel/proton-pack.c
> +++ b/arch/arm64/kernel/proton-pack.c
> @@ -845,52 +845,68 @@ static unsigned long system_bhb_mitigations;
>   * This must be called with SCOPE_LOCAL_CPU for each type of CPU, before any
>   * SCOPE_SYSTEM call will give the right answer.
>   */
> -u8 spectre_bhb_loop_affected(int scope)
> +static bool is_spectre_bhb_safe(int scope)
> +{
> +       static const struct midr_range spectre_bhb_safe_list[] = {
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A35),
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A53),
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A55),
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A510),
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A520),
> +               {},
> +       };
> +       static bool all_safe = true;
> +
> +       if (scope != SCOPE_LOCAL_CPU)
> +               return all_safe;
> +
> +       if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_safe_list))
> +               return true;
> +
> +       all_safe = false;
> +
> +       return false;
> +}
> +
> +static u8 spectre_bhb_loop_affected(void)
>  {
>         u8 k = 0;
> -       static u8 max_bhb_k;
> -
> -       if (scope == SCOPE_LOCAL_CPU) {
> -               static const struct midr_range spectre_bhb_k32_list[] = {
> -                       MIDR_ALL_VERSIONS(MIDR_CORTEX_A78),
> -                       MIDR_ALL_VERSIONS(MIDR_CORTEX_A78AE),
> -                       MIDR_ALL_VERSIONS(MIDR_CORTEX_A78C),
> -                       MIDR_ALL_VERSIONS(MIDR_CORTEX_X1),
> -                       MIDR_ALL_VERSIONS(MIDR_CORTEX_A710),
> -                       MIDR_ALL_VERSIONS(MIDR_CORTEX_X2),
> -                       MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N2),
> -                       MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V1),
> -                       {},
> -               };
> -               static const struct midr_range spectre_bhb_k24_list[] = {
> -                       MIDR_ALL_VERSIONS(MIDR_CORTEX_A76),
> -                       MIDR_ALL_VERSIONS(MIDR_CORTEX_A77),
> -                       MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
> -                       {},
> -               };
> -               static const struct midr_range spectre_bhb_k11_list[] = {
> -                       MIDR_ALL_VERSIONS(MIDR_AMPERE1),
> -                       {},
> -               };
> -               static const struct midr_range spectre_bhb_k8_list[] = {
> -                       MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
> -                       MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
> -                       {},
> -               };
> -
> -               if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k32_list))
> -                       k = 32;
> -               else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k24_list))
> -                       k = 24;
> -               else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k11_list))
> -                       k = 11;
> -               else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k8_list))
> -                       k =  8;
> -
> -               max_bhb_k = max(max_bhb_k, k);
> -       } else {
> -               k = max_bhb_k;
> -       }
> +
> +       static const struct midr_range spectre_bhb_k32_list[] = {
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A78),
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A78AE),
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A78C),
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_X1),
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A710),
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_X2),
> +               MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N2),
> +               MIDR_ALL_VERSIONS(MIDR_NEOVERSE_V1),
> +               {},
> +       };
> +       static const struct midr_range spectre_bhb_k24_list[] = {
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A76),
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A77),
> +               MIDR_ALL_VERSIONS(MIDR_NEOVERSE_N1),
> +               {},
> +       };
> +       static const struct midr_range spectre_bhb_k11_list[] = {
> +               MIDR_ALL_VERSIONS(MIDR_AMPERE1),
> +               {},
> +       };
> +       static const struct midr_range spectre_bhb_k8_list[] = {
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
> +               MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
> +               {},
> +       };
> +
> +       if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k32_list))
> +               k = 32;
> +       else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k24_list))
> +               k = 24;
> +       else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k11_list))
> +               k = 11;
> +       else if (is_midr_in_range_list(read_cpuid_id(), spectre_bhb_k8_list))
> +               k =  8;
>
>         return k;
>  }
> @@ -916,9 +932,8 @@ static enum mitigation_state spectre_bhb_get_cpu_fw_mitigation_state(void)
>         }
>  }
>
> -static bool is_spectre_bhb_fw_affected(int scope)
> +static bool is_spectre_bhb_fw_affected(void)
>  {
> -       static bool system_affected;
>         enum mitigation_state fw_state;
>         bool has_smccc = arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_NONE;
>         static const struct midr_range spectre_bhb_firmware_mitigated_list[] = {
> @@ -929,16 +944,8 @@ static bool is_spectre_bhb_fw_affected(int scope)
>         bool cpu_in_list = is_midr_in_range_list(read_cpuid_id(),
>                                          spectre_bhb_firmware_mitigated_list);
>
> -       if (scope != SCOPE_LOCAL_CPU)
> -               return system_affected;
> -
>         fw_state = spectre_bhb_get_cpu_fw_mitigation_state();
> -       if (cpu_in_list || (has_smccc && fw_state == SPECTRE_MITIGATED)) {
> -               system_affected = true;
> -               return true;
> -       }
> -
> -       return false;
> +       return cpu_in_list || (has_smccc && fw_state == SPECTRE_MITIGATED);
>  }
>
>  static bool supports_ecbhb(int scope)
> @@ -954,6 +961,8 @@ static bool supports_ecbhb(int scope)
>                                                     ID_AA64MMFR1_EL1_ECBHB_SHIFT);
>  }
>
> +static u8 max_bhb_k;
> +
>  bool is_spectre_bhb_affected(const struct arm64_cpu_capabilities *entry,
>                              int scope)
>  {
> @@ -962,16 +971,18 @@ bool is_spectre_bhb_affected(const struct arm64_cpu_capabilities *entry,
>         if (supports_csv2p3(scope))
>                 return false;
>
> -       if (supports_clearbhb(scope))
> -               return true;
> -
> -       if (spectre_bhb_loop_affected(scope))
> -               return true;
> +       if (is_spectre_bhb_safe(scope))
> +               return false;
>
> -       if (is_spectre_bhb_fw_affected(scope))
> -               return true;
> +       /*
> +        * At this point the core isn't known to be "safe" so we're going to
> +        * assume it's vulnerable. We still need to update `max_bhb_k` though,
> +        * but only if we aren't mitigating with clearbhb though.
> +        */
> +       if (scope == SCOPE_LOCAL_CPU && !supports_clearbhb(SCOPE_LOCAL_CPU))
> +               max_bhb_k = max(max_bhb_k, spectre_bhb_loop_affected());
>
> -       return false;
> +       return true;
>  }
>
>  static void this_cpu_set_vectors(enum arm64_bp_harden_el1_vectors slot)
> @@ -1028,7 +1039,7 @@ void spectre_bhb_enable_mitigation(const struct arm64_cpu_capabilities *entry)
>                 this_cpu_set_vectors(EL1_VECTOR_BHB_CLEAR_INSN);
>                 state = SPECTRE_MITIGATED;
>                 set_bit(BHB_INSN, &system_bhb_mitigations);
> -       } else if (spectre_bhb_loop_affected(SCOPE_LOCAL_CPU)) {
> +       } else if (spectre_bhb_loop_affected()) {
>                 /*
>                  * Ensure KVM uses the indirect vector which will have the
>                  * branchy-loop added. A57/A72-r0 will already have selected
> @@ -1041,7 +1052,7 @@ void spectre_bhb_enable_mitigation(const struct arm64_cpu_capabilities *entry)
>                 this_cpu_set_vectors(EL1_VECTOR_BHB_LOOP);
>                 state = SPECTRE_MITIGATED;
>                 set_bit(BHB_LOOP, &system_bhb_mitigations);
> -       } else if (is_spectre_bhb_fw_affected(SCOPE_LOCAL_CPU)) {
> +       } else if (is_spectre_bhb_fw_affected()) {
>                 fw_state = spectre_bhb_get_cpu_fw_mitigation_state();
>                 if (fw_state == SPECTRE_MITIGATED) {
>                         /*
> @@ -1100,7 +1111,6 @@ void noinstr spectre_bhb_patch_loop_iter(struct alt_instr *alt,
>  {
>         u8 rd;
>         u32 insn;
> -       u16 loop_count = spectre_bhb_loop_affected(SCOPE_SYSTEM);
>
>         BUG_ON(nr_inst != 1); /* MOV -> MOV */
>
> @@ -1109,7 +1119,7 @@ void noinstr spectre_bhb_patch_loop_iter(struct alt_instr *alt,
>
>         insn = le32_to_cpu(*origptr);
>         rd = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RD, insn);
> -       insn = aarch64_insn_gen_movewide(rd, loop_count, 0,
> +       insn = aarch64_insn_gen_movewide(rd, max_bhb_k, 0,
>                                          AARCH64_INSN_VARIANT_64BIT,
>                                          AARCH64_INSN_MOVEWIDE_ZERO);
>         *updptr++ = cpu_to_le32(insn);
> --
> 2.47.1.613.gc27f4b7a9f-goog
>


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

* Re: [PATCH v3 1/3] arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre BHB
  2024-12-19 20:53 ` [PATCH v3 1/3] arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre BHB Douglas Anderson
  2024-12-20 16:33   ` Julius Werner
@ 2025-01-06 22:32   ` Doug Anderson
  2025-01-06 22:48   ` Florian Fainelli
  2 siblings, 0 replies; 7+ messages in thread
From: Doug Anderson @ 2025-01-06 22:32 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland
  Cc: Roxana Bradescu, Julius Werner, bjorn.andersson, linux-arm-kernel,
	linux-arm-msm, Jeffrey Hugo, Trilok Soni, stable, James Morse,
	Kent Overstreet, Suren Baghdasaryan, linux-kernel

Hi,

On Thu, Dec 19, 2024 at 12:54 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> @@ -916,9 +932,8 @@ static enum mitigation_state spectre_bhb_get_cpu_fw_mitigation_state(void)
>         }
>  }
>
> -static bool is_spectre_bhb_fw_affected(int scope)
> +static bool is_spectre_bhb_fw_affected(void)
>  {
> -       static bool system_affected;
>         enum mitigation_state fw_state;
>         bool has_smccc = arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_NONE;
>         static const struct midr_range spectre_bhb_firmware_mitigated_list[] = {
> @@ -929,16 +944,8 @@ static bool is_spectre_bhb_fw_affected(int scope)
>         bool cpu_in_list = is_midr_in_range_list(read_cpuid_id(),
>                                          spectre_bhb_firmware_mitigated_list);
>
> -       if (scope != SCOPE_LOCAL_CPU)
> -               return system_affected;
> -
>         fw_state = spectre_bhb_get_cpu_fw_mitigation_state();
> -       if (cpu_in_list || (has_smccc && fw_state == SPECTRE_MITIGATED)) {
> -               system_affected = true;
> -               return true;
> -       }
> -
> -       return false;
> +       return cpu_in_list || (has_smccc && fw_state == SPECTRE_MITIGATED);

Upon looking at this again, I realized that I can fully get rid of
`cpu_in_list` here and the whole `spectre_bhb_firmware_mitigated_list`
variable. After my patch there's only one caller to this function and
it only cares whether the firmware call can be used to mitigate, so I
can rename this function to has_spectre_bhb_fw_mitigation() and
simplify it and the caller.

I'll plan to spin this in the next day or two and also include the
proper loop value for Kryo-400 cores, since I've got that now.

I'll plan to keep Julius's review tag unless told not to.

-Doug


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

* Re: [PATCH v3 1/3] arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre BHB
  2024-12-19 20:53 ` [PATCH v3 1/3] arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre BHB Douglas Anderson
  2024-12-20 16:33   ` Julius Werner
  2025-01-06 22:32   ` Doug Anderson
@ 2025-01-06 22:48   ` Florian Fainelli
  2 siblings, 0 replies; 7+ messages in thread
From: Florian Fainelli @ 2025-01-06 22:48 UTC (permalink / raw)
  To: Douglas Anderson, Catalin Marinas, Will Deacon, Mark Rutland
  Cc: Roxana Bradescu, Julius Werner, bjorn.andersson, linux-arm-kernel,
	linux-arm-msm, Jeffrey Hugo, Trilok Soni, stable, James Morse,
	Kent Overstreet, Suren Baghdasaryan, linux-kernel

On 12/19/24 12:53, Douglas Anderson wrote:
> The code for detecting CPUs that are vulnerable to Spectre BHB was
> based on a hardcoded list of CPU IDs that were known to be affected.
> Unfortunately, the list mostly only contained the IDs of standard ARM
> cores. The IDs for many cores that are minor variants of the standard
> ARM cores (like many Qualcomm Kyro CPUs) weren't listed. This led the
> code to assume that those variants were not affected.
> 
> Flip the code on its head and instead assume that a core is vulnerable
> if it doesn't have CSV2_3 but is unrecognized as being safe. This
> involves creating a "Spectre BHB safe" list.
> 
> As of right now, the only CPU IDs added to the "Spectre BHB safe" list
> are ARM Cortex A35, A53, A55, A510, and A520. This list was created by
> looking for cores that weren't listed in ARM's list [1] as per review
> feedback on v2 of this patch [2].
> 
> NOTE: this patch will not actually _mitigate_ anyone, it will simply
> cause them to report themselves as vulnerable. If any cores in the
> system are reported as vulnerable but not mitigated then the whole
> system will be reported as vulnerable though the system will attempt
> to mitigate with the information it has about the known cores.
> 
> [1] https://developer.arm.com/Arm%20Security%20Center/Spectre-BHB
> [2] https://lore.kernel.org/r/20241219175128.GA25477@willie-the-truck
> 
> 
> Fixes: 558c303c9734 ("arm64: Mitigate spectre style branch history side channels")
> Cc: stable@vger.kernel.org
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
> Changes in v3:
> - Don't guess the mitigation; just report unknown cores as vulnerable.
> - Restructure the code since is_spectre_bhb_affected() defaults to true
> 
> Changes in v2:
> - New
> 
>   arch/arm64/include/asm/spectre.h |   1 -
>   arch/arm64/kernel/proton-pack.c  | 144 +++++++++++++++++--------------
>   2 files changed, 77 insertions(+), 68 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/spectre.h b/arch/arm64/include/asm/spectre.h
> index 0c4d9045c31f..f1524cdeacf1 100644
> --- a/arch/arm64/include/asm/spectre.h
> +++ b/arch/arm64/include/asm/spectre.h
> @@ -97,7 +97,6 @@ enum mitigation_state arm64_get_meltdown_state(void);
>   
>   enum mitigation_state arm64_get_spectre_bhb_state(void);
>   bool is_spectre_bhb_affected(const struct arm64_cpu_capabilities *entry, int scope);
> -u8 spectre_bhb_loop_affected(int scope);
>   void spectre_bhb_enable_mitigation(const struct arm64_cpu_capabilities *__unused);
>   bool try_emulate_el1_ssbs(struct pt_regs *regs, u32 instr);
>   
> diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c
> index da53722f95d4..06e04c9e6480 100644
> --- a/arch/arm64/kernel/proton-pack.c
> +++ b/arch/arm64/kernel/proton-pack.c
> @@ -845,52 +845,68 @@ static unsigned long system_bhb_mitigations;
>    * This must be called with SCOPE_LOCAL_CPU for each type of CPU, before any
>    * SCOPE_SYSTEM call will give the right answer.
>    */
> -u8 spectre_bhb_loop_affected(int scope)
> +static bool is_spectre_bhb_safe(int scope)
> +{
> +	static const struct midr_range spectre_bhb_safe_list[] = {
> +		MIDR_ALL_VERSIONS(MIDR_CORTEX_A35),
> +		MIDR_ALL_VERSIONS(MIDR_CORTEX_A53),

You can add MIDR_ALL_VERSIONS(MIDR_BRAHMA_B53) here as well. Thanks
-- 
Florian



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

end of thread, other threads:[~2025-01-06 22:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-19 20:53 [PATCH v3 0/3] arm64: errata: Rework Spectre BHB mitigations to not assume "safe" Douglas Anderson
2024-12-19 20:53 ` [PATCH v3 1/3] arm64: errata: Assume that unknown CPUs _are_ vulnerable to Spectre BHB Douglas Anderson
2024-12-20 16:33   ` Julius Werner
2025-01-06 22:32   ` Doug Anderson
2025-01-06 22:48   ` Florian Fainelli
2024-12-19 20:53 ` [PATCH v3 2/3] arm64: cputype: Add MIDR_CORTEX_A76AE Douglas Anderson
2024-12-19 20:53 ` [PATCH v3 3/3] arm64: errata: Add newer ARM cores to the spectre_bhb_loop_affected() lists Douglas Anderson

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