[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Hirokazu Takahashi <taka@xxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: "Orzel, Michal" <michal.orzel@xxxxxxx>
  • Date: Thu, 17 Sep 2026 14:15:52 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=temperror (sender ip is 165.204.84.17) smtp.rcpttodomain=valinux.co.jp smtp.mailfrom=amd.com; dmarc=temperror action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=u8O8o+6GognuIehsqoEsV+kIhBgi+XgprtCO7CZ1tAg=; b=jS2ZrkrK46djc8R9aN/uKnMlvengEb6eiRHIFxOon9Gf6jhEBwCD9ALMW4ZUQfiVIkjSUseV8nwxDdfIqZWY3iOimLSm+ky3zu/FAb4MPsfoK626pNDY7GFz2n6pSLcPY3h5MnTi/jesZnmCmG2lqo6hyWP36//sncGtBsaZyPSH7ZIoBt+W2a12ZVcEFd7ku+nznhLh736/pnWMztinM73dN3CaBoFZMlztx5ontk9AYydSY1FNuXrl2vhLz41AWnDYXTIW6QG7fQRhggnt6Txtmew1Y/39P6wKMA+U2Dz+hKggsCpNVhdvsZsura7LWm/Dvvu29nKZj3mS1ZpOJg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=IGlDCTHqdiaYCJ1zHnb7i+/Q36sp4C/NZLAJxmkvaIoGHB9aiOS2cAPSZQm9r4WNBdcvSW59mQa47z9acCkCaTiDze5P5qfMpTgu3/V6pcT0AGrBZULN4+qD9hx+r3FPTWb4A6/iUjb4YTZeEcUFDA3teRL7w4ba/FqsmMnzjfnq9vGGVKXHVSAsc/SMx2CItl6ovQUr5uN5ziofvJ/wlXI/ZmW1En2isGeLtHUKancV9PwKIlL8F5IYW7UJ+gPTHXzOviSqFGH0q8EmFbyRANvH5Iy4El0bI557GFXljDKMjyKt7kcG/UXzR6u3EdIOtf3FLdyGG9mOtvCqBjaPGQ==
  • Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=selector1 header.d=amd.com header.i="@amd.com" header.h="From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck"
  • Cc: <Mykyta_Poturai@xxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Roger Pau Monné <roger@xxxxxxxxxxxxxx>
  • Delivery-date: Thu, 17 Sep 2026 12:16:06 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


On 11-Sep-26 11:42, Hirokazu Takahashi wrote:
> Parse the 'cpu-map' node in the Device Tree to extract CPU topology
> information. If the 'cpu-map' node is absent, simply ignore it.
> 
> Signed-off-by: Hirokazu Takahashi <taka@xxxxxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> # common, acpi
> ---
> Changes in v10:
>  - Postpone the invocation of init_cpu_topology() until after nr_cpu_ids
>    is finalized.
>  - Improve the Device Tree cpu-map node parsing logic:
>    * Rename functions and variables to accurately reflect the DT nodes
>      being processed.
>    * Simplify the parsing algorithm by eliminating recursive calls and
>      avoiding passing INVALID_TOPO_ID as an argument.
>  - Unify all license headers to GPL-2.0-only.
>  - Return an error instead of triggering an ASSERT() when duplicate CPU
>    definitions are detected.
>  - Replace BUG_ON() with ASSERT() for condition checks in
>    dt_init_cpu_topology().
>  - Update commit messages to remove details that no longer match the
>    current implementation.
> 
>  xen/arch/arm/Kconfig                  |   1 +
>  xen/arch/arm/setup.c                  |   3 +
>  xen/arch/arm/smpboot.c                |   5 +
>  xen/common/Kconfig                    |  22 ++
>  xen/common/Makefile                   |   1 +
>  xen/common/cpu-topology.c             |  62 +++++
>  xen/common/cpu.c                      |   5 +
>  xen/common/device-tree/Makefile       |   1 +
>  xen/common/device-tree/cpu-topology.c | 347 ++++++++++++++++++++++++++
>  xen/drivers/acpi/Makefile             |   1 +
>  xen/drivers/acpi/topology.c           |  41 +++
>  xen/include/xen/acpi.h                |  13 +
>  xen/include/xen/cpu-topology.h        |  34 +++
>  xen/include/xen/dt-cpu-topology.h     |  35 +++
>  14 files changed, 571 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 843a43897e..1e0fd4957e 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -19,6 +19,7 @@ config ARM
>       select HAS_ALTERNATIVE if HAS_VMAP
>       select HAS_DEVICE_TREE_DISCOVERY
>       select HAS_DOM0LESS
> +     select HAS_GENERIC_CPU_TOPOLOGY
>       select HAS_GRANT_CACHE_FLUSH if GRANT_TABLE
>       select HAS_STACK_PROTECTOR
>       select HAS_STATIC_MEMORY
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 86532d0a35..108f1c703e 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -10,6 +10,7 @@
>  
>  #include <xen/bootinfo.h>
>  #include <xen/compile.h>
> +#include <xen/cpu-topology.h>
>  #include <xen/device_tree.h>
>  #include <xen/dom0less-build.h>
>  #include <xen/domain_page.h>
> @@ -387,6 +388,8 @@ void asmlinkage __init noreturn start_xen(unsigned long 
> fdt_paddr)
>      nr_cpu_ids = smp_get_max_cpus();
>      printk(XENLOG_INFO "SMP: Allowing %u CPUs\n", nr_cpu_ids);
>  
> +    init_cpu_topology();
> +
>      /*
>       * Some errata relies on SMCCC version which is detected by psci_init()
>       * (called from smp_init_cpus()).
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 1806c47a08..904130efdf 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>
> @@ -244,6 +246,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);
This should be moved to the above else branch which is taken on a success only.

>      }
>  
>      if ( !bootcpu_valid )
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index da80fdba84..29879a9131 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -140,6 +140,9 @@ config HAS_EX_TABLE
>  config HAS_FAST_MULTIPLY
>       bool
>  
> +config HAS_GENERIC_CPU_TOPOLOGY
> +     bool
> +
>  config HAS_IOPORTS
>       bool
>  
> @@ -191,6 +194,25 @@ config VM_EVENT
>  config NEEDS_LIBELF
>       bool
>  
> +config GENERIC_CPU_TOPOLOGY
> +     bool
> +
> +config DT_CPU_TOPOLOGY
> +     bool "Device tree based CPU topology support (UNSUPPORTED)"
Can you please explain why unsupported?

> +     depends on HAS_GENERIC_CPU_TOPOLOGY && DEVICE_TREE_PARSE && UNSUPPORTED
> +     select GENERIC_CPU_TOPOLOGY
> +     help
> +       Retrieve CPU topology information from the device tree to optimize
> +       vCPU scheduling.
> +
> +config ACPI_CPU_TOPOLOGY
> +     bool "ACPI based CPU topology support (UNSUPPORTED)"
> +     depends on HAS_GENERIC_CPU_TOPOLOGY && ACPI && UNSUPPORTED
> +     select GENERIC_CPU_TOPOLOGY
> +     help
> +       Retrieve CPU topology information from the ACPI PPTT to optimize
> +       vCPU scheduling.
> +
>  config NUMA
>       bool
>  
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index 6018e25614..901bb37925 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.init.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..f784afbd3d
> --- /dev/null
> +++ b/xen/common/cpu-topology.c
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/acpi.h>
> +#include <xen/cpu-topology.h>
> +#include <xen/cpumask.h>
> +#include <xen/dt-cpu-topology.h>
> +#include <xen/init.h>
> +#include <xen/xvmalloc.h>
> +
> +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);
> +    }
> +
> +    XVFREE(cpu_topology);
> +}
> +
> +void __init init_cpu_topology(void)
> +{
> +    unsigned int cpu;
> +    int ret;
> +
> +    cpu_topology = xvzalloc_array(struct cpu_topology, nr_cpu_ids);
> +    if ( !cpu_topology )
> +        return;
I don't think this is ok for this function not to let the caller about the
error. That's especially important for failures from actual DT/ACPI topology
initialization. You don't even print anything.

> +
> +    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();
> +            return;
> +        }
> +    }
> +
> +    if ( acpi_disabled )
> +        ret = dt_init_cpu_topology();
> +    else
> +        ret = acpi_init_cpu_topology();
> +
> +    /* Free the CPU topology table if initialization fails. */
> +    if ( ret != 0 )
> +        free_topology_table();
> +}
> +
> +/*
> + * 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/cpu.c b/xen/common/cpu.c
> index f09af0444b..9591ada60a 100644
> --- a/xen/common/cpu.c
> +++ b/xen/common/cpu.c
> @@ -1,5 +1,6 @@
>  #include <xen/cpumask.h>
>  #include <xen/cpu.h>
> +#include <xen/cpu-topology.h>
>  #include <xen/event.h>
>  #include <xen/init.h>
>  #include <xen/sched.h>
> @@ -46,6 +47,10 @@ const unsigned long 
> cpu_bit_bitmap[BITS_PER_LONG+1][BITS_TO_LONGS(NR_CPUS)] = {
>  #undef MASK_DECLARE_2
>  #undef MASK_DECLARE_1
>  
> +#ifdef CONFIG_GENERIC_CPU_TOPOLOGY
> +struct cpu_topology *__ro_after_init cpu_topology;
> +#endif /* CONFIG_GENERIC_CPU_TOPOLOGY */
> +
>  static DEFINE_RWLOCK(cpu_add_remove_lock);
>  
>  bool get_cpu_maps(void)
> 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..ce9da0bb36
> --- /dev/null
> +++ b/xen/common/device-tree/cpu-topology.c
> @@ -0,0 +1,347 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * 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>
> +
> +#define INVALID_TOPO_ID (~0U)
> +
> +struct cpu_map {
> +    unsigned int thread_id;
> +    unsigned int core_id;
> +    unsigned int cluster_id;
> +    unsigned int socket_id;
> +};
> +
> +static struct cpu_map __initdata cpu_map[NR_CPUS] = {
> +    [0 ... NR_CPUS - 1] = {
> +        .thread_id = INVALID_TOPO_ID,
> +        .core_id = INVALID_TOPO_ID,
> +        .cluster_id = INVALID_TOPO_ID,
> +        .socket_id = INVALID_TOPO_ID,
> +    },
> +};
> +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 cluster, 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_cpu > cpu )
> +            continue;
> +
> +        if ( target_map->socket_id != map->socket_id )
> +            continue;
> +
> +        cpumask_set_cpu(target_cpu, cpu_topo->core_sibling);
> +        cpumask_set_cpu(cpu, target_topo->core_sibling);
This does not compile.

