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

[Cédric Le Goater]

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 at 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 at linux.ibm.com>
>>>>> Signed-off-by: Pratik Rajesh Sampat <psampat at linux.ibm.com>
>>>>> Signed-off-by: Vasant Hegde <hegdevasant at 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