All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] powerpc/powernv/idle: Exclude mfspr on HID1,4,5 on P9 and above
@ 2020-07-03 12:46 ` Pratik Rajesh Sampat
  0 siblings, 0 replies; 9+ messages in thread
From: Pratik Rajesh Sampat @ 2020-07-03 12:46 UTC (permalink / raw)
  To: mpe, linuxppc-dev, linux-kernel, benh, paulus, ego, svaidy,
	psampat, pratik.r.sampat

POWER9 onwards the support for the registers HID1, HID4, HID5 has been
receded.
Although mfspr on the above registers worked in Power9, In Power10
simulator is unrecognized. Moving their assignment under the
check for machines lower than Power9

Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/idle.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 2dd467383a88..19d94d021357 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -73,9 +73,6 @@ static int pnv_save_sprs_for_deep_states(void)
 	 */
 	uint64_t lpcr_val	= mfspr(SPRN_LPCR);
 	uint64_t hid0_val	= mfspr(SPRN_HID0);
-	uint64_t hid1_val	= mfspr(SPRN_HID1);
-	uint64_t hid4_val	= mfspr(SPRN_HID4);
-	uint64_t hid5_val	= mfspr(SPRN_HID5);
 	uint64_t hmeer_val	= mfspr(SPRN_HMEER);
 	uint64_t msr_val = MSR_IDLE;
 	uint64_t psscr_val = pnv_deepest_stop_psscr_val;
@@ -117,6 +114,9 @@ static int pnv_save_sprs_for_deep_states(void)
 
 			/* Only p8 needs to set extra HID regiters */
 			if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
+				uint64_t hid1_val = mfspr(SPRN_HID1);
+				uint64_t hid4_val = mfspr(SPRN_HID4);
+				uint64_t hid5_val = mfspr(SPRN_HID5);
 
 				rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
 				if (rc != 0)
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 1/2] powerpc/powernv/idle: Exclude mfspr on HID1, 4, 5 on P9 and above
@ 2020-07-03 12:46 ` Pratik Rajesh Sampat
  0 siblings, 0 replies; 9+ messages in thread
From: Pratik Rajesh Sampat @ 2020-07-03 12:46 UTC (permalink / raw)
  To: mpe, linuxppc-dev, linux-kernel, benh, paulus, ego, svaidy,
	psampat, pratik.r.sampat

POWER9 onwards the support for the registers HID1, HID4, HID5 has been
receded.
Although mfspr on the above registers worked in Power9, In Power10
simulator is unrecognized. Moving their assignment under the
check for machines lower than Power9

Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/idle.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 2dd467383a88..19d94d021357 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -73,9 +73,6 @@ static int pnv_save_sprs_for_deep_states(void)
 	 */
 	uint64_t lpcr_val	= mfspr(SPRN_LPCR);
 	uint64_t hid0_val	= mfspr(SPRN_HID0);
-	uint64_t hid1_val	= mfspr(SPRN_HID1);
-	uint64_t hid4_val	= mfspr(SPRN_HID4);
-	uint64_t hid5_val	= mfspr(SPRN_HID5);
 	uint64_t hmeer_val	= mfspr(SPRN_HMEER);
 	uint64_t msr_val = MSR_IDLE;
 	uint64_t psscr_val = pnv_deepest_stop_psscr_val;
@@ -117,6 +114,9 @@ static int pnv_save_sprs_for_deep_states(void)
 
 			/* Only p8 needs to set extra HID regiters */
 			if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
