Date: Tue, 30 Jan 2018 07:52:55 -0800 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: Wojciech Macek <wma@semihalf.com> Cc: Wojciech Macek <wma@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org, Patryk Duda <pdk@semihalf.com> Subject: Re: svn commit: r328537 - in head/sys/powerpc: aim powernv Message-ID: <1aeae6a0-dcc6-0c67-7ce0-30aadb02dc72@freebsd.org> In-Reply-To: <CANsEV8e1adG00C-L4x%2BF5r5uRx-ME9VTry=JyaLzOcTPEhO2qw@mail.gmail.com> References: <201801290927.w0T9R2ot008700@repo.freebsd.org> <2b1dc9be-493e-57fb-d012-0af52dc7475b@freebsd.org> <CANsEV8e1adG00C-L4x%2BF5r5uRx-ME9VTry=JyaLzOcTPEhO2qw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 01/30/18 01:04, Wojciech Macek wrote: > The LPCR register must be set very early. The best is to do it in > cpudep_bootstrap as this is the first function being run on the AP > after start. > As soon as the AP completes pmap_cpu_bootstrap, we must guarantee that > DSI exceptions are working fine. We can't do this with LPCR set > incorrectly, this it contains a base addres of an exception table. Hmm, yes. I had forgotten this ran after pmap_cpu_bootstrap(). Thanks for the explanation! > The code from powernv_smp_ap_init was moved to attach, as we don't set > LPCR twice during AP startup - it's not an error of course, but I like > to have this in one place only. > > I can revert this patch if you insist, but to fix non-powernv boards > we can just add "if (mfmsr() & PSL_HV)" check to early_bootstrap. What > do you think? Let's just add the PSL_HV check. We could add another platform_early() method, but I don't see much point in it here. -Nathan > > Regards, > Wojtek > > 2018-01-29 16:46 GMT+01:00 Nathan Whitehorn <nwhitehorn@freebsd.org > <mailto:nwhitehorn@freebsd.org>>: > > Can you explain why this is necessary? Both functions are run in > the same context and this way of doing things breaks important > abstraction barriers. > > Since it also breaks booting on pHyp systems, I would appreciate > it if you could revert this pending review. > -Nathan > > > On 01/29/18 01:27, Wojciech Macek wrote: > > Author: wma > Date: Mon Jan 29 09:27:02 2018 > New Revision: 328537 > URL: https://svnweb.freebsd.org/changeset/base/328537 > <https://svnweb.freebsd.org/changeset/base/328537>; > > Log: > PowerNV: move LPCR and LPID altering to > cpudep_ap_early_bootstrap > It turns out that under some circumstances we can get > DSI or DSE before we set > LPCR and LPID so we should set it as early as possible. > Authored by: Patryk Duda <pdk@semihalf.com > <mailto:pdk@semihalf.com>> > Submitted by: Wojciech Macek <wma@semihalf.com > <mailto:wma@semihalf.com>> > Obtained from: Semihalf > Sponsored by: IBM, QCM Technologies > > Modified: > head/sys/powerpc/aim/mp_cpudep.c > head/sys/powerpc/powernv/platform_powernv.c > > Modified: head/sys/powerpc/aim/mp_cpudep.c > ============================================================================== > --- head/sys/powerpc/aim/mp_cpudep.c Mon Jan 29 09:24:28 > 2018 (r328536) > +++ head/sys/powerpc/aim/mp_cpudep.c Mon Jan 29 09:27:02 > 2018 (r328537) > @@ -64,9 +64,6 @@ cpudep_ap_early_bootstrap(void) > register_t reg; > #endif > - __asm __volatile("mtsprg 0, %0" :: "r"(ap_pcpu)); > - powerpc_sync(); > - > switch (mfpvr() >> 16) { > case IBM970: > case IBM970FX: > @@ -86,7 +83,20 @@ cpudep_ap_early_bootstrap(void) > #endif > powerpc_sync(); > break; > + case IBMPOWER8: > + case IBMPOWER8E: > + isync(); > + /* Direct interrupts to SRR instead of HSRR > and reset LPCR otherwise */ > + mtspr(SPR_LPID, 0); > + isync(); > + > + mtspr(SPR_LPCR, LPCR_LPES); > + isync(); > + break; > } > + > + __asm __volatile("mtsprg 0, %0" :: "r"(ap_pcpu)); > + powerpc_sync(); > } > uintptr_t > > Modified: head/sys/powerpc/powernv/platform_powernv.c > ============================================================================== > --- head/sys/powerpc/powernv/platform_powernv.c Mon Jan 29 > 09:24:28 2018 (r328536) > +++ head/sys/powerpc/powernv/platform_powernv.c Mon Jan 29 > 09:27:02 2018 (r328537) > @@ -128,6 +128,7 @@ powernv_attach(platform_t plat) > pcell_t prop; > phandle_t cpu; > int res, len, node, idx; > + register_t msr; > /* Ping OPAL again just to make sure */ > opal_check(); > @@ -141,6 +142,19 @@ powernv_attach(platform_t plat) > cpu_idle_hook = powernv_cpu_idle; > powernv_boot_pir = mfspr(SPR_PIR); > + /* LPID must not be altered when PSL_DR or PSL_IR is > set */ > + msr = mfmsr(); > + mtmsr(msr & ~(PSL_DR | PSL_IR)); > + > + /* Direct interrupts to SRR instead of HSRR and reset > LPCR otherwise */ > + mtspr(SPR_LPID, 0); > + isync(); > + > + mtmsr(msr); > + > + mtspr(SPR_LPCR, LPCR_LPES); > + isync(); > + > /* Init CPU bits */ > powernv_smp_ap_init(plat); > @@ -444,21 +458,6 @@ powernv_reset(platform_t platform) > static void > powernv_smp_ap_init(platform_t platform) > { > - register_t msr; > - > - /* LPID must not be altered when PSL_DR or PSL_IR is > set */ > - msr = mfmsr(); > - mtmsr(msr & ~(PSL_DR | PSL_IR)); > - > - isync(); > - /* Direct interrupts to SRR instead of HSRR and reset > LPCR otherwise */ > - mtspr(SPR_LPID, 0); > - isync(); > - > - mtmsr(msr); > - > - mtspr(SPR_LPCR, LPCR_LPES); > - isync(); > } > static void > > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1aeae6a0-dcc6-0c67-7ce0-30aadb02dc72>