RE: [PATCH v2 3/3] xen/sched: Make cpu_nr_siblings() architecture-specific

[Hirokazu Takahashi <taka@xxxxxxxxxxxxx>]

Hello,

> On 17.06.2026 09:12, Hirokazu Takahashi wrote:
> > --- a/xen/arch/x86/include/asm/processor.h
> > +++ b/xen/arch/x86/include/asm/processor.h
> > @@ -106,6 +106,7 @@ extern void intel_init_arat(void);
> >
> >  #define cpu_to_core(_cpu)   (cpu_data[_cpu].cpu_core_id)
> >  #define cpu_to_socket(_cpu) (cpu_data[_cpu].phys_proc_id)
> > +#define cpu_nr_siblings(_cpu) (cpu_data[_cpu].x86_num_siblings)
> 
> This is uniformly available when building x86. An earlier patch adds an
> #include of the new cpu-topology.h ...

"The cpu-topology.h header is intended for non-x86 architectures where topology
information is retrieved from Device Tree or ACPI. 
For x86, I will ensure that it is properly disabled to avoid any conflicts."

> > --- a/xen/common/sched/credit2.c
> > +++ b/xen/common/sched/credit2.c
> > @@ -29,22 +29,6 @@
> >  /* #define d2printk printk */
> >  #define d2printk(x...)
> >
> > -/*
> > - * TODO: Abstract this properly, and figure out what Credit2 wants to do 
> > with
> > - *       the fact that x86_num_siblings doesn't even have the same meaning
> > - *       between x86 vendors.
> > - */
> > -static unsigned int cpu_nr_siblings(unsigned int cpu)
> > -{
> > -#ifdef CONFIG_X86
> > -    return cpu_data[cpu].x86_num_siblings;
> > -#elif defined(CONFIG_CPU_TOPOLOGY)
> > -    return cpu_topology[cpu].num_siblings;
> > -#else
> > -    return 1;
> > -#endif
> > -}
> 
> ... to this file, thus allowing for the static function to be dropped. 
> However,
> ...
> 
> > --- a/xen/include/xen/cpu-topology.h
> > +++ b/xen/include/xen/cpu-topology.h
> > @@ -24,6 +24,7 @@ void init_cpu_topology(void);
> >
> >  #define cpu_to_core(cpu)   (cpu_topology[cpu].phys_core_id)
> >  #define cpu_to_socket(cpu)   (cpu_topology[cpu].phys_socket_id)
> > +#define cpu_nr_siblings(cpu) (cpu_topology[cpu].num_siblings)
> >
> >  #else /* CONFIG_CPU_TOPOLOGY */
> >
> > @@ -31,6 +32,7 @@ static inline void init_cpu_topology(void) {}
> >
> >  #define cpu_to_core(cpu)   (0U)
> >  #define cpu_to_socket(cpu) (0U)
> > +#define cpu_nr_siblings(cpu) (1U)
> >
> >  #endif /* CONFIG_CPU_TOPOLOGY */
> 
> ... one of the two #define-s will take effect here. Whichever one it is, it'll
> conflict with x86'es (when building for x86). Am I overlooking something here,
> or did you simply not build-test x86? Looks like a problem of the same kind
> may actually be introduced already by patch 2.

Oops, you are completely right, and I sincerely apologize. I did not properly 
build-test on x86.

Thank you,
Hirokazu Takahashi.