Regression in 3.1 causes Xen to use wrong idle routine

DISCUSSION Stefan Bader 4 participants 15 messages Oct 26, 2011 – Nov 14, 2011

Subsystems (3)

x86 ACPI CPU idle time management framework

lore

Share & tools

lore

Stefan Bader

author

2011-10-26 10:24 UTC #0 lore raw

The following commit changes calls to pm_idle into first trying cpuidle_call_idle() and if that returns non-zero to fall back to call pm_idle().

commit a0bfa1373859e9d11dc92561a8667588803e42d8

Author: Len Brown <[email protected]>
Date:   Fri Apr 1 19:34:59 2011 -0400

cpuidle: stop depending on pm_idle

However cpuidle_call_idle() will return -ENODEV if it is supposed to be disabled by cpuidle.off. Which then causes pm_idle() to be called.

This has some bad interaction with the following change that tries to make use of disabling cpuidle in Xen to fall back to hlt.

commit d91ee5863b71e8c90eaf6035bff3078a85e2e7b5

Author: Len Brown <[email protected]>
Date:   Fri Apr 1 18:28:35 2011 -0400

cpuidle: replace xen access to x86 pm_idle and default_idle

The problem I see is that select_idle_routine() is called from arch/x86/kernel/cpu/common.c and since Xen setup does not set pm_idle anymore, it can cause mwait_idle or amd_e400_idle functions to be selected. In testing it seem amd_e400_idle in PVM domU at least does not immediately cause problems, but mwait_idle just causes crashes. From the reports I have this may be related to older hypervisors (3.1 and older) not clearing the mwait capability. But overall there seems something wrong in the interaction.

I am not really sure whether the logic of calling pm_idle() on all errors from cpuidle_call_idle() is already flawed or the assumption in the Xen patch about being able to prevent the wrong idle function by turning cpuidle off is incorrect. One quick fix could be to add some Xen case into select_idle_routine() which picks default_idle...

-Stefan

Konrad Rzeszutek Wilk

2011-10-26 13:30 UTC #1 lore raw

Quoted 17 lines · Stefan Bader

The following commit changes calls to pm_idle into first trying cpuidle_call_idle() and if that returns non-zero to fall back to call pm_idle().
commit a0bfa1373859e9d11dc92561a8667588803e42d8
Author: Len Brown <[email protected]>
Date:   Fri Apr 1 19:34:59 2011 -0400
cpuidle: stop depending on pm_idle
However cpuidle_call_idle() will return -ENODEV if it is supposed to be disabled by cpuidle.off. Which then causes pm_idle() to be called.
This has some bad interaction with the following change that tries to make use of disabling cpuidle in Xen to fall back to hlt.
commit d91ee5863b71e8c90eaf6035bff3078a85e2e7b5

Quoted 8 lines

Author: Len Brown <[email protected]>
Date:   Fri Apr 1 18:28:35 2011 -0400
cpuidle: replace xen access to x86 pm_idle and default_idle
The problem I see is that select_idle_routine() is called from arch/x86/kernel/cpu/common.c and since Xen setup does not set pm_idle anymore, it can cause mwait_idle or amd_e400_idle functions to be selected.

Right, b/c that is what d91ee5863b71e8c90eaf6035bff3078a85e2e7b5 was suppose to do - " xen scribble on pm_idle and access default_idle,
have it simply disable_cpuidle() so acpi_idle will not load and architecture default HLT will be used."

But it seems that select_idle_routine() was not thought off.

Quoted 10 lines

In testing it seem amd_e400_idle in PVM domU at least does not immediately cause problems, but mwait_idle just causes crashes. From the reports I have this may be related to older hypervisors (3.1 and older) not clearing the mwait capability. But overall there seems something wrong in the interaction.
I am not really sure whether the logic of calling pm_idle() on all errors from cpuidle_call_idle() is already flawed or the assumption in the Xen patch about being able to prevent the wrong idle function by turning cpuidle off is incorrect. One quick fix could be to add some Xen case into select_idle_routine() which picks default_idle...

What about using the cpuidle_disabled() functionality and adhere to that? As so:

arch/x86/kernel/process.c +5 -0

