[PATCH v2] x86/topology: Unify srat_detect_node among amd and hygon

Nikola Z. Ivanov posted 1 patch 1 week, 2 days ago
arch/x86/kernel/cpu/amd.c    | 74 ------------------------------------
arch/x86/kernel/cpu/common.c | 73 +++++++++++++++++++++++++++++++++++
arch/x86/kernel/cpu/cpu.h    |  1 +
arch/x86/kernel/cpu/hygon.c  | 73 -----------------------------------
arch/x86/kernel/cpu/intel.c  |  4 +-
5 files changed, 76 insertions(+), 149 deletions(-)
[PATCH v2] x86/topology: Unify srat_detect_node among amd and hygon
Posted by Nikola Z. Ivanov 1 week, 2 days ago
Since the amd and hygon srat_detect_node implementation
is practically identical the code can be shared.

Move the implementation to common.c
and expose it in arch/x86/kernel/cpu/cpu.h for code
in arch/x86/kernel/cpu/ to use.

For intel the separate implementation is kept to
stay consistent with its behavior.

Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
---
Changes since v1:
  - Abandon the idea for using the same function for intel,
    also move the function to another file and limit its scope to x86
  https://lore.kernel.org/all/dcfd7282-f722-46ea-b35c-2ab1333ca0d8@gmail.com/

 arch/x86/kernel/cpu/amd.c    | 74 ------------------------------------
 arch/x86/kernel/cpu/common.c | 73 +++++++++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/cpu.h    |  1 +
 arch/x86/kernel/cpu/hygon.c  | 73 -----------------------------------
 arch/x86/kernel/cpu/intel.c  |  4 +-
 5 files changed, 76 insertions(+), 149 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 2f8e8ff2d000..f317c6bdcf5d 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -277,80 +277,6 @@ static void init_amd_k7(struct cpuinfo_x86 *c)
 #endif
 }
 
-#ifdef CONFIG_NUMA
-/*
- * To workaround broken NUMA config.  Read the comment in
- * srat_detect_node().
- */
-static int nearby_node(int apicid)
-{
-	int i, node;
-
-	for (i = apicid - 1; i >= 0; i--) {
-		node = __apicid_to_node[i];
-		if (node != NUMA_NO_NODE && node_online(node))
-			return node;
-	}
-	for (i = apicid + 1; i < MAX_LOCAL_APIC; i++) {
-		node = __apicid_to_node[i];
-		if (node != NUMA_NO_NODE && node_online(node))
-			return node;
-	}
-	return first_node(node_online_map); /* Shouldn't happen */
-}
-#endif
-
-static void srat_detect_node(struct cpuinfo_x86 *c)
-{
-#ifdef CONFIG_NUMA
-	int cpu = smp_processor_id();
-	int node;
-	unsigned apicid = c->topo.apicid;
-
-	node = numa_cpu_node(cpu);
-	if (node == NUMA_NO_NODE)
-		node = per_cpu_llc_id(cpu);
-
-	/*
-	 * On multi-fabric platform (e.g. Numascale NumaChip) a
-	 * platform-specific handler needs to be called to fixup some
-	 * IDs of the CPU.
-	 */
-	if (x86_cpuinit.fixup_cpu_id)
-		x86_cpuinit.fixup_cpu_id(c, node);
-
-	if (!node_online(node)) {
-		/*
-		 * Two possibilities here:
-		 *
-		 * - The CPU is missing memory and no node was created.  In
-		 *   that case try picking one from a nearby CPU.
-		 *
-		 * - The APIC IDs differ from the HyperTransport node IDs
-		 *   which the K8 northbridge parsing fills in.  Assume
-		 *   they are all increased by a constant offset, but in
-		 *   the same order as the HT nodeids.  If that doesn't
-		 *   result in a usable node fall back to the path for the
-		 *   previous case.
-		 *
-		 * This workaround operates directly on the mapping between
-		 * APIC ID and NUMA node, assuming certain relationship
-		 * between APIC ID, HT node ID and NUMA topology.  As going
-		 * through CPU mapping may alter the outcome, directly
-		 * access __apicid_to_node[].
-		 */
-		int ht_nodeid = c->topo.initial_apicid;
-
-		if (__apicid_to_node[ht_nodeid] != NUMA_NO_NODE)
-			node = __apicid_to_node[ht_nodeid];
-		/* Pick a nearby node */
-		if (!node_online(node))
-			node = nearby_node(apicid);
-	}
-	numa_set_node(cpu, node);
-#endif
-}
-
 static void bsp_determine_snp(struct cpuinfo_x86 *c)
 {
 #ifdef CONFIG_ARCH_HAS_CC_PLATFORM
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index a4268c47f2bc..a0d7c7c47153 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2518,6 +2518,79 @@ void cpu_init(void)
 	load_fixmap_gdt(cpu);
 }
 
