Re: [PATCH 1/3] xen/device-tree: Parse 'cpu-map' node for CPU topology exploration

[Julien Grall <julien@xxxxxxx>]

Hi,

On 10/06/2026 12:13, Hirokazu Takahashi wrote:
> diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile
> index 9036e455d6..38bc5d5306 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.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..bbdf0d1fe8
> --- /dev/null
> +++ b/xen/common/device-tree/cpu-topology.c
> @@ -0,0 +1,342 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Derived from Linux kernel 7.0's $drivers/base/arch_topology.c
> + * Parse cpu topology information.
> + *
> + * Copyright (c) 2026 VA Linux Systems Japan K.K.
> + * Author: Hirokazu Takahashi <taka@xxxxxxxxxxxxx>
We don't commonly have copyright header in the tree and instead use the 
commit message to keep track of Copyright. That said, if you want to 
keep it, I think you ought to keep the copyright from Linux because your 
code is based on it.
> + */
> +
> +#include <xen/cpu.h>
> +#include <xen/cpumask.h>
> +#include <xen/delay.h>
> +#include <xen/device_tree.h>
> +#include <xen/cpu-topology.h>
> +#include <xen/numa.h>
> +#include <xen/domain_page.h>
> +#include <xen/errno.h>
> +#include <xen/init.h>
> +
> +struct cpu_map {
> +    unsigned int thread_id;
> +    unsigned int core_id;
> +    unsigned int cluster_id;
> +    unsigned int package_id;
> +};
> +
> +struct cpu_topology *cpu_topology;
Looking at the use in the other patch, you seem to unconditionally use 
cpu_topology when CONFIG_DT_CPU_TOPOLOGY. However, you don't seem to 
fill it when the system is using ACPI.
So I think this either needs to be moved to common code and filled by 
ACPI or we need to make clear in the name that this is DT specific.
[...]

> +void __init dt_init_cpu_topology(void)
> +{
> +    unsigned int cpu;
> +    const unsigned int nr_cpus = cpumask_last(&cpu_possible_map) + 1U;
> +
> +    cpu_topology = xzalloc_array(struct cpu_topology, nr_cpus);
> +    if ( !cpu_topology )
> +        panic("Failed to allocate memory for cpu_topology array\n");
> +
> +    if (parse_dt_topology())
Style: AFAICT, this is following Xen style. So it wants to be:

if ( ... )

> +        fixup_topology();
> +
> +    for_each_possible_cpu( cpu )
> +        setup_siblings_masks(cpu);
> +}
> diff --git a/xen/include/xen/cpu-topology.h b/xen/include/xen/cpu-topology.h
> new file mode 100644
> index 0000000000..1c03f4deaa
> --- /dev/null
> +++ b/xen/include/xen/cpu-topology.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef XEN_CPU_TOPOLOGY_H
> +#define XEN_CPU_TOPOLOGY_H
> +
> +#include <xen/types.h>
> +#include <xen/device_tree.h>
> +
> +struct cpu_topology {
> +    cpumask_t thread_sibling;
> +    cpumask_t core_sibling;
> +    cpumask_t cluster_sibling;
> +};
> +
> +
> +#ifdef CONFIG_DT_CPU_TOPOLOGY
> +
> +extern struct cpu_topology *cpu_topology;
> +void map_cpuid_to_node(unsigned int cpuid, struct dt_device_node *cpu_node);
> +void dt_init_cpu_topology(void);
> +
> +#elif CONFIG_DEVICE_TREE_PARSE
> +
> +static inline void map_cpuid_to_node(unsigned int cpuid, struct dt_device_node 
> *cpu_node) {}
> +static inline void dt_init_cpu_topology(void) {}
> +
> +#endif /* CONFIG_DEVICE_TREE_PARSE */
> +
> +#endif /* XEN_CPU_TOPOLOGY_H */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
--
Julien Grall