Re: [PATCH] x86/mwait_idle: Allow setting the max cstate to C1

[Ross Lagerwall <ross.lagerwall@citrix.com>]

On 06/02/2014 04:46 PM, Jan Beulich wrote:
>>>> On 02.06.14 at 16:43, <ross.lagerwall@citrix.com> wrote:
>> From: Ross Lagerwall <rosslagerwall@citrix.com>
>>
>> Following 91413b519631 ("x86/mwait_idle: export both C1 and C1E"), when
>> setting the max cstate to C1, the C1E cstate is used as well. This is
>> because MWAIT_HINT2CSTATE returns the same value for C1 and C1E.
>> Instead, when limiting the cstate, compare max_cstate with the position
>> in the states array, as the acpi cpu_idle driver does.
>>
>> Without this patch, there's no way of setting the max cstate to C1 when using
>> the mwait_idle driver.
>
> But it was intentionally this way from the beginning of the existence of
> the mwait idle driver - the other approach makes the value to be passed
> really platform dependent (i.e. "max_cstate=2" doesn't universally mean
> what one would expect: maximum C-state is C2).

Except that is how xenpm and xl debugkeys c already display their 
output. E.g:
C0                   : transition [             3431722]
                        residency  [              131101 ms]
C1                   : transition [                 588]
                        residency  [                3514 ms]
C2                   : transition [                 465]
                        residency  [                 497 ms]
C3                   : transition [                 176]
                        residency  [                 299 ms]
C4                   : transition [                  15]
                        residency  [                   5 ms]
C5                   : transition [             3430478]
                        residency  [            57685073 ms]

In the above, C1 == hardware C1 and C2 == hardware C1E.  Changing it so 
that "xenpm set-max-cstate 1" sets it to xenpm's notion of C1 rather 
than actual C1 (as the patch does) seems consistent to me.

The alternative would be to fix up xenpm, xl debugkeys c, and any other 
consumers of C-states to correctly display substates and take a new 
substate parameter.  IMHO, there's little gain in doing this as C-states 
are really platform dependent anyway.  If the next Intel processor has a 
selectable sub-sub-C-state, do we really want to have to change 
everything again?

Cheers
-- 
Ross Lagerwall