2023-08-02 11:24:32

by Thomas Gleixner

[permalink] [raw]
Subject: [patch V3 30/40] x86/cpu: Provide an AMD/HYGON specific topology parser

AMD/HYGON uses various methods for topology evaluation:

- Leaf 0x80000008 and 0x8000001e based with an optional leaf 0xb,
which is the preferred variant for modern CPUs.

Leaf 0xb will be superseded by leaf 0x80000026 soon, which is just
another variant of the Intel 0x1f leaf for whatever reasons.

- Subleaf 0x80000008 and NODEID_MSR base

- Legacy fallback

That code is following the principle of random bits and pieces all over the
place which results in multiple evaluations and impenetrable code flows in
the same way as the Intel parsing did.

Provide a sane implementation by clearly separating the three variants and
bringing them in the proper preference order in one place.

This provides the parsing for both AMD and HYGON because there is no point
in having a separate HYGON parser which only differs by 3 lines of
code. Any further divergence between AMD and HYGON can be handled in
different functions, while still sharing the existing parsers.

Signed-off-by: Thomas Gleixner <[email protected]>
---
V3: Fix the off by one with leaf 0x8000001e::ebx::threads_per_cu - Michael
---
arch/x86/include/asm/topology.h | 2
arch/x86/kernel/cpu/Makefile | 2
arch/x86/kernel/cpu/amd.c | 2
arch/x86/kernel/cpu/cacheinfo.c | 4
arch/x86/kernel/cpu/cpu.h | 2
arch/x86/kernel/cpu/debugfs.c | 2
arch/x86/kernel/cpu/topology.h | 6 +
arch/x86/kernel/cpu/topology_amd.c | 179 ++++++++++++++++++++++++++++++++++
arch/x86/kernel/cpu/topology_common.c | 19 +++
9 files changed, 211 insertions(+), 7 deletions(-)

