Re: [PATCH v3 6/6] xen/arm: mpu: Implement a dummy enable_secondary_cpu_mm

[Julien Grall <julien@xxxxxxx>]

Hi Ayan,

On 10/10/2024 15:03, Ayan Kumar Halder wrote:
> Secondary cpus initialization is not yet supported. Thus, we print an
> appropriate message and put the secondary cpus in WFE state.
> And we introduce to BUILD_BUG_ON to prevent users using from building Xen
> on multiprocessor based MPU systems.
>
> In Arm, there is no clean way to disable SMP. As of now, we wish to support
> MPU on UNP only. So, we have defined the default range of NR_CPUs to be 1 for
> MPU.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> ---
> Changes from :-
>
> v1 - 1. NR_CPUS is defined as 1 for MPU
>
> 2. Added a message in enable_secondary_cpu_mm()
>
> v2 - 1. Added the range
>
> 2. Clarified in the commit message why/how we have disabled SMP.
>
>   xen/arch/Kconfig              |  2 ++
>   xen/arch/arm/arm64/mpu/head.S | 10 ++++++++++
>   xen/arch/arm/setup.c          |  5 +++++
>   3 files changed, 17 insertions(+)
>
> diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
> index 308ce129a8..6053e048fa 100644
> --- a/xen/arch/Kconfig
> +++ b/xen/arch/Kconfig
> @@ -6,11 +6,13 @@ config PHYS_ADDR_T_32
>   
>   config NR_CPUS
>         int "Maximum number of CPUs"
> +       range 1 1 if MPU
>         range 1 16383
>         default "256" if X86
>         default "8" if ARM && RCAR3
>         default "4" if ARM && QEMU
>         default "4" if ARM && MPSOC
> +       default "1" if ARM && MPU
>         default "128" if ARM
>         help
>           Controls the build-time size of various arrays and bitmaps
> diff --git a/xen/arch/arm/arm64/mpu/head.S b/xen/arch/arm/arm64/mpu/head.S
> index e354f4552b..4d36a8c9bc 100644
> --- a/xen/arch/arm/arm64/mpu/head.S
> +++ b/xen/arch/arm/arm64/mpu/head.S
> @@ -147,6 +147,16 @@ fail:
>       b   1b
>   END(enable_boot_cpu_mm)
>   
> +/*
> + * We don't yet support secondary CPUs bring-up. Implement a dummy helper to
> + * please the common code.
> + */
> +ENTRY(enable_secondary_cpu_mm)
> +    PRINT("- SMP not enabled yet -\r\n")
> +1:  wfe
> +    b 1b
> +ENDPROC(enable_secondary_cpu_mm)
> +
>   /*
>    * Local variables:
>    * mode: ASM
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 0203771164..5a0d343f5b 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -296,6 +296,11 @@ void asmlinkage __init start_xen(unsigned long fdt_paddr)
>       int rc, i;
>   
>   #ifdef CONFIG_MPU
> +    /*
> +     * Currently, SMP is not enabled on MPU based systems.
> +     */
> +    BUILD_BUG_ON(NR_CPUS > 1);
I think this is a odd place to add the BUILD_BUG_ON. It would better 
belong to smp.c (if not mpu/smpboot.c). Maybe in a build_assertions() 
function.
Cheers,

--
Julien Grall