Re: [Xen-devel] [PATCH v1 1/2] x86/cpu: maintain a parked CPU bitmap

[Julien Grall <julien@xxxxxxx>]

Hi,

On 20/11/2019 23:05, Chao Gao wrote:
> It helps to distinguish parked CPUs from those are really offlined or
> hot-added. We need to know the parked CPUs in order to do a special
> check against them to ensure that all CPUs, except those are really
> offlined or hot-added, have the same ucode version.
>
> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> ---
> Note that changes on ARM side are untested.
> ---
>   xen/arch/arm/smpboot.c    | 1 +
>   xen/arch/x86/cpu/common.c | 4 ++++
>   xen/arch/x86/smpboot.c    | 1 +
>   xen/common/cpu.c          | 4 ++++
>   xen/include/asm-arm/smp.h | 1 +
>   xen/include/asm-x86/smp.h | 1 +
>   xen/include/xen/cpumask.h | 1 +
>   7 files changed, 13 insertions(+)
>
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 00b64c3..1b57ba4 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -39,6 +39,7 @@
>   cpumask_t cpu_online_map;
>   cpumask_t cpu_present_map;
>   cpumask_t cpu_possible_map;
> +cpumask_var_t cpu_parked_map;
You define cpu_parked_map but AFAIK it will never get allocated. The 
risk here is any access to that variable will result to a fault.
Looking at the changes below, it looks like access in common code will 
be protected by park_offline_cpus. This is always false on Arm, so the 
compiler should remove any access to cpu_parked_map.
With that in mind, I think it would be best to only provide a prototype 
for cpu_parked_map and so the linker can warn if someone used it.

> diff --git a/xen/include/asm-arm/smp.h b/xen/include/asm-arm/smp.h
> index fdbcefa..4b392fa 100644
> --- a/xen/include/asm-arm/smp.h
> +++ b/xen/include/asm-arm/smp.h
> @@ -19,6 +19,7 @@ DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask);
>    * would otherwise prefer them to be off?
>    */
>   #define park_offline_cpus false
> +extern cpumask_var_t cpu_parked_map;
The prototype should be the same for all architectures. So is there any 
reason to duplicate it?
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel