Re: [PATCH v2 6/8] leon3: implement multiprocessor

[Clément Chigot]

On Tue, Jan 30, 2024 at 12:43 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Hi Clément,
>
> On 16/1/24 14:02, Clément Chigot wrote:
> > This allows to register more than one CPU on the leon3_generic machine.
> >
> > Co-developed-by: Frederic Konrad <konrad.frederic@yahoo.fr>
> > Signed-off-by: Clément Chigot <chigot@adacore.com>
> > ---
> >   hw/sparc/leon3.c | 106 +++++++++++++++++++++++++++++++++--------------
> >   1 file changed, 74 insertions(+), 32 deletions(-)
> >
> > diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> > index 7866f0a049..eacd85ee4f 100644
> > --- a/hw/sparc/leon3.c
> > +++ b/hw/sparc/leon3.c
> > @@ -54,6 +54,8 @@
> >   #define LEON3_PROM_OFFSET    (0x00000000)
> >   #define LEON3_RAM_OFFSET     (0x40000000)
> >
> > +#define MAX_CPUS  4
> > +
> >   #define LEON3_UART_OFFSET  (0x80000100)
> >   #define LEON3_UART_IRQ     (3)
> >
> > @@ -67,9 +69,12 @@
> >   #define LEON3_AHB_PNP_OFFSET (0xFFFFF000)
> >
> >   typedef struct ResetData {
> > -    SPARCCPU *cpu;
> > -    uint32_t  entry;            /* save kernel entry in case of reset */
> > -    target_ulong sp;            /* initial stack pointer */
> > +    struct CPUResetData {
> > +        int id;
> > +        SPARCCPU *cpu;
> > +        target_ulong sp;  /* initial stack pointer */
> > +    } info[MAX_CPUS];
> > +    uint32_t entry;             /* save kernel entry in case of reset */
> >   } ResetData;
> >
> >   static uint32_t *gen_store_u32(uint32_t *code, hwaddr addr, uint32_t val)
> > @@ -125,18 +130,19 @@ static void write_bootloader(CPUSPARCState *env, 
> > uint8_t *base,
> >       stl_p(p++, 0x01000000); /* nop */
> >   }
> >
> > -static void main_cpu_reset(void *opaque)
> > +static void leon3_cpu_reset(void *opaque)
> >   {
> > -    ResetData *s   = (ResetData *)opaque;
> > -    CPUState *cpu = CPU(s->cpu);
> > -    CPUSPARCState  *env = &s->cpu->env;
> > +    struct CPUResetData *info = (struct CPUResetData *) opaque;
> > +    int id = info->id;
> > +    ResetData *s = (ResetData *)DO_UPCAST(ResetData, info[id], info);
> > +    CPUState *cpu = CPU(s->info[id].cpu);
> > +    CPUSPARCState *env = cpu_env(cpu);
> >
> >       cpu_reset(cpu);
> > -
> > -    cpu->halted = 0;
> > -    env->pc     = s->entry;
> > -    env->npc    = s->entry + 4;
> > -    env->regbase[6] = s->sp;
> > +    cpu->halted = cpu->cpu_index != 0;
> > +    env->pc = s->entry;
> > +    env->npc = s->entry + 4;
> > +    env->regbase[6] = s->info[id].sp;
>
> You take care to initialize with different stack, ...
>
> >   }
> >
> >   static void leon3_cache_control_int(CPUSPARCState *env)
> > @@ -170,8 +176,8 @@ static void leon3_cache_control_int(CPUSPARCState *env)
> >
> >   static void leon3_irq_ack(CPUSPARCState *env, int intno)
> >   {
> > -    /* No SMP support yet, only CPU #0 available so far.  */
> > -    grlib_irqmp_ack(env->irq_manager, 0, intno);
> > +    CPUState *cpu = CPU(env_cpu(env));
> > +    grlib_irqmp_ack(env->irq_manager, cpu->cpu_index, intno);
> >   }
> >
> >   /*
> > @@ -213,6 +219,20 @@ static void leon3_set_pil_in(void *opaque, int n, int 
> > level)
> >       }
> >   }
> >
> > +static void leon3_start_cpu_async_work(CPUState *cpu, run_on_cpu_data data)
> > +{
> > +    cpu->halted = 0;
> > +}
> > +
> > +static void leon3_start_cpu(void *opaque, int n, int level)
> > +{
> > +    CPUState *cs = CPU(opaque);
> > +
> > +    if (level) {
> > +        async_run_on_cpu(cs, leon3_start_cpu_async_work, RUN_ON_CPU_NULL);
> > +    }
> > +}
> > +
> >   static void leon3_irq_manager(CPUSPARCState *env, int intno)
> >   {
> >       leon3_irq_ack(env, intno);
> > @@ -238,17 +258,21 @@ static void leon3_generic_hw_init(MachineState 
> > *machine)
> >       AHBPnp *ahb_pnp;
> >       APBPnp *apb_pnp;
> >
> > -    /* Init CPU */
> > -    cpu = SPARC_CPU(cpu_create(machine->cpu_type));
> > -    env = &cpu->env;
> > +    reset_info = g_malloc0(sizeof(ResetData));
> >
> > -    cpu_sparc_set_id(env, 0);
> > +    for (i = 0; i < machine->smp.cpus; i++) {
> > +        /* Init CPU */
> > +        cpu = SPARC_CPU(cpu_create(machine->cpu_type));
> > +        env = &cpu->env;
> >
> > -    /* Reset data */
> > -    reset_info        = g_new0(ResetData, 1);
> > -    reset_info->cpu   = cpu;
> > -    reset_info->sp    = LEON3_RAM_OFFSET + ram_size;
> > -    qemu_register_reset(main_cpu_reset, reset_info);
> > +        cpu_sparc_set_id(env, i);
> > +
> > +        /* Reset data */
> > +        reset_info->info[i].id = i;
> > +        reset_info->info[i].cpu = cpu;
> > +        reset_info->info[i].sp = LEON3_RAM_OFFSET + ram_size;
>
> ... but all CPUs are initialized with the same stack. Is this
> expected?

Indeed, I've just blindly updated the existing code.
The official doc (see [1] §4.2.15) does not mention anything about SP
when a reset occurs. The program loaded should take care of their
initialization.
I'll remove that. Thanks for the notice.

[1] https://gaisler.com/doc/gr712rc-usermanual.pdf

> > +        qemu_register_reset(leon3_cpu_reset, &reset_info->info[i]);
> > +    }
> >
> >       ahb_pnp = GRLIB_AHB_PNP(qdev_new(TYPE_GRLIB_AHB_PNP));
> >       sysbus_realize_and_unref(SYS_BUS_DEVICE(ahb_pnp), &error_fatal);
> > @@ -266,14 +290,28 @@ static void leon3_generic_hw_init(MachineState 
> > *machine)
> >
> >       /* Allocate IRQ manager */
> >       irqmpdev = qdev_new(TYPE_GRLIB_IRQMP);
> > +    object_property_set_int(OBJECT(irqmpdev), "ncpus", machine->smp.cpus,
> > +                            &error_fatal);
> >       sysbus_realize_and_unref(SYS_BUS_DEVICE(irqmpdev), &error_fatal);
> > -    qdev_init_gpio_in_named_with_opaque(DEVICE(cpu), leon3_set_pil_in,
> > -                                        env, "pil", 1);
> > -    qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", 0,
> > -                                qdev_get_gpio_in_named(DEVICE(cpu), "pil", 
> > 0));
> > +
> > +    for (i = 0; i < machine->smp.cpus; i++) {
> > +        cpu = reset_info->info[i].cpu;
> > +        env = &cpu->env;
> > +        qdev_init_gpio_in_named_with_opaque(DEVICE(cpu), leon3_start_cpu,
> > +                                            cpu, "start_cpu", 1);
> > +        qdev_connect_gpio_out_named(irqmpdev, "grlib-start-cpu", i,
> > +                                    qdev_get_gpio_in_named(DEVICE(cpu),
> > +                                                           "start_cpu", 
> > 0));
> > +        qdev_init_gpio_in_named_with_opaque(DEVICE(cpu), leon3_set_pil_in,
> > +                                            env, "pil", 1);
> > +        qdev_connect_gpio_out_named(irqmpdev, "grlib-irq", i,
> > +                                    qdev_get_gpio_in_named(DEVICE(cpu),
> > +                                                           "pil", 0));
> > +        env->irq_manager = irqmpdev;
> > +        env->qemu_irq_ack = leon3_irq_manager;
> > +    }
> > +
> >       sysbus_mmio_map(SYS_BUS_DEVICE(irqmpdev), 0, LEON3_IRQMP_OFFSET);
> > -    env->irq_manager = irqmpdev;
> > -    env->qemu_irq_ack = leon3_irq_manager;
> >       grlib_apb_pnp_add_entry(apb_pnp, LEON3_IRQMP_OFFSET, 0xFFF,
> >                               GRLIB_VENDOR_GAISLER, GRLIB_IRQMP_DEV,
> >                               2, 0, GRLIB_APBIO_AREA);
> > @@ -347,10 +385,13 @@ static void leon3_generic_hw_init(MachineState 
> > *machine)
> >               uint8_t *bootloader_entry;
> >
> >               bootloader_entry = memory_region_get_ram_ptr(prom);
> > -            write_bootloader(env, bootloader_entry, entry);
> > -            env->pc = LEON3_PROM_OFFSET;
> > -            env->npc = LEON3_PROM_OFFSET + 4;
> > +            write_bootloader(&reset_info->info[0].cpu->env, 
> > bootloader_entry,
> > +                             entry);
> >               reset_info->entry = LEON3_PROM_OFFSET;
> > +            for (i = 0; i < machine->smp.cpus; i++) {
> > +                reset_info->info[i].cpu->env.pc = LEON3_PROM_OFFSET;
> > +                reset_info->info[i].cpu->env.npc = LEON3_PROM_OFFSET + 4;
> > +            }
> >           }
> >       }
> >
> > @@ -389,6 +430,7 @@ static void leon3_generic_machine_init(MachineClass *mc)
> >       mc->init = leon3_generic_hw_init;
> >       mc->default_cpu_type = SPARC_CPU_TYPE_NAME("LEON3");
> >       mc->default_ram_id = "leon3.ram";
> > +    mc->max_cpus = MAX_CPUS;
> >   }
> >
> >   DEFINE_MACHINE("leon3_generic", leon3_generic_machine_init)
>