2024-01-23 13:23:39

by Thomas Gleixner

[permalink] [raw]
Subject: [patch v2 25/30] x86/cpu/topology: Use topology logical mapping mechanism

From: Thomas Gleixner <[email protected]>

Replace the logical package and die management functionality and retrieve
the logical IDs from the topology bitmaps.

Signed-off-by: Thomas Gleixner <[email protected]>



---
arch/x86/include/asm/topology.h | 15 ++--
arch/x86/kernel/cpu/common.c | 13 ---
arch/x86/kernel/cpu/topology_common.c | 4 +
arch/x86/kernel/smpboot.c | 111 ----------------------------------
4 files changed, 12 insertions(+), 131 deletions(-)
---
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -172,6 +172,13 @@ static inline int topology_get_logical_i
#define topology_core_cpumask(cpu) (per_cpu(cpu_core_map, cpu))
#define topology_sibling_cpumask(cpu) (per_cpu(cpu_sibling_map, cpu))

+
+static inline int topology_phys_to_logical_pkg(unsigned int pkg)
+{
+ return topology_get_logical_id(pkg << x86_topo_system.dom_shifts[TOPO_PKG_DOMAIN],
+ TOPO_PKG_DOMAIN);
+}
+
extern int __max_smt_threads;

static inline int topology_max_smt_threads(void)
@@ -181,10 +188,6 @@ static inline int topology_max_smt_threa

#include <linux/cpu_smt.h>

-int topology_update_package_map(unsigned int apicid, unsigned int cpu);
-int topology_update_die_map(unsigned int dieid, unsigned int cpu);
-int topology_phys_to_logical_pkg(unsigned int pkg);
-
extern unsigned int __amd_nodes_per_pkg;

static inline unsigned int topology_amd_nodes_per_pkg(void)
@@ -205,10 +208,6 @@ static inline bool topology_is_primary_t
}

#else /* CONFIG_SMP */
-static inline int
-topology_update_package_map(unsigned int apicid, unsigned int cpu) { return 0; }
-static inline int
-topology_update_die_map(unsigned int dieid, unsigned int cpu) { return 0; }
static inline int topology_phys_to_logical_pkg(unsigned int pkg) { return 0; }
static inline int topology_max_smt_threads(void) { return 1; }
static inline bool topology_is_primary_thread(unsigned int cpu) { return true; }
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1718,18 +1718,6 @@ static void generic_identify(struct cpui
#endif
}

