Skip to contentSkip to conversation

x86/amd: Two fixes for downcored machines

PATCH New Borislav Petkov 2 participants 5 messages Jul 31, 2017 – Aug 10, 2017 x86

lore
Share & tools
lore

Borislav Petkov

author maintainer
#0 lore raw

cover x86/amd: Two fixes for downcored machines

From: Borislav Petkov <[email protected]>

Hi guys,

here are two fixes for downcored F17h machines. The respective commit messages explain the situation adequately, IMO. Both tip:x86/cpu stuff.

Please queue,
thanks.

Suravee Suthikulpanit (2):
x86/amd: Limit cpu_core_id fixup to families older than F17h x86/amd: Derive L3 shared_cpu_map from cpu_llc_shared_mask

 arch/x86/kernel/cpu/amd.c             | 24 +++++++++++++++++-------
 arch/x86/kernel/cpu/intel_cacheinfo.c | 32 ++++++++++++++++++--------------
 2 files changed, 35 insertions(+), 21 deletions(-)
Signature · 1 line
2.14.0.rc0
author maintainer
#1 lore raw

PATCH 1/2 x86/amd: Limit cpu_core_id fixup to families older than F17h

From: Suravee Suthikulpanit <[email protected]>

Current cpu_core_id fixup causes downcored F17h configurations to be incorrect:

NODE: 0

  processor  0 core id : 0
  processor  1 core id : 1
  processor  2 core id : 2
  processor  3 core id : 4
  processor  4 core id : 5
  processor  5 core id : 0

NODE: 1

  processor  6 core id : 2
  processor  7 core id : 3
  processor  8 core id : 4
  processor  9 core id : 0

processor 10 core id : 1
processor 11 core id : 2

Code that relies on the cpu_core_id, like match_smt(), for example, which builds the thread siblings masks used by the scheduler, is mislead.

So, limit the fixup to pre-F17h machines. The new value for cpu_core_id for F17h and later will represent the CPUID_Fn8000001E_EBX[CoreId], which is guaranteed to be unique for each core within a socket.

This way we have:

NODE: 0

  processor  0 core id : 0
  processor  1 core id : 1
  processor  2 core id : 2
  processor  3 core id : 4
  processor  4 core id : 5
  processor  5 core id : 6

NODE: 1

  processor  6 core id : 8
  processor  7 core id : 9
  processor  8 core id : 10
  processor  9 core id : 12

processor 10 core id : 13
processor 11 core id : 14

Signed-off-by Suravee Suthikulpanit <[email protected]>
Cc Yazen Ghannam <[email protected]>
Cc x86-ml <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
[ Heavily massaged. ]
Signed-off-by Borislav Petkov <[email protected]>
---
 arch/x86/kernel/cpu/amd.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)
arch/x86/kernel/cpu/amd.c +17 -7
index 110ca5d2bb87..aa3cf163fd19 100644
@@ -297,6 +297,22 @@ static int nearby_node(int apicid)
 }
 #endif
 
+/*
+ * Fix up cpu_core_id for pre-F17h systems to be in the
+ * [0 .. cores_per_node - 1] range. Not really needed but
+ * kept so as not to break existing setups.
+ */
+static void legacy_fixup_core_id(struct cpuinfo_x86 *c)
+{
+	u32 cus_per_node;
+
+	if (c->x86 >= 0x17)
+		return;
+
+	cus_per_node = c->x86_max_cores / nodes_per_socket;
+	c->cpu_core_id %= cus_per_node;
+}
+
 /*
  * Fixup core topology information for
  * (1) AMD multi-node processors

@@ -354,15 +370,9 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
 	} else
 		return;
 
-	/* fixup multi-node processor information */
 	if (nodes_per_socket > 1) {
-		u32 cus_per_node;
-
 		set_cpu_cap(c, X86_FEATURE_AMD_DCM);
-		cus_per_node = c->x86_max_cores / nodes_per_socket;
-
-		/* core id has to be in the [0 .. cores_per_node - 1] range */
-		c->cpu_core_id %= cus_per_node;
+		legacy_fixup_core_id(c);
 	}
 }
 #endif
Signature · 1 line
2.14.0.rc0
merge bot
#2 lore raw

[tip:x86/cpu] x86/cpu/amd: Limit cpu_core_id fixup to families older than F17h

x86/cpu/amd: Limit cpu_core_id fixup to families older than F17h
Commit-ID:  b89b41d0b8414690ec0030c134b8bde209e6d06c
Gitweb:     http://git.kernel.org/tip/b89b41d0b8414690ec0030c134b8bde209e6d06c
Author:     Suravee Suthikulpanit <[email protected]>
AuthorDate: Mon, 31 Jul 2017 10:51:58 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 10 Aug 2017 17:37:43 +0200

