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

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Ross Lagerwall <ross.lagerwall@xxxxxxxxxx> 06/18/14 10:13 AM >>>
>On 06/02/2014 04:46 PM, Jan Beulich wrote:
>>>>> On 02.06.14 at 16:43, <ross.lagerwall@xxxxxxxxxx> wrote:
>>> From: Ross Lagerwall <rosslagerwall@xxxxxxxxxx>
>>>
>>> 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.

And I always considered it wrong for it to confuse things like this. 
Nevertheless
I agree that the utility's output and accepted input ought to be the same. Yet 
I'm
getting the impression that you forget that there's also a hypervisor command
line option to consider here, and that definitely ought to reflect reality, not 
some
tool's questionable view of the world.

>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?

Yes, I think this would be the right direction - far better than using 
confusing naming.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel