From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ross Lagerwall Subject: Re: [PATCH] x86/mwait_idle: Allow setting the max cstate to C1 Date: Wed, 18 Jun 2014 09:13:48 +0100 Message-ID: <53A14A3C.10001@citrix.com> References: <1401720230-3147-1-git-send-email-ross.lagerwall@citrix.com> <538CB88E0200007800016FC3@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WxB0C-00007k-Oq for xen-devel@lists.xenproject.org; Wed, 18 Jun 2014 08:13:52 +0000 In-Reply-To: <538CB88E0200007800016FC3@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel@lists.xenproject.org, Keir Fraser List-Id: xen-devel@lists.xenproject.org On 06/02/2014 04:46 PM, Jan Beulich wrote: >>>> On 02.06.14 at 16:43, wrote: >> From: Ross Lagerwall >> >> 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