x86/cpu/amd: Limit cpu_core_id fixup to families older than F17h

Current cpu_core_id fixup causes downcored F17h configurations to be incorrect:

NODE: 0

  processor  0 core id : 0
  processor  1 core id : 1
  processor  2 core id : 2
  processor  3 core id : 4
  processor  4 core id : 5
  processor  5 core id : 0

NODE: 1

  processor  6 core id : 2
  processor  7 core id : 3
  processor  8 core id : 4
  processor  9 core id : 0

processor 10 core id : 1
processor 11 core id : 2

Code that relies on the cpu_core_id, like match_smt(), for example, which builds the thread siblings masks used by the scheduler, is mislead.

So, limit the fixup to pre-F17h machines. The new value for cpu_core_id for F17h and later will represent the CPUID_Fn8000001E_EBX[CoreId], which is guaranteed to be unique for each core within a socket.

This way we have:

NODE: 0

  processor  0 core id : 0
  processor  1 core id : 1
  processor  2 core id : 2
  processor  3 core id : 4
  processor  4 core id : 5
  processor  5 core id : 6

NODE: 1

  processor  6 core id : 8
  processor  7 core id : 9
  processor  8 core id : 10
  processor  9 core id : 12

processor 10 core id : 13
processor 11 core id : 14

Signed-off-by Suravee Suthikulpanit <[email protected]>
[ Heavily massaged. ]
Signed-off-by Borislav Petkov <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Yazen Ghannam <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Ingo Molnar <[email protected]>
---
 arch/x86/kernel/cpu/amd.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)
arch/x86/kernel/cpu/amd.c +17 -7
index 3b9e220..82d571c 100644
@@ -298,6 +298,22 @@ static int nearby_node(int apicid)
 #endif
 
 /*
+ * Fix up cpu_core_id for pre-F17h systems to be in the
+ * [0 .. cores_per_node - 1] range. Not really needed but
+ * kept so as not to break existing setups.
+ */
+static void legacy_fixup_core_id(struct cpuinfo_x86 *c)
+{
+	u32 cus_per_node;
+
+	if (c->x86 >= 0x17)
+		return;
+
+	cus_per_node = c->x86_max_cores / nodes_per_socket;
+	c->cpu_core_id %= cus_per_node;
+}
+
+/*
  * Fixup core topology information for
  * (1) AMD multi-node processors
  *     Assumption: Number of cores in each internal node is the same.

@@ -354,15 +370,9 @@ static void amd_get_topology(struct cpuinfo_x86 *c)
 	} else
 		return;
 
-	/* fixup multi-node processor information */
 	if (nodes_per_socket > 1) {
-		u32 cus_per_node;
-
 		set_cpu_cap(c, X86_FEATURE_AMD_DCM);
-		cus_per_node = c->x86_max_cores / nodes_per_socket;
-
-		/* core id has to be in the [0 .. cores_per_node - 1] range */
-		c->cpu_core_id %= cus_per_node;
+		legacy_fixup_core_id(c);
 	}
 }
 #endif
author maintainer
#3 lore raw

PATCH 2/2 x86/amd: Derive L3 shared_cpu_map from cpu_llc_shared_mask

In reply to Borislav Petkov

From: Suravee Suthikulpanit <[email protected]>

For systems with X86_FEATURE_TOPOEXT, current logic uses the APIC ID to calculate shared_cpu_map. However, APIC IDs are not guaranteed to be contiguous for cores across different L3s (e.g. family17h system w/ downcore configuration). This breaks the logic, and results in an incorrect L3 shared_cpu_map.

Instead, always use the previously calculated cpu_llc_shared_mask of each CPU to derive the L3 shared_cpu_map.

Signed-off-by Suravee Suthikulpanit <[email protected]>
Cc x86-ml <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Borislav Petkov <[email protected]>
---
 arch/x86/kernel/cpu/intel_cacheinfo.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)
arch/x86/kernel/cpu/intel_cacheinfo.c +18 -14
index c55fb2cb2acc..24f749324c0f 100644
@@ -811,7 +811,24 @@ static int __cache_amd_cpumap_setup(unsigned int cpu, int index,
 	struct cacheinfo *this_leaf;
 	int i, sibling;
 
-	if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
+	/*
+	 * For L3, always use the pre-calculated cpu_llc_shared_mask
+	 * to derive shared_cpu_map.
+	 */
+	if (index == 3) {
+		for_each_cpu(i, cpu_llc_shared_mask(cpu)) {
+			this_cpu_ci = get_cpu_cacheinfo(i);
+			if (!this_cpu_ci->info_list)
+				continue;
+			this_leaf = this_cpu_ci->info_list + index;
+			for_each_cpu(sibling, cpu_llc_shared_mask(cpu)) {
+				if (!cpu_online(sibling))
+					continue;
+				cpumask_set_cpu(sibling,
+						&this_leaf->shared_cpu_map);
+			}
+		}
+	} else if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
 		unsigned int apicid, nshared, first, last;
 
 		this_leaf = this_cpu_ci->info_list + index;

@@ -839,19 +856,6 @@ static int __cache_amd_cpumap_setup(unsigned int cpu, int index,
 						&this_leaf->shared_cpu_map);
 			}
 		}