-static void update_package_map(struct cpuinfo_x86 *c)
-{
-#ifdef CONFIG_SMP
- unsigned int cpu = smp_processor_id();
-
- BUG_ON(topology_update_package_map(c->topo.pkg_id, cpu));
- BUG_ON(topology_update_die_map(c->topo.die_id, cpu));
-#else
- c->topo.logical_pkg_id = 0;
-#endif
-}
-
/*
* This does the hard work of actually picking apart the CPU stuff...
*/
@@ -1913,7 +1901,6 @@ void identify_secondary_cpu(struct cpuin
#ifdef CONFIG_X86_32
enable_sep_cpu();
#endif
- update_package_map(c);
x86_spec_ctrl_setup_ap();
update_srbds_msr();
if (boot_cpu_has_bug(X86_BUG_GDS))
--- a/arch/x86/kernel/cpu/topology_common.c
+++ b/arch/x86/kernel/cpu/topology_common.c
@@ -10,6 +10,7 @@
#include "cpu.h"

struct x86_topology_system x86_topo_system __ro_after_init;
+EXPORT_SYMBOL_GPL(x86_topo_system);

unsigned int __amd_nodes_per_pkg __ro_after_init;
EXPORT_SYMBOL_GPL(__amd_nodes_per_pkg);
@@ -147,6 +148,9 @@ static void topo_set_ids(struct topo_sca
c->topo.pkg_id = topo_shift_apicid(apicid, TOPO_PKG_DOMAIN);
c->topo.die_id = topo_shift_apicid(apicid, TOPO_DIE_DOMAIN);

+ c->topo.logical_pkg_id = topology_get_logical_id(apicid, TOPO_PKG_DOMAIN);
+ c->topo.logical_die_id = topology_get_logical_id(apicid, TOPO_DIE_DOMAIN);
+
/* Package relative core ID */
c->topo.core_id = (apicid & topo_domain_mask(TOPO_PKG_DOMAIN)) >>
x86_topo_system.dom_shifts[TOPO_SMT_DOMAIN];
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -125,23 +125,6 @@ struct mwait_cpu_dead {
*/
static DEFINE_PER_CPU_ALIGNED(struct mwait_cpu_dead, mwait_cpu_dead);

-/* Logical package management. */
-struct logical_maps {
- u32 phys_pkg_id;
- u32 phys_die_id;
- u32 logical_pkg_id;
- u32 logical_die_id;
-};
-
-/* Temporary workaround until the full topology mechanics is in place */
-static DEFINE_PER_CPU_READ_MOSTLY(struct logical_maps, logical_maps) = {
- .phys_pkg_id = U32_MAX,
- .phys_die_id = U32_MAX,
-};
-
-static unsigned int logical_packages __read_mostly;
-static unsigned int logical_die __read_mostly;
-
/* Maximum number of SMT threads on any online core */
int __read_mostly __max_smt_threads = 1;

@@ -334,103 +317,11 @@ static void notrace start_secondary(void
cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
}

-/**
- * topology_phys_to_logical_pkg - Map a physical package id to a logical
- * @phys_pkg: The physical package id to map
- *
- * Returns logical package id or -1 if not found
- */
-int topology_phys_to_logical_pkg(unsigned int phys_pkg)
-{
- int cpu;
-
- for_each_possible_cpu(cpu) {
- if (per_cpu(logical_maps.phys_pkg_id, cpu) == phys_pkg)
- return per_cpu(logical_maps.logical_pkg_id, cpu);
- }
- return -1;
-}
-EXPORT_SYMBOL(topology_phys_to_logical_pkg);
-
-/**
- * topology_phys_to_logical_die - Map a physical die id to logical
- * @die_id: The physical die id to map
- * @cur_cpu: The CPU for which the mapping is done
- *
- * Returns logical die id or -1 if not found
- */
-static int topology_phys_to_logical_die(unsigned int die_id, unsigned int cur_cpu)
-{
- int cpu, proc_id = cpu_data(cur_cpu).topo.pkg_id;
-
- for_each_possible_cpu(cpu) {
- if (per_cpu(logical_maps.phys_pkg_id, cpu) == proc_id &&
- per_cpu(logical_maps.phys_die_id, cpu) == die_id)
- return per_cpu(logical_maps.logical_die_id, cpu);
- }
- return -1;
-}
-
-/**
- * topology_update_package_map - Update the physical to logical package map
- * @pkg: The physical package id as retrieved via CPUID
- * @cpu: The cpu for which this is updated
- */
-int topology_update_package_map(unsigned int pkg, unsigned int cpu)
-{
- int new;
-
- /* Already available somewhere? */
- new = topology_phys_to_logical_pkg(pkg);
- if (new >= 0)
- goto found;
-
- new = logical_packages++;
- if (new != pkg) {
- pr_info("CPU %u Converting physical %u to logical package %u\n",
- cpu, pkg, new);
- }
-found:
- per_cpu(logical_maps.phys_pkg_id, cpu) = pkg;
- per_cpu(logical_maps.logical_pkg_id, cpu) = new;
- cpu_data(cpu).topo.logical_pkg_id = new;
- return 0;
-}
-/**
- * topology_update_die_map - Update the physical to logical die map
- * @die: The die id as retrieved via CPUID
- * @cpu: The cpu for which this is updated
- */
-int topology_update_die_map(unsigned int die, unsigned int cpu)
-{
- int new;
-
- /* Already available somewhere? */
- new = topology_phys_to_logical_die(die, cpu);
- if (new >= 0)
- goto found;
-
- new = logical_die++;
- if (new != die) {
- pr_info("CPU %u Converting physical %u to logical die %u\n",
- cpu, die, new);
- }
-found:
- per_cpu(logical_maps.phys_die_id, cpu) = die;
- per_cpu(logical_maps.logical_die_id, cpu) = new;
- cpu_data(cpu).topo.logical_die_id = new;
- return 0;
-}
-
static void __init smp_store_boot_cpu_info(void)
{
- int id = 0; /* CPU 0 */
- struct cpuinfo_x86 *c = &cpu_data(id);
+ struct cpuinfo_x86 *c = &cpu_data(0);

*c = boot_cpu_data;
- c->cpu_index = id;
- topology_update_package_map(c->topo.pkg_id, id);
- topology_update_die_map(c->topo.die_id, id);
c->initialized = true;
}




2024-02-01 22:32:12

by Sohil Mehta

[permalink] [raw]
Subject: Re: [patch v2 25/30] x86/cpu/topology: Use topology logical mapping mechanism

On 1/23/2024 5:11 AM, Thomas Gleixner wrote:

> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -172,6 +172,13 @@ static inline int topology_get_logical_i
> #define topology_core_cpumask(cpu) (per_cpu(cpu_core_map, cpu))
> #define topology_sibling_cpumask(cpu) (per_cpu(cpu_sibling_map, cpu))
>
> +

This additional new line can be avoided.

> +static inline int topology_phys_to_logical_pkg(unsigned int pkg)
> +{
> + return topology_get_logical_id(pkg << x86_topo_system.dom_shifts[TOPO_PKG_DOMAIN],
> + TOPO_PKG_DOMAIN);
> +}
> +


2024-02-02 06:49:41

by Zhang, Rui

[permalink] [raw]
Subject: Re: [patch v2 25/30] x86/cpu/topology: Use topology logical mapping mechanism

Hi, Thomas,

> @@ -147,6 +148,9 @@ static void topo_set_ids(struct topo_sca
>         c->topo.pkg_id = topo_shift_apicid(apicid, TOPO_PKG_DOMAIN);
>         c->topo.die_id = topo_shift_apicid(apicid, TOPO_DIE_DOMAIN);
>  
> +       c->topo.logical_pkg_id = topology_get_logical_id(apicid,
> TOPO_PKG_DOMAIN);
> +       c->topo.logical_die_id = topology_get_logical_id(apicid,
> TOPO_DIE_DOMAIN);
> +

Just wondering if we could have logical_core_id.

drivers/hwmon/coretemp.c uses an array to save per core temperature
information. We cannot use core_id as array index because it can be
sparse. Currently, to get the temperature info for a specified core,
we need to traverse the array to know which core each entry maps to.

Ideally, we could have a global logical_core_id, and use that as the
array index directly.
This can also simplify kernel code in many places when checking if two
cpus are in the same core or not.

For now, I don't see a need to expose this info to userspace.

thanks,
rui

2024-02-12 16:21:49

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch v2 25/30] x86/cpu/topology: Use topology logical mapping mechanism

On Fri, Feb 02 2024 at 06:45, Rui Zhang wrote:
>> @@ -147,6 +148,9 @@ static void topo_set_ids(struct topo_sca
>>         c->topo.pkg_id = topo_shift_apicid(apicid, TOPO_PKG_DOMAIN);
>>         c->topo.die_id = topo_shift_apicid(apicid, TOPO_DIE_DOMAIN);
>>  
>> +       c->topo.logical_pkg_id = topology_get_logical_id(apicid,
>> TOPO_PKG_DOMAIN);
>> +       c->topo.logical_die_id = topology_get_logical_id(apicid,
>> TOPO_DIE_DOMAIN);
>> +
>
> Just wondering if we could have logical_core_id.
>
> drivers/hwmon/coretemp.c uses an array to save per core temperature
> information. We cannot use core_id as array index because it can be
> sparse. Currently, to get the temperature info for a specified core,
> we need to traverse the array to know which core each entry maps to.
>
> Ideally, we could have a global logical_core_id, and use that as the
> array index directly.
> This can also simplify kernel code in many places when checking if two
> cpus are in the same core or not.

That's trivial to do now :)

It's an orthogonal change and we can put it on top once this pile is
merged.

Thanks,

tglx