[Skiboot] [PATCH] OCC: Increase max pstate check on P9 to 255

[Stewart Smith]

Stewart Smith <stewart at linux.vnet.ibm.com> writes:
> Vaidyanathan Srinivasan <svaidy at linux.vnet.ibm.com> writes:
>> * Stewart Smith <stewart at linux.vnet.ibm.com> [2017-11-17 09:34:03]:
>>
>>> This has changed from P8, we can now have > 127 pstates.
>>> 
>>> This was observed on Boston during WoF bringup.
>>> 
>>> Reported-by: Minda Wei <minda.wei at ibm.com>
>>
>> Reported-by: Francesco A Campisano <campisan at us.ibm.com>
>>>
>>>
>>> Signed-off-by: Stewart Smith <stewart at linux.vnet.ibm.com>
>>
>> Reviewed-by: Vaidyanathan Srinivasan <svaidy at linux.vnet.ibm.com>
>>
>>
>>> ---
>>>  hw/occ.c | 10 ++++++----
>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/hw/occ.c b/hw/occ.c
>>> index 78c6a6a356e3..8ad0dfe421d7 100644
>>> --- a/hw/occ.c
>>> +++ b/hw/occ.c
>>> @@ -612,13 +612,15 @@ static bool add_cpu_pstate_properties(int *pstate_nom)
>>>  	nr_pstates = labs(pmax - pmin) + 1;
>>>  	prlog(PR_DEBUG, "OCC: Version %x Min %d Nom %d Max %d Nr States %d\n",
>>>  	      occ_data->version, pmin, pnom, pmax, nr_pstates);
>>> -	if (nr_pstates <= 1 || nr_pstates > 128) {
>>> +	if ((occ_data->version == 0x90 && (nr_pstates <= 1)) ||
>>> +	    (occ_data->version <= 0x02 &&
>>> +	     (nr_pstates <= 1 || nr_pstates > 128))) {
>>>  		/**
>>>  		 * @fwts-label OCCInvalidPStateRange
>>>  		 * @fwts-advice The number of pstates is outside the valid
>>> -		 * range (currently <=1 or > 128), so OPAL has not added
>>> -		 * pstates to the device tree. This means that OCC (On Chip
>>> -		 * Controller) will be non-functional. This means
>>> +		 * range (currently <=1 or > 128 on p8, >255 on P9), so OPAL
>>> +		 * has not added pstates to the device tree. This means that
>>> +		 * OCC (On Chip Controller) will be non-functional. This means
>>>  		 * that CPU idle states and CPU frequency scaling
>>>  		 * will not be functional.
>>>  		 */
>>
>> Thanks for the fix to increase the limit.
>>
>> Checking for occ_data->version is the right check to decide permitted
>> range between P8 vs P9.
>>
>> In case occ_data->version is bumped up, then we do not know the range
>> and we will fail in an earlier check and not export any pstates.
>>
>> We need to update the right range for future versions here.
>>
>> However Linux could have reporting problems with PState IDs more than
>> 128 and also with large number of PStates.  This change does not as
>> such crash Linux, but more work is needed in Linux driver to support
>> large number of PStates.
>
> I'll pull the patch in and let's see how it goes for a bit... see if
> people see this as a real problem or not. We can always pull it out and
> do a dance with filtering pstates to limit to 128 and put the full list
> somewhere else that the kernel doesn't poke at by default.
>
> mpe, what are your thoughts here?

I've merged to master as of
701556d2ea9b890a389183907ae484b78c05c665.... let's see if this is a
terrible idea :)

-- 
Stewart Smith
OPAL Architect, IBM.