-	} else if (index == 3) {
-		for_each_cpu(i, cpu_llc_shared_mask(cpu)) {
-			this_cpu_ci = get_cpu_cacheinfo(i);
-			if (!this_cpu_ci->info_list)
-				continue;
-			this_leaf = this_cpu_ci->info_list + index;
-			for_each_cpu(sibling, cpu_llc_shared_mask(cpu)) {
-				if (!cpu_online(sibling))
-					continue;
-				cpumask_set_cpu(sibling,
-						&this_leaf->shared_cpu_map);
-			}
-		}
 	} else
 		return 0;
Signature · 1 line
2.14.0.rc0
merge bot
#4 lore raw

[tip:x86/cpu] x86/cpu/amd: Derive L3 shared_cpu_map from cpu_llc_shared_mask

x86/cpu/amd: Derive L3 shared_cpu_map from cpu_llc_shared_mask
Commit-ID:  2b83809a5e6d619a780876fcaf68cdc42b50d28c
Gitweb:     http://git.kernel.org/tip/2b83809a5e6d619a780876fcaf68cdc42b50d28c
Author:     Suravee Suthikulpanit <[email protected]>
AuthorDate: Mon, 31 Jul 2017 10:51:59 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 10 Aug 2017 17:37:43 +0200

x86/cpu/amd: Derive L3 shared_cpu_map from cpu_llc_shared_mask

For systems with X86_FEATURE_TOPOEXT, current logic uses the APIC ID to calculate shared_cpu_map. However, APIC IDs are not guaranteed to be contiguous for cores across different L3s (e.g. family17h system w/ downcore configuration). This breaks the logic, and results in an incorrect L3 shared_cpu_map.

Instead, always use the previously calculated cpu_llc_shared_mask of each CPU to derive the L3 shared_cpu_map.

Signed-off-by Suravee Suthikulpanit <[email protected]>
Signed-off-by Borislav Petkov <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Ingo Molnar <[email protected]>
---
 arch/x86/kernel/cpu/intel_cacheinfo.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)
arch/x86/kernel/cpu/intel_cacheinfo.c +18 -14
index c55fb2c..24f74932 100644
@@ -811,7 +811,24 @@ static int __cache_amd_cpumap_setup(unsigned int cpu, int index,
 	struct cacheinfo *this_leaf;
 	int i, sibling;
 
-	if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
+	/*
+	 * For L3, always use the pre-calculated cpu_llc_shared_mask
+	 * to derive shared_cpu_map.
+	 */
+	if (index == 3) {
+		for_each_cpu(i, cpu_llc_shared_mask(cpu)) {
+			this_cpu_ci = get_cpu_cacheinfo(i);
+			if (!this_cpu_ci->info_list)
+				continue;
+			this_leaf = this_cpu_ci->info_list + index;
+			for_each_cpu(sibling, cpu_llc_shared_mask(cpu)) {
+				if (!cpu_online(sibling))
+					continue;
+				cpumask_set_cpu(sibling,
+						&this_leaf->shared_cpu_map);
+			}
+		}
+	} else if (boot_cpu_has(X86_FEATURE_TOPOEXT)) {
 		unsigned int apicid, nshared, first, last;
 
 		this_leaf = this_cpu_ci->info_list + index;

@@ -839,19 +856,6 @@ static int __cache_amd_cpumap_setup(unsigned int cpu, int index,
 						&this_leaf->shared_cpu_map);
 			}
 		}
-	} else if (index == 3) {
-		for_each_cpu(i, cpu_llc_shared_mask(cpu)) {
-			this_cpu_ci = get_cpu_cacheinfo(i);
-			if (!this_cpu_ci->info_list)
-				continue;
-			this_leaf = this_cpu_ci->info_list + index;
-			for_each_cpu(sibling, cpu_llc_shared_mask(cpu)) {
-				if (!cpu_online(sibling))
-					continue;
-				cpumask_set_cpu(sibling,
-						&this_leaf->shared_cpu_map);
-			}
-		}
 	} else
 		return 0;