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 <[email protected]>
---
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);
+ }
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);
--
2.25.4
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 <[email protected]> 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 <[email protected]>
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 <[email protected]> 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 <[email protected]>
>
> 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