| Message ID | 20200504081953.245912-1-joel@jms.id.au |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] aspeed: Add boot stub for smp booting | expand |
On Mon, 4 May 2020 at 08:20, Joel Stanley <joel@jms.id.au> wrote: ... > v3: Use WFI instead of WFE > v2: test for number of CPUs > +static void aspeed_write_smpboot(ARMCPU *cpu, > + const struct arm_boot_info *info) > +{ > + static const uint32_t poll_mailbox_ready[] = { > + /* > + * r2 = per-cpu go sign value > + * r1 = AST_SMP_MBOX_FIELD_ENTRY > + * r0 = AST_SMP_MBOX_FIELD_GOSIGN > + */ > + 0xee100fb0, /* mrc p15, 0, r0, c0, c0, 5 */ > + 0xe21000ff, /* ands r0, r0, #255 */ > + 0xe59f201c, /* ldr r2, [pc, #28] */ > + 0xe1822000, /* orr r2, r2, r0 */ > + > + 0xe59f1018, /* ldr r1, [pc, #24] */ > + 0xe59f0018, /* ldr r0, [pc, #24] */ > + > + 0xe320f003, /* wfi */ I was wrong, wfi does not work in this case. I must have made a mistake when testing. Cédric, can you please confirm my testing is correct? I was using today's linux next, but anything newer than Linux 5.4 should have the same result. Peter, can you please take v2 of this patch set instead of v3? If you've already added it to you queue I can send a follow up to fix it. Cheers, Joel
On 5/8/20 8:52 AM, Joel Stanley wrote: > On Mon, 4 May 2020 at 08:20, Joel Stanley <joel@jms.id.au> wrote: > ... > >> v3: Use WFI instead of WFE >> v2: test for number of CPUs > >> +static void aspeed_write_smpboot(ARMCPU *cpu, >> + const struct arm_boot_info *info) >> +{ >> + static const uint32_t poll_mailbox_ready[] = { >> + /* >> + * r2 = per-cpu go sign value >> + * r1 = AST_SMP_MBOX_FIELD_ENTRY >> + * r0 = AST_SMP_MBOX_FIELD_GOSIGN >> + */ >> + 0xee100fb0, /* mrc p15, 0, r0, c0, c0, 5 */ >> + 0xe21000ff, /* ands r0, r0, #255 */ >> + 0xe59f201c, /* ldr r2, [pc, #28] */ >> + 0xe1822000, /* orr r2, r2, r0 */ >> + >> + 0xe59f1018, /* ldr r1, [pc, #24] */ >> + 0xe59f0018, /* ldr r0, [pc, #24] */ >> + >> + 0xe320f003, /* wfi */ > > I was wrong, wfi does not work in this case. I must have made a > mistake when testing. > > Cédric, can you please confirm my testing is correct? I was using > today's linux next, but anything newer than Linux 5.4 should have the > same result. Indeed, with OpenBMC kernel v5.4.32-260-g7dc9442bbe7d and wfi (patch v3), [ 0.045214] smp: Bringing up secondary CPUs ... [ 1.178127] CPU1: failed to come online [ 1.187309] smp: Brought up 1 node, 1 CPU [ 1.187590] SMP: Total of 1 processors activated (2250.00 BogoMIPS). [ 1.187786] CPU: All CPU(s) started in HYP mode. [ 1.187850] CPU: Virtualization extensions available. When using wfe (patch v2), [ 0.091092] smp: Bringing up secondary CPUs ... [ 0.096628] smp: Brought up 1 node, 2 CPUs [ 0.096718] SMP: Total of 2 processors activated (4500.00 BogoMIPS). [ 0.096768] CPU: All CPU(s) started in HYP mode. [ 0.096785] CPU: Virtualization extensions available. Cheers, C.
On Fri, 8 May 2020 at 15:27, Cédric Le Goater <clg@kaod.org> wrote: > Indeed, with OpenBMC kernel v5.4.32-260-g7dc9442bbe7d and wfi (patch v3), > > [ 0.045214] smp: Bringing up secondary CPUs ... > [ 1.178127] CPU1: failed to come online > [ 1.187309] smp: Brought up 1 node, 1 CPU > [ 1.187590] SMP: Total of 1 processors activated (2250.00 BogoMIPS). > [ 1.187786] CPU: All CPU(s) started in HYP mode. > [ 1.187850] CPU: Virtualization extensions available. > > When using wfe (patch v2), > > [ 0.091092] smp: Bringing up secondary CPUs ... > [ 0.096628] smp: Brought up 1 node, 2 CPUs > [ 0.096718] SMP: Total of 2 processors activated (4500.00 BogoMIPS). > [ 0.096768] CPU: All CPU(s) started in HYP mode. > [ 0.096785] CPU: Virtualization extensions available. OK, I've applied patch v2 to target-arm.next. v3 looked a bit odd anyway -- a wfi-based secondary loop generally needs to prod the GIC to enable interrupts for the core before going into its loop (compare the 32-bit smpboot[] code in hw/arm/boot.c). But looking at the code in the kernel's arch/arm/mach-aspeed/platsmp.c, it uses dsb_sev() to prod the secondary core, so only wfe will work here. If you find that you are often in situations where the secondary cores are not started and so QEMU is wasting host CPU in this busy-loop, you could look at implementing a non-noop wfe/sev in QEMU, though it's not completely trivial because of all the things other than sev that are 'wfe wake-up events'. thanks -- PMM
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 88bcd6ff3fbd..93970502b8a6 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -116,6 +116,58 @@ static const MemoryRegionOps max_ram_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; +#define AST_SMP_MAILBOX_BASE 0x1e6e2180 +#define AST_SMP_MBOX_FIELD_ENTRY (AST_SMP_MAILBOX_BASE + 0x0) +#define AST_SMP_MBOX_FIELD_GOSIGN (AST_SMP_MAILBOX_BASE + 0x4) +#define AST_SMP_MBOX_FIELD_READY (AST_SMP_MAILBOX_BASE + 0x8) +#define AST_SMP_MBOX_FIELD_POLLINSN (AST_SMP_MAILBOX_BASE + 0xc) +#define AST_SMP_MBOX_CODE (AST_SMP_MAILBOX_BASE + 0x10) +#define AST_SMP_MBOX_GOSIGN 0xabbaab00 + +static void aspeed_write_smpboot(ARMCPU *cpu, + const struct arm_boot_info *info) +{ + static const uint32_t poll_mailbox_ready[] = { + /* + * r2 = per-cpu go sign value + * r1 = AST_SMP_MBOX_FIELD_ENTRY + * r0 = AST_SMP_MBOX_FIELD_GOSIGN + */ + 0xee100fb0, /* mrc p15, 0, r0, c0, c0, 5 */ + 0xe21000ff, /* ands r0, r0, #255 */ + 0xe59f201c, /* ldr r2, [pc, #28] */ + 0xe1822000, /* orr r2, r2, r0 */ + + 0xe59f1018, /* ldr r1, [pc, #24] */ + 0xe59f0018, /* ldr r0, [pc, #24] */ + + 0xe320f003, /* wfi */ + 0xe5904000, /* ldr r4, [r0] */ + 0xe1520004, /* cmp r2, r4 */ + 0x1afffffb, /* bne <wfi> */ + 0xe591f000, /* ldr pc, [r1] */ + AST_SMP_MBOX_GOSIGN, + AST_SMP_MBOX_FIELD_ENTRY, + AST_SMP_MBOX_FIELD_GOSIGN, + }; + + rom_add_blob_fixed("aspeed.smpboot", poll_mailbox_ready, + sizeof(poll_mailbox_ready), + info->smp_loader_start); +} + +static void aspeed_reset_secondary(ARMCPU *cpu, + const struct arm_boot_info *info) +{ + AddressSpace *as = arm_boot_address_space(cpu, info); + CPUState *cs = CPU(cpu); + + /* info->smp_bootreg_addr */ + address_space_stl_notdirty(as, AST_SMP_MBOX_FIELD_GOSIGN, 0, + MEMTXATTRS_UNSPECIFIED, NULL); + cpu_set_pc(cs, info->smp_loader_start); +} + #define FIRMWARE_ADDR 0x0 static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size, @@ -270,6 +322,19 @@ static void aspeed_machine_init(MachineState *machine) } } + if (machine->kernel_filename && bmc->soc.num_cpus > 1) { + /* With no u-boot we must set up a boot stub for the secondary CPU */ + MemoryRegion *smpboot = g_new(MemoryRegion, 1); + memory_region_init_ram(smpboot, OBJECT(bmc), "aspeed.smpboot", + 0x80, &error_abort); + memory_region_add_subregion(get_system_memory(), + AST_SMP_MAILBOX_BASE, smpboot); + + aspeed_board_binfo.write_secondary_boot = aspeed_write_smpboot; + aspeed_board_binfo.secondary_cpu_reset_hook = aspeed_reset_secondary; + aspeed_board_binfo.smp_loader_start = AST_SMP_MBOX_CODE; + } + aspeed_board_binfo.ram_size = ram_size; aspeed_board_binfo.loader_start = sc->memmap[ASPEED_SDRAM]; aspeed_board_binfo.nb_cpus = bmc->soc.num_cpus;
This is a boot stub that is similar to the code u-boot runs, allowing the kernel to boot the secondary CPU. u-boot works as follows: 1. Initialises the SMP mailbox area in the SCU at 0x1e6e2180 with default values 2. Copies a stub named 'mailbox_insn' from flash to the SCU, just above the mailbox area 3. Sets AST_SMP_MBOX_FIELD_READY to a magic value to indicate the secondary can begin execution from the stub 4. The stub waits until the AST_SMP_MBOX_FIELD_GOSIGN register is set to a magic value 5. Jumps to the address in AST_SMP_MBOX_FIELD_ENTRY, starting Linux Linux indicates it is ready by writing the address of its entrypoint function to AST_SMP_MBOX_FIELD_ENTRY and the 'go' magic number to AST_SMP_MBOX_FIELD_GOSIGN. The secondary CPU sees this at step 4 and breaks out of it's loop. To be compatible, a fixed qemu stub is loaded into the mailbox area. As qemu can ensure the stub is loaded before execution starts, we do not need to emulate the AST_SMP_MBOX_FIELD_READY behaviour of u-boot. The secondary CPU's program counter points to the beginning of the stub, allowing qemu to start secondaries at step four. Reboot behaviour is preserved by resetting AST_SMP_MBOX_FIELD_GOSIGN when the secondaries are reset. This is only configured when the system is booted with -kernel and qemu does not execute u-boot first. Reviewed-by: Cédric Le Goater <clg@kaod.org> Tested-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Joel Stanley <joel@jms.id.au> --- v3: Use WFI instead of WFE v2: test for number of CPUs --- hw/arm/aspeed.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+)