[PATCH] xen/arm: Fix sibling mask setup to only include online CPUs

[PATCH] xen/arm: Fix sibling mask setup to only include online CPUs

[Hirokazu Takahashi]

Populate the CPU sibling map only with CPUs that are brought
online. Currently, all CPUs in cpu_possible_map are added,
which leaves CPUs that failed to boot in the sibling masks and
may lead the scheduler to make incorrect decisions.

Fixes: 701190abb3d5 ("xen/arm: Set correct per-cpu cpu_core_mask")
Signed-off-by: Hirokazu Takahashi <taka@valinux.co.jp>
---
 xen/arch/arm/smpboot.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 8c9072f3c1..2d01896cf0 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -91,15 +91,24 @@ static int setup_cpu_sibling_map(int cpu)
          !zalloc_cpumask_var(&per_cpu(cpu_core_mask, cpu)) )
         return -ENOMEM;
 
+    return 0;
+}
+
+static void init_cpu_sibling_map(int cpu)
+{
+    unsigned int sibling;
+
     /*
      * Currently we assume there is no multithread and NUMA, so
      * a CPU is a sibling with itself, and the all possible CPUs
-     * are supposed to belong to the same socket (NUMA node).
+     * are supposed to belong to the same socket.
      */
     cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
-    cpumask_copy(per_cpu(cpu_core_mask, cpu), &cpu_possible_map);
+    cpumask_copy(per_cpu(cpu_core_mask, cpu), &cpu_online_map);
+    cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
 
-    return 0;
+    for_each_online_cpu(sibling)
+        cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, sibling));
 }
 
 static void remove_cpu_sibling_map(int cpu)
@@ -314,6 +323,7 @@ smp_prepare_cpus(void)
     if ( rc )
         panic("Unable to allocate CPU sibling/core maps\n");
 
+    init_cpu_sibling_map(0);
 }
 
 /* Boot the current CPU */
@@ -385,6 +395,8 @@ void asmlinkage noreturn start_secondary(void)
 
     set_current(idle_vcpu[cpuid]);
 
+    init_cpu_sibling_map(cpuid);
+
     /* Run local notifiers */
     notify_cpu_starting(cpuid);
     /*
-- 
2.43.0

Re: [PATCH] xen/arm: Fix sibling mask setup to only include online CPUs

[Volodymyr Babchuk]

Hello Hirokazu-san,

Hirokazu Takahashi <taka@valinux.co.jp> writes:

> Populate the CPU sibling map only with CPUs that are brought
> online. Currently, all CPUs in cpu_possible_map are added,
> which leaves CPUs that failed to boot in the sibling masks and
> may lead the scheduler to make incorrect decisions.

I think that this is a scheduler issue then. A CPU has siblings
regardless of if they are running or not. Also, your change will affect
CPU Hotplug functionality, as with your patch sibling mask will change
in runtime. But partially - only in cpu_up, but not on cpu_down. I am
not sure that schedulers can handle updates to siblings mask in runtime.

Could you please elaborate how it affects scheduler behaviour right now?

>
> Fixes: 701190abb3d5 ("xen/arm: Set correct per-cpu cpu_core_mask")
> Signed-off-by: Hirokazu Takahashi <taka@valinux.co.jp>
> ---
>  xen/arch/arm/smpboot.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 8c9072f3c1..2d01896cf0 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -91,15 +91,24 @@ static int setup_cpu_sibling_map(int cpu)
>           !zalloc_cpumask_var(&per_cpu(cpu_core_mask, cpu)) )
>          return -ENOMEM;
>  
> +    return 0;
> +}
> +
> +static void init_cpu_sibling_map(int cpu)
> +{
> +    unsigned int sibling;
> +
>      /*
>       * Currently we assume there is no multithread and NUMA, so
>       * a CPU is a sibling with itself, and the all possible CPUs
> -     * are supposed to belong to the same socket (NUMA node).
> +     * are supposed to belong to the same socket.
>       */
>      cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
> -    cpumask_copy(per_cpu(cpu_core_mask, cpu), &cpu_possible_map);
> +    cpumask_copy(per_cpu(cpu_core_mask, cpu), &cpu_online_map);
> +    cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
>  
> -    return 0;
> +    for_each_online_cpu(sibling)
> +        cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, sibling));
>  }
>  
>  static void remove_cpu_sibling_map(int cpu)
> @@ -314,6 +323,7 @@ smp_prepare_cpus(void)
>      if ( rc )
>          panic("Unable to allocate CPU sibling/core maps\n");
>  
> +    init_cpu_sibling_map(0);
>  }
>  
>  /* Boot the current CPU */
> @@ -385,6 +395,8 @@ void asmlinkage noreturn start_secondary(void)
>  
>      set_current(idle_vcpu[cpuid]);
>  
> +    init_cpu_sibling_map(cpuid);
> +
>      /* Run local notifiers */
>      notify_cpu_starting(cpuid);
>      /*

-- 
WBR, Volodymyr

RE: [PATCH] xen/arm: Fix sibling mask setup to only include online CPUs

[Hirokazu Takahashi]

Hello Volodymyr,

> -----Original Message-----
> From: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> Sent: Wednesday, October 28, 2026 11:13 AM
> To: Hirokazu Takahashi <taka@valinux.co.jp>
> Cc: xen-devel@lists.xenproject.org; michal.orzel@amd.com; Stefano Stabellini
> <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Bertrand Marquis
> <bertrand.marquis@arm.com>
> Subject: Re: [PATCH] xen/arm: Fix sibling mask setup to only include online
> CPUs
> 
> Hello Hirokazu-san,
> 
> Hirokazu Takahashi <taka@valinux.co.jp> writes:
> 
> > Populate the CPU sibling map only with CPUs that are brought
> > online. Currently, all CPUs in cpu_possible_map are added,
> > which leaves CPUs that failed to boot in the sibling masks and
> > may lead the scheduler to make incorrect decisions.
> 
> I think that this is a scheduler issue then. A CPU has siblings
> regardless of if they are running or not. Also, your change will affect
> CPU Hotplug functionality, as with your patch sibling mask will change
> in runtime. But partially - only in cpu_up, but not on cpu_down. I am
> not sure that schedulers can handle updates to siblings mask in runtime.
> 
> Could you please elaborate how it affects scheduler behaviour right now?

In x86 Xen, __cpu_disable() is called in the cpu_down() path, where the target
CPU is removed from the CPU sibling map (via remove_siblinginfo()).
I think ARM64 Xen should follow suit.

> > Fixes: 701190abb3d5 ("xen/arm: Set correct per-cpu cpu_core_mask")
> > Signed-off-by: Hirokazu Takahashi <taka@valinux.co.jp>
> > ---
> >  xen/arch/arm/smpboot.c | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> > index 8c9072f3c1..2d01896cf0 100644
> > --- a/xen/arch/arm/smpboot.c
> > +++ b/xen/arch/arm/smpboot.c

Thank you,
Hirokazu Takahashi.