2006-10-29 02:45:43

by Chris Wright

[permalink] [raw]
Subject: [PATCH 4/7] Allow selected bug checks to be skipped by paravirt kernels

Allow selected bug checks to be skipped by paravirt kernels. The two most
important are the F00F workaround (which is either done by the hypervisor,
or not required), and the 'hlt' instruction check, which can break under
some hypervisors.

Signed-off-by: Zachary Amsden <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: Jeremy Fitzhardinge <[email protected]>

---
arch/i386/kernel/cpu/intel.c | 2 +-
include/asm-i386/bugs.h | 4 ++++
2 files changed, 5 insertions(+), 1 deletion(-)

--- linux-2.6-pv.orig/arch/i386/kernel/cpu/intel.c
+++ linux-2.6-pv/arch/i386/kernel/cpu/intel.c
@@ -107,7 +107,7 @@ static void __cpuinit init_intel(struct
* Note that the workaround only should be initialized once...
*/
c->f00f_bug = 0;
- if ( c->x86 == 5 ) {
+ if (!paravirt_enabled() && c->x86 == 5) {
static int f00f_workaround_enabled = 0;

c->f00f_bug = 1;
--- linux-2.6-pv.orig/include/asm-i386/bugs.h
+++ linux-2.6-pv/include/asm-i386/bugs.h
@@ -21,6 +21,7 @@
#include <asm/processor.h>
#include <asm/i387.h>
#include <asm/msr.h>
+#include <asm/paravirt.h>

static int __init no_halt(char *s)
{
@@ -91,6 +92,9 @@ static void __init check_fpu(void)

static void __init check_hlt(void)
{
+ if (paravirt_enabled())
+ return;
+
printk(KERN_INFO "Checking 'hlt' instruction... ");
if (!boot_cpu_data.hlt_works_ok) {
printk("disabled\n");

--


2006-11-01 12:18:09

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 4/7] Allow selected bug checks to be skipped by paravirt kernels

On Sat 2006-10-28 00:00:04, Chris Wright wrote:
> Allow selected bug checks to be skipped by paravirt kernels. The two most
> important are the F00F workaround (which is either done by the hypervisor,
> or not required), and the 'hlt' instruction check, which can break under
> some hypervisors.

How can hlt check break? It is hlt;hlt;hlt, IIRC, that looks fairly
innocent to me.

> --- linux-2.6-pv.orig/arch/i386/kernel/cpu/intel.c
> +++ linux-2.6-pv/arch/i386/kernel/cpu/intel.c
> @@ -107,7 +107,7 @@ static void __cpuinit init_intel(struct
> * Note that the workaround only should be initialized once...
> */
> c->f00f_bug = 0;
> - if ( c->x86 == 5 ) {
> + if (!paravirt_enabled() && c->x86 == 5) {

I'd do x86==5 check first... pentiums are not common any more.

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2006-11-01 22:41:39

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH 4/7] Allow selected bug checks to be skipped by paravirt kernels

On Wed, Nov 01, 2006 at 01:17:53PM +0100, Pavel Machek wrote:

> > +++ linux-2.6-pv/arch/i386/kernel/cpu/intel.c
> > @@ -107,7 +107,7 @@ static void __cpuinit init_intel(struct
> > * Note that the workaround only should be initialized once...
> > */
> > c->f00f_bug = 0;
> > - if ( c->x86 == 5 ) {
> > + if (!paravirt_enabled() && c->x86 == 5) {
>
> I'd do x86==5 check first... pentiums are not common any more.

It's not like paravirt_enabled will be common-case either,
and is this isn't exactly a performance critical piece of code,
it doesn't really matter which way around the checks are done.

Dave

--
http://www.codemonkey.org.uk

2006-11-01 23:24:46

by Zachary Amsden

[permalink] [raw]
Subject: Re: [PATCH 4/7] Allow selected bug checks to be skipped by paravirt kernels

Pavel Machek wrote:
> On Sat 2006-10-28 00:00:04, Chris Wright wrote:
>
>> Allow selected bug checks to be skipped by paravirt kernels. The two most
>> important are the F00F workaround (which is either done by the hypervisor,
>> or not required), and the 'hlt' instruction check, which can break under
>> some hypervisors.
>>
>
> How can hlt check break? It is hlt;hlt;hlt, IIRC, that looks fairly
> innocent to me.
>

Not if you use tickless timers that don't generate interrupts to unhalt
you, or if you delay ticks until the next scheduled timeout and you
haven't yet scheduled any timeout. Both are likely in a hypervisor.

Zach

2006-11-02 10:21:14

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 4/7] Allow selected bug checks to be skipped by paravirt kernels

Hi!

> >>Allow selected bug checks to be skipped by paravirt kernels. The two most
> >>important are the F00F workaround (which is either done by the hypervisor,
> >>or not required), and the 'hlt' instruction check, which can break under
> >>some hypervisors.
> >>
> >
> >How can hlt check break? It is hlt;hlt;hlt, IIRC, that looks fairly
> >innocent to me.
> >
>
> Not if you use tickless timers that don't generate interrupts to unhalt
> you, or if you delay ticks until the next scheduled timeout and you
> haven't yet scheduled any timeout. Both are likely in a hypervisor.

Well.. but you are working around problem, instead of fixing it.

Tickless kernels are possible on normal machines, too.

Please fix it properly... probably by requesting timer 10msec in
advance or something.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2006-11-02 11:04:51

by Zachary Amsden

[permalink] [raw]
Subject: Re: [PATCH 4/7] Allow selected bug checks to be skipped by paravirt kernels

Pavel Machek wrote:
>>> How can hlt check break? It is hlt;hlt;hlt, IIRC, that looks fairly
>>> innocent to me.
>>>
>>>
>> Not if you use tickless timers that don't generate interrupts to unhalt
>> you, or if you delay ticks until the next scheduled timeout and you
>> haven't yet scheduled any timeout. Both are likely in a hypervisor.
>>
>
> Well.. but you are working around problem, instead of fixing it.
>
> Tickless kernels are possible on normal machines, too.
>
> Please fix it properly... probably by requesting timer 10msec in
> advance or something.
> Pavel
>

Well, I agree in spirit, but there is something to be said for keeping
the code less complicated by removing these workarounds for broken
processors. Preferably, we could remove the hlt check entirely, but
then those people with these broken processors would not get the
expected behavior of stalling during boot - that is the expected
behavior of failure, correct? In any case, I added this workaround for
the case when running under Xen. I would rather not add a dependence on
timer scheduling to legacy bug checking code when the number of timer
sources and tickless variations available is proliferating and the
number of legacy processors that would even need this check is rapidly
approaching zero.

Zach