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

[Stewart Smith]

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?

-- 
Stewart Smith
OPAL Architect, IBM.