+				uint64_t hid1_val = mfspr(SPRN_HID1);
+				uint64_t hid4_val = mfspr(SPRN_HID4);
+				uint64_t hid5_val = mfspr(SPRN_HID5);
 
 				rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
 				if (rc != 0)
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] powerpc/powernv/idle: save-restore DAWR0,DAWRX0 for P10
  2020-07-03 12:46 ` [PATCH 1/2] powerpc/powernv/idle: Exclude mfspr on HID1, 4, 5 " Pratik Rajesh Sampat
  (?)
@ 2020-07-03 12:46 ` Pratik Rajesh Sampat
  2020-07-09  9:09     ` Gautham R Shenoy
  -1 siblings, 1 reply; 9+ messages in thread
From: Pratik Rajesh Sampat @ 2020-07-03 12:46 UTC (permalink / raw)
  To: mpe, linuxppc-dev, linux-kernel, benh, paulus, ego, svaidy,
	psampat, pratik.r.sampat

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(+)

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] powerpc/powernv/idle: Exclude mfspr on HID1,4,5 on P9 and above
  2020-07-03 12:46 ` [PATCH 1/2] powerpc/powernv/idle: Exclude mfspr on HID1, 4, 5 " Pratik Rajesh Sampat
@ 2020-07-09  9:01   ` Gautham R Shenoy
  -1 siblings, 0 replies; 9+ messages in thread
From: Gautham R Shenoy @ 2020-07-09  9:01 UTC (permalink / raw)
  To: Pratik Rajesh Sampat
  Cc: mpe, linuxppc-dev, linux-kernel, benh, paulus, ego, svaidy,
	pratik.r.sampat

On Fri, Jul 03, 2020 at 06:16:39PM +0530, Pratik Rajesh Sampat wrote:
> POWER9 onwards the support for the registers HID1, HID4, HID5 has been
> receded.
> Although mfspr on the above registers worked in Power9, In Power10
> simulator is unrecognized. Moving their assignment under the
> check for machines lower than Power9
> 
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>

Nice catch.

Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>

> ---
>  arch/powerpc/platforms/powernv/idle.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 2dd467383a88..19d94d021357 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -73,9 +73,6 @@ static int pnv_save_sprs_for_deep_states(void)
>  	 */
>  	uint64_t lpcr_val	= mfspr(SPRN_LPCR);
>  	uint64_t hid0_val	= mfspr(SPRN_HID0);
> -	uint64_t hid1_val	= mfspr(SPRN_HID1);
> -	uint64_t hid4_val	= mfspr(SPRN_HID4);
> -	uint64_t hid5_val	= mfspr(SPRN_HID5);
>  	uint64_t hmeer_val	= mfspr(SPRN_HMEER);
>  	uint64_t msr_val = MSR_IDLE;
>  	uint64_t psscr_val = pnv_deepest_stop_psscr_val;
> @@ -117,6 +114,9 @@ static int pnv_save_sprs_for_deep_states(void)
> 
>  			/* Only p8 needs to set extra HID regiters */
>  			if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
> +				uint64_t hid1_val = mfspr(SPRN_HID1);
> +				uint64_t hid4_val = mfspr(SPRN_HID4);
> +				uint64_t hid5_val = mfspr(SPRN_HID5);
> 
>  				rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
>  				if (rc != 0)
> -- 
> 2.25.4
> 
--
Thanks and Regards
gautham.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] powerpc/powernv/idle: Exclude mfspr on HID1,4,5 on P9 and above
@ 2020-07-09  9:01   ` Gautham R Shenoy
  0 siblings, 0 replies; 9+ messages in thread
From: Gautham R Shenoy @ 2020-07-09  9:01 UTC (permalink / raw)
  To: Pratik Rajesh Sampat
  Cc: ego, pratik.r.sampat, linux-kernel, paulus, linuxppc-dev

On Fri, Jul 03, 2020 at 06:16:39PM +0530, Pratik Rajesh Sampat wrote:
> POWER9 onwards the support for the registers HID1, HID4, HID5 has been
> receded.
> Although mfspr on the above registers worked in Power9, In Power10
> simulator is unrecognized. Moving their assignment under the
> check for machines lower than Power9
> 
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>

Nice catch.

Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>

