On 18/6/26 12:37, Alex Bennée wrote:
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

Indeed. I'll let Nuvoton folks confirm that then.



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:

Oh I missed the specific smpboot[].


   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);



Reply via email to