--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -162,6 +162,8 @@ int topology_update_die_map(unsigned int
int topology_phys_to_logical_pkg(unsigned int pkg);
bool topology_smt_supported(void);

+extern unsigned int __amd_nodes_per_pkg;
+
static inline unsigned int topology_amd_nodes_per_pkg(void)
{
return __max_die_per_package;
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -18,7 +18,7 @@ KMSAN_SANITIZE_common.o := n
KCSAN_SANITIZE_common.o := n

obj-y := cacheinfo.o scattered.o
-obj-y += topology_common.o topology_ext.o topology.o
+obj-y += topology_common.o topology_ext.o topology_amd.o topology.o
obj-y += common.o
obj-y += rdrand.o
obj-y += match.o
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -423,7 +423,7 @@ static void amd_get_topology(struct cpui
if (!err)
c->x86_coreid_bits = get_count_order(c->x86_max_cores);

- cacheinfo_amd_init_llc_id(c);
+ cacheinfo_amd_init_llc_id(c, c->topo.die_id);

} else if (cpu_has(c, X86_FEATURE_NODEID_MSR)) {
u64 value;
--- a/arch/x86/kernel/cpu/cacheinfo.c
+++ b/arch/x86/kernel/cpu/cacheinfo.c
@@ -661,7 +661,7 @@ static int find_num_cache_leaves(struct
return i;
}

-void cacheinfo_amd_init_llc_id(struct cpuinfo_x86 *c)
+void cacheinfo_amd_init_llc_id(struct cpuinfo_x86 *c, u16 die_id)
{
/*
* We may have multiple LLCs if L3 caches exist, so check if we
@@ -672,7 +672,7 @@ void cacheinfo_amd_init_llc_id(struct cp

if (c->x86 < 0x17) {
/* LLC is at the node level. */
- c->topo.llc_id = c->topo.die_id;
+ c->topo.llc_id = die_id;
} else if (c->x86 == 0x17 && c->x86_model <= 0x1F) {
/*
* LLC is at the core complex level.
--- a/arch/x86/kernel/cpu/cpu.h
+++ b/arch/x86/kernel/cpu/cpu.h
@@ -79,7 +79,7 @@ extern void init_hygon_cacheinfo(struct
extern int detect_extended_topology(struct cpuinfo_x86 *c);
extern void check_null_seg_clears_base(struct cpuinfo_x86 *c);

-void cacheinfo_amd_init_llc_id(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);

unsigned int aperfmperf_get_khz(int cpu);
--- a/arch/x86/kernel/cpu/debugfs.c
+++ b/arch/x86/kernel/cpu/debugfs.c
@@ -26,6 +26,8 @@ static int cpu_debug_show(struct seq_fil
seq_printf(m, "logical_die_id: %u\n", c->topo.logical_die_id);
seq_printf(m, "llc_id: %u\n", c->topo.llc_id);
seq_printf(m, "l2c_id: %u\n", c->topo.l2c_id);
+ seq_printf(m, "amd_node_id: %u\n", c->topo.amd_node_id);
+ seq_printf(m, "amd_nodes_per_pkg: %u\n", topology_amd_nodes_per_pkg());
seq_printf(m, "max_cores: %u\n", c->x86_max_cores);
seq_printf(m, "max_die_per_pkg: %u\n", __max_die_per_package);
seq_printf(m, "smp_num_siblings: %u\n", smp_num_siblings);
--- a/arch/x86/kernel/cpu/topology.h
+++ b/arch/x86/kernel/cpu/topology.h
@@ -9,6 +9,10 @@ struct topo_scan {

// Legacy CPUID[1]:EBX[23:16] number of logical processors
unsigned int ebx1_nproc_shift;
+
+ // AMD specific node ID which cannot be mapped into APIC space.
+ u16 amd_nodes_per_pkg;
+ u16 amd_node_id;
};

bool topo_is_converted(struct cpuinfo_x86 *c);
@@ -17,6 +21,8 @@ void cpu_parse_topology(struct cpuinfo_x
void topology_set_dom(struct topo_scan *tscan, enum x86_topology_domains dom,
unsigned int shift, unsigned int ncpus);
bool cpu_parse_topology_ext(struct topo_scan *tscan);
+void cpu_parse_topology_amd(struct topo_scan *tscan);
+void cpu_topology_fixup_amd(struct topo_scan *tscan);

static inline u32 topo_shift_apicid(u32 apicid, enum x86_topology_domains dom)
{
--- /dev/null
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -0,0 +1,179 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/cpu.h>
+
+#include <asm/apic.h>
+#include <asm/memtype.h>
+#include <asm/processor.h>
+
+#include "cpu.h"
+
+static bool parse_8000_0008(struct topo_scan *tscan)
+{
+ struct {
+ u32 ncores : 8,
+ __rsvd0 : 4,
+ apicidsize : 4,
+ perftscsize : 2,
+ __rsvd1 : 14;
+ } ecx;
+ unsigned int sft;
+
+ if (tscan->c->extended_cpuid_level < 0x80000008)
+ return false;
+
+ cpuid_leaf_reg(0x80000008, CPUID_ECX, &ecx);
+
+ /* If the APIC ID size is 0, then get the shift value from ecx.ncores */
+ sft = ecx.apicidsize;
+ if (!sft)
+ sft = get_count_order(ecx.ncores + 1);
+
+ topology_set_dom(tscan, TOPO_CORE_DOMAIN, sft, ecx.ncores + 1);
+ return true;
+}
+
+static void store_node(struct topo_scan *tscan, unsigned int nr_nodes, u16 node_id)
+{
+ /*
+ * Starting with Fam 17h the DIE domain could probably be used to
+ * retrieve the node info on AMD/HYGON. Analysis of CPUID dumps
+ * suggests it's the topmost bit(s) of the CPU cores area, but
+ * that's guess work and neither enumerated nor documented.
+ *
+ * Up to Fam 16h this does not work at all and the legacy node ID
+ * has to be used.
+ */
+ tscan->amd_nodes_per_pkg = nr_nodes;
+ tscan->amd_node_id = node_id;
+}
+
+static bool parse_8000_001e(struct topo_scan *tscan, bool has_0xb)
+{
+ struct {
+ // eax
+ u32 x2apic_id : 32;
+ // ebx
+ u32 cuid : 8,
+ threads_per_cu : 8,
+ __rsvd0 : 16;
+ // ecx
+ u32 nodeid : 8,
+ nodes_per_pkg : 3,
+ __rsvd1 : 21;
+ // edx
+ u32 __rsvd2 : 32;
+ } leaf;
+
+ if (!boot_cpu_has(X86_FEATURE_TOPOEXT))
+ return false;
+
+ cpuid_leaf(0x8000001e, &leaf);
+
+ tscan->c->topo.initial_apicid = leaf.x2apic_id;
+
+ /*
+ * If leaf 0xb is available, then SMT shift is set already. If not
+ * take it from ecx.threads_per_cu and use topo_update_dom() -
+ * topology_set_dom() would propagate and overwrite the already
+ * propagated CORE level.
+ */
+ if (!has_0xb) {
+ topology_update_dom(tscan, TOPO_SMT_DOMAIN, get_count_order(leaf.threads_per_cu),
+ leaf.threads_per_cu + 1);
+ }
+
+ store_node(tscan, leaf.nodes_per_pkg + 1, leaf.nodeid);
+
+ if (tscan->c->x86_vendor == X86_VENDOR_AMD) {
+ if (tscan->c->x86 == 0x15)
+ tscan->c->topo.cu_id = leaf.cuid;
+
+ cacheinfo_amd_init_llc_id(tscan->c, leaf.nodeid);
+ } else {
+ /*
+ * Package ID is ApicId[6..] on Hygon CPUs. See commit
+ * e0ceeae708ce for explanation. The topology info is
+ * screwed up: The package shift is always 6 and the node
+ * ID is bit [4:5]. Don't touch the latter without
+ * confirmation from the Hygon developers.
+ */
+ topology_set_dom(tscan, TOPO_CORE_DOMAIN, 6, tscan->dom_ncpus[TOPO_CORE_DOMAIN]);
+ cacheinfo_hygon_init_llc_id(tscan->c);
+ }
+ return true;
+}
+
+static bool parse_fam10h_node_id(struct topo_scan *tscan)
+{
+ struct {
+ union {
+ u64 node_id : 3,
+ nodes_per_pkg : 3,
+ unused : 58;
+ u64 msr;
+ };
+ } nid;
+
+ if (!boot_cpu_has(X86_FEATURE_NODEID_MSR))
+ return false;
+
+ rdmsrl(MSR_FAM10H_NODE_ID, nid.msr);
+ store_node(tscan, nid.nodes_per_pkg + 1, nid.node_id);
+ tscan->c->topo.llc_id = nid.node_id;
+ return true;
+}
+
+static void legacy_set_llc(struct topo_scan *tscan)
+{
+ unsigned int apicid = tscan->c->topo.initial_apicid;
+
+ /* parse_8000_0008() set everything up except llc_id */
+ tscan->c->topo.llc_id = apicid >> tscan->dom_shifts[TOPO_CORE_DOMAIN];
+}
+
+static void parse_topology_amd(struct topo_scan *tscan)
+{
+ bool has_0xb = false;
+
+ /*
+ * If the extended topology leaf 0x8000_001e is available
+ * try to get SMT and CORE shift from leaf 0xb first, then
+ * try to get the CORE shift from leaf 0x8000_0008.
+ */
+ if (boot_cpu_has(X86_FEATURE_TOPOEXT))
+ has_0xb = cpu_parse_topology_ext(tscan);
+
+ if (!has_0xb && !parse_8000_0008(tscan))
+ return;
+
+ /* Prefer leaf 0x8000001e if available */
+ if (parse_8000_001e(tscan, has_0xb))
+ return;
+
+ /* Try the NODEID MSR */
+ if (parse_fam10h_node_id(tscan))
+ return;
+
+ legacy_set_llc(tscan);
+}
+
+void cpu_parse_topology_amd(struct topo_scan *tscan)
+{
+ tscan->amd_nodes_per_pkg = 1;
+ parse_topology_amd(tscan);
+
+ if (tscan->amd_nodes_per_pkg > 1)
+ set_cpu_cap(tscan->c, X86_FEATURE_AMD_DCM);
+}
+
+void cpu_topology_fixup_amd(struct topo_scan *tscan)
+{
+ struct cpuinfo_x86 *c = tscan->c;
+
+ /*
+ * Adjust the core_id relative to the node when there is more than
+ * one node.
+ */
+ if (tscan->c->x86 < 0x17 && tscan->amd_nodes_per_pkg > 1)
+ c->topo.core_id %= tscan->dom_ncpus[TOPO_CORE_DOMAIN] / tscan->amd_nodes_per_pkg;
+}
--- a/arch/x86/kernel/cpu/topology_common.c
+++ b/arch/x86/kernel/cpu/topology_common.c
@@ -11,11 +11,13 @@

struct x86_topology_system x86_topo_system __ro_after_init;

+unsigned int __amd_nodes_per_pkg __ro_after_init;
+EXPORT_SYMBOL_GPL(__amd_nodes_per_pkg);
+
void topology_set_dom(struct topo_scan *tscan, enum x86_topology_domains dom,
unsigned int shift, unsigned int ncpus)
{
- tscan->dom_shifts[dom] = shift;
- tscan->dom_ncpus[dom] = ncpus;
+ topology_update_dom(tscan, dom, shift, ncpus);

/* Propagate to the upper levels */
for (dom++; dom < TOPO_MAX_DOMAIN; dom++) {
@@ -152,6 +154,13 @@ static void topo_set_ids(struct topo_sca

/* Relative core ID */
c->topo.core_id = topo_relative_domain_id(apicid, TOPO_CORE_DOMAIN);
+
+ /* Temporary workaround */
+ if (tscan->amd_nodes_per_pkg)
+ c->topo.amd_node_id = c->topo.die_id = tscan->amd_node_id;
+
+ if (c->x86_vendor == X86_VENDOR_AMD)
+ cpu_topology_fixup_amd(tscan);
}

static void topo_set_max_cores(struct topo_scan *tscan)
@@ -236,4 +245,10 @@ void __init cpu_init_topology(struct cpu
*/
__max_die_per_package = tscan.dom_ncpus[TOPO_DIE_DOMAIN] /
tscan.dom_ncpus[TOPO_DIE_DOMAIN - 1];
+ /*
+ * AMD systems have Nodes per package which cannot be mapped to
+ * APIC ID (yet).
+ */
+ if (c->x86_vendor == X86_VENDOR_AMD || c->x86_vendor == X86_VENDOR_HYGON)
+ __amd_nodes_per_pkg = __max_die_per_package = tscan.amd_nodes_per_pkg;
}



2023-08-02 20:25:38

by Thomas Gleixner

[permalink] [raw]
Subject: [patch V3a 30/40] x86/cpu: Provide an AMD/HYGON specific topology parser

AMD/HYGON uses various methods for topology evaluation:

- Leaf 0x80000008 and 0x8000001e based with an optional leaf 0xb,
which is the preferred variant for modern CPUs.

Leaf 0xb will be superseded by leaf 0x80000026 soon, which is just
another variant of the Intel 0x1f leaf for whatever reasons.

- Subleaf 0x80000008 and NODEID_MSR base

- Legacy fallback

That code is following the principle of random bits and pieces all over the
place which results in multiple evaluations and impenetrable code flows in
the same way as the Intel parsing did.

Provide a sane implementation by clearly separating the three variants and
bringing them in the proper preference order in one place.

This provides the parsing for both AMD and HYGON because there is no point
in having a separate HYGON parser which only differs by 3 lines of
code. Any further divergence between AMD and HYGON can be handled in
different functions, while still sharing the existing parsers.

Signed-off-by: Thomas Gleixner <[email protected]>
---
V3: Fix the off by one with leaf 0x8000001e::ebx::threads_per_cu - Michael
V3a: Fix it for real.
---
arch/x86/include/asm/topology.h | 2
arch/x86/kernel/cpu/Makefile | 2
arch/x86/kernel/cpu/amd.c | 2
arch/x86/kernel/cpu/cacheinfo.c | 4
arch/x86/kernel/cpu/cpu.h | 2
arch/x86/kernel/cpu/debugfs.c | 2
arch/x86/kernel/cpu/topology.h | 6 +
arch/x86/kernel/cpu/topology_amd.c | 180 ++++++++++++++++++++++++++++++++++
arch/x86/kernel/cpu/topology_common.c | 19 +++
9 files changed, 212 insertions(+), 7 deletions(-)

--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -162,6 +162,8 @@ int topology_update_die_map(unsigned int
int topology_phys_to_logical_pkg(unsigned int pkg);
bool topology_smt_supported(void);

+extern unsigned int __amd_nodes_per_pkg;
+
static inline unsigned int topology_amd_nodes_per_pkg(void)
{
return __max_die_per_package;
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -18,7 +18,7 @@ KMSAN_SANITIZE_common.o := n
KCSAN_SANITIZE_common.o := n

obj-y := cacheinfo.o scattered.o
-obj-y += topology_common.o topology_ext.o topology.o
+obj-y += topology_common.o topology_ext.o topology_amd.o topology.o
obj-y += common.o
obj-y += rdrand.o
obj-y += match.o
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -423,7 +423,7 @@ static void amd_get_topology(struct cpui
if (!err)
c->x86_coreid_bits = get_count_order(c->x86_max_cores);

- cacheinfo_amd_init_llc_id(c);
+ cacheinfo_amd_init_llc_id(c, c->topo.die_id);

} else if (cpu_has(c, X86_FEATURE_NODEID_MSR)) {
u64 value;
--- a/arch/x86/kernel/cpu/cacheinfo.c
+++ b/arch/x86/kernel/cpu/cacheinfo.c
@@ -661,7 +661,7 @@ static int find_num_cache_leaves(struct
return i;
}

-void cacheinfo_amd_init_llc_id(struct cpuinfo_x86 *c)
+void cacheinfo_amd_init_llc_id(struct cpuinfo_x86 *c, u16 die_id)
{
/*
* We may have multiple LLCs if L3 caches exist, so check if we
@@ -672,7 +672,7 @@ void cacheinfo_amd_init_llc_id(struct cp

if (c->x86 < 0x17) {
/* LLC is at the node level. */
- c->topo.llc_id = c->topo.die_id;
+ c->topo.llc_id = die_id;
} else if (c->x86 == 0x17 && c->x86_model <= 0x1F) {
/*
* LLC is at the core complex level.
--- a/arch/x86/kernel/cpu/cpu.h
+++ b/arch/x86/kernel/cpu/cpu.h
@@ -79,7 +79,7 @@ extern void init_hygon_cacheinfo(struct
extern int detect_extended_topology(struct cpuinfo_x86 *c);
extern void check_null_seg_clears_base(struct cpuinfo_x86 *c);

-void cacheinfo_amd_init_llc_id(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);

unsigned int aperfmperf_get_khz(int cpu);
--- a/arch/x86/kernel/cpu/debugfs.c
+++ b/arch/x86/kernel/cpu/debugfs.c
@@ -26,6 +26,8 @@ static int cpu_debug_show(struct seq_fil
seq_printf(m, "logical_die_id: %u\n", c->topo.logical_die_id);
seq_printf(m, "llc_id: %u\n", c->topo.llc_id);
seq_printf(m, "l2c_id: %u\n", c->topo.l2c_id);
+ seq_printf(m, "amd_node_id: %u\n", c->topo.amd_node_id);
+ seq_printf(m, "amd_nodes_per_pkg: %u\n", topology_amd_nodes_per_pkg());
seq_printf(m, "max_cores: %u\n", c->x86_max_cores);
seq_printf(m, "max_die_per_pkg: %u\n", __max_die_per_package);
seq_printf(m, "smp_num_siblings: %u\n", smp_num_siblings);
--- a/arch/x86/kernel/cpu/topology.h
+++ b/arch/x86/kernel/cpu/topology.h
@@ -9,6 +9,10 @@ struct topo_scan {

// Legacy CPUID[1]:EBX[23:16] number of logical processors
unsigned int ebx1_nproc_shift;
+
+ // AMD specific node ID which cannot be mapped into APIC space.
+ u16 amd_nodes_per_pkg;
+ u16 amd_node_id;
};

bool topo_is_converted(struct cpuinfo_x86 *c);
@@ -17,6 +21,8 @@ void cpu_parse_topology(struct cpuinfo_x
void topology_set_dom(struct topo_scan *tscan, enum x86_topology_domains dom,
unsigned int shift, unsigned int ncpus);
bool cpu_parse_topology_ext(struct topo_scan *tscan);
+void cpu_parse_topology_amd(struct topo_scan *tscan);
+void cpu_topology_fixup_amd(struct topo_scan *tscan);

static inline u32 topo_shift_apicid(u32 apicid, enum x86_topology_domains dom)
{
--- /dev/null
+++ b/arch/x86/kernel/cpu/topology_amd.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/cpu.h>
+
+#include <asm/apic.h>
+#include <asm/memtype.h>
+#include <asm/processor.h>
+
+#include "cpu.h"
+
+static bool parse_8000_0008(struct topo_scan *tscan)
+{
+ struct {
+ u32 ncores : 8,
+ __rsvd0 : 4,
+ apicidsize : 4,
+ perftscsize : 2,
+ __rsvd1 : 14;
+ } ecx;
+ unsigned int sft;
+
+ if (tscan->c->extended_cpuid_level < 0x80000008)
+ return false;
+
+ cpuid_leaf_reg(0x80000008, CPUID_ECX, &ecx);
+
+ /* If the APIC ID size is 0, then get the shift value from ecx.ncores */
+ sft = ecx.apicidsize;
+ if (!sft)
+ sft = get_count_order(ecx.ncores + 1);
+
+ topology_set_dom(tscan, TOPO_CORE_DOMAIN, sft, ecx.ncores + 1);
+ return true;
+}
+
+static void store_node(struct topo_scan *tscan, unsigned int nr_nodes, u16 node_id)
+{
+ /*
+ * Starting with Fam 17h the DIE domain could probably be used to
+ * retrieve the node info on AMD/HYGON. Analysis of CPUID dumps
+ * suggests it's the topmost bit(s) of the CPU cores area, but
+ * that's guess work and neither enumerated nor documented.
+ *
+ * Up to Fam 16h this does not work at all and the legacy node ID
+ * has to be used.
+ */
+ tscan->amd_nodes_per_pkg = nr_nodes;
+ tscan->amd_node_id = node_id;
+}
+
+static bool parse_8000_001e(struct topo_scan *tscan, bool has_0xb)
+{
+ struct {
+ // eax
+ u32 x2apic_id : 32;
+ // ebx
+ u32 cuid : 8,
+ threads_per_cu : 8,
+ __rsvd0 : 16;
+ // ecx
+ u32 nodeid : 8,
+ nodes_per_pkg : 3,
+ __rsvd1 : 21;
+ // edx
+ u32 __rsvd2 : 32;
+ } leaf;
+
+ if (!boot_cpu_has(X86_FEATURE_TOPOEXT))
+ return false;
+
+ cpuid_leaf(0x8000001e, &leaf);
+
+ tscan->c->topo.initial_apicid = leaf.x2apic_id;
+
+ /*
+ * If leaf 0xb is available, then SMT shift is set already. If not
+ * take it from ecx.threads_per_cu and use topo_update_dom() -
+ * topology_set_dom() would propagate and overwrite the already
+ * propagated CORE level.
+ */
+ if (!has_0xb) {
+ unsigned int nthreads = leaf.threads_per_cu + 1;
+
+ topology_update_dom(tscan, TOPO_SMT_DOMAIN, get_count_order(nthreads), nthreads);
+ }
+
+ store_node(tscan, leaf.nodes_per_pkg + 1, leaf.nodeid);
+
+ if (tscan->c->x86_vendor == X86_VENDOR_AMD) {
+ if (tscan->c->x86 == 0x15)
+ tscan->c->topo.cu_id = leaf.cuid;
+
+ cacheinfo_amd_init_llc_id(tscan->c, leaf.nodeid);
+ } else {
+ /*
+ * Package ID is ApicId[6..] on Hygon CPUs. See commit
+ * e0ceeae708ce for explanation. The topology info is
+ * screwed up: The package shift is always 6 and the node
+ * ID is bit [4:5]. Don't touch the latter without
+ * confirmation from the Hygon developers.
+ */
+ topology_set_dom(tscan, TOPO_CORE_DOMAIN, 6, tscan->dom_ncpus[TOPO_CORE_DOMAIN]);
+ cacheinfo_hygon_init_llc_id(tscan->c);
+ }
+ return true;
+}
+
+static bool parse_fam10h_node_id(struct topo_scan *tscan)
+{
+ struct {
+ union {
+ u64 node_id : 3,
+ nodes_per_pkg : 3,
+ unused : 58;
+ u64 msr;
+ };
+ } nid;
+
+ if (!boot_cpu_has(X86_FEATURE_NODEID_MSR))
+ return false;
+
+ rdmsrl(MSR_FAM10H_NODE_ID, nid.msr);
+ store_node(tscan, nid.nodes_per_pkg + 1, nid.node_id);
+ tscan->c->topo.llc_id = nid.node_id;
+ return true;
+}
+
+static void legacy_set_llc(struct topo_scan *tscan)
+{
+ unsigned int apicid = tscan->c->topo.initial_apicid;
+
+ /* parse_8000_0008() set everything up except llc_id */
+ tscan->c->topo.llc_id = apicid >> tscan->dom_shifts[TOPO_CORE_DOMAIN];
+}
+
+static void parse_topology_amd(struct topo_scan *tscan)
+{
+ bool has_0xb = false;
+
+ /*
+ * If the extended topology leaf 0x8000_001e is available
+ * try to get SMT and CORE shift from leaf 0xb first, then
+ * try to get the CORE shift from leaf 0x8000_0008.
+ */
+ if (boot_cpu_has(X86_FEATURE_TOPOEXT))
+ has_0xb = cpu_parse_topology_ext(tscan);
+
+ if (!has_0xb && !parse_8000_0008(tscan))
+ return;
+
+ /* Prefer leaf 0x8000001e if available */
+ if (parse_8000_001e(tscan, has_0xb))
+ return;
+
+ /* Try the NODEID MSR */
+ if (parse_fam10h_node_id(tscan))
+ return;
+
+ legacy_set_llc(tscan);
+}
+
+void cpu_parse_topology_amd(struct topo_scan *tscan)
+{
+ tscan->amd_nodes_per_pkg = 1;
+ parse_topology_amd(tscan);
+
+ if (tscan->amd_nodes_per_pkg > 1)
+ set_cpu_cap(tscan->c, X86_FEATURE_AMD_DCM);
+}
+
+void cpu_topology_fixup_amd(struct topo_scan *tscan)
+{
+ struct cpuinfo_x86 *c = tscan->c;
+
+ /*
+ * Adjust the core_id relative to the node when there is more than
+ * one node.
+ */
+ if (tscan->c->x86 < 0x17 && tscan->amd_nodes_per_pkg > 1)
+ c->topo.core_id %= tscan->dom_ncpus[TOPO_CORE_DOMAIN] / tscan->amd_nodes_per_pkg;
+}
--- a/arch/x86/kernel/cpu/topology_common.c
+++ b/arch/x86/kernel/cpu/topology_common.c
@@ -11,11 +11,13 @@

struct x86_topology_system x86_topo_system __ro_after_init;

+unsigned int __amd_nodes_per_pkg __ro_after_init;
+EXPORT_SYMBOL_GPL(__amd_nodes_per_pkg);
+
void topology_set_dom(struct topo_scan *tscan, enum x86_topology_domains dom,
unsigned int shift, unsigned int ncpus)
{
- tscan->dom_shifts[dom] = shift;
- tscan->dom_ncpus[dom] = ncpus;
+ topology_update_dom(tscan, dom, shift, ncpus);

/* Propagate to the upper levels */
for (dom++; dom < TOPO_MAX_DOMAIN; dom++) {
@@ -152,6 +154,13 @@ static void topo_set_ids(struct topo_sca

/* Relative core ID */
c->topo.core_id = topo_relative_domain_id(apicid, TOPO_CORE_DOMAIN);
+
+ /* Temporary workaround */
+ if (tscan->amd_nodes_per_pkg)
+ c->topo.amd_node_id = c->topo.die_id = tscan->amd_node_id;
+
+ if (c->x86_vendor == X86_VENDOR_AMD)
+ cpu_topology_fixup_amd(tscan);
}

static void topo_set_max_cores(struct topo_scan *tscan)
@@ -236,4 +245,10 @@ void __init cpu_init_topology(struct cpu
*/
__max_die_per_package = tscan.dom_ncpus[TOPO_DIE_DOMAIN] /
tscan.dom_ncpus[TOPO_DIE_DOMAIN - 1];
+ /*
+ * AMD systems have Nodes per package which cannot be mapped to
+ * APIC ID (yet).
+ */
+ if (c->x86_vendor == X86_VENDOR_AMD || c->x86_vendor == X86_VENDOR_HYGON)
+ __amd_nodes_per_pkg = __max_die_per_package = tscan.amd_nodes_per_pkg;
}

2023-08-02 20:51:22

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [patch V3 30/40] x86/cpu: Provide an AMD/HYGON specific topology parser

From: Thomas Gleixner <[email protected]> Sent: Wednesday, August 2, 2023 3:22 AM
>
> AMD/HYGON uses various methods for topology evaluation:
>
> - Leaf 0x80000008 and 0x8000001e based with an optional leaf 0xb,
> which is the preferred variant for modern CPUs.
>
> Leaf 0xb will be superseded by leaf 0x80000026 soon, which is just
> another variant of the Intel 0x1f leaf for whatever reasons.
>
> - Subleaf 0x80000008 and NODEID_MSR base
>
> - Legacy fallback
>
> That code is following the principle of random bits and pieces all over the
> place which results in multiple evaluations and impenetrable code flows in
> the same way as the Intel parsing did.
>
> Provide a sane implementation by clearly separating the three variants and
> bringing them in the proper preference order in one place.
>
> This provides the parsing for both AMD and HYGON because there is no point
> in having a separate HYGON parser which only differs by 3 lines of
> code. Any further divergence between AMD and HYGON can be handled in
> different functions, while still sharing the existing parsers.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> V3: Fix the off by one with leaf 0x8000001e::ebx::threads_per_cu - Michael
> ---

[snip]

> +
> +static bool parse_8000_001e(struct topo_scan *tscan, bool has_0xb)
> +{
> + struct {
> + // eax
> + u32 x2apic_id : 32;
> + // ebx
> + u32 cuid : 8,
> + threads_per_cu : 8,
> + __rsvd0 : 16;
> + // ecx
> + u32 nodeid : 8,
> + nodes_per_pkg : 3,
> + __rsvd1 : 21;
> + // edx
> + u32 __rsvd2 : 32;
> + } leaf;
> +
> + if (!boot_cpu_has(X86_FEATURE_TOPOEXT))
> + return false;
> +
> + cpuid_leaf(0x8000001e, &leaf);
> +
> + tscan->c->topo.initial_apicid = leaf.x2apic_id;
> +
> + /*
> + * If leaf 0xb is available, then SMT shift is set already. If not
> + * take it from ecx.threads_per_cu and use topo_update_dom() -
> + * topology_set_dom() would propagate and overwrite the already
> + * propagated CORE level.
> + */
> + if (!has_0xb) {
> + topology_update_dom(tscan, TOPO_SMT_DOMAIN, get_count_order(leaf.threads_per_cu),
> + leaf.threads_per_cu + 1);

Isn't the +1 also needed on the argument to get_count_order()?
I haven't actually run the config, but if hyper-threading is disabled,
presumably threads_per_cu is 0, and get_count_order returns -1.

Michael

> + }
> +
> + store_node(tscan, leaf.nodes_per_pkg + 1, leaf.nodeid);
> +
> + if (tscan->c->x86_vendor == X86_VENDOR_AMD) {
> + if (tscan->c->x86 == 0x15)
> + tscan->c->topo.cu_id = leaf.cuid;
> +
> + cacheinfo_amd_init_llc_id(tscan->c, leaf.nodeid);
> + } else {
> + /*
> + * Package ID is ApicId[6..] on Hygon CPUs. See commit
> + * e0ceeae708ce for explanation. The topology info is
> + * screwed up: The package shift is always 6 and the node
> + * ID is bit [4:5]. Don't touch the latter without
> + * confirmation from the Hygon developers.
> + */
> + topology_set_dom(tscan, TOPO_CORE_DOMAIN, 6, tscan->dom_ncpus[TOPO_CORE_DOMAIN]);
> + cacheinfo_hygon_init_llc_id(tscan->c);
> + }
> + return true;
> +}
> +

2023-08-02 20:54:15

by Thomas Gleixner

[permalink] [raw]
Subject: RE: [patch V3 30/40] x86/cpu: Provide an AMD/HYGON specific topology parser

On Wed, Aug 02 2023 at 19:28, Michael Kelley (LINUX) wrote:
> From: Thomas Gleixner <[email protected]> Sent: Wednesday, August 2, 2023 3:22 AM
>> + /*
>> + * If leaf 0xb is available, then SMT shift is set already. If not
>> + * take it from ecx.threads_per_cu and use topo_update_dom() -
>> + * topology_set_dom() would propagate and overwrite the already
>> + * propagated CORE level.
>> + */
>> + if (!has_0xb) {
>> + topology_update_dom(tscan, TOPO_SMT_DOMAIN, get_count_order(leaf.threads_per_cu),
>> + leaf.threads_per_cu + 1);
>
> Isn't the +1 also needed on the argument to get_count_order()?
> I haven't actually run the config, but if hyper-threading is disabled,
> presumably threads_per_cu is 0, and get_count_order returns -1.

Bah. I'm sure that I wanted to do that. No idea how I forgot about it
again.


2023-08-11 15:05:13

by Pu Wen

[permalink] [raw]
Subject: Re: [patch V3a 30/40] x86/cpu: Provide an AMD/HYGON specific topology parser

On 2023/8/3 3:51, Thomas Gleixner wrote:
> + if (tscan->c->x86_vendor == X86_VENDOR_AMD) {
> + if (tscan->c->x86 == 0x15)
> + tscan->c->topo.cu_id = leaf.cuid;
> +
> + cacheinfo_amd_init_llc_id(tscan->c, leaf.nodeid);
> + } else {
> + /*
> + * Package ID is ApicId[6..] on Hygon CPUs. See commit
> + * e0ceeae708ce for explanation. The topology info is
> + * screwed up: The package shift is always 6 and the node
> + * ID is bit [4:5]. Don't touch the latter without
> + * confirmation from the Hygon developers.
> + */
> + topology_set_dom(tscan, TOPO_CORE_DOMAIN, 6, tscan->dom_ncpus[TOPO_CORE_DOMAIN]);

Hygon updated CPUs will not always shift 6, and shift 6 is not good for
running guests.
So suggest to modify like this:
if (!boot_cpu_has(X86_FEATURE_HYPERVISOR) && tscan->c->x86_model <=
0x3)
topology_set_dom(tscan, TOPO_CORE_DOMAIN, 6,
tscan->dom_ncpus[TOPO_CORE_DOMAIN]);

--
Regards,
Pu Wen

> + cacheinfo_hygon_init_llc_id(tscan->c);
> + }
> + return true;
> +}
> +


2023-08-11 17:58:25

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch V3a 30/40] x86/cpu: Provide an AMD/HYGON specific topology parser

On Fri, Aug 11 2023 at 20:58, Pu Wen wrote:
> On 2023/8/3 3:51, Thomas Gleixner wrote:
>> + if (tscan->c->x86_vendor == X86_VENDOR_AMD) {
>> + if (tscan->c->x86 == 0x15)
>> + tscan->c->topo.cu_id = leaf.cuid;
>> +
>> + cacheinfo_amd_init_llc_id(tscan->c, leaf.nodeid);
>> + } else {
>> + /*
>> + * Package ID is ApicId[6..] on Hygon CPUs. See commit
>> + * e0ceeae708ce for explanation. The topology info is
>> + * screwed up: The package shift is always 6 and the node
>> + * ID is bit [4:5]. Don't touch the latter without
>> + * confirmation from the Hygon developers.
>> + */
>> + topology_set_dom(tscan, TOPO_CORE_DOMAIN, 6, tscan->dom_ncpus[TOPO_CORE_DOMAIN]);
>
> Hygon updated CPUs will not always shift 6, and shift 6 is not good for
> running guests.
> So suggest to modify like this:
> if (!boot_cpu_has(X86_FEATURE_HYPERVISOR) && tscan->c->x86_model <=
> 0x3)
> topology_set_dom(tscan, TOPO_CORE_DOMAIN, 6,
> tscan->dom_ncpus[TOPO_CORE_DOMAIN]);

This is exactly what the existing code does today. Can you please send a
delta patch on top of this with a proper explanation?

Thanks,

tglx

2023-08-12 05:37:57

by Pu Wen

[permalink] [raw]
Subject: Re: [patch V3a 30/40] x86/cpu: Provide an AMD/HYGON specific topology parser

On 2023/8/12 1:11, Thomas Gleixner wrote:
> On Fri, Aug 11 2023 at 20:58, Pu Wen wrote:
>> On 2023/8/3 3:51, Thomas Gleixner wrote:
>>> + if (tscan->c->x86_vendor == X86_VENDOR_AMD) {
>>> + if (tscan->c->x86 == 0x15)
>>> + tscan->c->topo.cu_id = leaf.cuid;
>>> +
>>> + cacheinfo_amd_init_llc_id(tscan->c, leaf.nodeid);
>>> + } else {
>>> + /*
>>> + * Package ID is ApicId[6..] on Hygon CPUs. See commit
>>> + * e0ceeae708ce for explanation. The topology info is
>>> + * screwed up: The package shift is always 6 and the node
>>> + * ID is bit [4:5]. Don't touch the latter without
>>> + * confirmation from the Hygon developers.
>>> + */
>>> + topology_set_dom(tscan, TOPO_CORE_DOMAIN, 6, tscan->dom_ncpus[TOPO_CORE_DOMAIN]);
>>
>> Hygon updated CPUs will not always shift 6, and shift 6 is not good for
>> running guests.
>> So suggest to modify like this:
>> if (!boot_cpu_has(X86_FEATURE_HYPERVISOR) && tscan->c->x86_model <=
>> 0x3)
>> topology_set_dom(tscan, TOPO_CORE_DOMAIN, 6,
>> tscan->dom_ncpus[TOPO_CORE_DOMAIN]);
>
> This is exactly what the existing code does today. Can you please send a
> delta patch on top of this with a proper explanation?

Will.

And I think it's better to send a prerequisite patch(is attached) after
patch 02 of this series. Since it can be individually back-ported to the
stable releases.

--
Regards,
Pu Wen



Attachments:
x86-cpu-Refine-the-CPU-topology-deriving-method-for-Hygon.patch (1.44 kB)
Subject: [tip: x86/core] x86/cpu/hygon: Fix the CPU topology evaluation for real

The following commit has been merged into the x86/core branch of tip:

Commit-ID: ee545b94d39a00c93dc98b1dbcbcf731d2eadeb4
Gitweb: https://git.kernel.org/tip/ee545b94d39a00c93dc98b1dbcbcf731d2eadeb4
Author: Pu Wen <[email protected]>
AuthorDate: Mon, 14 Aug 2023 10:18:26 +02:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Tue, 10 Oct 2023 14:38:16 +02:00

x86/cpu/hygon: Fix the CPU topology evaluation for real

Hygon processors with a model ID > 3 have CPUID leaf 0xB correctly
populated and don't need the fixed package ID shift workaround. The fixup
is also incorrect when running in a guest.

Fixes: e0ceeae708ce ("x86/CPU/hygon: Fix phys_proc_id calculation logic for multi-die processors")
Signed-off-by: Pu Wen <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/cpu/hygon.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/hygon.c b/arch/x86/kernel/cpu/hygon.c
index defdc59..a7b3ef4 100644
--- a/arch/x86/kernel/cpu/hygon.c
+++ b/arch/x86/kernel/cpu/hygon.c
@@ -87,8 +87,12 @@ static void hygon_get_topology(struct cpuinfo_x86 *c)
if (!err)
c->x86_coreid_bits = get_count_order(c->x86_max_cores);

- /* Socket ID is ApicId[6] for these processors. */
- c->phys_proc_id = c->apicid >> APICID_SOCKET_ID_BIT;
+ /*
+ * Socket ID is ApicId[6] for the processors with model <= 0x3
+ * when running on host.
+ */
+ if (!boot_cpu_has(X86_FEATURE_HYPERVISOR) && c->x86_model <= 0x3)
+ c->phys_proc_id = c->apicid >> APICID_SOCKET_ID_BIT;

cacheinfo_hygon_init_llc_id(c, cpu);
} else if (cpu_has(c, X86_FEATURE_NODEID_MSR)) {