diff mbox series

[59/61] xive2: Add NCU_SPEC_BAR to stop engine for restore

Message ID 20210719132012.150948-60-hegdevasant@linux.vnet.ibm.com
State Superseded
Headers show
Series P10 Enablement | expand

Commit Message

Vasant Hegde July 19, 2021, 1:20 p.m. UTC
From: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>

P10 Stop engines have apis similar to P9 to set xscom restores
after wakeup from deep-sleep states.

This xscom restore will be used to support STOP11 on P10.

Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
 hw/xive2.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

Comments

Cédric Le Goater July 21, 2021, 1:35 p.m. UTC | #1
On 7/19/21 3:20 PM, Vasant Hegde wrote:
> From: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
> 
> P10 Stop engines have apis similar to P9 to set xscom restores
> after wakeup from deep-sleep states.
> 
> This xscom restore will be used to support STOP11 on P10.
> 
> Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> ---
>  hw/xive2.c | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/xive2.c b/hw/xive2.c
> index f559b0f9a..d2fc2ad69 100644
> --- a/hw/xive2.c
> +++ b/hw/xive2.c
> @@ -20,8 +20,8 @@
>  #include <bitmap.h>
>  #include <buddy.h>
>  #include <phys-map.h>
> -#include <p9_stop_api.H> /* TODO (p10): need P10 stop state engine */
> -
> +#include <p9_stop_api.H>
> +#include <p10_stop_api.H>

Why do we need both ? 

C. 

