Re: [PATCH v3 1/3] x86: Allow limiting the max C-state sub-state

["Jan Beulich" <JBeulich@suse.com>]

>>> On 25.06.14 at 17:52, <ross.lagerwall@citrix.com> wrote:
> On 06/25/2014 01:37 PM, Jan Beulich wrote:
>>>>> On 23.06.14 at 13:09, <ross.lagerwall@citrix.com> wrote:
>>> --- a/xen/arch/x86/cpu/mwait-idle.c
>>> +++ b/xen/arch/x86/cpu/mwait-idle.c
>>> @@ -330,7 +330,9 @@ static void mwait_idle(void)
>>>   	    (next_state = cpuidle_current_governor->select(power)) > 0) {
>>>   		do {
>>>   			cx = &power->states[next_state];
>>> -		} while (cx->type > max_cstate && --next_state);
>>> +		} while ((cx->type > max_cstate || (cx->type == max_cstate &&
>>> +			  MWAIT_HINT2SUBSTATE(cx->address) > max_csubstate)) &&
>>> +			 --next_state);
>>
>> In the context of the above comment it then is questionable
>> whether here (and similarly in acpi_processor_idle()) using the
>> MWAIT parameter value for the comparison here is really
>> suitable: If you look at hsw_cstates[] and atom_cstates[] you'll
>> see that there we have states with just a single non-zero sub-
>> state (which the logic here would exclude in certain cases when
>> one would expect it to be permitted).
>>
> 
> When would one expect them to be permitted that this logic would exclude?
> 
> C7s-HSW has a C-state of 4 and a sub-state of 2.  If you set max_cstate 
> = 4, then no C-state > 4 will be selected.
> Similarly, if you select max_csubstate = 2, then no sub C-state > 2 will 
> be selected (if max_cstate = 4). This seems congruous to me.

Actually I think I got it the wrong way round: Taking your example, if
one sets max_csubstate = 1, one may expect the 1st (not necessarily
the one numbered "1") to still be permitted (much like we also don't tie
max_cstate to actual values to be passed to MWAIT). Just look at how
much more interesting the sub-states get with the ports from recent
Linux that I posted earlier today.

Jan