> +
> +        if ( target_map->cluster_id != map->cluster_id )
> +            continue;
> +
> +        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 const struct dt_device_node *__init dt_find_child_node_by_name(
> +    const struct dt_device_node *dt,
> +    const char *name)
> +{
> +    const struct dt_device_node *np;
> +
> +    dt_for_each_child_node(dt, np)
> +        if ( np->name && (dt_node_cmp(np->name, name) == 0) )
> +            return np;
Please use dt_node_name_is_equal() here.

> +
> +    return NULL;
> +}
> +
> +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 %u exceeds the max cpus %zu\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 Xen 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 bool __init is_cpu_map_empty( unsigned int cpu )
Stray space at the end "cpu )"

> +{
> +    return (cpu_map[cpu].socket_id == INVALID_TOPO_ID) &&
> +           (cpu_map[cpu].cluster_id == INVALID_TOPO_ID) &&
> +           (cpu_map[cpu].core_id == INVALID_TOPO_ID) &&
> +           (cpu_map[cpu].thread_id == INVALID_TOPO_ID);
> +}
> +
> +static int __init parse_core(const struct dt_device_node *core,
> +                             unsigned int socket_id,
> +                             unsigned int cluster_id,
> +                             unsigned int core_id)
> +{
> +    bool leaf = true;
> +    unsigned int thread_id;
> +    unsigned int cpu;
> +
> +    for ( thread_id = 0; ; thread_id++ )
> +    {
> +        const struct dt_device_node *thread;
> +        char name[20];
> +
> +        snprintf(name, sizeof(name), "thread%u", thread_id);
> +        thread = dt_find_child_node_by_name(core, name);
> +
> +        if ( !thread )
> +            break;
> +
> +        leaf = false;
> +        cpu = get_cpu_for_node(thread);
> +
> +        if ( cpu == INVALID_TOPO_ID )
> +        {
> +            printk(XENLOG_ERR
> +                   "ERROR: %s: Can't get CPU for thread\n", 
> dt_node_name(thread));
No need for the ERROR/WARNING prefixes. You already use correct xenlog levels.

> +            return -EINVAL;
> +        }
> +
> +        if ( !is_cpu_map_empty(cpu) )
> +        {
> +            printk(XENLOG_ERR
> +                   "ERROR: Duplicate CPU definition for CPU%u\n", cpu);
> +            return -EINVAL;
> +        }
> +
> +        cpu_map[cpu].socket_id = socket_id;
> +        cpu_map[cpu].cluster_id = cluster_id;
> +        cpu_map[cpu].core_id = core_id;
> +        cpu_map[cpu].thread_id = thread_id;
> +    }
> +
> +    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;
> +        }
> +
> +        if ( !is_cpu_map_empty(cpu) )
> +        {
> +            printk(XENLOG_ERR
> +                   "ERROR: Duplicate CPU definition for CPU%u\n", cpu);
> +            return -EINVAL;
> +        }
> +
> +        cpu_map[cpu].socket_id = socket_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 socket_id,
> +                                unsigned int cluster_id)
> +{
> +    bool has_cores = false;
> +    int ret = 0;
> +
> +    if ( dt_find_child_node_by_name(cluster, "cluster0") )
> +    {
> +        printk(XENLOG_WARNING
XENLOG_ERROR?

> +               "WARNING: Topology for clusters of clusters not yet 
> supported\n");
> +        return -EINVAL;
> +    }
> +
> +    for ( unsigned int core_id = 0; ; core_id++ )
> +    {
> +        const struct dt_device_node *core;
> +        char name[20];
> +
> +        snprintf(name, sizeof(name), "core%u", core_id);
> +        core = dt_find_child_node_by_name(cluster, name);
> +
> +        if ( !core )
> +            break;
> +
> +        has_cores = true;
> +
> +        ret = parse_core(core, socket_id, cluster_id, core_id);
> +        if ( ret != 0 )
> +            return ret;
> +    }
> +
> +    if ( !has_cores )
> +        printk(XENLOG_WARNING "WARNING: %s: empty cluster\n",
> +               dt_node_name(cluster));
> +
> +    return ret;
> +}
> +
> +static int __init parse_socket(const struct dt_device_node *socket,
> +                               unsigned int socket_id)
> +{
> +    bool has_cluster = false;
> +    int ret = 0;
> +
> +    for ( unsigned int cluster_id = 0; ; cluster_id++ )
> +    {
> +        const struct dt_device_node *cluster;
> +        char name[20];
> +
> +        snprintf(name, sizeof(name), "cluster%u", cluster_id);
> +        cluster = dt_find_child_node_by_name(socket, name);
> +
> +        if ( !cluster )
> +            break;
> +
> +        has_cluster = true;
> +        ret = parse_cluster(cluster, socket_id, cluster_id);
> +        if ( ret != 0 )
> +            return ret;
> +    }
> +
> +    /*
> +     * If no cluster node is defined, assume the socket has
> +     * a single cluster.
> +     */
> +    if ( !has_cluster )
> +        ret = parse_cluster(socket, socket_id, 0);
> +
> +    return ret;
> +}
> +
> +static int __init parse_package(const struct dt_device_node *package)
> +{
> +    bool has_socket = false;
> +    int ret = 0;
> +
> +    for ( unsigned int socket_id = 0; ; socket_id++ )
> +    {
> +        const struct dt_device_node *socket;
> +        char name[20];
> +
> +        snprintf(name, sizeof(name), "socket%u", socket_id);
> +        socket = dt_find_child_node_by_name(package, name);
> +
> +        if ( !socket )
> +            break;
> +
> +        has_socket = true;
> +        ret = parse_socket(socket, socket_id);
> +        if ( ret != 0 )
> +            return ret;
> +    }
> +
> +    /*
> +     * If no socket node is defined, assume all clusters reside under
> +     * a single socket.
> +     */
> +    if ( !has_socket )
> +        ret = parse_socket(package, 0);
> +
> +    return ret;
> +}
> +
> +static 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 )
> +        return -ENOENT;
> +
> +    map = dt_find_child_node_by_name(cpus, "cpu-map");
> +    if ( !map )
> +        return -ENOENT;
> +
> +    return parse_package(map);
Linux ends this function with for_each_possible_cpu(). What's the reason for
dropping it for our case?

