Re: [PATCH v2] aspeed: Add boot stub for smp booting

[Joel Stanley]

On Thu, 16 Apr 2020 at 16:48, Peter Maydell <address@hidden> wrote:
>
> On Thu, 9 Apr 2020 at 07:31, Joel Stanley <address@hidden> wrote:
> >
> > This is a boot stub that is similar to the code u-boot runs, allowing
> > the kernel to boot the secondary CPU.
>
> > +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]         */
> > +
> > +        0xe320f002,  /* wfe                           */
> > +        0xe5904000,  /* ldr     r4, [r0]              */
> > +        0xe1520004,  /* cmp     r2, r4                */
> > +        0x1afffffb,  /* bne     <wfe>                 */
>
> Note that unlike "wfi", QEMU's "wfe" implementation is merely
> a 'yield', so a secondary-CPU boot loop that has wfe in it
> will basically be a busy-loop of those vcpu threads.
> (This is why the smpboot code in hw/arm/boot.c uses wfi.)
>
> I don't suppose the secondary boot protocol on these boards
> is such that a wfi loop will work ? (Depends on what the
> primary code in the kernel does to prod the secondary after
> writing the magic value.)

Thanks for the review.

I chose wfe as it's what aspeed's u-boot has. I don't have a strong
understand of this area of ARM processors, nor do I have any insight
into why that choice was made.

I did some testing and wfi in the qemu stub works fine with Linux so I
will respin with that instead.

Cheers,

Joel