> ---
>  arch/powerpc/platforms/powernv/idle.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 2dd467383a88..19d94d021357 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -73,9 +73,6 @@ static int pnv_save_sprs_for_deep_states(void)
>  	 */
>  	uint64_t lpcr_val	= mfspr(SPRN_LPCR);
>  	uint64_t hid0_val	= mfspr(SPRN_HID0);
> -	uint64_t hid1_val	= mfspr(SPRN_HID1);
> -	uint64_t hid4_val	= mfspr(SPRN_HID4);
> -	uint64_t hid5_val	= mfspr(SPRN_HID5);
>  	uint64_t hmeer_val	= mfspr(SPRN_HMEER);
>  	uint64_t msr_val = MSR_IDLE;
>  	uint64_t psscr_val = pnv_deepest_stop_psscr_val;
> @@ -117,6 +114,9 @@ static int pnv_save_sprs_for_deep_states(void)
> 
>  			/* Only p8 needs to set extra HID regiters */
>  			if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
> +				uint64_t hid1_val = mfspr(SPRN_HID1);
> +				uint64_t hid4_val = mfspr(SPRN_HID4);
> +				uint64_t hid5_val = mfspr(SPRN_HID5);
> 
>  				rc = opal_slw_set_reg(pir, SPRN_HID1, hid1_val);
>  				if (rc != 0)
> -- 
> 2.25.4
> 
--
Thanks and Regards
gautham.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] powerpc/powernv/idle: save-restore DAWR0,DAWRX0 for P10
  2020-07-03 12:46 ` [PATCH 2/2] powerpc/powernv/idle: save-restore DAWR0,DAWRX0 for P10 Pratik Rajesh Sampat
@ 2020-07-09  9:09     ` Gautham R Shenoy
  0 siblings, 0 replies; 9+ messages in thread
From: Gautham R Shenoy @ 2020-07-09  9:09 UTC (permalink / raw)
  To: Pratik Rajesh Sampat
  Cc: mpe, linuxppc-dev, linux-kernel, benh, paulus, ego, svaidy,
	pratik.r.sampat, Ravi Bangoria

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
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] powerpc/powernv/idle: save-restore DAWR0,DAWRX0 for P10
@ 2020-07-09  9:09     ` Gautham R Shenoy
  0 siblings, 0 replies; 9+ messages in thread
From: Gautham R Shenoy @ 2020-07-09  9:09 UTC (permalink / raw)
  To: Pratik Rajesh Sampat
  Cc: ego, pratik.r.sampat, linux-kernel, paulus, linuxppc-dev, Ravi Bangoria

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
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] powerpc/powernv/idle: save-restore DAWR0,DAWRX0 for P10
  2020-07-09  9:09     ` Gautham R Shenoy
@ 2020-07-09 11:21       ` Pratik Sampat
  -1 siblings, 0 replies; 9+ messages in thread
From: Pratik Sampat @ 2020-07-09 11:21 UTC (permalink / raw)
  To: ego
  Cc: mpe, linuxppc-dev, linux-kernel, benh, paulus, svaidy,
	pratik.r.sampat, Ravi Bangoria



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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] powerpc/powernv/idle: save-restore DAWR0,DAWRX0 for P10
@ 2020-07-09 11:21       ` Pratik Sampat
  0 siblings, 0 replies; 9+ messages in thread
From: Pratik Sampat @ 2020-07-09 11:21 UTC (permalink / raw)
  To: ego; +Cc: Ravi Bangoria, pratik.r.sampat, linux-kernel, paulus, linuxppc-dev



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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-07-09 11:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-03 12:46 [PATCH 1/2] powerpc/powernv/idle: Exclude mfspr on HID1,4,5 on P9 and above Pratik Rajesh Sampat
2020-07-03 12:46 ` [PATCH 1/2] powerpc/powernv/idle: Exclude mfspr on HID1, 4, 5 " Pratik Rajesh Sampat
2020-07-03 12:46 ` [PATCH 2/2] powerpc/powernv/idle: save-restore DAWR0,DAWRX0 for P10 Pratik Rajesh Sampat
2020-07-09  9:09   ` Gautham R Shenoy
2020-07-09  9:09     ` Gautham R Shenoy
2020-07-09 11:21     ` Pratik Sampat
2020-07-09 11:21       ` Pratik Sampat
2020-07-09  9:01 ` [PATCH 1/2] powerpc/powernv/idle: Exclude mfspr on HID1,4,5 on P9 and above Gautham R Shenoy
2020-07-09  9:01   ` Gautham R Shenoy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.