Re: [PATCH v1 02/14] xen/riscv: introduce smp_clear_cpu_maps()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.04.2025 17:57, Oleksii Kurochko wrote:
> Initialize cpu_{possible, online, present}_map by using smp_clear_cpu_maps().
> 
> Drop DEFINE_PER_CPU(unsigned int, cpu_id) from stubs.c as this variable isn't
> expected to be used in RISC-V at all.
> 
> Move declaration of cpu_{possible,online,present}_map from stubs.c to 
> smpboot.c
> as now smpboot.c is now introduced.
> Other defintions keep in stubs.c as they are not initialized and not needed, 
> at
> the moment.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
>  xen/arch/riscv/Makefile          |  1 +
>  xen/arch/riscv/include/asm/smp.h |  2 ++
>  xen/arch/riscv/setup.c           |  2 ++
>  xen/arch/riscv/smpboot.c         | 15 +++++++++++++++
>  xen/arch/riscv/stubs.c           |  6 ------
>  5 files changed, 20 insertions(+), 6 deletions(-)
>  create mode 100644 xen/arch/riscv/smpboot.c
> 
> diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
> index 0c6c4a38a3..f551bf32a2 100644
> --- a/xen/arch/riscv/Makefile
> +++ b/xen/arch/riscv/Makefile
> @@ -10,6 +10,7 @@ obj-y += sbi.o
>  obj-y += setup.o
>  obj-y += shutdown.o
>  obj-y += smp.o
> +obj-y += smpboot.o
>  obj-y += stubs.o
>  obj-y += time.o
>  obj-y += traps.o
> diff --git a/xen/arch/riscv/include/asm/smp.h 
> b/xen/arch/riscv/include/asm/smp.h
> index 5e170b57b3..188c033718 100644
> --- a/xen/arch/riscv/include/asm/smp.h
> +++ b/xen/arch/riscv/include/asm/smp.h
> @@ -26,6 +26,8 @@ static inline void set_cpuid_to_hartid(unsigned long cpuid,
>  
>  void setup_tp(unsigned int cpuid);
>  
> +void smp_clear_cpu_maps(void);
> +
>  #endif
>  
>  /*
> diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
> index 4e416f6e44..7f68f3f5b7 100644
> --- a/xen/arch/riscv/setup.c
> +++ b/xen/arch/riscv/setup.c
> @@ -72,6 +72,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id,
>  
>      remove_identity_mapping();
>  
> +    smp_clear_cpu_maps();
> +
>      set_processor_id(0);
>  
>      set_cpuid_to_hartid(0, bootcpu_id);
> diff --git a/xen/arch/riscv/smpboot.c b/xen/arch/riscv/smpboot.c
> new file mode 100644
> index 0000000000..0f4dcc28e1
> --- /dev/null
> +++ b/xen/arch/riscv/smpboot.c
> @@ -0,0 +1,15 @@
> +#include <xen/cpumask.h>
> +#include <xen/init.h>
> +
> +cpumask_t cpu_online_map;
> +cpumask_t cpu_present_map;
> +cpumask_t cpu_possible_map;

__read_mostly for all of them, perhaps (if CPU hotplug isn't expected to
be supported) even __ro_after_init for the latter two?

As to cpu_possible_map - do you predict that you'll actually use it? Arm
does (and instead has only a fake cpu_present_map), but on x86 we get away
without.

> +void __init smp_clear_cpu_maps(void)
> +{
> +    cpumask_clear(&cpu_possible_map);
> +    cpumask_clear(&cpu_online_map);

What's the point of these? All three maps start out fully zeroed.

> +    cpumask_set_cpu(0, &cpu_possible_map);
> +    cpumask_set_cpu(0, &cpu_online_map);

These are contradicting the name of the function. The somewhat equivalent
function we have on x86 is smp_prepare_boot_cpu().

> +    cpumask_copy(&cpu_present_map, &cpu_possible_map);

Another cpumask_set_cpu() is probably cheaper here then.

Jan