Re: [PATCH] xen/arm: Set correct per-cpu cpu_core_mask

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Henry,

On 26/02/2024 04:01, Henry Wang wrote:
> In the common sysctl command XEN_SYSCTL_physinfo, the value of
> cores_per_socket is calculated based on the cpu_core_mask of CPU0.
> Currently on Arm this is a fixed value 1 (can be checked via xl info),
> which is not correct. This is because during the Arm CPU online
> process at boot time, setup_cpu_sibling_map() only sets the per-cpu
> cpu_core_mask for itself.
> 
> cores_per_socket refers to the number of cores that belong to the same
> socket (NUMA node). Currently Xen on Arm does not support physical
> CPU hotplug and NUMA. Therefore if the MT bit (bit 24) in MPIDR_EL1
> is 0, cores_per_socket means all possible CPUs detected from the device
> tree. Setting the per-cpu cpu_core_mask in setup_cpu_sibling_map()
> accordingly. Drop the in-code comment which seems to be outdated.
> 
> Signed-off-by: Henry Wang <xin.wang2@xxxxxxx>
NIT: You first sent this patch as part of NUMA series:
https://lore.kernel.org/xen-devel/20231120025431.14845-16-Henry.Wang@xxxxxxx/
Shouldn't you retain the Arm's authorship?

> ---
>  xen/arch/arm/smpboot.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index a84e706d77..d616778655 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -66,7 +66,6 @@ static bool cpu_is_dead;
>  
>  /* ID of the PCPU we're running on */
>  DEFINE_PER_CPU(unsigned int, cpu_id);
> -/* XXX these seem awfully x86ish... */
>  /* representing HT siblings of each logical CPU */
>  DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_mask);
>  /* representing HT and core siblings of each logical CPU */
> @@ -89,6 +88,11 @@ static int setup_cpu_sibling_map(int cpu)
>      cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
>      cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
>  
> +    /* PE not implemented using a multithreading type approach. */
> +    if ( system_cpuinfo.mpidr.mt == 0 )
Do we need this check? It mt was true, cpu_sibling_mask would be incorrect 
anyway (it would still be 1).

~Michal