| Message ID | 20200703124640.42820-2-psampat@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] powerpc/powernv/idle: Exclude mfspr on HID1, 4, 5 on P9 and above | expand |
On Fri, Jul 03, 2020 at 06:16:40PM +0530, Pratik Rajesh Sampat wrote: > Additional registers DAWR0, DAWRX0 may be lost on Power 10 for > stop levels < 4. Adding Ravi Bangoria <ravi.bangoria@linux.ibm.com> to the cc. > Therefore save the values of these SPRs before entering a "stop" > state and restore their values on wakeup. > > Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com> The saving and restoration looks good to me. > --- > arch/powerpc/platforms/powernv/idle.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c > index 19d94d021357..471d4a65b1fa 100644 > --- a/arch/powerpc/platforms/powernv/idle.c > +++ b/arch/powerpc/platforms/powernv/idle.c > @@ -600,6 +600,8 @@ struct p9_sprs { > u64 iamr; > u64 amor; > u64 uamor; > + u64 dawr0; > + u64 dawrx0; > }; > > static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on) > @@ -677,6 +679,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on) > sprs.tscr = mfspr(SPRN_TSCR); > if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) > sprs.ldbar = mfspr(SPRN_LDBAR); > + if (cpu_has_feature(CPU_FTR_ARCH_31)) { > + sprs.dawr0 = mfspr(SPRN_DAWR0); > + sprs.dawrx0 = mfspr(SPRN_DAWRX0); > + } > But this is within the if condition which says if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level) This if condition is meant for stop4 and stop5 since these are stop levels that have OPAL_PM_LOSE_HYP_CONTEXT set. Since we can lose DAWR*, on states that lose limited hypervisor context, such as stop0-2, we need to unconditionally save them like AMR, IAMR etc. > sprs_saved = true; > > @@ -792,6 +798,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on) > mtspr(SPRN_MMCR2, sprs.mmcr2); > if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) > mtspr(SPRN_LDBAR, sprs.ldbar); > + if (cpu_has_feature(CPU_FTR_ARCH_31)) { > + mtspr(SPRN_DAWR0, sprs.dawr0); > + mtspr(SPRN_DAWRX0, sprs.dawrx0); > + } Likewise, we need to unconditionally restore these SPRs. > > mtspr(SPRN_SPRG3, local_paca->sprg_vdso); > > -- > 2.25.4 >
On 09/07/20 2:39 pm, Gautham R Shenoy wrote: > On Fri, Jul 03, 2020 at 06:16:40PM +0530, Pratik Rajesh Sampat wrote: >> Additional registers DAWR0, DAWRX0 may be lost on Power 10 for >> stop levels < 4. > Adding Ravi Bangoria <ravi.bangoria@linux.ibm.com> to the cc. > >> Therefore save the values of these SPRs before entering a "stop" >> state and restore their values on wakeup. >> >> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com> > > The saving and restoration looks good to me. >> --- >> arch/powerpc/platforms/powernv/idle.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c >> index 19d94d021357..471d4a65b1fa 100644 >> --- a/arch/powerpc/platforms/powernv/idle.c >> +++ b/arch/powerpc/platforms/powernv/idle.c >> @@ -600,6 +600,8 @@ struct p9_sprs { >> u64 iamr; >> u64 amor; >> u64 uamor; >> + u64 dawr0; >> + u64 dawrx0; >> }; >> >> static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on) >> @@ -677,6 +679,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on) >> sprs.tscr = mfspr(SPRN_TSCR); >> if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) >> sprs.ldbar = mfspr(SPRN_LDBAR); >> + if (cpu_has_feature(CPU_FTR_ARCH_31)) { >> + sprs.dawr0 = mfspr(SPRN_DAWR0); >> + sprs.dawrx0 = mfspr(SPRN_DAWRX0); >> + } >> > > But this is within the if condition which says > > if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level) > > This if condition is meant for stop4 and stop5 since these are stop > levels that have OPAL_PM_LOSE_HYP_CONTEXT set. > > Since we can lose DAWR*, on states that lose limited hypervisor > context, such as stop0-2, we need to unconditionally save them > like AMR, IAMR etc. > Right, shallow states too loose DAWR/X. Thanks for pointing it out. I'll fix this and resend. >> sprs_saved = true; >> >> @@ -792,6 +798,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on) >> mtspr(SPRN_MMCR2, sprs.mmcr2); >> if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) >> mtspr(SPRN_LDBAR, sprs.ldbar); >> + if (cpu_has_feature(CPU_FTR_ARCH_31)) { >> + mtspr(SPRN_DAWR0, sprs.dawr0); >> + mtspr(SPRN_DAWRX0, sprs.dawrx0); >> + } > > Likewise, we need to unconditionally restore these SPRs. > > >> mtspr(SPRN_SPRG3, local_paca->sprg_vdso); >> >> -- >> 2.25.4 >> Thanks Pratik
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c index 19d94d021357..471d4a65b1fa 100644 --- a/arch/powerpc/platforms/powernv/idle.c +++ b/arch/powerpc/platforms/powernv/idle.c @@ -600,6 +600,8 @@ struct p9_sprs { u64 iamr; u64 amor; u64 uamor; + u64 dawr0; + u64 dawrx0; }; static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on) @@ -677,6 +679,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on) sprs.tscr = mfspr(SPRN_TSCR); if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) sprs.ldbar = mfspr(SPRN_LDBAR); + if (cpu_has_feature(CPU_FTR_ARCH_31)) { + sprs.dawr0 = mfspr(SPRN_DAWR0); + sprs.dawrx0 = mfspr(SPRN_DAWRX0); + } sprs_saved = true; @@ -792,6 +798,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on) mtspr(SPRN_MMCR2, sprs.mmcr2); if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) mtspr(SPRN_LDBAR, sprs.ldbar); + if (cpu_has_feature(CPU_FTR_ARCH_31)) { + mtspr(SPRN_DAWR0, sprs.dawr0); + mtspr(SPRN_DAWRX0, sprs.dawrx0); + } mtspr(SPRN_SPRG3, local_paca->sprg_vdso);
Additional registers DAWR0, DAWRX0 may be lost on Power 10 for stop levels < 4. Therefore save the values of these SPRs before entering a "stop" state and restore their values on wakeup. Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com> --- arch/powerpc/platforms/powernv/idle.c | 10 ++++++++++ 1 file changed, 10 insertions(+)