From: Len Brown <[email protected]>
The BIOS exports deep C-states on modern Intel processors
as "C3-type" to satisfy various legacy Operating Systems.
However, the hardware actually supports C2-type, and does
not require the extra costs of C3-type.
One of the costs is to check the BM_STS (Bus Master Status)
bit before entering C3, and instead choose a shallower C-state
if there was "recent bus master activity".
We have found a number of systems in the field that erroneously
set BM_STS and prevent entry into deep C-states.
Re-define BIOS presented C3-type states as C2-type states
on modern processors to avoid this issue.
If a device in the system really does want to prevent use
of a deep C-state, its Linux driver should register its
constraints via pm_qos_add_request().
https://bugzilla.kernel.org/show_bug.cgi?id=15886
Signed-off-by: Len Brown <[email protected]>
---
drivers/acpi/processor_idle.c | 38 ++++++++++++++++++++++++++++++++++++++
1 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index b1b3856..14d1a0c 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -607,6 +607,38 @@ static void acpi_processor_power_verify_c3(struct acpi_processor *pr,
return;
}
+/*
+ * Modern Intel processors support only ACPI C2-type C-states.
+ * But the BIOS tends to report its deepest C-state as C3-type
+ * to satisfy various old operating systems. We can skip
+ * C3 OS overhead by treating the deep-states as C2-type.
+ * Also, we can avoid checking BM_STS, which on some systems
+ * erroneously prevents entry into C3-type states.
+ */
+static int acpi_c3type_is_really_c2type(void) {
+
+ if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
+ return 0;
+
+ if (boot_cpu_data.x86 != 6)
+ return 0;
+
+ switch(boot_cpu_data.x86_model) {
+ case 0x1A: /* Core i7, Xeon 5500 series */
+ case 0x1E: /* Core i7 and i5 Processor */
+ case 0x1F: /* Core i7 and i5 Processor */
+ case 0x2E: /* NHM-EX Xeon */
+ case 0x2F: /* WSM-EX Xeon */
+ case 0x25: /* WSM */
+ case 0x2C: /* WSM */
+ case 0x2A: /* SNB */
+ case 0x2D: /* SNB Xeon */
+ return 1;
+ default:
+ return 0;
+ }
+}
+
static int acpi_processor_power_verify(struct acpi_processor *pr)
{
unsigned int i;
@@ -617,6 +649,12 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
for (i = 1; i < ACPI_PROCESSOR_MAX_POWER && i <= max_cstate; i++) {
struct acpi_processor_cx *cx = &pr->power.states[i];
+ if ((cx->type == ACPI_STATE_C3)
+ && acpi_c3type_is_really_c2type()) {
+ ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Redefining C3-type to C2\n"));
+ cx->type = ACPI_STATE_C2;
+ }
+
switch (cx->type) {
case ACPI_STATE_C1:
cx->valid = 1;
--
1.7.2.rc3.43.g24e7a
On Wed, Jul 21, 2010 at 2:31 PM, Len Brown <[email protected]> wrote:
> From: Len Brown <[email protected]>
>
> The BIOS exports deep C-states on modern Intel processors
> as "C3-type" to satisfy various legacy Operating Systems.
>
> However, the hardware actually supports C2-type, and does
> not require the extra costs of C3-type.
>
> One of the costs is to check the BM_STS (Bus Master Status)
> bit before entering C3, and instead choose a shallower C-state
> if there was "recent bus master activity".
>
> We have found a number of systems in the field that erroneously
> set BM_STS and prevent entry into deep C-states.
> Re-define BIOS presented C3-type states as C2-type states
> on modern processors to avoid this issue.
>
> If a device in the system really does want to prevent use
> of a deep C-state, its Linux driver should register its
> constraints via pm_qos_add_request().
>
> https://bugzilla.kernel.org/show_bug.cgi?id=15886
>
Agree with the intent. But, I think its cleaner to keep all arch model
checks in arch/x86/kernel/acpi/cstate.c.
Thanks,
Venki
> Signed-off-by: Len Brown <[email protected]>
> ---
> ?drivers/acpi/processor_idle.c | ? 38 ++++++++++++++++++++++++++++++++++++++
> ?1 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index b1b3856..14d1a0c 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -607,6 +607,38 @@ static void acpi_processor_power_verify_c3(struct acpi_processor *pr,
> ? ? ? ?return;
> ?}
>
> +/*
> + * Modern Intel processors support only ACPI C2-type C-states.
> + * But the BIOS tends to report its deepest C-state as C3-type
> + * to satisfy various old operating systems. ?We can skip
> + * C3 OS overhead by treating the deep-states as C2-type.
> + * Also, we can avoid checking BM_STS, which on some systems
> + * erroneously prevents entry into C3-type states.
> + */
> +static int acpi_c3type_is_really_c2type(void) {
> +
> + ? ? ? if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> + ? ? ? ? ? ? ? return 0;
> +
> + ? ? ? if (boot_cpu_data.x86 != 6)
> + ? ? ? ? ? ? ? return 0;
> +
> + ? ? ? switch(boot_cpu_data.x86_model) {
> + ? ? ? case 0x1A: ? ? ?/* Core i7, Xeon 5500 series */
> + ? ? ? case 0x1E: ? ? ?/* Core i7 and i5 Processor */
> + ? ? ? case 0x1F: ? ? ?/* Core i7 and i5 Processor */
> + ? ? ? case 0x2E: ? ? ?/* NHM-EX Xeon */
> + ? ? ? case 0x2F: ? ? ?/* WSM-EX Xeon */
> + ? ? ? case 0x25: ? ? ?/* WSM */
> + ? ? ? case 0x2C: ? ? ?/* WSM */
> + ? ? ? case 0x2A: ? ? ?/* SNB */
> + ? ? ? case 0x2D: ? ? ?/* SNB Xeon */
> + ? ? ? ? ? ? ? return 1;
> + ? ? ? default:
> + ? ? ? ? ? ? ? return 0;
> + ? ? ? }
> +}
> +
> ?static int acpi_processor_power_verify(struct acpi_processor *pr)
> ?{
> ? ? ? ?unsigned int i;
> @@ -617,6 +649,12 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
> ? ? ? ?for (i = 1; i < ACPI_PROCESSOR_MAX_POWER && i <= max_cstate; i++) {
> ? ? ? ? ? ? ? ?struct acpi_processor_cx *cx = &pr->power.states[i];
>
> + ? ? ? ? ? ? ? if ((cx->type == ACPI_STATE_C3)
> + ? ? ? ? ? ? ? ? ? ? ? && acpi_c3type_is_really_c2type()) {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Redefining C3-type to C2\n"));
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? cx->type = ACPI_STATE_C2;
> + ? ? ? ? ? ? ? }
> +
> ? ? ? ? ? ? ? ?switch (cx->type) {
> ? ? ? ? ? ? ? ?case ACPI_STATE_C1:
> ? ? ? ? ? ? ? ? ? ? ? ?cx->valid = 1;
> --
> 1.7.2.rc3.43.g24e7a
>
>
>
> If a device in the system really does want to prevent use
> of a deep C-state, its Linux driver should register its
> constraints via pm_qos_add_request().
I reviewed the patch and it looks good to me.
I would suggest to have a command line option for this too,
in case someone wants to run an older kernel on a new system not
known by your patch yet.
-Andi
This patch is kaput.
As detailed in the bug report
https://bugzilla.kernel.org/show_bug.cgi?id=15886
we should be able to fix some of these boxes
by paying attention to an ACPI flag we didn't
realize existed until yesterday.
I'll follow-up with a new patch today.
However, we'll still have issues with systems
like the HP DL360 G6 which explicity set the
flag to ask for BM_STS checking and configure
the chipset such that BM_STS is active.
That may require a BIOS fix, or we may
have to run intel_idle on that box --
since intel_idle ignores BM_STS always
and instead relies on drivers to use pm_qos
to register device latency constraints.
thanks,
Len Brown, Intel Open Source Technology Center
From: Len Brown <[email protected]>
It turns out that there is a bit in the _CST for Intel FFH C3
that tells the OS if we should be checking BM_STS or not.
Linux has been unconditionally checking BM_STS.
If the chip-set is configured to enable BM_STS,
it can retard or completely prevent entry into
deep C-states -- as illustrated by turbostat:
http://userweb.kernel.org/~lenb/acpi/utils/pmtools/turbostat/
ref: Intel Processor Vendor-Specific ACPI Interface Specification
table 4 "_CST FFH GAS Field Encoding"
Bit 1: Set to 1 if OSPM should use Bus Master avoidance for this C-state
https://bugzilla.kernel.org/show_bug.cgi?id=15886
Signed-off-by: Len Brown <[email protected]>
---
arch/x86/kernel/acpi/cstate.c | 9 +++++++++
drivers/acpi/processor_idle.c | 2 +-
include/acpi/processor.h | 3 ++-
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c
index 2e837f5..fb7a5f0 100644
--- a/arch/x86/kernel/acpi/cstate.c
+++ b/arch/x86/kernel/acpi/cstate.c
@@ -145,6 +145,15 @@ int acpi_processor_ffh_cstate_probe(unsigned int cpu,
percpu_entry->states[cx->index].eax = cx->address;
percpu_entry->states[cx->index].ecx = MWAIT_ECX_INTERRUPT_BREAK;
}
+
+ /*
+ * For _CST FFH on Intel, if GAS.access_size bit 1 is cleared,
+ * then we should skip checking BM_STS for this C-state.
+ * ref: "Intel Processor Vendor-Specific ACPI Interface Specification"
+ */
+ if ((c->x86_vendor == X86_VENDOR_INTEL) && !(reg->access_size & 0x2))
+ cx->bm_sts_skip = 1;
+
return retval;
}
EXPORT_SYMBOL_GPL(acpi_processor_ffh_cstate_probe);
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index b1b3856..b351342 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -947,7 +947,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
if (acpi_idle_suspend)
return(acpi_idle_enter_c1(dev, state));
- if (acpi_idle_bm_check()) {
+ if (!cx->bm_sts_skip && acpi_idle_bm_check()) {
if (dev->safe_state) {
dev->last_state = dev->safe_state;
return dev->safe_state->enter(dev, dev->safe_state);
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index da565a4..a68ca8a 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -48,7 +48,7 @@ struct acpi_power_register {
u8 space_id;
u8 bit_width;
u8 bit_offset;
- u8 reserved;
+ u8 access_size;
u64 address;
} __attribute__ ((packed));
@@ -63,6 +63,7 @@ struct acpi_processor_cx {
u32 power;
u32 usage;
u64 time;
+ u8 bm_sts_skip;
char desc[ACPI_CX_DESC_LEN];
};
--
1.7.2
Len Brown wrote:
> However, we'll still have issues with systems
> like the HP DL360 G6 which explicity set the
> flag to ask for BM_STS checking and configure
> the chipset such that BM_STS is active.
> That may require a BIOS fix, or we may
> have to run intel_idle on that box --
> since intel_idle ignores BM_STS always
> and instead relies on drivers to use pm_qos
> to register device latency constraints.
I'm curious as to why you see a problem with the DL380G6 as the one I have here happily sits in C6 when idle.
your turbostat util shows:
CPU GHz TSC %c0 %c1 %c3 %c6 %pc3 %pc6
avg 1.64 2.27 0.16 0.12 0.00 99.71 0.00 90.15
and powertop has results like:
Cn Avg residency P-states (frequencies)
C0 (cpu running) ( 0,1%) Turbo Mode 0,0%
polling 0,0ms ( 0,0%) 2,27 Ghz 0,0%
C1 mwait 0,1ms ( 0,0%) 2,13 Ghz 0,0%
C2 mwait 1,0ms ( 0,0%) 2,00 Ghz 0,0%
C3 mwait 90,4ms (99,9%) 1,60 Ghz 100,0%
this is with v2.6.35-rc5-176-gcd5b8f8 and using acpi_idle. I've deliberately disabled intel_idle to test, however using intel_idle
gives almost identical results.
Looking at the bug 15886, the Access Size 0x03 entries you mentioned are all 0x01 on this system. I've also uploaded the acpidump
from this DL380G6 to that bug so that you can check I've not just looked in the wrong place.
Did the first acpidump come from a system with the 'HP Power Regulator' setting in the bios set to OS Control mode ? My system is
set this way and it seems to work as expected.
The other settings for this option appear to be designed to override OS power management controls, for example the description of
the 'Static High Performance' option suggests it'll somehow force the CPU to operate in the highest performance mode all of the
time: "HP Static High Performance Mode: Processors will run in their maximum power/performance state at all times regardless of the
OS power management policy".
If this does turn out to be as simple as a bios setting, should we really be trying to workaround what may be a legitimate decision
by the servers admin ?
Iain
From: Len Brown <[email protected]>
processor.bm_check_disable=1" prevents Linux from checking BM_STS
before entering C3-type cpu power states.
This may be useful for a system running acpi_idle
where the BIOS exports FADT C-states, _CST IO C-states,
or _CST FFH C-states with the BM_STS bit set;
while configuring the chipset to set BM_STS
more frequently than perhaps is optimal.
Note that such systems may have been developed
using a tickful OS that would quickly clear BM_STS,
rather than a tickless OS that may go for some time
between checking and clearing BM_STS.
Note also that an alternative for newer systems
is to use the intel_idle driver, which always
ignores BM_STS, relying Linux device drivers
to register constraints explicitly via PM_QOS.
https://bugzilla.kernel.org/show_bug.cgi?id=15886
Signed-off-by: Len Brown <[email protected]>
---
drivers/acpi/processor_idle.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index b351342..1d41048 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -76,6 +76,8 @@ static unsigned int max_cstate __read_mostly = ACPI_PROCESSOR_MAX_POWER;
module_param(max_cstate, uint, 0000);
static unsigned int nocst __read_mostly;
module_param(nocst, uint, 0000);
+static int bm_check_disable __read_mostly;
+module_param(bm_check_disable, uint, 0000);
static unsigned int latency_factor __read_mostly = 2;
module_param(latency_factor, uint, 0644);
@@ -763,6 +765,9 @@ static int acpi_idle_bm_check(void)
{
u32 bm_status = 0;
+ if (bm_check_disable)
+ return 0;
+
acpi_read_bit_register(ACPI_BITREG_BUS_MASTER_STATUS, &bm_status);
if (bm_status)
acpi_write_bit_register(ACPI_BITREG_BUS_MASTER_STATUS, 1);
--
1.7.2
Iain wrote:
> Len Brown wrote:
>> However, we'll still have issues with systems
>> like the HP DL360 G6 which explicity set the
>
> I'm curious as to why you see a problem with the DL380G6 as the one I have here happily sits in C6 when idle.
Please ignore me, apologies for the noise.
Just noticed the problem system was a DL360 and mine is a DL380. Long day spent working with both 360's and 380's - I don't seem to
be able to tell them apart anymore..
Iain
> I'm curious as to why you see a problem with the DL380G6 as the one I have
> here happily sits in C6 when idle.
yay!
> your turbostat util shows:
>
> CPU GHz TSC %c0 %c1 %c3 %c6 %pc3 %pc6
> avg 1.64 2.27 0.16 0.12 0.00 99.71 0.00 90.15
> Looking at the bug 15886, the Access Size 0x03 entries you mentioned are all
> 0x01 on this system. I've also uploaded the acpidump from this DL380G6 to that
> bug so that you can check I've not just looked in the wrong place.
You read it correctly, your BIOS does not request BM_STS, mjg59's does.
> Did the first acpidump come from a system with the 'HP Power Regulator'
> setting in the bios set to OS Control mode ? My system is set this way and it
> seems to work as expected.
I expect that is to enable PCC, which would change P-states,
but unlikely would have an effect on C-states.
If you can try it both ways that might be good to know.
(include powertop display once again)
Of course, the default setting is what 99% of customers use...
> The other settings for this option appear to be designed to override OS power
> management controls, for example the description of the 'Static High
> Performance' option suggests it'll somehow force the CPU to operate in the
> highest performance mode all of the time: "HP Static High Performance Mode:
> Processors will run in their maximum power/performance state at all times
> regardless of the OS power management policy".
This is BIOS writer "value add".
Unclear how it migh be an improvement over what Linux has been shipping
for years.
> If this does turn out to be as simple as a bios setting, should we really be
> trying to workaround what may be a legitimate decision by the servers admin ?
Ideally we will do exactly as the BIOS requests.
However, somtimes what they request makes lots of sense
on some version of Windows, and may make less sense
when running Linux.
Please upload the output from dmidecode to the bug report.
I am hopeful that you have a current BIOS and that
Matthew may have an pre-production BIOS.
thanks,
Len Brown, Intel Open Source Technology Center
Len Brown wrote:
> You read it correctly, your BIOS does not request BM_STS, mjg59's does.
Right, and on a DL360 G6 with the 07-24-2009 bios version I saw the same.
> I expect that is to enable PCC, which would change P-states,
> but unlikely would have an effect on C-states.
I found another option in the bios to limit or disable the C-states today, so plenty of opportunity to configure the system into an
odd state.
> If you can try it both ways that might be good to know.
> (include powertop display once again)
> Of course, the default setting is what 99% of customers use...
I'll upload an archive to the bugzilla entry with the details. What seems to happen is that when you set the default Balanced Power
and Performance mode the CST code vanishes completely and the processor manages to get to c6 some of the time. Enable OS Control
mode and the bad CST code appears.
> This is BIOS writer "value add".
> Unclear how it migh be an improvement over what Linux has been shipping
> for years.
Well yes, having Linux and the bios fighting for control probably isn't going to help.
> Please upload the output from dmidecode to the bug report.
> I am hopeful that you have a current BIOS and that
> Matthew may have an pre-production BIOS.
I've uploaded an archive with dmidecode, turbostat and powertop dumps. There are dumps with the bios set to the default, and to OS
Control mode.
The original bios on my DL360G6 was 07-24-2009 and has the same issue as Matthew. I upgraded the machine to the latest 2010.05.15
and repeated the tests.
Good news is that the new bios has fixed the CST code so that the Access length values are all 0x01 when they're present and the
dumps show the processor getting into c6 much more.
So you were correct, bios fix was needed.
Iain
Len Brown <[email protected]> writes:
>
> Note also that an alternative for newer systems
> is to use the intel_idle driver, which always
> ignores BM_STS, relying Linux device drivers
> to register constraints explicitly via PM_QOS.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=15886
Thanks. I don't fully understand why the check for this option
is in a different place than the register check in the earlier patch?
This needs to be also documented in Documentation/kernel-parameters.txt
Other than that it looks good.
-Andi
--
[email protected] -- Speaking for myself only.
> > Note also that an alternative for newer systems
> > is to use the intel_idle driver, which always
> > ignores BM_STS, relying Linux device drivers
> > to register constraints explicitly via PM_QOS.
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=15886
>
> Thanks. I don't fully understand why the check for this option
> is in a different place than the register check in the earlier patch?
Technically, it could have been.
There are a comple of constraints in the layout of this code.
The _CST flag is x86 (actually Intel) specific -- so the detection
went into arch/x86/kernel/acpi/cstate.c
However, the operation of the that flag is per C-state,
not necessarily per system -- so we remember the flag
in in a cx->bm_sts_skip flag and check it in the
'acpi generic' drivers/acpi/processor_idle.c
But we can't test a per cx flag inside acpi_idle_bm_check()
because it doesn't have access to the cx, so i put that
test at the site of its only caller.
In this 2nd patch...
we added a 'generic' ACPI bootparam that applies
to all C-states. So it overrides any per-cstate flag
and it is static to the processor_idle.c file,
so it seemed cleanest (to me)
to push it down inside acpi_idle_bm_check()
rather than in its only caller.
> This needs to be also documented in Documentation/kernel-parameters.txt
I thought about that and decided against it.
While we do document some driver specific modparams
in kernel-parameters.txt, I do not expect this one to
be used that often -- mostly for diagnosis of BIOS bugs.
I know of two machines that need it,
and both of those machines have a BIOS update
or a BIOS update in progress that make it unnecessary.
thanks for caring.
Len Brown, Intel Open Source Technology Center.
> > This needs to be also documented in Documentation/kernel-parameters.txt
>
> I thought about that and decided against it.
> While we do document some driver specific modparams
> in kernel-parameters.txt, I do not expect this one to
> be used that often -- mostly for diagnosis of BIOS bugs.
> I know of two machines that need it,
> and both of those machines have a BIOS update
> or a BIOS update in progress that make it unnecessary.
I think even obscure parameters should be documented.
-Andi
--
[email protected] -- Speaking for myself only.
On Tue, 27 Jul 2010, Andi Kleen wrote:
> > > This needs to be also documented in Documentation/kernel-parameters.txt
> >
> > I thought about that and decided against it.
> > While we do document some driver specific modparams
> > in kernel-parameters.txt, I do not expect this one to
> > be used that often -- mostly for diagnosis of BIOS bugs.
> > I know of two machines that need it,
> > and both of those machines have a BIOS update
> > or a BIOS update in progress that make it unnecessary.
>
> I think even obscure parameters should be documented.
Where?
kernel-parameters.txt seems to be mostly about core kernel parameters.
While there are some key driver parameters in there, they
appear to be the exception.
-Len
On Wed 2010-07-21 17:31:27, Len Brown wrote:
> From: Len Brown <[email protected]>
>
> The BIOS exports deep C-states on modern Intel processors
> as "C3-type" to satisfy various legacy Operating Systems.
Can you elaborate on this? I though the only difference between
C2-type and C3-type is busmastering...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Tue, Aug 03, 2010 at 08:55:14AM +0200, Pavel Machek wrote:
> On Wed 2010-07-21 17:31:27, Len Brown wrote:
> > From: Len Brown <[email protected]>
> >
> > The BIOS exports deep C-states on modern Intel processors
> > as "C3-type" to satisfy various legacy Operating Systems.
>
> Can you elaborate on this? I though the only difference between
> C2-type and C3-type is busmastering...
The main difference is latency.
-Andi
--
[email protected] -- Speaking for myself only.