>  
>  /* Verbose debug */
>  #undef XIVE_VERBOSE_DEBUG
> @@ -3022,11 +3022,32 @@ static void xive_configure_ex_special_bar(struct xive *x, struct cpu_thread *c)
>  
>  void xive2_late_init(void)
>  {
> +	struct cpu_thread *c;
> +
>  	prlog(PR_INFO, "SLW: Configuring self-restore for NCU_SPEC_BAR\n");
> -			/*
> -			 * TODO (p10): need P10 stop state engine and fix for STOP11
> -			 */
> +	for_each_present_cpu(c) {
> +		if(cpu_is_thread0(c)) {
> +			struct proc_chip *chip = get_chip(c->chip_id);
> +			struct xive *x = chip->xive;
> +			uint64_t xa, val, rc;
> +			xa = XSCOM_ADDR_P10_NCU(pir_to_core_id(c->pir), P10_NCU_SPEC_BAR);
> +			val = (uint64_t)x->tm_base | P10_NCU_SPEC_BAR_ENABLE;
> +			/* Bail out if wakeup engine has already failed */
> +			if (wakeup_engine_state != WAKEUP_ENGINE_PRESENT) {
> +				prlog(PR_ERR, "XIVE proc_stop_api fail detected\n");
> +				break;
> +			}
> +			rc = proc_stop_save_scom((void *)chip->homer_base, xa, val,
> +				PROC_STOP_SCOM_REPLACE, PROC_STOP_SECTION_L3);
> +			if (rc) {
> +				xive_cpu_err(c, "proc_stop_save_scom failed for NCU_SPEC_BAR rc=%lld\n",
> +					     rc);
> +				wakeup_engine_state = WAKEUP_ENGINE_FAILED;
> +			}
> +		}
> +	}
>  }
> +
>  static void xive_provision_cpu(struct xive_cpu_state *xs, struct cpu_thread *c)
>  {
>  	struct xive *x;
>
Vasant Hegde July 22, 2021, 9:31 a.m. UTC | #2
On 7/21/21 7:05 PM, Cédric Le Goater wrote:
> On 7/19/21 3:20 PM, Vasant Hegde wrote:
>> From: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
>>
>> P10 Stop engines have apis similar to P9 to set xscom restores
>> after wakeup from deep-sleep states.
>>
>> This xscom restore will be used to support STOP11 on P10.
>>
>> Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
>> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>> ---
>>   hw/xive2.c | 31 ++++++++++++++++++++++++++-----
>>   1 file changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/xive2.c b/hw/xive2.c
>> index f559b0f9a..d2fc2ad69 100644
>> --- a/hw/xive2.c
>> +++ b/hw/xive2.c
>> @@ -20,8 +20,8 @@
>>   #include <bitmap.h>
>>   #include <buddy.h>
>>   #include <phys-map.h>
>> -#include <p9_stop_api.H> /* TODO (p10): need P10 stop state engine */
>> -
>> +#include <p9_stop_api.H>
>> +#include <p10_stop_api.H>
> 
> Why do we need both ?
> 

Because p10_stop_api.H uses definitions from p9_stop_api.H. May be we should 
move common
definitions to separate header file (say stop_api.h ) and then include that in 
P[9/10]_stop_api.H file?

-Vasant
Pratik Sampat July 23, 2021, 3:03 p.m. UTC | #3
On 22/07/21 3:01 pm, Vasant Hegde wrote:
> On 7/21/21 7:05 PM, Cédric Le Goater wrote:
>> On 7/19/21 3:20 PM, Vasant Hegde wrote:
>>> From: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
>>>
>>> P10 Stop engines have apis similar to P9 to set xscom restores
>>> after wakeup from deep-sleep states.
>>>
>>> This xscom restore will be used to support STOP11 on P10.
>>>
>>> Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
>>> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
>>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>>> ---
>>>   hw/xive2.c | 31 ++++++++++++++++++++++++++-----
>>>   1 file changed, 26 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/xive2.c b/hw/xive2.c
>>> index f559b0f9a..d2fc2ad69 100644
>>> --- a/hw/xive2.c
>>> +++ b/hw/xive2.c
>>> @@ -20,8 +20,8 @@
>>>   #include <bitmap.h>
>>>   #include <buddy.h>
>>>   #include <phys-map.h>
>>> -#include <p9_stop_api.H> /* TODO (p10): need P10 stop state engine */
>>> -
>>> +#include <p9_stop_api.H>
>>> +#include <p10_stop_api.H>
>>
>> Why do we need both ?
>>
>
> Because p10_stop_api.H uses definitions from p9_stop_api.H. May be we 
> should move common
> definitions to separate header file (say stop_api.h ) and then include 
> that in P[9/10]_stop_api.H file?
>
I agree that there are a lot of common definitions around both P9 and P10 stop
API header, and could definitely be cleaned up and merged. However the headers
are direct mirror from hostboot and they maintain different code-bases for
P9 and P10.

My cause of concern stems from the fact that if we refactor the libraries here
and if hostboot updates their libraries we would manually each time have
to code those changes up in our refactored library too.

Thanks,
Pratik

> -Vasant
>
Vasant Hegde July 27, 2021, 10:50 a.m. UTC | #4
On 7/23/21 8:33 PM, Pratik Sampat wrote:
> 
> 
> On 22/07/21 3:01 pm, Vasant Hegde wrote:
>> On 7/21/21 7:05 PM, Cédric Le Goater wrote:
>>> On 7/19/21 3:20 PM, Vasant Hegde wrote:
>>>> From: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
>>>>
>>>> P10 Stop engines have apis similar to P9 to set xscom restores
>>>> after wakeup from deep-sleep states.
>>>>
>>>> This xscom restore will be used to support STOP11 on P10.
>>>>
>>>> Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
>>>> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
>>>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>>>> ---
>>>>   hw/xive2.c | 31 ++++++++++++++++++++++++++-----
>>>>   1 file changed, 26 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/hw/xive2.c b/hw/xive2.c
>>>> index f559b0f9a..d2fc2ad69 100644
>>>> --- a/hw/xive2.c
>>>> +++ b/hw/xive2.c
>>>> @@ -20,8 +20,8 @@
>>>>   #include <bitmap.h>
>>>>   #include <buddy.h>
>>>>   #include <phys-map.h>
>>>> -#include <p9_stop_api.H> /* TODO (p10): need P10 stop state engine */
>>>> -
>>>> +#include <p9_stop_api.H>
>>>> +#include <p10_stop_api.H>
>>>
>>> Why do we need both ?
>>>
>>
>> Because p10_stop_api.H uses definitions from p9_stop_api.H. May be we should 
>> move common
>> definitions to separate header file (say stop_api.h ) and then include that in 
>> P[9/10]_stop_api.H file?
>>
> I agree that there are a lot of common definitions around both P9 and P10 stop
> API header, and could definitely be cleaned up and merged. However the headers
> are direct mirror from hostboot and they maintain different code-bases for
> P9 and P10.
> 
> My cause of concern stems from the fact that if we refactor the libraries here
> and if hostboot updates their libraries we would manually each time have
> to code those changes up in our refactored library too.

I'm referring to include/p10_stop_api.H here. AFAIC this file is already modified.

Something like below works?


diff --git a/hw/xive2.c b/hw/xive2.c
index 674005895..aece99a0d 100644
--- a/hw/xive2.c
+++ b/hw/xive2.c
@@ -20,7 +20,6 @@
  #include <bitmap.h>
  #include <buddy.h>
  #include <phys-map.h>
-#include <p9_stop_api.H>
  #include <p10_stop_api.H>

  /* Verbose debug */
diff --git a/include/p10_stop_api.H b/include/p10_stop_api.H
index f3854fefc..3a8772195 100644
--- a/include/p10_stop_api.H
+++ b/include/p10_stop_api.H
@@ -23,6 +23,7 @@

  #ifdef __SKIBOOT__
      #include <skiboot.h>
+    #include <p9_stop_api.H>
  #endif


-Vasant
Cédric Le Goater July 28, 2021, 6:10 a.m. UTC | #5
On 7/27/21 12:50 PM, Vasant Hegde wrote:
> On 7/23/21 8:33 PM, Pratik Sampat wrote:
>>
>>
>> On 22/07/21 3:01 pm, Vasant Hegde wrote:
>>> On 7/21/21 7:05 PM, Cédric Le Goater wrote:
>>>> On 7/19/21 3:20 PM, Vasant Hegde wrote:
>>>>> From: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
>>>>>
>>>>> P10 Stop engines have apis similar to P9 to set xscom restores
>>>>> after wakeup from deep-sleep states.
>>>>>
>>>>> This xscom restore will be used to support STOP11 on P10.
>>>>>
>>>>> Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
>>>>> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
>>>>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>>>>> ---
>>>>>   hw/xive2.c | 31 ++++++++++++++++++++++++++-----
>>>>>   1 file changed, 26 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/hw/xive2.c b/hw/xive2.c
>>>>> index f559b0f9a..d2fc2ad69 100644
>>>>> --- a/hw/xive2.c
>>>>> +++ b/hw/xive2.c
>>>>> @@ -20,8 +20,8 @@
>>>>>   #include <bitmap.h>
>>>>>   #include <buddy.h>
>>>>>   #include <phys-map.h>
>>>>> -#include <p9_stop_api.H> /* TODO (p10): need P10 stop state engine */
>>>>> -
>>>>> +#include <p9_stop_api.H>
>>>>> +#include <p10_stop_api.H>
>>>>
>>>> Why do we need both ?
>>>>
>>>
>>> Because p10_stop_api.H uses definitions from p9_stop_api.H. May be we should move common
>>> definitions to separate header file (say stop_api.h ) and then include that in P[9/10]_stop_api.H file?
>>>
>> I agree that there are a lot of common definitions around both P9 and P10 stop
>> API header, and could definitely be cleaned up and merged. However the headers
>> are direct mirror from hostboot and they maintain different code-bases for
>> P9 and P10.
>>
>> My cause of concern stems from the fact that if we refactor the libraries here
>> and if hostboot updates their libraries we would manually each time have
>> to code those changes up in our refactored library too.
> 
> I'm referring to include/p10_stop_api.H here. AFAIC this file is already modified.
> 
> Something like below works?

Looks good to me.

Thanks,

C.

> 
> 
> diff --git a/hw/xive2.c b/hw/xive2.c
> index 674005895..aece99a0d 100644
> --- a/hw/xive2.c
> +++ b/hw/xive2.c
> @@ -20,7 +20,6 @@
>  #include <bitmap.h>
>  #include <buddy.h>
>  #include <phys-map.h>
> -#include <p9_stop_api.H>
>  #include <p10_stop_api.H>
> 
>  /* Verbose debug */
> diff --git a/include/p10_stop_api.H b/include/p10_stop_api.H
> index f3854fefc..3a8772195 100644
> --- a/include/p10_stop_api.H
> +++ b/include/p10_stop_api.H
> @@ -23,6 +23,7 @@
> 
>  #ifdef __SKIBOOT__
>      #include <skiboot.h>
> +    #include <p9_stop_api.H>
>  #endif
> 
> 
> -Vasant
Pratik Sampat July 28, 2021, 6:12 a.m. UTC | #6
On 28/07/21 11:40 am, Cédric Le Goater wrote:
> On 7/27/21 12:50 PM, Vasant Hegde wrote:
>> On 7/23/21 8:33 PM, Pratik Sampat wrote:
>>>
>>> On 22/07/21 3:01 pm, Vasant Hegde wrote:
>>>> On 7/21/21 7:05 PM, Cédric Le Goater wrote:
>>>>> On 7/19/21 3:20 PM, Vasant Hegde wrote:
>>>>>> From: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
>>>>>>
>>>>>> P10 Stop engines have apis similar to P9 to set xscom restores
>>>>>> after wakeup from deep-sleep states.
>>>>>>
>>>>>> This xscom restore will be used to support STOP11 on P10.
>>>>>>
>>>>>> Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
>>>>>> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
>>>>>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>>>>>> ---
>>>>>>    hw/xive2.c | 31 ++++++++++++++++++++++++++-----
>>>>>>    1 file changed, 26 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/hw/xive2.c b/hw/xive2.c
>>>>>> index f559b0f9a..d2fc2ad69 100644
>>>>>> --- a/hw/xive2.c
>>>>>> +++ b/hw/xive2.c
>>>>>> @@ -20,8 +20,8 @@
>>>>>>    #include <bitmap.h>
>>>>>>    #include <buddy.h>
>>>>>>    #include <phys-map.h>
>>>>>> -#include <p9_stop_api.H> /* TODO (p10): need P10 stop state engine */
>>>>>> -
>>>>>> +#include <p9_stop_api.H>
>>>>>> +#include <p10_stop_api.H>
>>>>> Why do we need both ?
>>>>>
>>>> Because p10_stop_api.H uses definitions from p9_stop_api.H. May be we should move common
>>>> definitions to separate header file (say stop_api.h ) and then include that in P[9/10]_stop_api.H file?
>>>>
>>> I agree that there are a lot of common definitions around both P9 and P10 stop
>>> API header, and could definitely be cleaned up and merged. However the headers
>>> are direct mirror from hostboot and they maintain different code-bases for
>>> P9 and P10.
>>>
>>> My cause of concern stems from the fact that if we refactor the libraries here
>>> and if hostboot updates their libraries we would manually each time have
>>> to code those changes up in our refactored library too.
>> I'm referring to include/p10_stop_api.H here. AFAIC this file is already modified.
>>
>> Something like below works?
> Looks good to me.

Sure, I can call the P9 headers in P10 that way.
Thanks!

> Thanks,
>
> C.
>
>>
>> diff --git a/hw/xive2.c b/hw/xive2.c
>> index 674005895..aece99a0d 100644
>> --- a/hw/xive2.c
>> +++ b/hw/xive2.c
>> @@ -20,7 +20,6 @@
>>   #include <bitmap.h>
>>   #include <buddy.h>
>>   #include <phys-map.h>
>> -#include <p9_stop_api.H>
>>   #include <p10_stop_api.H>
>>
>>   /* Verbose debug */
>> diff --git a/include/p10_stop_api.H b/include/p10_stop_api.H
>> index f3854fefc..3a8772195 100644
>> --- a/include/p10_stop_api.H
>> +++ b/include/p10_stop_api.H
>> @@ -23,6 +23,7 @@
>>
>>   #ifdef __SKIBOOT__
>>       #include <skiboot.h>
>> +    #include <p9_stop_api.H>
>>   #endif
>>
>>
>> -Vasant
diff mbox series

Patch

diff --git a/hw/xive2.c b/hw/xive2.c
index f559b0f9a..d2fc2ad69 100644
--- a/hw/xive2.c
+++ b/hw/xive2.c
@@ -20,8 +20,8 @@ 
 #include <bitmap.h>
 #include <buddy.h>
 #include <phys-map.h>
-#include <p9_stop_api.H> /* TODO (p10): need P10 stop state engine */
-
+#include <p9_stop_api.H>
+#include <p10_stop_api.H>
 
 /* Verbose debug */
 #undef XIVE_VERBOSE_DEBUG
@@ -3022,11 +3022,32 @@  static void xive_configure_ex_special_bar(struct xive *x, struct cpu_thread *c)
 
 void xive2_late_init(void)
 {
+	struct cpu_thread *c;
+
 	prlog(PR_INFO, "SLW: Configuring self-restore for NCU_SPEC_BAR\n");
-			/*
-			 * TODO (p10): need P10 stop state engine and fix for STOP11
-			 */
+	for_each_present_cpu(c) {
+		if(cpu_is_thread0(c)) {
+			struct proc_chip *chip = get_chip(c->chip_id);
+			struct xive *x = chip->xive;
+			uint64_t xa, val, rc;
+			xa = XSCOM_ADDR_P10_NCU(pir_to_core_id(c->pir), P10_NCU_SPEC_BAR);
+			val = (uint64_t)x->tm_base | P10_NCU_SPEC_BAR_ENABLE;
+			/* Bail out if wakeup engine has already failed */
+			if (wakeup_engine_state != WAKEUP_ENGINE_PRESENT) {
+				prlog(PR_ERR, "XIVE proc_stop_api fail detected\n");
+				break;
+			}
+			rc = proc_stop_save_scom((void *)chip->homer_base, xa, val,
+				PROC_STOP_SCOM_REPLACE, PROC_STOP_SECTION_L3);
+			if (rc) {
+				xive_cpu_err(c, "proc_stop_save_scom failed for NCU_SPEC_BAR rc=%lld\n",
+					     rc);
+				wakeup_engine_state = WAKEUP_ENGINE_FAILED;
+			}
+		}
+	}
 }
+
 static void xive_provision_cpu(struct xive_cpu_state *xs, struct cpu_thread *c)
 {
 	struct xive *x;