Philippe Mathieu-Daudé <[email protected]> writes: > On 18/6/26 11:36, Alex Bennée wrote: >> This should point at the base of the Cortex-A9 MPCore private address >> space. If it doesn't we will confuse the Linux kernel as it probes the >> SCU registers and erroneously assumes the system is a buggy Aegis SOC >> and nerf the emission of SEV instructions, deadlocking any WFE's in >> the kernel (or QEMU smpboot code). > > I'm confused... > > The Configuration Base Address Register holds the physical base > address of the memory-mapped GIC CPU interface registers.
For which CPU? These are cortex-a9's not a57's and the behaviour is IMPDEF. See: https://developer.arm.com/documentation/ddi0407/i/introduction/private-memory-region > > NPCM7XX_GIC_CPU_IF_ADDR is the correct address of "the memory-mapped > GIC CPU interface", NPCM7XX_CPUP_BA is the A9MPCORE SCU MMIO address. > AFAIU the modelling looks correct. > > Per your description, the Linux guest tries to access GIC_CPU_IF_ADDR > but ends up accessing SCU_ADDR? Maybe the issue is with the > arm_boot_info::gic_cpu_if_addr address we pass to the guest? Maybe > the secondary reset code is corrupted? > > static const ARMInsnFixup smpboot[] = { > { 0xe59f2028 }, /* ldr r2, gic_cpu_if */ > { 0xe59f0028 }, /* ldr r0, bootreg_addr */ > { 0xe3a01001 }, /* mov r1, #1 */ > { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */ > { 0xe3a010ff }, /* mov r1, #0xff */ > { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */ > { 0, FIXUP_DSB }, /* dsb */ > { 0xe320f003 }, /* wfi */ > { 0xe5901000 }, /* ldr r1, [r0] */ > { 0xe1110001 }, /* tst r1, r1 */ > { 0x0afffffb }, /* beq <wfi> */ > { 0xe12fff11 }, /* bx r1 */ > { 0, FIXUP_GIC_CPU_IF }, /* gic_cpu_if: .word 0x.... */ > > Can you check ^^^ this value? I don't think we use this for npcm7xx: static void npcm7xx_write_secondary_boot(ARMCPU *cpu, const struct arm_boot_info *info) { /* * The default smpboot stub halts the secondary CPU with a 'wfi' * instruction, but the arch/arm/mach-npcm/platsmp.c in the Linux kernel * does not send an IPI to wake it up, so the second CPU fails to boot. So * we need to provide our own smpboot stub that can not use 'wfi', it has * to spin the secondary CPU until the first CPU writes to the SCRPAD reg. */ uint32_t smpboot[] = { 0xe59f2018, /* ldr r2, bootreg_addr */ 0xe3a00000, /* mov r0, #0 */ 0xe5820000, /* str r0, [r2] */ 0xe320f002, /* wfe */ 0xe5921000, /* ldr r1, [r2] */ 0xe1110001, /* tst r1, r1 */ 0x0afffffb, /* beq <wfe> */ 0xe12fff11, /* bx r1 */ NPCM7XX_SMP_BOOTREG_ADDR, }; int i; for (i = 0; i < ARRAY_SIZE(smpboot); i++) { smpboot[i] = tswap32(smpboot[i]); } rom_add_blob_fixed("smpboot", smpboot, sizeof(smpboot), NPCM7XX_SMP_LOADER_START); } > > Maybe this code has to be updated for your WFE use case? In earlier attempts I did drop the wfe in favour of a NOP until I figured out why the SEV's were not flowing. > >> Signed-off-by: Alex Bennée <[email protected]> >> Suggested-by: Arnd Bergmann <[email protected]> >> --- >> hw/arm/npcm7xx.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c >> index c2bbcd89dbc..c27f149c04a 100644 >> --- a/hw/arm/npcm7xx.c >> +++ b/hw/arm/npcm7xx.c >> @@ -492,7 +492,7 @@ static void npcm7xx_realize(DeviceState *dev, Error >> **errp) >> /* CPUs */ >> for (i = 0; i < nc->num_cpus; i++) { >> object_property_set_int(OBJECT(&s->cpu[i]), "reset-cbar", >> - NPCM7XX_GIC_CPU_IF_ADDR, &error_abort); >> + NPCM7XX_CPUP_BA, &error_abort); >> object_property_set_bool(OBJECT(&s->cpu[i]), "reset-hivecs", true, >> &error_abort); >> -- Alex Bennée Virtualisation Tech Lead @ Linaro
