[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJvTdKk0qsFsN7RPQGVQ7mWbd2N9jW7kZbH4htSnMBxvwo8M2w@mail.gmail.com>
Date: Wed, 23 Mar 2016 16:05:20 -0400
From: Len Brown <lenb@...nel.org>
To: Prarit Bhargava <prarit@...hat.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
X86 ML <x86@...nel.org>,
"Peter Zijlstra (Intel)" <peterz@...radead.org>,
Borislav Petkov <bp@...e.de>, Andi Kleen <ak@...ux.intel.com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Dasaratharaman Chandramouli
<dasaratharaman.chandramouli@...el.com>,
Linux PM list <linux-pm@...r.kernel.org>
Subject: Re: [PATCH 0/3] idle, Honor Hardware Disabled States
This patch assumes that if a package state is disabled,
the corresponding core state must be disabled.
That assumption is false.
Indeed, that is a very popular and useful configuration.
But even if that were not the case, this software is not necessary,
since the hardware handles demotion "c-state clipping" automatically.
Yes, there is a case where a certain version of a certain processor
has broken demotion, but this isn't the right fix for that.
The right fix for that is here:
https://bugzilla.kernel.org/show_bug.cgi?id=109081
and will be upstream shortly.
thanks,
-Len
On Mon, Mar 21, 2016 at 8:49 AM, Prarit Bhargava <prarit@...hat.com> wrote:
> Some Intel processors have cstates limited by MSR_PKG_CST_CONFIG_CONTROL
> (0xE2), which specifies the lowest processor-specific C-state code name
> (consuming the least power). The intel_idle driver does not query this MSR to
> see if states are limited by hardware. In the case of the i7-6700HQ the value
> of bits 3:0 on the MSR are 0x6, meaning that C9-SKL (0x7) and C10-SKL (0x8)
> should have been disabled.
>
> This patch adds a limit field to the cpuidle_state struct to hold the
> cstate encodings for each processor, and queries the values the values
> to confirm support.
>
> After applying this patchset the system now boots and reports that
> some states have been disabled:
>
> [root@...p40-125 ~]# dmesg | grep intel_idle
> [ 1.163367] intel_idle: MWAIT substates: 0x11142120
> [ 1.163368] intel_idle: v0.4 model 0x5E
> [ 1.163369] intel_idle: lapic_timer_reliable_states 0xffffffff
> [ 1.163370] intel_idle: state C9-SKL (0x7) is disabled. Max Package limit is 0x6.
> [ 1.163371] intel_idle: state C10-SKL (0x8) is disabled. Max Package limit is 0x6.
>
> Cc: x86@...nel.org
> Cc: Len Brown <lenb@...nel.org>
> Cc: "Peter Zijlstra (Intel)" <peterz@...radead.org>
> Cc: Borislav Petkov <bp@...e.de>
> Cc: Andi Kleen <ak@...ux.intel.com>
> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> Cc: Dasaratharaman Chandramouli <dasaratharaman.chandramouli@...el.com>
> Cc: linux-pm@...r.kernel.org
> Signed-off-by: Prarit Bhargava <prarit@...hat.com>
>
> Prarit Bhargava (3):
> idle, rename MSR_NHM_SNB_PKG_CST_CFG_CTL to
> MSR_PKG_CST_CONFIG_CONTROL
> intel_idle, Introduce cstate limits and fix hardware cstate disable
> value
> cpuidle, Prevent users from enabling cstates that are disabled in
> Hardware
>
> arch/x86/include/asm/msr-index.h | 2 +-
> drivers/cpuidle/governors/ladder.c | 3 ++
> drivers/cpuidle/governors/menu.c | 4 ++
> drivers/cpuidle/sysfs.c | 34 +++++++++++-
> drivers/idle/intel_idle.c | 93 ++++++++++++++++++++++++++++++++-
> include/linux/cpuidle.h | 11 +++-
> tools/power/x86/turbostat/turbostat.8 | 2 +-
> tools/power/x86/turbostat/turbostat.c | 10 ++--
> 8 files changed, 146 insertions(+), 13 deletions(-)
>
> --
> 1.7.9.3
>
--
Len Brown, Intel Open Source Technology Center
Powered by blists - more mailing lists