Date: Tue, 30 Jan 2018 10:04:22 +0100 From: Wojciech Macek <wma@semihalf.com> To: Nathan Whitehorn <nwhitehorn@freebsd.org> 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: <CANsEV8e1adG00C-L4x%2BF5r5uRx-ME9VTry=JyaLzOcTPEhO2qw@mail.gmail.com> In-Reply-To: <2b1dc9be-493e-57fb-d012-0af52dc7475b@freebsd.org> References: <201801290927.w0T9R2ot008700@repo.freebsd.org> <2b1dc9be-493e-57fb-d012-0af52dc7475b@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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. 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? Regards, Wojtek 2018-01-29 16:46 GMT+01:00 Nathan Whitehorn <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 >> >> 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> >> Submitted by: Wojciech Macek <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?CANsEV8e1adG00C-L4x%2BF5r5uRx-ME9VTry=JyaLzOcTPEhO2qw>