Re: [PATCH 1/3] powerpc/powernv: Avoid the secondary hold spinloop for OPAL boot

On Wed, 11 Oct 2017 22:27:23 +1100
Michael Ellerman [off-list ref] wrote:

Nicholas Piggin [off-list ref] writes:

quoted

On Wed, 11 Oct 2017 01:58:28 +1000
Nicholas Piggin [off-list ref] wrote:

 

quoted

Ahh okay, pseries is using the start-cpu RTAS call to enter at
generic_secondary_smp_init() as well. So we can take it out for
pseries as well.  

This patch seems to do the trick for pseries guests too:

powerpc/64s: Avoid waiting for secondary hold spinloop if it is not used

OPAL and some RTAS boot does not insert secondaries at 0x60 to wait at
the secondary hold spinloop. Instead they are started later, at
generic_secondary_smp_init(), which is after the secondary hold
spinloop.

Avoid waiting on this spinloop when booting with OPAL firmware, or
when the RTAS boot does not use this loop. This wait always times
out in those cases.

This saves 100ms boot time on bare metal (10s of seconds of real time
when booting on the simulator in SMP), and 100ms on modern pseries
guests.  

My instinct was to say "huh, that's not how it works on pseries!".

But then I see this was all changed in:

  dbe78b401186 ("powerpc/pseries: Do not start secondaries in Open Firmware") (Sep 2013)


So that is where my confusion comes from. Most of the code and comments
still talk about secondaries coming in at 0x60, but that's really only
on "legacy" machines.

I guess I can merge this, but really this code needs a proper cleanup. I
dislike all this platform specific knowledge ending up in setup_64.c.

If we had an smp_ops->spinning_secondaries() that tells the spin
loop how many secondaries to wait for, it could all go in platform code
I think.

Yeah, not sure how best to do it. What I wanted to do was just increment
spinning_secondaries in prom_init as we inserted them to 0x60 (or the
0x100 for pmac or whatever). But prom_init doesn't like referencing outside
variables so there goes that.

The check for use_spinloop() would just become a short-circuit check of
spinning_secondaries == 0.

Yeah maybe that would be enough. I don't know if half that setup_arch
could be per-platformirized, including smp_release_cpus().

The other issue is kexec. IIRC when we kexec on pseries we don't return
the CPUs to RTAS, so then they *are* spinning at 0x60. But maybe that's
changed since I last looked at it too :)

Oh I might have forgotten to test that on pseries, so I'll try that.

Thanks,
Nick