+#ifdef CONFIG_NUMA
+/*
+ * To workaround broken NUMA config.  Read the comment in
+ * srat_detect_node().
+ */
+static int nearby_node(int apicid)
+{
+	int i, node;
+
+	for (i = apicid - 1; i >= 0; i--) {
+		node = __apicid_to_node[i];
+		if (node != NUMA_NO_NODE && node_online(node))
+			return node;
+	}
+	for (i = apicid + 1; i < MAX_LOCAL_APIC; i++) {
+		node = __apicid_to_node[i];
+		if (node != NUMA_NO_NODE && node_online(node))
+			return node;
+	}
+	return first_node(node_online_map); /* Shouldn't happen */
+}
+#endif
+
+void srat_detect_node(struct cpuinfo_x86 *c)
+{
+#ifdef CONFIG_NUMA
+	int cpu = smp_processor_id();
+	int node;
+	unsigned int apicid = c->topo.apicid;
+
+	node = numa_cpu_node(cpu);
+	if (node == NUMA_NO_NODE)
+		node = c->topo.llc_id;
+
+	/*
+	 * On multi-fabric platform (e.g. Numascale NumaChip) a
+	 * platform-specific handler needs to be called to fixup some
+	 * IDs of the CPU.
+	 */
+	if (x86_cpuinit.fixup_cpu_id)
+		x86_cpuinit.fixup_cpu_id(c, node);
+
+	if (!node_online(node)) {
+		/*
+		 * Two possibilities here:
+		 *
+		 * - The CPU is missing memory and no node was created.  In
+		 *   that case try picking one from a nearby CPU.
+		 *
+		 * - The APIC IDs differ from the HyperTransport node IDs.
+		 *   Assume they are all increased by a constant offset, but
+		 *   in the same order as the HT nodeids.  If that doesn't
+		 *   result in a usable node fall back to the path for the
+		 *   previous case.
+		 *
+		 * This workaround operates directly on the mapping between
+		 * APIC ID and NUMA node, assuming certain relationship
+		 * between APIC ID, HT node ID and NUMA topology.  As going
+		 * through CPU mapping may alter the outcome, directly
+		 * access __apicid_to_node[].
+		 */
+		int ht_nodeid = c->topo.initial_apicid;
+
+		if (__apicid_to_node[ht_nodeid] != NUMA_NO_NODE)
+			node = __apicid_to_node[ht_nodeid];
+		/* Pick a nearby node */
+		if (!node_online(node))
+			node = nearby_node(apicid);
+	}
+	numa_set_node(cpu, node);
+#endif
+}
+
 #ifdef CONFIG_MICROCODE_LATE_LOADING
 /**
  * store_cpu_caps() - Store a snapshot of CPU capabilities
diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h
index 5c7a3a71191a..04e73ff9a153 100644
--- a/arch/x86/kernel/cpu/cpu.h
+++ b/arch/x86/kernel/cpu/cpu.h
@@ -65,6 +65,7 @@ extern void check_null_seg_clears_base(struct cpuinfo_x86 *c);
 
 void cacheinfo_amd_init_llc_id(struct cpuinfo_x86 *c, u16 die_id);
 void cacheinfo_hygon_init_llc_id(struct cpuinfo_x86 *c);
+void srat_detect_node(struct cpuinfo_x86 *c);
 
 #if defined(CONFIG_AMD_NB) && defined(CONFIG_SYSFS)
 struct amd_northbridge *amd_init_l3_cache(int index);
diff --git a/arch/x86/kernel/cpu/hygon.c b/arch/x86/kernel/cpu/hygon.c
index 7f95a74e4c65..a33735094843 100644
--- a/arch/x86/kernel/cpu/hygon.c
+++ b/arch/x86/kernel/cpu/hygon.c
@@ -20,79 +20,6 @@
 
 #include "cpu.h"
 
-#ifdef CONFIG_NUMA
-/*
- * To workaround broken NUMA config.  Read the comment in
- * srat_detect_node().
- */
-static int nearby_node(int apicid)
-{
-	int i, node;
-
-	for (i = apicid - 1; i >= 0; i--) {
-		node = __apicid_to_node[i];
-		if (node != NUMA_NO_NODE && node_online(node))
-			return node;
-	}
-	for (i = apicid + 1; i < MAX_LOCAL_APIC; i++) {
-		node = __apicid_to_node[i];
-		if (node != NUMA_NO_NODE && node_online(node))
-			return node;
-	}
-	return first_node(node_online_map); /* Shouldn't happen */
-}
-#endif
-
-static void srat_detect_node(struct cpuinfo_x86 *c)
-{
-#ifdef CONFIG_NUMA
-	int cpu = smp_processor_id();
-	int node;
-	unsigned int apicid = c->topo.apicid;
-
-	node = numa_cpu_node(cpu);
-	if (node == NUMA_NO_NODE)
-		node = c->topo.llc_id;
-
-	/*
-	 * On multi-fabric platform (e.g. Numascale NumaChip) a
-	 * platform-specific handler needs to be called to fixup some
-	 * IDs of the CPU.
-	 */
-	if (x86_cpuinit.fixup_cpu_id)
-		x86_cpuinit.fixup_cpu_id(c, node);
-
-	if (!node_online(node)) {
-		/*
-		 * Two possibilities here:
-		 *
-		 * - The CPU is missing memory and no node was created.  In
-		 *   that case try picking one from a nearby CPU.
-		 *
-		 * - The APIC IDs differ from the HyperTransport node IDs.
-		 *   Assume they are all increased by a constant offset, but
-		 *   in the same order as the HT nodeids.  If that doesn't
-		 *   result in a usable node fall back to the path for the
-		 *   previous case.
-		 *
-		 * This workaround operates directly on the mapping between
-		 * APIC ID and NUMA node, assuming certain relationship
-		 * between APIC ID, HT node ID and NUMA topology.  As going
-		 * through CPU mapping may alter the outcome, directly
-		 * access __apicid_to_node[].
-		 */
-		int ht_nodeid = c->topo.initial_apicid;
-
-		if (__apicid_to_node[ht_nodeid] != NUMA_NO_NODE)
-			node = __apicid_to_node[ht_nodeid];
-		/* Pick a nearby node */
-		if (!node_online(node))
-			node = nearby_node(apicid);
-	}
-	numa_set_node(cpu, node);
-#endif
-}
-
 static void bsp_init_hygon(struct cpuinfo_x86 *c)
 {
 	if (cpu_has(c, X86_FEATURE_CONSTANT_TSC)) {
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index f28c0efb7c8f..0c34ec32baec 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -468,7 +468,7 @@ static void intel_workarounds(struct cpuinfo_x86 *c)
 }
 #endif
 
-static void srat_detect_node(struct cpuinfo_x86 *c)
+static void srat_detect_node_intel(struct cpuinfo_x86 *c)
 {
 #ifdef CONFIG_NUMA
 	unsigned node;
@@ -614,7 +614,7 @@ static void init_intel(struct cpuinfo_x86 *c)
 		set_cpu_cap(c, X86_FEATURE_PREFER_YMM);
 
 	/* Work around errata */
-	srat_detect_node(c);
+	srat_detect_node_intel(c);
 
 	init_ia32_feat_ctl(c);
 
-- 
2.53.0
Re: [PATCH v2] x86/topology: Unify srat_detect_node among amd and hygon
Posted by Borislav Petkov 1 week, 1 day ago
On Fri, May 15, 2026 at 07:31:05PM +0300, Nikola Z. Ivanov wrote:
> Since the amd and hygon srat_detect_node implementation
> is practically identical the code can be shared.

There's a downside to that: when one of the vendors wants to change things and
then the ifdeffery ugliness starts.

So let's keep 'em separate for the sanity of everyone involved.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2] x86/topology: Unify srat_detect_node among amd and hygon
Posted by Ahmed S. Darwish 1 week, 2 days ago
On Fri, 15 May 2026, Nikola Z. Ivanov wrote:
>
> --- a/arch/x86/kernel/cpu/cpu.h
> +++ b/arch/x86/kernel/cpu/cpu.h
> @@ -65,6 +65,7 @@ extern void check_null_seg_clears_base(struct cpuinfo_x86 *c);
>
>  void cacheinfo_amd_init_llc_id(struct cpuinfo_x86 *c, u16 die_id);
>  void cacheinfo_hygon_init_llc_id(struct cpuinfo_x86 *c);
> +void srat_detect_node(struct cpuinfo_x86 *c);
>

This is very confusing from the perspective of common.c.

The moved srat_detect_node() function there has zero indication that it is
AMD/Hygon specific.

When I was refactoring the cacheinfo code some time ago, I marked the
vendor-specific cacheinfo.c functions with amd and/or hygon tags to
identify which is which.  You see that clearly in the two lines above your
new function declaration.

IMHO, you need to do something similar.  Please continue below.

>
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -468,7 +468,7 @@ static void intel_workarounds(struct cpuinfo_x86 *c)
>  }
>  #endif
>
> -static void srat_detect_node(struct cpuinfo_x86 *c)
> +static void srat_detect_node_intel(struct cpuinfo_x86 *c)
>  {
>  #ifdef CONFIG_NUMA
>  	unsigned node;
> @@ -614,7 +614,7 @@ static void init_intel(struct cpuinfo_x86 *c)
>  		set_cpu_cap(c, X86_FEATURE_PREFER_YMM);
>
>  	/* Work around errata */
> -	srat_detect_node(c);
> +	srat_detect_node_intel(c);
>

I think it would be nice to have:

    amd_hygon_srat_detect_node()
    intel_srat_detect_node()

This would also go better in line with the existing intel.c functions like
intel_unlock_cpuid_leafs(), intel_tlb_lookup(), intel_detect_tlb(), etc.

Thanks,
Ahmed