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

[Henry Wang <xin.wang2@xxxxxxx>]

Hi All,

Gentle ping since it has been a couple of months, any comments on this 
updated patch? Thanks!
Kind regards,
Henry

On 3/21/2024 11:57 AM, 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, also we assume there is no multithread. Therefore
> 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. Modify the in-code comment which seems to be outdated. Add
> a warning to users if Xen is running on processors with multithread
> support.
>
> Signed-off-by: Henry Wang <Henry.Wang@xxxxxxx>
> Signed-off-by: Henry Wang <xin.wang2@xxxxxxx>
> ---
> v3:
> - Use cpumask_copy() to set cpu_core_mask and drop the unnecessary
>    cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu)).
> - In-code comment adjustments.
> - Add a warning for multithread.
> v2:
> - Do not do the multithread check.
> ---
>   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 a84e706d77..b6268be27a 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -66,7 +66,11 @@ 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... */
> +/*
> + * Although multithread is part of the Arm spec, there are not many
> + * processors support multithread and current Xen on Arm assumes there
> + * is no multithread.
> + */
>   /* 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 */
> @@ -85,9 +89,13 @@ static int setup_cpu_sibling_map(int cpu)
>            !zalloc_cpumask_var(&per_cpu(cpu_core_mask, cpu)) )
>           return -ENOMEM;
>   
> -    /* A CPU is a sibling with itself and is always on its own core. */
> +    /*
> +     * 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).
> +     */
>       cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
> -    cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
> +    cpumask_copy(per_cpu(cpu_core_mask, cpu), &cpu_possible_map);
>   
>       return 0;
>   }
> @@ -277,6 +285,10 @@ void __init smp_init_cpus(void)
>           warning_add("WARNING: HMP COMPUTING HAS BEEN ENABLED.\n"
>                       "It has implications on the security and stability of the 
> system,\n"
>                       "unless the cpu affinity of all domains is specified.\n");
> +
> +    if ( system_cpuinfo.mpidr.mt == 1 )
> +        warning_add("WARNING: MULTITHREADING HAS BEEN DETECTED ON THE 
> PROCESSOR.\n"
> +                    "It might impact the security of the system.\n");
>   }
>   
>   unsigned int __init smp_get_max_cpus(void)