On Wed Jan 29, 2025 at 4:57 PM AEST, Paul Mackerras wrote:
> On Wed, Jan 29, 2025 at 04:21:26PM +1000, Nicholas Piggin wrote:
>> 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 <pau...@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?
>
> There's a field in the SYSCON register which tells you how many cores
> there are. microwatt_init_smp() looks at that field and only starts
> the CPUs that are there.
Ah, nice.
> Oops, sorry, I see that I forgot to do 'git add' on
> arch/powerpc/platforms/microwatt/smp.c. Here it is (I'll include it
> properly in v2, of course):
>
> // SPDX-License-Identifier: GPL-2.0-or-later
>
> /*
> * SMP support functions for Microwatt
> * Copyright 2025 Paul Mackerras <pau...@ozlabs.org>
> */
>
> #include <linux/kernel.h>
> #include <linux/smp.h>
> #include <linux/io.h>
> #include <asm/early_ioremap.h>
> #include <asm/xics.h>
>
> #include "microwatt.h"
>
> static void __init microwatt_smp_probe(void)
> {
> xics_smp_probe();
> }
>
> static void microwatt_smp_setup_cpu(int cpu)
> {
> if (cpu != 0)
> xics_setup_cpu();
> }
>
> static struct smp_ops_t microwatt_smp_ops = {
> .probe = microwatt_smp_probe,
> .message_pass = NULL, /* Use smp_muxed_ipi_message_pass */
> .kick_cpu = smp_generic_kick_cpu,
> .setup_cpu = microwatt_smp_setup_cpu,
> };
>
> /* XXX get from device tree */
> #define SYSCON_BASE 0xc0000000
#define SYSCON_LENGTH 0x100
?
>
> #define SYSCON_CPU_CTRL 0x58
>
> void __init microwatt_init_smp(void)
> {
> volatile unsigned char __iomem *syscon;
> int ncpus;
> int timeout;
>
> syscon = early_ioremap(SYSCON_BASE, 0x100);
ioremap is not up by SMP init time? I always have to
trawl through init spaghetti to work it out. I guess it's
early SMP init.
> if (syscon == NULL) {
> pr_err("Failed to map SYSCON\n");
> return;
> }
> ncpus = (readl(syscon + SYSCON_CPU_CTRL) >> 8) & 0xff;
> if (ncpus < 2)
> goto out;
>
> smp_ops = µwatt_smp_ops;
>
> /*
> * Write two instructions at location 0:
> * mfspr r3, PIR
> * b __secondary_hold
> */
> *(unsigned int *)KERNELBASE = 0x7c7ffaa6;
> *(unsigned int *)(KERNELBASE+4) = 0x4800005c;
Could move constants to PPC_INST_ ?
>
> /* enable the other CPUs, they start at location 0 */
> writel((1ul << ncpus) - 1, syscon + SYSCON_CPU_CTRL);
>
> timeout = 10000;
> while (!__secondary_hold_acknowledge) {
> if (--timeout == 0)
> break;
> barrier();
> }
>
> out:
> early_iounmap((void *)syscon, 0x100);
> }
Looks okay otherwise.
Thanks,
Nick