index e7e3b01..1f7f8c8 100644
@@ -14,6 +14,7 @@
 #include <linux/utsname.h>
 #include <trace/events/power.h>
 #include <linux/hw_breakpoint.h>
+#include <linux/cpuidle.h>
 #include <asm/cpu.h>
 #include <asm/system.h>
 #include <asm/apic.h>

@@ -587,6 +588,10 @@ void __cpuinit select_idle_routine(const struct cpuinfo_x86 *c)
 	if (pm_idle)
 		return;
 
+	if (cpuidle_disabled()) {
+		pm_idle = default_idle;
+		return;
+	}
 	if (cpu_has(c, X86_FEATURE_MWAIT) && mwait_usable(c)) {
 		/*
 		 * One CPU supports mwait => All CPUs supports mwait

include/linux/cpuidle.h +2 -0

index b51629e..123fe9e 100644
@@ -122,6 +122,7 @@ struct cpuidle_driver {
 };
 
 #ifdef CONFIG_CPU_IDLE
+extern int cpuidle_disabled(void);
 extern void disable_cpuidle(void);
 extern int cpuidle_idle_call(void);

@@ -137,6 +138,7 @@ extern int cpuidle_enable_device(struct cpuidle_device *dev);
 extern void cpuidle_disable_device(struct cpuidle_device *dev);
 
 #else
+static inline int cpuidle_disabled(void) { return 1; }
 static inline void disable_cpuidle(void) { }
 static inline int cpuidle_idle_call(void) { return -ENODEV; }

Stefan Bader

author

2011-10-26 13:36 UTC #2 lore raw

Quoted 96 lines · Konrad Rzeszutek Wilk

Quoted 17 lines · Stefan Bader
The following commit changes calls to pm_idle into first trying cpuidle_call_idle() and if that returns non-zero to fall back to call pm_idle().
commit a0bfa1373859e9d11dc92561a8667588803e42d8
Author: Len Brown <[email protected]>
Date:   Fri Apr 1 19:34:59 2011 -0400
cpuidle: stop depending on pm_idle
However cpuidle_call_idle() will return -ENODEV if it is supposed to be disabled by cpuidle.off. Which then causes pm_idle() to be called.
This has some bad interaction with the following change that tries to make use of disabling cpuidle in Xen to fall back to hlt.
commit d91ee5863b71e8c90eaf6035bff3078a85e2e7b5
Quoted 8 lines
Author: Len Brown <[email protected]>
Date:   Fri Apr 1 18:28:35 2011 -0400
cpuidle: replace xen access to x86 pm_idle and default_idle
The problem I see is that select_idle_routine() is called from arch/x86/kernel/cpu/common.c and since Xen setup does not set pm_idle anymore, it can cause mwait_idle or amd_e400_idle functions to be selected.
Right, b/c that is what d91ee5863b71e8c90eaf6035bff3078a85e2e7b5 was suppose to do - " xen scribble on pm_idle and access default_idle,
have it simply disable_cpuidle() so acpi_idle will not load and architecture default HLT will be used."
But it seems that select_idle_routine() was not thought off.
Quoted 10 lines
In testing it seem amd_e400_idle in PVM domU at least does not immediately cause problems, but mwait_idle just causes crashes. From the reports I have this may be related to older hypervisors (3.1 and older) not clearing the mwait capability. But overall there seems something wrong in the interaction.
I am not really sure whether the logic of calling pm_idle() on all errors from cpuidle_call_idle() is already flawed or the assumption in the Xen patch about being able to prevent the wrong idle function by turning cpuidle off is incorrect. One quick fix could be to add some Xen case into select_idle_routine() which picks default_idle...
What about using the cpuidle_disabled() functionality and adhere to that? As so:
arch/x86/kernel/process.c +5 -0
index e7e3b01..1f7f8c8 100644
@@ -14,6 +14,7 @@
 #include <linux/utsname.h>
 #include <trace/events/power.h>
 #include <linux/hw_breakpoint.h>
+#include <linux/cpuidle.h>
 #include <asm/cpu.h>
 #include <asm/system.h>
 #include <asm/apic.h>
@@ -587,6 +588,10 @@ void __cpuinit select_idle_routine(const struct cpuinfo_x86 *c)
 	if (pm_idle)
 		return;
 
+	if (cpuidle_disabled()) {
+		pm_idle = default_idle;
+		return;
+	}
 	if (cpu_has(c, X86_FEATURE_MWAIT) && mwait_usable(c)) {
 		/*
 		 * One CPU supports mwait => All CPUs supports mwait
include/linux/cpuidle.h +2 -1
index b51629e..123fe9e 100644
@@ -122,6 +122,7 @@ struct cpuidle_driver {
 };
 
 #ifdef CONFIG_CPU_IDLE
+extern int cpuidle_disabled(void);
 extern void disable_cpuidle(void);
 extern int cpuidle_idle_call(void);
 
@@ -137,6 +138,7 @@ extern int cpuidle_enable_device(struct cpuidle_device *dev);
 extern void cpuidle_disable_device(struct cpuidle_device *dev);
 
 #else
+static inline int cpuidle_disabled(void) { return 1; }
 static inline void disable_cpuidle(void) { }
 static inline int cpuidle_idle_call(void) { return -ENODEV; }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 1 line

From reading over it, this should work. Though I would be interested to hear

from the linux-acpi folks. Also to double check that calling pm_idle when cpuidle.off was specified really is what is intended.

-Stefan

Konrad Rzeszutek Wilk

2011-10-26 13:48 UTC #3 lore raw

Quoted 3 lines

Quoted 3 lines
What about using the cpuidle_disabled() functionality and adhere to that? As so:

.. snip..

Quoted 4 lines

Quoted 2 lines
From reading over it, this should work. Though I would be interested to hear
from the linux-acpi folks. Also to double check that calling pm_idle when cpuidle.off was specified really is what is intended.

Oh yeah, definitly need the input from linux-acpi folks. And also to be actually tested :-)

Stefan Bader

author

2011-10-26 13:57 UTC #4 lore raw

Quoted 11 lines · Konrad Rzeszutek Wilk

Quoted 3 lines
Quoted 3 lines
What about using the cpuidle_disabled() functionality and adhere to that? As so:
.. snip..
Quoted 4 lines
Quoted 2 lines
From reading over it, this should work. Though I would be interested to hear
from the linux-acpi folks. Also to double check that calling pm_idle when cpuidle.off was specified really is what is intended.
Oh yeah, definitly need the input from linux-acpi folks. And also to be actually tested :-)

I can volunteer to do the testing. But I am lazy enough to hold back a bit as someone may tell us this is completely the wrong way to fix it. :)

Konrad Rzeszutek Wilk

2011-11-09 17:58 UTC #5 lore raw

Quoted 7 lines · Stefan Bader

Quoted 6 lines · Konrad Rzeszutek Wilk
Quoted 3 lines
Quoted 3 lines
What about using the cpuidle_disabled() functionality and adhere to that? As so:
.. snip..
Quoted 2 lines
Quoted 2 lines
From reading over it, this should work. Though I would be interested to hear

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
from the linux-acpi folks. Also to double check that calling pm_idle when cpuidle.off was specified really is what is intended.
Oh yeah, definitly need the input from linux-acpi folks. And also to be actually tested :-)
I can volunteer to do the testing. But I am lazy enough to hold back a bit as someone may tell us this is completely the wrong way to fix it. :)

So the other option is to use 'idle=halt' on the Linux command line. That should provide the workaround for the folks reporting this (is there a BZ for it?).

At least until a good solution is hammered out.

Stefan Bader

author

2011-11-10 08:33 UTC #6 lore raw

Quoted 11 lines · Konrad Rzeszutek Wilk

Quoted 10 lines · Stefan Bader
Quoted 9 lines · Konrad Rzeszutek Wilk
Quoted 3 lines
Quoted 3 lines
What about using the cpuidle_disabled() functionality and adhere to that? As so:
.. snip..
Quoted 4 lines
Quoted 2 lines
From reading over it, this should work. Though I would be interested to hear
from the linux-acpi folks. Also to double check that calling pm_idle when cpuidle.off was specified really is what is intended.

Quoted 9 lines

Quoted 5 lines
Quoted 2 lines
Oh yeah, definitly need the input from linux-acpi folks. And also to be actually tested :-)
I can volunteer to do the testing. But I am lazy enough to hold back a bit as someone may tell us this is completely the wrong way to fix it. :)
So the other option is to use 'idle=halt' on the Linux command line. That should provide the workaround for the folks reporting this (is there a BZ for it?).

Believe upstream bz is still down. Got the issue reported here, though:

http://bugs.launchpad.net/bugs/881076

Hm, as a workaround probably. I guess it could be added when the test images are done. Unfortunately, when running your image in the cloud it is kind of hard to recover. Even more as you could have tested on an AMD based host.

Wonder whether it would be an option to automatically mask the mwait capability off when running in paravirt mode...

-Stefan

Quoted 6 lines

At least until a good solution is hammered out.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Konrad Rzeszutek Wilk

2011-11-10 14:55 UTC #7 lore raw

Quoted 18 lines · Stefan Bader

Quoted 17 lines · Konrad Rzeszutek Wilk
Quoted 15 lines · Stefan Bader
Quoted 11 lines · Konrad Rzeszutek Wilk
Quoted 3 lines
Quoted 3 lines
What about using the cpuidle_disabled() functionality and adhere to that? As so:
.. snip..
Quoted 4 lines
Quoted 2 lines
From reading over it, this should work. Though I would be interested to hear
from the linux-acpi folks. Also to double check that calling pm_idle when cpuidle.off was specified really is what is intended.
Oh yeah, definitly need the input from linux-acpi folks. And also to be actually tested :-)
I can volunteer to do the testing. But I am lazy enough to hold back a bit as someone may tell us this is completely the wrong way to fix it. :)

Quoted 6 lines

Quoted 3 lines
So the other option is to use 'idle=halt' on the Linux command line. That should provide the workaround for the folks reporting this (is there a BZ for it?).
Believe upstream bz is still down. Got the issue reported here, though:
http://bugs.launchpad.net/bugs/881076

Ok, added that to the patch description.

Quoted 4 lines

Hm, as a workaround probably. I guess it could be added when the test images are done. Unfortunately, when running your image in the cloud it is kind of hard to recover. Even more as you could have tested on an AMD based host.

Sadly my AMD boxes did not explode when I tested Len's patches :-(.

Quoted 3 lines

Wonder whether it would be an option to automatically mask the mwait capability off when running in paravirt mode...

We could, but why not strive to do it correctly first.

Stefan Bader

author

2011-11-10 15:35 UTC #8 lore raw

Quoted 26 lines · Konrad Rzeszutek Wilk

Quoted 18 lines · Stefan Bader
Quoted 17 lines · Konrad Rzeszutek Wilk
Quoted 15 lines · Stefan Bader
Quoted 11 lines · Konrad Rzeszutek Wilk
Quoted 3 lines
Quoted 3 lines
What about using the cpuidle_disabled() functionality and adhere to that? As so:
.. snip..
Quoted 4 lines
Quoted 2 lines
From reading over it, this should work. Though I would be interested to hear
from the linux-acpi folks. Also to double check that calling pm_idle when cpuidle.off was specified really is what is intended.
Oh yeah, definitly need the input from linux-acpi folks. And also to be actually tested :-)
I can volunteer to do the testing. But I am lazy enough to hold back a bit as someone may tell us this is completely the wrong way to fix it. :)
Quoted 6 lines
Quoted 3 lines
So the other option is to use 'idle=halt' on the Linux command line. That should provide the workaround for the folks reporting this (is there a BZ for it?).
Believe upstream bz is still down. Got the issue reported here, though:
http://bugs.launchpad.net/bugs/881076

Quoted 8 lines

Ok, added that to the patch description.
Quoted 4 lines
Hm, as a workaround probably. I guess it could be added when the test images are done. Unfortunately, when running your image in the cloud it is kind of hard to recover. Even more as you could have tested on an AMD based host.
Sadly my AMD boxes did not explode when I tested Len's patches :-(.

No, to my knowledge AMD CPUs do not have mwait. So the only alternate idle to happen would be the e400 aware one (which is basically default_idle with conditionally programming a broadcast timer interrupt to get out of c1e). I saw this happen on my box, too. And there was no visible badness caused by it.

Quoted 6 lines

Quoted 3 lines
Wonder whether it would be an option to automatically mask the mwait capability off when running in paravirt mode...
We could, but why not strive to do it correctly first.

Just seemed (from looking at those replies about wanting mwait idle even when cpuidle is disabled) that there is a slight controversy about what correctly is. I personally would also think that a state of cpuidle disabled means no special idle function. But that does not have to be the correct interpretation.

So I was reasoning that if the mwait function gets selected because the caps say its available, but it does not work in paravirt, then maybe claiming its availability is wrong as well.

Of course there is also the initial question about calling a generic function and if that fails call the idle function hook directly. Which, because having cpuidle disabled is returned as an error, ended up calling the hook directly again. I really have trouble understanding the reasoning there...

Len Brown

reviewer

2011-11-13 03:46 UTC #9 lore raw

In reply to Stefan Bader

Quoted 22 lines · Stefan Bader

The following commit changes calls to pm_idle into first trying cpuidle_call_idle() and if that returns non-zero to fall back to call pm_idle().
commit a0bfa1373859e9d11dc92561a8667588803e42d8
Author: Len Brown <[email protected]>
Date: Fri Apr 1 19:34:59 2011 -0400
cpuidle: stop depending on pm_idle
However cpuidle_call_idle() will return -ENODEV if it is supposed to be disabled by cpuidle.off. Which then causes pm_idle() to be called.
This has some bad interaction with the following change that tries to make use of disabling cpuidle in Xen to fall back to hlt.
commit d91ee5863b71e8c90eaf6035bff3078a85e2e7b5
Author: Len Brown <[email protected]>
Date: Fri Apr 1 18:28:35 2011 -0400
cpuidle: replace xen access to x86 pm_idle and default_idle

Quoted 7 lines

The problem I see is that select_idle_routine() is called from arch/x86/kernel/cpu/common.c and since Xen setup does not set pm_idle anymore, it can cause mwait_idle or amd_e400_idle functions to be selected. In testing it seem amd_e400_idle in PVM domU at least does not immediately cause problems, but mwait_idle just causes crashes. From the reports I have this may be related to older hypervisors (3.1 and older) not clearing the mwait capability. But overall there seems something wrong in the interaction.

Why is Xen advertising X86_FEATURE_MWAIT and then crashing when the dom0 (or other guests) use what it advertises?

What versions of Xen have this bug?

Quoted 3 lines

I am not really sure whether the logic of calling pm_idle() on all errors from cpuidle_call_idle() is already flawed or the assumption in the Xen patch about being able to prevent the wrong idle function by turning cpuidle off is incorrect.

The patches above appear to be operating as intended. What wasn't expected, was that some version of Xen is deployed that advertises the MWAIT feature, but crashes when it is used.

Quoted 2 lines

One quick fix could be to add some Xen case into select_idle_routine() which picks default_idle...

No.

Working around this Xen bug for a newly compiled Dom0 is insufficient.

All guests that also look for MWAIT support w/o asking ACPI (ie. all versions of Linux that use intel_idle, such as the last few Fedora's, RHEL, SLES etc.)
will trip over the same Xen bug, even if Dom0 doesn't.

Xen must not advertises MWAIT support if it doesn't have MWAIT support.

thanks,
Len Brown, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Keir Fraser

2011-11-13 16:59 UTC #10 lore raw

Quoted 10 lines · Len Brown

Quoted 9 lines
The problem I see is that select_idle_routine() is called from arch/x86/kernel/cpu/common.c and since Xen setup does not set pm_idle anymore, it can cause mwait_idle or amd_e400_idle functions to be selected. In testing it seem amd_e400_idle in PVM domU at least does not immediately cause
problems, but mwait_idle just causes crashes. From the reports I have this may be related to older hypervisors (3.1 and older) not clearing the mwait
capability. But overall there seems something wrong in the interaction.

Quoted 4 lines

Why is Xen advertising X86_FEATURE_MWAIT and then crashing when the dom0 (or other guests) use what it advertises?
What versions of Xen have this bug?

Xen doesn't advertise MWAIT. Possibly Xen-pv_ops is lying to the rest of the kernel via the cpuid pv_ops hook. This would probably be because Xen is relying on the OSPM in dom0 kernel to parse out Cx/Px info which Xen itself *can* use.

-- Keir

Konrad Rzeszutek Wilk

2011-11-14 18:19 UTC #11 lore raw

Quoted 12 lines · Keir Fraser

Quoted 10 lines · Len Brown
Quoted 9 lines
The problem I see is that select_idle_routine() is called from arch/x86/kernel/cpu/common.c and since Xen setup does not set pm_idle anymore, it can cause mwait_idle or amd_e400_idle functions to be selected. In testing it seem amd_e400_idle in PVM domU at least does not immediately cause
problems, but mwait_idle just causes crashes. From the reports I have this may be related to older hypervisors (3.1 and older) not clearing the mwait
capability. But overall there seems something wrong in the interaction.

Quoted 7 lines

Quoted 4 lines
Why is Xen advertising X86_FEATURE_MWAIT and then crashing when the dom0 (or other guests) use what it advertises?
What versions of Xen have this bug?
Xen doesn't advertise MWAIT. Possibly Xen-pv_ops is lying to the rest of the kernel via the cpuid pv_ops hook. This would probably be because Xen is

I can't seem to find anything in there advertising the MWAIT feature.

Quoted 2 lines

relying on the OSPM in dom0 kernel to parse out Cx/Px info which Xen itself *can* use.

The Cx/Px patches that would parse the Cx/Px and then percolate those up to the hypervisor are in mainline. So that is not it.

Konrad Rzeszutek Wilk

2011-11-14 20:11 UTC #12 lore raw

Quoted 21 lines · Konrad Rzeszutek Wilk

Quoted 12 lines · Keir Fraser
Quoted 10 lines · Len Brown
Quoted 9 lines
The problem I see is that select_idle_routine() is called from arch/x86/kernel/cpu/common.c and since Xen setup does not set pm_idle anymore, it can cause mwait_idle or amd_e400_idle functions to be selected. In testing it seem amd_e400_idle in PVM domU at least does not immediately cause
problems, but mwait_idle just causes crashes. From the reports I have this may be related to older hypervisors (3.1 and older) not clearing the mwait
capability. But overall there seems something wrong in the interaction.
Quoted 7 lines
Quoted 4 lines
Why is Xen advertising X86_FEATURE_MWAIT and then crashing when the dom0 (or other guests) use what it advertises?
What versions of Xen have this bug?
Xen doesn't advertise MWAIT. Possibly Xen-pv_ops is lying to the rest of the kernel via the cpuid pv_ops hook. This would probably be because Xen is

Quoted 7 lines

I can't seem to find anything in there advertising the MWAIT feature.
Quoted 2 lines
relying on the OSPM in dom0 kernel to parse out Cx/Px info which Xen itself *can* use.
The Cx/Px patches that would parse the Cx/Px and then percolate those up to the hypervisor are in mainline. So that is not it.

<sigh> I meant to say "are _not_ in mainline". Sorry for the confusion.

Konrad Rzeszutek Wilk

2011-11-14 14:31 UTC #13 lore raw

In reply to Len Brown

Hey Len,

Quoted 10 lines

Quoted 7 lines
The problem I see is that select_idle_routine() is called from arch/x86/kernel/cpu/common.c and since Xen setup does not set pm_idle anymore, it can cause mwait_idle or amd_e400_idle functions to be selected. In testing it seem amd_e400_idle in PVM domU at least does not immediately cause problems, but mwait_idle just causes crashes. From the reports I have this may be related to older hypervisors (3.1 and older) not clearing the mwait capability. But overall there seems something wrong in the interaction.
Why is Xen advertising X86_FEATURE_MWAIT and then crashing when the dom0 (or other guests) use what it advertises?

The only case where I've seen this is with Amazon EC2. The other newer hypervisors (4.1.1 and such) do not trigger this.

Quoted 2 lines

What versions of Xen have this bug?

Whatever Amazon is using. I think they are RHEL5 based hypervisor.

Quoted 8 lines

Quoted 4 lines
I am not really sure whether the logic of calling pm_idle() on all errors from cpuidle_call_idle() is already flawed or the assumption in the Xen patch about being able to prevent the wrong idle function by turning cpuidle off is incorrect.
The patches above appear to be operating as intended. What wasn't expected, was that some version of Xen is deployed that advertises the MWAIT feature, but crashes when it is used.

How does that work with AMD? On those machines it ends up calling amd_e400_idle instead of the default_idle. Granted it does not "BUG" out but it does lead to extra trap-n-emulate (the MSR operation) in to the hypervisor which is not good.

Quoted 6 lines

Quoted 3 lines
One quick fix could be to add some Xen case into select_idle_routine() which picks default_idle...
No.

Quoted 8 lines

Working around this Xen bug for a newly compiled Dom0 is insufficient.
All guests that also look for MWAIT support w/o asking ACPI (ie. all versions of Linux that use intel_idle, such as the last few Fedora's, RHEL, SLES etc.)
will trip over the same Xen bug, even if Dom0 doesn't.
Xen must not advertises MWAIT support if it doesn't have MWAIT support.

How does work out when we figure MWAIT support from the CPUID? Or are you saying that it is correct - if the CPU advertises it, then yes advertise it to the Linux kernel?

Konrad Rzeszutek Wilk

2011-11-14 18:26 UTC #14 lore raw

Quoted 13 lines · Konrad Rzeszutek Wilk

Hey Len,
Quoted 10 lines
Quoted 7 lines
The problem I see is that select_idle_routine() is called from arch/x86/kernel/cpu/common.c and since Xen setup does not set pm_idle anymore, it can cause mwait_idle or amd_e400_idle functions to be selected. In testing it seem amd_e400_idle in PVM domU at least does not immediately cause problems, but mwait_idle just causes crashes. From the reports I have this may be related to older hypervisors (3.1 and older) not clearing the mwait capability. But overall there seems something wrong in the interaction.
Why is Xen advertising X86_FEATURE_MWAIT and then crashing when the dom0 (or other guests) use what it advertises?

Quoted 7 lines

The only case where I've seen this is with Amazon EC2. The other newer hypervisors (4.1.1 and such) do not trigger this.
Quoted 2 lines
What versions of Xen have this bug?
Whatever Amazon is using. I think they are RHEL5 based hypervisor.

Vincent,

Would you have ideas of what might be happening? It looks as if some Amazon instances are advertisting the MWAIT flag but not really supporting it. Any ideas of what might be going on?

Quoted 14 lines

Quoted 9 lines
Quoted 5 lines
I am not really sure whether the logic of calling pm_idle() on all errors from cpuidle_call_idle() is already flawed or the assumption in the Xen patch about being able to prevent the wrong idle function by turning cpuidle off is incorrect.
The patches above appear to be operating as intended. What wasn't expected, was that some version of Xen is deployed that advertises the MWAIT feature, but crashes when it is used.
How does that work with AMD? On those machines it ends up calling amd_e400_idle instead of the default_idle. Granted it does not "BUG" out but it does lead to extra trap-n-emulate (the MSR operation) in to the hypervisor which is not good.

.. snip..

Quoted 1 line

Quoted 1 line
Working around this Xen bug for a newly compiled Dom0 is insufficient.

It looks to affect PV guests as well.

Quoted 5 lines

Quoted 5 lines
All guests that also look for MWAIT support w/o asking ACPI (ie. all versions of Linux that use intel_idle, such as the last few Fedora's, RHEL, SLES etc.)
will trip over the same Xen bug, even if Dom0 doesn't.

Don't think I answered this one:

The intel_idle has not been used in those cases b/c the default_idle had been used instead.

Quoted 11 lines

Quoted 2 lines
Xen must not advertises MWAIT support if it doesn't have MWAIT support.
How does work out when we figure MWAIT support from the CPUID? Or are you saying that it is correct - if the CPU advertises it, then yes advertise it to the Linux kernel?
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel