svn commit: r339316 - projects/power8_bringup_hacks/sys/powerpc/powerpc

[Leandro]

Ok, I'm done for today, but I'll get back to it on Monday.

> At first I thought it was a fairly innocuous change, because "nothing"
could be doing a PCPU_GET() for pcpu[0], right?
Yes, there are some combinations of platforms/OFW where SPRG0 gets
initialized before cpu_pcpu_init() being called, but that was not the case
for POWER8/PowerNV/FDT.

> So, with this change you're making a memcpy() overwrite itself (which, I
hope, is a real nop in code?).
Yes, with this change memcpy() is overwriting the same data, that doesn't
make much sense and may not end up being turned into a nop in code (need to
check that).
The problem is that I'm not sure if removing this memcpy() won't break some
other combo.
Do you have any suggestion to detect this case and skip the copy?

For a local POWER8 machine that I have access, this was causing kexec to
hang most times.
So my basic debug strategy was to place debug prints (with OPAL for early
ones), flush them and check how far the boot would go before the machine
hanged.
Later I can give you more details on IRC if you are interested :)

- Leandro

On Thu, Oct 11, 2018 at 5:08 PM Justin Hibbits <chmeeedalf at gmail.com> wrote:

> On Thu, 11 Oct 2018 19:18:06 +0000 (UTC)
> Leandro Lupori <luporl at FreeBSD.org> wrote:
>
> > Author: luporl
> > Date: Thu Oct 11 19:18:06 2018
> > New Revision: 339316
> > URL: https://svnweb.freebsd.org/changeset/base/339316
> >
> > Log:
> >   Initialize SPRG0 before its first possible use.
> >
> > Modified:
> >   projects/power8_bringup_hacks/sys/powerpc/powerpc/machdep.c
> >
> > Modified: projects/power8_bringup_hacks/sys/powerpc/powerpc/machdep.c
> >
> ==============================================================================
> > ---
> > projects/power8_bringup_hacks/sys/powerpc/powerpc/machdep.c
> > Thu Oct 11 19:06:54 2018      (r339315) +++
> > projects/power8_bringup_hacks/sys/powerpc/powerpc/machdep.c
> > Thu Oct 11 19:18:06 2018      (r339316) @@ -383,12 +383,12 @@
> > powerpc_init(vm_offset_t fdt, vm_offset_t toc, vm_offs if
> > (platform_smp_get_bsp(&bsp) != 0) bsp.cr_cpuid = 0; pc =
> > &__pcpu[bsp.cr_cpuid];
> > +     __asm __volatile("mtsprg 0, %0" :: "r"(pc));
> >       pcpu_init(pc, bsp.cr_cpuid, sizeof(struct pcpu));
> >       pc->pc_curthread = &thread0;
> >       thread0.td_oncpu = bsp.cr_cpuid;
> >       pc->pc_cpuid = bsp.cr_cpuid;
> >       pc->pc_hwref = bsp.cr_hwref;
> > -     __asm __volatile("mtsprg 0, %0" :: "r"(pc));
> >
> >       /*
> >        * Init KDB
> >
>
> Wow, that's quite some sleuthing!
>
> Consider this approved by me, and email re@ to get it into head.  Then
> hop on IRC and give me the play-by-play on how you figured it out.  I'm
> really curious!
>
> At first I thought it was a fairly innocuous change, because "nothing"
> could be doing a PCPU_GET() for pcpu[0], right?  Then I saw AIM's
> cpu_pcpu_init() doing a memcpy() from PCPU_GET(aim.slb).  So, with this
> change you're making a memcpy() overwrite itself (which, I hope, is a
> real nop in code?).
>
> - Justin
>