Re: [PATCH 5/5] powerpc/microwatt: Add SMP support

["Nicholas Piggin" <npiggin@gmail.com>]

On Wed Jan 29, 2025 at 8:55 AM AEST, Paul Mackerras wrote:
> This adds support for Microwatt systems with more than one core, and
> updates the device tree for a 2-core version.  (This does not prevent
> the kernel from running on a single-core system.)
>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>

Well, I'm impressed you added SMP :)

What happens with a 1 CPU system? Do we time out waiting for secondaries
and continue, or is there something more graceful?

> ---
>  arch/powerpc/boot/dts/microwatt.dts          | 34 ++++++++++++++++++--
>  arch/powerpc/platforms/microwatt/Kconfig     |  2 +-
>  arch/powerpc/platforms/microwatt/Makefile    |  1 +
>  arch/powerpc/platforms/microwatt/microwatt.h |  1 +
>  arch/powerpc/platforms/microwatt/setup.c     | 10 ++++++
>  5 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/microwatt.dts b/arch/powerpc/boot/dts/microwatt.dts
> index 6e575e841a7b..89281dc975b3 100644
> --- a/arch/powerpc/boot/dts/microwatt.dts
> +++ b/arch/powerpc/boot/dts/microwatt.dts
> @@ -142,6 +142,36 @@ PowerPC,Microwatt@0 {
>  			ibm,mmu-lpid-bits = <12>;
>  			ibm,mmu-pid-bits = <20>;
>  		};
> +
> +		PowerPC,Microwatt@1 {
> +			i-cache-sets = <2>;
> +			ibm,dec-bits = <64>;
> +			reservation-granule-size = <64>;
> +			clock-frequency = <100000000>;
> +			timebase-frequency = <100000000>;
> +			i-tlb-sets = <1>;
> +			ibm,ppc-interrupt-server#s = <1>;
> +			i-cache-block-size = <64>;
> +			d-cache-block-size = <64>;
> +			d-cache-sets = <2>;
> +			i-tlb-size = <64>;
> +			cpu-version = <0x990000>;
> +			status = "okay";
> +			i-cache-size = <0x1000>;
> +			ibm,processor-radix-AP-encodings = <0x0c 0xa0000010 0x20000015 0x4000001e>;
> +			tlb-size = <0>;
> +			tlb-sets = <0>;
> +			device_type = "cpu";
> +			d-tlb-size = <128>;
> +			d-tlb-sets = <2>;
> +			reg = <1>;
> +			general-purpose;
> +			64-bit;
> +			d-cache-size = <0x1000>;
> +			ibm,chip-id = <0>;
> +			ibm,mmu-lpid-bits = <12>;
> +			ibm,mmu-pid-bits = <20>;
> +		};
>  	};
>  
>  	soc@c0000000 {
> @@ -154,8 +184,8 @@ soc@c0000000 {
>  
>  		interrupt-controller@4000 {
>  			compatible = "openpower,xics-presentation", "ibm,ppc-xicp";
> -			ibm,interrupt-server-ranges = <0x0 0x1>;
> -			reg = <0x4000 0x100>;
> +			ibm,interrupt-server-ranges = <0x0 0x2>;
> +			reg = <0x4000 0x10 0x4010 0x10>;
>  		};
>  
>  		ICS: interrupt-controller@5000 {
> diff --git a/arch/powerpc/platforms/microwatt/Kconfig b/arch/powerpc/platforms/microwatt/Kconfig
> index 1d5cc1ae3636..8911ea7cf12e 100644
> --- a/arch/powerpc/platforms/microwatt/Kconfig
> +++ b/arch/powerpc/platforms/microwatt/Kconfig
> @@ -1,6 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  config PPC_MICROWATT
> -	depends on PPC_BOOK3S_64 && !SMP
> +	depends on PPC_BOOK3S_64
>  	bool "Microwatt SoC platform"
>  	select PPC_XICS
>  	select PPC_ICS_NATIVE
> diff --git a/arch/powerpc/platforms/microwatt/Makefile b/arch/powerpc/platforms/microwatt/Makefile
> index 116d6d3ad3f0..d973b2ab4042 100644
> --- a/arch/powerpc/platforms/microwatt/Makefile
> +++ b/arch/powerpc/platforms/microwatt/Makefile
> @@ -1 +1,2 @@
>  obj-y	+= setup.o rng.o
> +obj-$(CONFIG_SMP) += smp.o

Hmm.... `git add arch/powerpc/platforms/microwatt/smp.c` ?

> diff --git a/arch/powerpc/platforms/microwatt/microwatt.h b/arch/powerpc/platforms/microwatt/microwatt.h
> index 335417e95e66..891aa2800768 100644
> --- a/arch/powerpc/platforms/microwatt/microwatt.h
> +++ b/arch/powerpc/platforms/microwatt/microwatt.h
> @@ -3,5 +3,6 @@
>  #define _MICROWATT_H
>  
>  void microwatt_rng_init(void);
> +void microwatt_init_smp(void);
>  
>  #endif /* _MICROWATT_H */
> diff --git a/arch/powerpc/platforms/microwatt/setup.c b/arch/powerpc/platforms/microwatt/setup.c
> index 97828a99780d..d966bf1f57f7 100644
> --- a/arch/powerpc/platforms/microwatt/setup.c
> +++ b/arch/powerpc/platforms/microwatt/setup.c
> @@ -29,6 +29,15 @@ static int __init microwatt_populate(void)
>  }
>  machine_arch_initcall(microwatt, microwatt_populate);
>  
> +static int __init microwatt_probe(void)
> +{
> +	/* Main reason for having this is to start the other CPU(s) */
> +#ifdef CONFIG_SMP
> +	microwatt_init_smp();
> +#endif

Could use

	if (IS_ENABLED(CONFIG_SMP))
		microwatt_init_smp();

Thanks,
Nick

> +	return 1;
> +}
> +
>  static void __init microwatt_setup_arch(void)
>  {
>  	microwatt_rng_init();
> @@ -45,6 +54,7 @@ static void microwatt_idle(void)
>  define_machine(microwatt) {
>  	.name			= "microwatt",
>  	.compatible		= "microwatt-soc",
> +	.probe			= microwatt_probe,
>  	.init_IRQ		= microwatt_init_IRQ,
>  	.setup_arch		= microwatt_setup_arch,
>  	.progress		= udbg_progress,