[Skiboot] [PATCH] Bail out when booting in big core mode

[Joel Stanley]

On Wed, 29 Apr 2020 at 08:05, Vaidyanathan Srinivasan
<svaidy at linux.ibm.com> wrote:
>
> * Joel Stanley <joel at jms.id.au> [2020-04-29 12:48:06]:
>
> > This is not supported and will result in a misbehaving system later in
> > boot.
> >
> > We wait until the console is up so the developer can see what happened.
> >
> > Signed-off-by: Joel Stanley <joel at jms.id.au>
> > ---
> > I see that there are some patches on the list to support this mode. If
> > they don't go in soon, I ask that this change be added (and backported)
> > to help developers who accidentally boot in big core mode.
> >
> >  core/cpu.c          | 9 +++++++++
> >  core/init.c         | 3 +++
> >  include/cpu.h       | 3 +++
> >  include/processor.h | 2 ++
> >  4 files changed, 17 insertions(+)
> >
> > diff --git a/core/cpu.c b/core/cpu.c
> > index 37d9f41a8b79..f0c7e881e0f0 100644
> > --- a/core/cpu.c
> > +++ b/core/cpu.c
> > @@ -935,6 +935,15 @@ static void init_cpu_thread(struct cpu_thread *t,
> >       assert(pir == container_of(t, struct cpu_stack, cpu) - cpu_stacks);
> >  }
> >
> > +void cpu_check_bigcore(void)
> > +{
> > +     unsigned int pvr = mfspr(SPR_PVR);
> > +     if (PVR_TYPE(pvr) == PVR_TYPE_P9 && !(PVR_CHIP_TYPE(pvr) & 0x1)) {
> > +             prerror("CPU: Detected unsupported POWER9 in big core mode.\n");
> > +             abort();
> > +     }
> > +}
> > +
> >  static void enable_attn(void)
> >  {
> >       unsigned long hid0;
> > diff --git a/core/init.c b/core/init.c
> > index 39cfd3fbe1b1..73a6756ebdb3 100644
> > --- a/core/init.c
> > +++ b/core/init.c
> > @@ -1284,6 +1284,9 @@ void __noreturn __nomcount main_cpu_entry(const void *fdt)
> >       /* Install the OPAL Console handlers */
> >       init_opal_console();
> >
> > +     /* Now that the console is up, bail out if we do not support the machine */
> > +     cpu_check_bigcore();
> > +
>
> Hi Joel,
>
> Without the big core enablement patch we hit asset() in
> dt_add_cpufeatures() much earlier than this place.

I don't see that on the hardware I was testing on (witherspoon with
overrides set to boot the machine in big core mode).

I was also testing in mambo with PVR set:

 myconf config processor/initial/PVR 0x4e0202

I understand that is only testing the PVR check, and not other side effects.

> I like the idea of atleast detecting the platform and then printing on
> console and then abort().
>
> But I do not know on which platform or combination of setting you
> actually get to this point.  Share the PVR you ended up with.
>
> We will actually need to check in Core-Thread-State register bit 63 to
> confirm fused-core mode.

The PVR of the witherspoon system is 0x4e0202. This changes when you
set the overrides to boot in big core mode, with no other changes to
the system (same host firmware load, etc).

I haven't checked the other register.

> Let me split out the fused-core detection logic from the patch series
> and add this clean way of aborting with message.  That will solve
> cases where we unexpectedly boot in incorrect modes. This is a good
> idea.

Thanks! I'd like this patch to make its way to the stable tree if
possible, so making it minimal would be ideal.

Cheers,

Joel