Date: Thu, 11 Oct 2018 18:09:29 -0300 From: Leandro <leandro.lupori@gmail.com> To: Justin Hibbits <chmeeedalf@gmail.com> Cc: luporl@freebsd.org, src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: Re: svn commit: r339316 - projects/power8_bringup_hacks/sys/powerpc/powerpc Message-ID: <CAC7XEcKVDRFRXiKq1xbO=S8PNrRHksPh6usRYx5BpcQbjmyh1A@mail.gmail.com> In-Reply-To: <20181011150805.7234d85c@ralga.knownspace> References: <201810111918.w9BJI6Ak067450@repo.freebsd.org> <20181011150805.7234d85c@ralga.knownspace>
next in thread | previous in thread | raw e-mail | index | archive | help
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@gmail.com> wrote: > On Thu, 11 Oct 2018 19:18:06 +0000 (UTC) > Leandro Lupori <luporl@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 >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAC7XEcKVDRFRXiKq1xbO=S8PNrRHksPh6usRYx5BpcQbjmyh1A>