> +}
> +
> +int __init dt_init_cpu_topology(void)
> +{
> +    unsigned int cpu;
> +    int ret;
> +
> +    ASSERT(acpi_disabled);
> +    ASSERT(cpu_topology);
> +
> +    ret = parse_dt_topology();
> +    if ( ret == 0 )
> +        for_each_possible_cpu(cpu)
> +            setup_siblings_masks(cpu);
> +
> +    return ret;
> +}
> +
> +/*
> + * 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/Makefile b/xen/drivers/acpi/Makefile
> index 477408afbe..6d676e91d4 100644
> --- a/xen/drivers/acpi/Makefile
> +++ b/xen/drivers/acpi/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_ACPI_NUMA) += numa.o
>  obj-y += osl.o
>  obj-$(CONFIG_PM_STATS) += pmstat.o
>  obj-$(CONFIG_PM_OP) += pm-op.o
> +obj-$(CONFIG_ACPI_CPU_TOPOLOGY) += topology.init.o
>  
>  obj-$(CONFIG_X86) += hwregs.o
>  obj-$(CONFIG_X86) += reboot.o
> diff --git a/xen/drivers/acpi/topology.c b/xen/drivers/acpi/topology.c
> new file mode 100644
> index 0000000000..090c793f33
> --- /dev/null
> +++ b/xen/drivers/acpi/topology.c
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#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).
> + */
> +int __init acpi_init_cpu_topology(void)
> +{
> +    unsigned int cpu;
The correspondong DT function has two ASSERTs. Why the divergence here? At least
the one for cpu_topology existing is a good one (I don't consider the first
ASSERT very useful).

> +
> +    /*
> +     * 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);
> +        cpumask_copy(topo->cluster_sibling, &cpu_possible_map);
> +    }
> +
> +    return 0;
> +}
> +
> +/*
> + * 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..cbb02e0f35 100644
> --- a/xen/include/xen/acpi.h
> +++ b/xen/include/xen/acpi.h
> @@ -135,6 +135,19 @@ static inline int acpi_boot_table_init(void)
>  
>  #endif       /*!CONFIG_ACPI*/
>  
> +#ifdef CONFIG_ACPI_CPU_TOPOLOGY
> +
> +int acpi_init_cpu_topology(void);
> +
> +#else /* CONFIG_ACPI_CPU_TOPOLOGY */
> +
> +static inline int acpi_init_cpu_topology(void)
> +{
> +    return -EOPNOTSUPP;
> +}
> +
> +#endif /* CONFIG_ACPI_CPU_TOPOLOGY */
> +
>  int get_cpu_id(u32 acpi_id);
>  
>  unsigned int acpi_register_gsi (u32 gsi, int edge_level, int 
> active_high_low);
> diff --git a/xen/include/xen/cpu-topology.h b/xen/include/xen/cpu-topology.h
> new file mode 100644
> index 0000000000..7cfe3752cd
> --- /dev/null
> +++ b/xen/include/xen/cpu-topology.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef XEN_CPU_TOPOLOGY_H
> +#define XEN_CPU_TOPOLOGY_H
> +
> +#include <xen/cpumask.h>
You can move it under #ifdef where you actually use these types.

> +
> +#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 */
> +
> +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..72b35b3cf2
> --- /dev/null
> +++ b/xen/include/xen/dt-cpu-topology.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef XEN_DT_CPU_TOPOLOGY_H
> +#define XEN_DT_CPU_TOPOLOGY_H
> +
> +#include <xen/errno.h>
You can move it under #else which is where you actually use the errno code.

> +
> +struct dt_device_node;
> +
> +#ifdef CONFIG_DT_CPU_TOPOLOGY
> +
> +void map_cpu_to_dt_node(unsigned int cpu, struct dt_device_node *cpu_node);
> +int 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) {}
> +static inline int dt_init_cpu_topology(void)
> +{
> +    return -EOPNOTSUPP;
> +}
> +
> +#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:
> + */

~Michal




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.