Parse the 'cpu-map' node in the Device Tree to extract CPU topology
information. If the 'cpu-map' node is absent, fall back to
generating the topology data from the NUMA information. This
generation assumes exactly one socket per NUMA node and that SMT
is unsupported.
Signed-off-by: Hirokazu Takahashi <taka@valinux.co.jp>
---
Changes in v4:
- Only display the CPU topology configuration prompt in common/Kconfig
if the architecture defines HAS_GENERIC_CPU_TOPOLOGY.
- Move the definition of the global 'cpu_topology' pointer to
common/cpu-topology.c.
- Update the Makefile to explicitly build objects as .init.o when all
functions and data within a file are annotated with __init/__initdata,
ensuring their memory is reclaimed after system initialization.
- Add an error log in the CPU-to-node mapping function for out-of-bounds
cases.
- Use ARRAY_SIZE() instead of raw macros when guarding array accesses.
- Rename variables and functions to avoid ambiguous or misleading terms:
- Avoid 'cpuid' to prevent confusion with x86 CPUID features/instructions.
- Avoid 'node' where it could be confused with a NUMA node, explicitly
renaming them to clarify they refer to a Device Tree node.
- Move local variable declarations into the narrowest possible scope.
- Replace the unsupported "%pOF" printk format specifier with "%s" and
explicit node name retrieval.
- Remove #include <dt-cpu-topology.h> from cpu-topology.h, and ensure
the header directly includes only what its definitions require.
- Remove #include <xen/device_tree.h> from dt-cpu-topology.h, replacing
it with a forward declaration of 'struct dt_device_node'.
- Use 'const' qualifiers for pointer declarations where the pointed-to
structure is not modified.
Changes in v3:
- Use (nr_cpu_ids - 1) as the maximum CPU ID here. The fix for the sparse
map mismatch issue on ARM Xen has been split out into a separate patch.
- Switch topology sibling masks to cpumask_var_t for dynamic allocation.
- Allow the system to keep running with a degraded fallback even if
the topology table allocation fails.
- Clean up the code to follow the Xen coding style. Please let me know
if I missed any style nits!
- Verify successful builds across x86, RISC-V, and PPC environments.
Changes in v2:
- Generate topology information even when ACPI is enabled. Note that
this is a temporary implementation and doesn't yet parse the PPTT
(Processor Properties Topology Table).
- Added support for cpu-map node in Device Tree that doesn't contain
explicit cluster node definitions.
xen/arch/arm/Kconfig | 1 +
xen/arch/arm/smpboot.c | 7 +
xen/common/Kconfig | 18 ++
xen/common/Makefile | 1 +
xen/common/cpu-topology.c | 62 +++++
xen/common/device-tree/Makefile | 1 +
xen/common/device-tree/cpu-topology.c | 355 ++++++++++++++++++++++++++
xen/drivers/acpi/Kconfig | 3 +
xen/drivers/acpi/Makefile | 1 +
xen/drivers/acpi/topology.c | 38 +++
xen/include/xen/acpi.h | 2 +
xen/include/xen/cpu-topology.h | 35 +++
xen/include/xen/dt-cpu-topology.h | 29 +++
13 files changed, 553 insertions(+)
create mode 100644 xen/common/cpu-topology.c
create mode 100644 xen/common/device-tree/cpu-topology.c
create mode 100644 xen/drivers/acpi/topology.c
create mode 100644 xen/include/xen/cpu-topology.h
create mode 100644 xen/include/xen/dt-cpu-topology.h
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 5fa89fcb24..a390844432 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -22,6 +22,7 @@ config ARM
select HAS_GRANT_CACHE_FLUSH if GRANT_TABLE
select HAS_STACK_PROTECTOR
select HAS_UBSAN
+ select HAS_GENERIC_CPU_TOPOLOGY
config ARCH_DEFCONFIG
string
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 7f3cfa812e..5ce2bcf6ec 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -9,10 +9,12 @@
#include <xen/acpi.h>
#include <xen/cpu.h>
+#include <xen/cpu-topology.h>
#include <xen/cpumask.h>
#include <xen/delay.h>
#include <xen/device_tree.h>
#include <xen/domain_page.h>
+#include <xen/dt-cpu-topology.h>
#include <xen/errno.h>
#include <xen/init.h>
#include <xen/mm.h>
@@ -242,6 +244,9 @@ static void __init dt_smp_init_cpus(void)
}
else
tmp_map[i] = hwid;
+
+ /* Pass the info to dt_init_cpu_topology() */
+ map_cpu_to_dt_node(i, cpu);
}
if ( !bootcpu_valid )
@@ -279,6 +284,8 @@ void __init smp_init_cpus(void)
else
acpi_smp_init_cpus();
+ init_cpu_topology();
+
if ( opt_hmp_unsafe )
warning_add("WARNING: HMP COMPUTING HAS BEEN ENABLED.\n"
"It has implications on the security and stability of the system,\n"
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 5ff71480ee..6ee689cbdd 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -188,6 +188,24 @@ config VM_EVENT
config NEEDS_LIBELF
bool
+config HAS_GENERIC_CPU_TOPOLOGY
+ bool
+
+config DT_CPU_TOPOLOGY
+ bool
+
+config GENERIC_CPU_TOPOLOGY
+ bool "CPU topology support (UNSUPPORTED)" if UNSUPPORTED
+ depends on HAS_GENERIC_CPU_TOPOLOGY
+ select DT_CPU_TOPOLOGY if DEVICE_TREE_PARSE
+ select ACPI_CPU_TOPOLOGY if ACPI
+ help
+ Retrieve CPU topology information from the device tree to optimize
+ virtual CPU scheduling.
+
+ Note: Implementation for parsing CPU topology from the ACPI PPTT
+ is currently missing.
+
config NUMA
bool
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 6018e25614..34b333cdb3 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_GENERIC_BUG_FRAME) += bug.o
obj-$(CONFIG_HYPFS_CONFIG) += config_data.o
obj-$(CONFIG_CORE_PARKING) += core_parking.o
obj-y += cpu.o
+obj-$(CONFIG_GENERIC_CPU_TOPOLOGY) += cpu-topology.o
obj-$(CONFIG_DEBUG_TRACE) += debugtrace.o
obj-$(CONFIG_HAS_DEVICE_TREE_DISCOVERY) += device.o
obj-$(filter-out $(CONFIG_X86),$(CONFIG_ACPI)) += device.o
diff --git a/xen/common/cpu-topology.c b/xen/common/cpu-topology.c
new file mode 100644
index 0000000000..94cb9ba312
--- /dev/null
+++ b/xen/common/cpu-topology.c
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <xen/acpi.h>
+#include <xen/cpu-topology.h>
+#include <xen/cpumask.h>
+#include <xen/dt-cpu-topology.h>
+#include <xen/init.h>
+
+struct cpu_topology *__ro_after_init cpu_topology;
+
+static void __init free_topology_table(void)
+{
+ unsigned int cpu;
+
+ for ( cpu = 0; cpu < nr_cpu_ids; cpu++ )
+ {
+ free_cpumask_var(cpu_topology[cpu].thread_sibling);
+ free_cpumask_var(cpu_topology[cpu].core_sibling);
+ free_cpumask_var(cpu_topology[cpu].cluster_sibling);
+ }
+
+ XFREE(cpu_topology);
+}
+
+void __init init_cpu_topology(void)
+{
+ unsigned int cpu;
+
+ cpu_topology = xzalloc_array(struct cpu_topology, nr_cpu_ids);
+ if ( !cpu_topology )
+ {
+ printk(XENLOG_ERR "Failed to allocate memory for cpu_topology table\n");
+ return;
+ }
+
+ for ( cpu = 0; cpu < nr_cpu_ids; cpu++ )
+ {
+ if ( !zalloc_cpumask_var(&cpu_topology[cpu].thread_sibling) ||
+ !zalloc_cpumask_var(&cpu_topology[cpu].core_sibling) ||
+ !zalloc_cpumask_var(&cpu_topology[cpu].cluster_sibling) )
+ {
+ free_topology_table();
+ printk(XENLOG_ERR "Failed to allocate memory for cpu_topology table\n");
+ return;
+ }
+ }
+
+ if ( acpi_disabled )
+ dt_init_cpu_topology();
+ else
+ acpi_init_cpu_topology();
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile
index 9036e455d6..6ee670b5f4 100644
--- a/xen/common/device-tree/Makefile
+++ b/xen/common/device-tree/Makefile
@@ -1,6 +1,7 @@
obj-y += bootfdt.init.o
obj-$(CONFIG_HAS_DEVICE_TREE_DISCOVERY) += bootinfo-fdt.init.o
obj-$(CONFIG_HAS_DEVICE_TREE_DISCOVERY) += bootinfo.init.o
+obj-$(CONFIG_DT_CPU_TOPOLOGY) += cpu-topology.init.o
obj-y += device-tree.o
obj-$(CONFIG_DOMAIN_BUILD_HELPERS) += domain-build.init.o
obj-$(filter $(CONFIG_DOM0LESS_BOOT),$(CONFIG_HAS_DEVICE_TREE_DISCOVERY)) += dom0less-build.init.o
diff --git a/xen/common/device-tree/cpu-topology.c b/xen/common/device-tree/cpu-topology.c
new file mode 100644
index 0000000000..b653227ef4
--- /dev/null
+++ b/xen/common/device-tree/cpu-topology.c
@@ -0,0 +1,355 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Derived from Linux kernel 7.0's $drivers/base/arch_topology.c
+ * Parse cpu topology information.
+ */
+
+#include <xen/acpi.h>
+#include <xen/cpu-topology.h>
+#include <xen/cpumask.h>
+#include <xen/device_tree.h>
+#include <xen/errno.h>
+#include <xen/init.h>
+#include <xen/numa.h>
+
+struct cpu_map {
+ unsigned int thread_id;
+ unsigned int core_id;
+ unsigned int cluster_id;
+ unsigned int package_id;
+};
+
+static const unsigned int __initdata invalid_topo_id = ~0U;
+static struct cpu_map __initdata cpu_map[NR_CPUS] = {
+ [0 ... NR_CPUS - 1] = {invalid_topo_id, invalid_topo_id, invalid_topo_id, 0}
+};
+static struct dt_device_node *__initdata dt_cpu_table[NR_CPUS];
+
+static void __init setup_siblings_masks(unsigned int target_cpu)
+{
+ const struct cpu_topology *target_topo = &cpu_topology[target_cpu];
+ const struct cpu_map *target_map = &cpu_map[target_cpu];
+ unsigned int cpu;
+
+ /* Update core and thread sibling masks */
+ for_each_possible_cpu(cpu)
+ {
+ const struct cpu_topology *cpu_topo = &cpu_topology[cpu];
+ const struct cpu_map *map = &cpu_map[cpu];
+
+ if ( target_map->package_id != map->package_id )
+ continue;
+
+ cpumask_set_cpu(target_cpu, cpu_topo->core_sibling);
+ cpumask_set_cpu(cpu, target_topo->core_sibling);
+
+ if ( target_map->cluster_id != map->cluster_id )
+ continue;
+
+ if ( target_map->cluster_id != invalid_topo_id )
+ {
+ cpumask_set_cpu(target_cpu, cpu_topo->cluster_sibling);
+ cpumask_set_cpu(cpu, target_topo->cluster_sibling);
+ }
+
+ if ( target_map->core_id != map->core_id )
+ continue;
+
+ cpumask_set_cpu(target_cpu, cpu_topo->thread_sibling);
+ cpumask_set_cpu(cpu, target_topo->thread_sibling);
+ }
+}
+
+static struct dt_device_node *__init
+ dt_find_child_node_by_name(const struct dt_device_node *dt,
+ const char *name)
+{
+ struct dt_device_node *np;
+
+ dt_for_each_child_node(dt, np)
+ if ( np->name && (dt_node_cmp(np->name, name) == 0) )
+ break;
+
+ return np;
+}
+
+void __init map_cpu_to_dt_node(unsigned int cpu,
+ struct dt_device_node *cpu_node)
+{
+ if ( cpu < ARRAY_SIZE(dt_cpu_table) )
+ dt_cpu_table[cpu] = cpu_node;
+ else
+ printk(XENLOG_WARNING
+ "cpu id %u exceeds the max cores %lu\n",
+ cpu, ARRAY_SIZE(dt_cpu_table));
+}
+
+static unsigned int __init cpu_node_to_id(const struct dt_device_node *cpu_node)
+{
+ unsigned int cpu;
+
+ for_each_possible_cpu(cpu)
+ if ( cpu_node == dt_cpu_table[cpu] )
+ return cpu;
+
+ return invalid_topo_id;
+}
+
+/*
+ * This function returns the logical cpu number of the DT node.
+ */
+static unsigned int __init
+ get_cpu_for_node(const struct dt_device_node *dt_node)
+{
+ const struct dt_device_node *cpu_node = dt_parse_phandle(dt_node, "cpu", 0);
+
+ if ( !cpu_node )
+ return invalid_topo_id;
+
+ return cpu_node_to_id(cpu_node);
+}
+
+static int __init parse_core(const struct dt_device_node *core,
+ unsigned int package_id,
+ unsigned int cluster_id,
+ unsigned int core_id)
+{
+ bool leaf = true;
+ unsigned int i = 0;
+ unsigned int cpu;
+
+ do {
+ const struct dt_device_node *t;
+ char name[20];
+
+ snprintf(name, sizeof(name), "thread%u", i);
+ t = dt_find_child_node_by_name(core, name);
+
+ if ( !t )
+ break;
+
+ leaf = false;
+ cpu = get_cpu_for_node(t);
+ if ( cpu != invalid_topo_id )
+ {
+ cpu_map[cpu].package_id = package_id;
+ cpu_map[cpu].cluster_id = cluster_id;
+ cpu_map[cpu].core_id = core_id;
+ cpu_map[cpu].thread_id = i;
+ }
+ else
+ {
+ printk(XENLOG_ERR
+ "ERROR: %s: Can't get CPU for thread\n", dt_node_name(t));
+ return -EINVAL;
+ }
+ i++;
+ } while ( true );
+
+ cpu = get_cpu_for_node(core);
+
+ if ( cpu != invalid_topo_id )
+ {
+ if ( !leaf )
+ {
+ printk(XENLOG_ERR "ERROR: %s: Core has both threads and CPU\n",
+ dt_node_name(core));
+ return -EINVAL;
+ }
+
+ cpu_map[cpu].package_id = package_id;
+ cpu_map[cpu].cluster_id = cluster_id;
+ cpu_map[cpu].core_id = core_id;
+ cpu_map[cpu].thread_id = 0;
+ }
+ else if ( leaf )
+ {
+ printk(XENLOG_ERR
+ "ERROR: %s: Can't get CPU for leaf core\n", dt_node_name(core));
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int __init parse_cluster(const struct dt_device_node *cluster,
+ unsigned int package_id,
+ unsigned int cluster_id,
+ unsigned int depth)
+{
+ bool leaf = true;
+ bool has_cores = false;
+ unsigned int core_id = 0;
+ unsigned int i = 0;
+
+ /*
+ * First check for child clusters; we currently ignore any
+ * information about the nesting of clusters and present the
+ * scheduler with a flat list of them.
+ */
+ do {
+ const struct dt_device_node *c;
+ char name[20];
+ int ret;
+
+ snprintf(name, sizeof(name), "cluster%u", i);
+ c = dt_find_child_node_by_name(cluster, name);
+
+ if ( !c )
+ break;
+
+ leaf = false;
+ ret = parse_cluster(c, package_id, i, depth + 1);
+ if ( depth > 0 )
+ printk(XENLOG_WARNING
+ "WARNING: Topology for clusters of clusters not yet supported\n");
+ if ( ret != 0 )
+ return ret;
+ i++;
+ } while ( true );
+
+ /* Now check for cores */
+ i = 0;
+ do {
+ const struct dt_device_node *c;
+ char name[20];
+ int ret;
+
+ snprintf(name, sizeof(name), "core%u", i);
+ c = dt_find_child_node_by_name(cluster, name);
+
+ if ( !c )
+ break;
+
+ has_cores = true;
+
+ if ( depth == 0 )
+ {
+ printk(XENLOG_ERR
+ "ERROR: %s: cpu-map children should be clusters\n",
+ dt_node_name(c));
+ return -EINVAL;
+ }
+
+ if ( leaf )
+ {
+ ret = parse_core(c, package_id, cluster_id, core_id++);
+ if ( ret != 0 )
+ return ret;
+ }
+ else
+ {
+ printk(XENLOG_ERR "ERROR: %s: Non-leaf cluster with core %s\n",
+ dt_node_name(cluster), name);
+ return -EINVAL;
+ }
+
+ i++;
+ } while ( true );
+
+ if ( leaf && !has_cores )
+ printk(XENLOG_WARNING "WARNING: %s: empty cluster\n",
+ dt_node_name(cluster));
+
+ return 0;
+}
+
+static int __init parse_socket(const struct dt_device_node *socket)
+{
+ bool has_socket = false;
+ unsigned int package_id = 0;
+ int ret;
+
+ do {
+ const struct dt_device_node *c;
+ char name[20];
+
+ snprintf(name, sizeof(name), "socket%u", package_id);
+ c = dt_find_child_node_by_name(socket, name);
+
+ if ( !c )
+ break;
+
+ has_socket = true;
+ ret = parse_cluster(c, package_id, invalid_topo_id, 0);
+ if ( ret != 0 )
+ return ret;
+
+ package_id++;
+ } while ( true );
+
+ if ( !has_socket )
+ ret = parse_cluster(socket, 0, invalid_topo_id, 0);
+
+ return ret;
+}
+
+/*
+ * Generate cpu topology information when cpu-map node doesn't exist.
+ * It assumes that the cpu doesn't have SMT and all CPUs on a NUMA
+ * node belong to the same socket.
+ */
+static void __init fixup_topology(void)
+{
+ unsigned int cpu;
+ unsigned int clid = 0;
+ unsigned int pkgid = 0;
+
+ for_each_possible_cpu(cpu)
+ {
+ struct cpu_map *map = &cpu_map[cpu];
+
+ map->package_id = cpu_to_node(cpu);
+ if ( map->package_id != pkgid )
+ {
+ pkgid = map->package_id;
+ clid = 0;
+ }
+ map->cluster_id = clid++;
+ map->core_id = 0;
+ map->thread_id = 0;
+ }
+}
+
+int __init parse_dt_topology(void)
+{
+ const struct dt_device_node *cpus;
+ const struct dt_device_node *map;
+
+ cpus = dt_find_node_by_path("/cpus");
+ if ( !cpus )
+ {
+ printk(XENLOG_ERR "ERROR: No CPU information found in DT\n");
+ return -EINVAL;
+ }
+
+ map = dt_find_child_node_by_name(cpus, "cpu-map");
+ if ( !map )
+ return -ENOENT;
+
+ return parse_socket(map);
+}
+
+void __init dt_init_cpu_topology(void)
+{
+ unsigned int cpu;
+
+ BUG_ON(!acpi_disabled);
+ BUG_ON(!cpu_topology);
+
+ if ( parse_dt_topology() )
+ fixup_topology();
+
+ for_each_possible_cpu(cpu)
+ setup_siblings_masks(cpu);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/drivers/acpi/Kconfig b/xen/drivers/acpi/Kconfig
index e3f3d8f4b1..5277b7bf83 100644
--- a/xen/drivers/acpi/Kconfig
+++ b/xen/drivers/acpi/Kconfig
@@ -8,3 +8,6 @@ config ACPI_LEGACY_TABLES_LOOKUP
config ACPI_NUMA
bool
select NUMA
+
+config ACPI_CPU_TOPOLOGY
+ bool
diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile
index 477408afbe..bcdb1b5e1b 100644
--- a/xen/drivers/acpi/Makefile
+++ b/xen/drivers/acpi/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_PM_OP) += pm-op.o
obj-$(CONFIG_X86) += hwregs.o
obj-$(CONFIG_X86) += reboot.o
+obj-$(CONFIG_ACPI_CPU_TOPOLOGY) += topology.init.o
diff --git a/xen/drivers/acpi/topology.c b/xen/drivers/acpi/topology.c
new file mode 100644
index 0000000000..6bd2d96ebb
--- /dev/null
+++ b/xen/drivers/acpi/topology.c
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <xen/acpi.h>
+#include <xen/cpu-topology.h>
+#include <xen/cpumask.h>
+#include <xen/init.h>
+
+/*
+ * TODO: Populate the topology information by scanning the ACPI
+ * PPTT (Processor Properties Topology Table).
+ */
+void __init acpi_init_cpu_topology(void)
+{
+ unsigned int cpu;
+
+ /*
+ * Generate temporary cpu topology information for now.
+ * It assumes that the cpu doesn't have SMT and all CPUs
+ * belong to the same socket.
+ */
+ for_each_possible_cpu(cpu)
+ {
+ struct cpu_topology *topo = &cpu_topology[cpu];
+
+ cpumask_set_cpu(cpu, topo->thread_sibling);
+ cpumask_copy(topo->core_sibling, &cpu_possible_map);
+ }
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h
index 2fdf38cf74..d531c76370 100644
--- a/xen/include/xen/acpi.h
+++ b/xen/include/xen/acpi.h
@@ -101,6 +101,8 @@ void acpi_table_print (struct acpi_table_header *header, unsigned long phys_addr
void acpi_table_print_madt_entry (struct acpi_subtable_header *madt);
void acpi_table_print_srat_entry (struct acpi_subtable_header *srat);
+void acpi_init_cpu_topology(void);
+
/* the following four functions are architecture-dependent */
void acpi_numa_slit_init (struct acpi_table_slit *slit);
void acpi_numa_processor_affinity_init(const struct acpi_srat_cpu_affinity *);
diff --git a/xen/include/xen/cpu-topology.h b/xen/include/xen/cpu-topology.h
new file mode 100644
index 0000000000..f64820febf
--- /dev/null
+++ b/xen/include/xen/cpu-topology.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef XEN_CPU_TOPOLOGY_H
+#define XEN_CPU_TOPOLOGY_H
+
+#include <xen/cpumask.h>
+
+#ifdef CONFIG_GENERIC_CPU_TOPOLOGY
+
+struct cpu_topology {
+ cpumask_var_t thread_sibling;
+ cpumask_var_t core_sibling;
+ cpumask_var_t cluster_sibling;
+};
+
+extern struct cpu_topology *cpu_topology;
+void init_cpu_topology(void);
+
+#else /* CONFIG_GENERIC_CPU_TOPOLOGY */
+
+#define cpu_topology ((struct cpu_topology *)NULL)
+static inline void init_cpu_topology(void) {}
+
+#endif /* CONFIG_GENERIC_CPU_TOPOLOGY */
+
+#endif /* XEN_CPU_TOPOLOGY_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/dt-cpu-topology.h b/xen/include/xen/dt-cpu-topology.h
new file mode 100644
index 0000000000..cb8a266479
--- /dev/null
+++ b/xen/include/xen/dt-cpu-topology.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef XEN_DT_CPU_TOPOLOGY_H
+#define XEN_DT_CPU_TOPOLOGY_H
+
+struct dt_device_node;
+
+#ifdef CONFIG_DT_CPU_TOPOLOGY
+
+void map_cpu_to_dt_node(unsigned int cpu, struct dt_device_node *cpu_node);
+void dt_init_cpu_topology(void);
+
+#else /* CONFIG_DT_CPU_TOPOLOGY */
+
+static inline void map_cpu_to_dt_node(unsigned int cpu,
+ struct dt_device_node *cpu_node) {}
+
+#endif /* CONFIG_DT_CPU_TOPOLOGY */
+
+#endif /* XEN_DT_CPU_TOPOLOGY_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.43.0On 03.07.2026 11:15, Hirokazu Takahashi wrote:
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -188,6 +188,24 @@ config VM_EVENT
> config NEEDS_LIBELF
> bool
>
> +config HAS_GENERIC_CPU_TOPOLOGY
> + bool
A few lines up from here you'll find a long, sorted list of HAS_*. Please
insert this one at the appropriate spot there.
> +config DT_CPU_TOPOLOGY
> + bool
> +
> +config GENERIC_CPU_TOPOLOGY
> + bool "CPU topology support (UNSUPPORTED)" if UNSUPPORTED
> + depends on HAS_GENERIC_CPU_TOPOLOGY
> + select DT_CPU_TOPOLOGY if DEVICE_TREE_PARSE
> + select ACPI_CPU_TOPOLOGY if ACPI
I wonder what others think here; to me this looks backwards. "Generic" ought
to be selected by the more specific variants.
> + help
> + Retrieve CPU topology information from the device tree to optimize
> + virtual CPU scheduling.
This is DT help text, not generic one. Altogether something like the below?
config GENERIC_CPU_TOPOLOGY
bool
config DT_CPU_TOPOLOGY
bool "CPU topology support (UNSUPPORTED)" if UNSUPPORTED
depends on HAS_GENERIC_CPU_TOPOLOGY && DEVICE_TREE_PARSE
select GENERIC_CPU_TOPOLOGY
help
Retrieve CPU topology information from the device tree to optimize
virtual CPU scheduling.
(And then similarly for ACPI.)
> --- /dev/null
> +++ b/xen/common/cpu-topology.c
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include <xen/acpi.h>
> +#include <xen/cpu-topology.h>
> +#include <xen/cpumask.h>
> +#include <xen/dt-cpu-topology.h>
> +#include <xen/init.h>
> +
> +struct cpu_topology *__ro_after_init cpu_topology;
With both functions below being __init, this is the sole reason why the
file cannot (also) be built into cpu-topology.init.o. Perhaps this would
better move into e.g. cpu.c (inside an #ifdef there, of course)?
> --- /dev/null
> +++ b/xen/common/device-tree/cpu-topology.c
> @@ -0,0 +1,355 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Derived from Linux kernel 7.0's $drivers/base/arch_topology.c
> + * Parse cpu topology information.
> + */
> +
> +#include <xen/acpi.h>
> +#include <xen/cpu-topology.h>
> +#include <xen/cpumask.h>
> +#include <xen/device_tree.h>
> +#include <xen/errno.h>
> +#include <xen/init.h>
> +#include <xen/numa.h>
> +
> +struct cpu_map {
> + unsigned int thread_id;
> + unsigned int core_id;
> + unsigned int cluster_id;
> + unsigned int package_id;
> +};
> +
> +static const unsigned int __initdata invalid_topo_id = ~0U;
To better fit with the rest of the codebase, I think this will want to be
#define INVALID_TOPO_ID (~0U)
> +static struct cpu_map __initdata cpu_map[NR_CPUS] = {
> + [0 ... NR_CPUS - 1] = {invalid_topo_id, invalid_topo_id, invalid_topo_id, 0}
> +};
Why would 0 need spelling out? And why is it 0 for package_id, but
invalid_topo_id for everything else? (IOW: There may want to be a comment
here.)
> +static struct dt_device_node *__initdata dt_cpu_table[NR_CPUS];
> +
> +static void __init setup_siblings_masks(unsigned int target_cpu)
> +{
> + const struct cpu_topology *target_topo = &cpu_topology[target_cpu];
> + const struct cpu_map *target_map = &cpu_map[target_cpu];
> + unsigned int cpu;
> +
> + /* Update core and thread sibling masks */
> + for_each_possible_cpu(cpu)
> + {
> + const struct cpu_topology *cpu_topo = &cpu_topology[cpu];
> + const struct cpu_map *map = &cpu_map[cpu];
> +
> + if ( target_map->package_id != map->package_id )
> + continue;
> +
> + cpumask_set_cpu(target_cpu, cpu_topo->core_sibling);
> + cpumask_set_cpu(cpu, target_topo->core_sibling);
> +
> + if ( target_map->cluster_id != map->cluster_id )
> + continue;
> +
> + if ( target_map->cluster_id != invalid_topo_id )
> + {
> + cpumask_set_cpu(target_cpu, cpu_topo->cluster_sibling);
> + cpumask_set_cpu(cpu, target_topo->cluster_sibling);
> + }
> +
> + if ( target_map->core_id != map->core_id )
> + continue;
> +
> + cpumask_set_cpu(target_cpu, cpu_topo->thread_sibling);
> + cpumask_set_cpu(cpu, target_topo->thread_sibling);
> + }
> +}
> +
> +static struct dt_device_node *__init
> + dt_find_child_node_by_name(const struct dt_device_node *dt,
Bogus indentation (the function name wants to start at line start when
wrapped like this).
> + const char *name)
> +{
> + struct dt_device_node *np;
> +
> + dt_for_each_child_node(dt, np)
> + if ( np->name && (dt_node_cmp(np->name, name) == 0) )
> + break;
> +
> + return np;
> +}
I'm not a DT person, so I don't know how likely it is that the
implementation of dt_for_each_child_node(), to e.g. be similar to
list_for_each*(). Here you're assuming np to be NULL upon loop exit,
which however is an implementation detail of the macro.
> +void __init map_cpu_to_dt_node(unsigned int cpu,
> + struct dt_device_node *cpu_node)
> +{
> + if ( cpu < ARRAY_SIZE(dt_cpu_table) )
> + dt_cpu_table[cpu] = cpu_node;
> + else
> + printk(XENLOG_WARNING
> + "cpu id %u exceeds the max cores %lu\n",
> + cpu, ARRAY_SIZE(dt_cpu_table));
There's still "id" here, and "cores" also neither fits dt_cpu_table[],
nor is it an arch-independent synonym for "CPU" (and you use the term
for another purpose further down). %lu also isn't exactly the format to
use with size_t arguments.
> +static unsigned int __init cpu_node_to_id(const struct dt_device_node *cpu_node)
> +{
> + unsigned int cpu;
> +
> + for_each_possible_cpu(cpu)
> + if ( cpu_node == dt_cpu_table[cpu] )
> + return cpu;
> +
> + return invalid_topo_id;
> +}
> +
> +/*
> + * This function returns the logical cpu number of the DT node.
> + */
"logical" meaning what exactly here? To be unambiguous, maybe better "Xen
CPU number"?
> +static unsigned int __init
> + get_cpu_for_node(const struct dt_device_node *dt_node)
Bad indentation again.
> +static int __init parse_core(const struct dt_device_node *core,
> + unsigned int package_id,
> + unsigned int cluster_id,
> + unsigned int core_id)
> +{
> + bool leaf = true;
> + unsigned int i = 0;
> + unsigned int cpu;
> +
> + do {
> + const struct dt_device_node *t;
> + char name[20];
> +
> + snprintf(name, sizeof(name), "thread%u", i);
> + t = dt_find_child_node_by_name(core, name);
> +
> + if ( !t )
> + break;
> +
> + leaf = false;
> + cpu = get_cpu_for_node(t);
> + if ( cpu != invalid_topo_id )
> + {
> + cpu_map[cpu].package_id = package_id;
> + cpu_map[cpu].cluster_id = cluster_id;
> + cpu_map[cpu].core_id = core_id;
> + cpu_map[cpu].thread_id = i;
> + }
> + else
> + {
> + printk(XENLOG_ERR
> + "ERROR: %s: Can't get CPU for thread\n", dt_node_name(t));
> + return -EINVAL;
> + }
Less indentation is needed with
if ( cpu == invalid_topo_id )
{
printk(XENLOG_ERR
"ERROR: %s: Can't get CPU for thread\n", dt_node_name(t));
return -EINVAL;
}
cpu_map[cpu].package_id = package_id;
cpu_map[cpu].cluster_id = cluster_id;
cpu_map[cpu].core_id = core_id;
cpu_map[cpu].thread_id = i;
Also should you maybe assert up front that the slot is still vacant?
> + i++;
> + } while ( true );
Why not the more conventional
for ( i = 0; ; ++i )
? Then it will also be possible to use "continue" inside the loop (if ever
needed). Same elsewhere.
> +/*
> + * Generate cpu topology information when cpu-map node doesn't exist.
> + * It assumes that the cpu doesn't have SMT and all CPUs on a NUMA
> + * node belong to the same socket.
> + */
> +static void __init fixup_topology(void)
> +{
> + unsigned int cpu;
> + unsigned int clid = 0;
> + unsigned int pkgid = 0;
> +
> + for_each_possible_cpu(cpu)
> + {
> + struct cpu_map *map = &cpu_map[cpu];
> +
> + map->package_id = cpu_to_node(cpu);
> + if ( map->package_id != pkgid )
> + {
> + pkgid = map->package_id;
> + clid = 0;
> + }
> + map->cluster_id = clid++;
> + map->core_id = 0;
> + map->thread_id = 0;
> + }
> +}
The comment ahead of the function talks of NUMA nodes, but the function
shows no NUMA-awareness.
> +int __init parse_dt_topology(void)
static?
> --- /dev/null
> +++ b/xen/drivers/acpi/topology.c
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include <xen/acpi.h>
> +#include <xen/cpu-topology.h>
> +#include <xen/cpumask.h>
> +#include <xen/init.h>
> +
> +/*
> + * TODO: Populate the topology information by scanning the ACPI
> + * PPTT (Processor Properties Topology Table).
> + */
> +void __init acpi_init_cpu_topology(void)
> +{
> + unsigned int cpu;
> +
> + /*
> + * Generate temporary cpu topology information for now.
> + * It assumes that the cpu doesn't have SMT and all CPUs
> + * belong to the same socket.
> + */
> + for_each_possible_cpu(cpu)
> + {
> + struct cpu_topology *topo = &cpu_topology[cpu];
> +
> + cpumask_set_cpu(cpu, topo->thread_sibling);
> + cpumask_copy(topo->core_sibling, &cpu_possible_map);
> + }
What about ->cluster_sibling? That can't remain all empty, can it?
> --- /dev/null
> +++ b/xen/include/xen/cpu-topology.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef XEN_CPU_TOPOLOGY_H
> +#define XEN_CPU_TOPOLOGY_H
> +
> +#include <xen/cpumask.h>
> +
> +#ifdef CONFIG_GENERIC_CPU_TOPOLOGY
> +
> +struct cpu_topology {
> + cpumask_var_t thread_sibling;
> + cpumask_var_t core_sibling;
> + cpumask_var_t cluster_sibling;
> +};
> +
> +extern struct cpu_topology *cpu_topology;
> +void init_cpu_topology(void);
> +
> +#else /* CONFIG_GENERIC_CPU_TOPOLOGY */
> +
> +#define cpu_topology ((struct cpu_topology *)NULL)
As before: I consider it dangerous to expose an identifier which is
only ever NULL. The extern should be moved out of the #ifdef, and
DCE should be leveraged to eliminate all references when
GENERIC_CPU_TOPOLOGY=n.
Jan
© 2016 - 2026 Red Hat, Inc.