2008-07-29 17:40:34

by Suresh Siddha

[permalink] [raw]
Subject: [patch 0/9] x86, xsave: xsave/xrstor support

This patchset adds the support for xsave/xrstor infrastructure for x86.
xsave/xrstor manages the existing and future processor extended states in x86
architecutre.

More info on xsave/xrstor can be found in the Intel SDM's located at
http://www.intel.com/products/processor/manuals/index.htm

The layout of the xsave/xrstor area extends from the 512-byte FXSAVE/FXRSTOR
layout. xsave/xrstor area layout consists of:

- fxsave/fxrstor area (512 bytes)
- xsave header area (64 bytes)
- set of save areas, each corresponding to a processor extended state

The number of save areas, the offset and the size of each save area is
enumerated by CPUID leaf function 0xd.

This patch includes the basic xsave/xrstor infrastructure(supporting FP/SSE),
which includes:
- context switch support, extending traditional lazy restore mechanism
- signal handling support, extending(using the software reserved bytes
464..511 in the current 512byte layout of fxsave frame) the memory
layout pointed out by the fpstate pointer in the sigcontext

More details in the patches to follow.

Signed-off-by: Suresh Siddha <[email protected]>
---


2008-07-29 23:14:18

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

Suresh Siddha wrote:
> This patchset adds the support for xsave/xrstor infrastructure for x86.
> xsave/xrstor manages the existing and future processor extended states in x86
> architecutre.
>
> More info on xsave/xrstor can be found in the Intel SDM's located at
> http://www.intel.com/products/processor/manuals/index.htm
>
> The layout of the xsave/xrstor area extends from the 512-byte FXSAVE/FXRSTOR
> layout. xsave/xrstor area layout consists of:
>
> - fxsave/fxrstor area (512 bytes)
> - xsave header area (64 bytes)
> - set of save areas, each corresponding to a processor extended state
>
> The number of save areas, the offset and the size of each save area is
> enumerated by CPUID leaf function 0xd.
>
> This patch includes the basic xsave/xrstor infrastructure(supporting FP/SSE),
> which includes:
> - context switch support, extending traditional lazy restore mechanism
> - signal handling support, extending(using the software reserved bytes
> 464..511 in the current 512byte layout of fxsave frame) the memory
> layout pointed out by the fpstate pointer in the sigcontext
>
> More details in the patches to follow.
>
> Signed-off-by: Suresh Siddha <[email protected]>

Applied as tip:x86/xsave.

I should warn I had to do quite a bit of manual fixup in order to
disentangle it from other changes in -tip, so please test it out; I
compile-tested it but for obvious reasons can't do more than that at the
moment.

-hpa

2008-07-29 23:30:01

by Suresh Siddha

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

On Tue, Jul 29, 2008 at 04:09:12PM -0700, H. Peter Anvin wrote:
> Applied as tip:x86/xsave.
>
> I should warn I had to do quite a bit of manual fixup in order to
> disentangle it from other changes in -tip, so please test it out; I
> compile-tested it but for obvious reasons can't do more than that at the
> moment.

hpa, these patches just apply fine to tip/master. Can you please arrange
the tip/x86/xsave tree accordingly? or do I need to do something else
to smooth this process?

thanks,
suresh

2008-07-29 23:47:50

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

Suresh Siddha wrote:
> On Tue, Jul 29, 2008 at 04:09:12PM -0700, H. Peter Anvin wrote:
>> Applied as tip:x86/xsave.
>>
>> I should warn I had to do quite a bit of manual fixup in order to
>> disentangle it from other changes in -tip, so please test it out; I
>> compile-tested it but for obvious reasons can't do more than that at the
>> moment.
>
> hpa, these patches just apply fine to tip/master. Can you please arrange
> the tip/x86/xsave tree accordingly? or do I need to do something else
> to smooth this process?

This is awkward, since that means this is "derived topic". Most of the
changes are orthogonal and relatively trivial to fix up at merge time,
so I would prefer to keep them separate.

-hpa

2008-07-30 10:04:23

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support


* H. Peter Anvin <[email protected]> wrote:

>> hpa, these patches just apply fine to tip/master. Can you please
>> arrange the tip/x86/xsave tree accordingly? or do I need to do
>> something else to smooth this process?
>
> This is awkward, since that means this is "derived topic". Most of
> the changes are orthogonal and relatively trivial to fix up at merge
> time, so I would prefer to keep them separate.

Well, in this case the conflicts seem to be quite heavy, so i'd suggest
to use the method we have used for x86/x2apic and for xen-64bit:

Merge the affected topics into tip/x86/core. Then merge x86/core into
x86/xsave, and put the xsave patches ontop of that base.

This way x86/xsave is a 'derived' topic and optional until it's proven,
but one that is still mergable once all the dependent topics go
upstream. We'd only have to rebase it in the (unlikely) event of there
being some major problem with any of the topics merged into x86/core.

ok?

Ingo

2008-07-30 16:36:39

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

Ingo Molnar wrote:
> * H. Peter Anvin <[email protected]> wrote:
>
>>> hpa, these patches just apply fine to tip/master. Can you please
>>> arrange the tip/x86/xsave tree accordingly? or do I need to do
>>> something else to smooth this process?
>> This is awkward, since that means this is "derived topic". Most of
>> the changes are orthogonal and relatively trivial to fix up at merge
>> time, so I would prefer to keep them separate.
>
> Well, in this case the conflicts seem to be quite heavy, so i'd suggest
> to use the method we have used for x86/x2apic and for xen-64bit:
>
> Merge the affected topics into tip/x86/core. Then merge x86/core into
> x86/xsave, and put the xsave patches ontop of that base.
>
> This way x86/xsave is a 'derived' topic and optional until it's proven,
> but one that is still mergable once all the dependent topics go
> upstream. We'd only have to rebase it in the (unlikely) event of there
> being some major problem with any of the topics merged into x86/core.
>
> ok?

It somewhat concerns me, because one of the conflicts is generated by
collision with x2apic. The rest of them I don't think are too problematic.

Most of the conflicts are of the type "orthogonal transformations to the
same chunk of code", which doesn't make them less annoying.

-hpa

2008-07-30 17:08:48

by Suresh Siddha

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

On Wed, Jul 30, 2008 at 09:31:41AM -0700, H. Peter Anvin wrote:
> Ingo Molnar wrote:
> > * H. Peter Anvin <[email protected]> wrote:
> >
> >>> hpa, these patches just apply fine to tip/master. Can you please
> >>> arrange the tip/x86/xsave tree accordingly? or do I need to do
> >>> something else to smooth this process?
> >> This is awkward, since that means this is "derived topic". Most of
> >> the changes are orthogonal and relatively trivial to fix up at merge
> >> time, so I would prefer to keep them separate.
> >
> > Well, in this case the conflicts seem to be quite heavy, so i'd suggest
> > to use the method we have used for x86/x2apic and for xen-64bit:
> >
> > Merge the affected topics into tip/x86/core. Then merge x86/core into
> > x86/xsave, and put the xsave patches ontop of that base.
> >
> > This way x86/xsave is a 'derived' topic and optional until it's proven,
> > but one that is still mergable once all the dependent topics go
> > upstream. We'd only have to rebase it in the (unlikely) event of there
> > being some major problem with any of the topics merged into x86/core.
> >
> > ok?
>
> It somewhat concerns me, because one of the conflicts is generated by
> collision with x2apic. The rest of them I don't think are too problematic.

hpa, confilicts with x2apic branch are very small and related to cpuid bits.

commit 04df16d2465cbb59b84c9c57ad865dbbeebadad8
Author: Suresh Siddha <[email protected]>
Date: Tue Jul 29 10:29:18 2008 -0700

x86, xsave: xsave cpuid feature bits

Add xsave CPU feature bits.

and

commit 32e1d0a0651004f5fe47f85a2a5c725ad579a90c
Author: Suresh Siddha <[email protected]>
Date: Thu Jul 10 11:16:50 2008 -0700

x64, x2apic/intr-remap: cpuid bits for x2apic feature

cpuid feature for x2apic.

Both of these patches are straight forward, simple and can be moved
to x86/core(?) now, if that helps.

thanks,
suresh

2008-07-30 17:20:08

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

Suresh Siddha wrote:

>> It somewhat concerns me, because one of the conflicts is generated by
>> collision with x2apic. The rest of them I don't think are too problematic.
>
> hpa, confilicts with x2apic branch are very small and related to cpuid bits.
>
> commit 04df16d2465cbb59b84c9c57ad865dbbeebadad8
> Author: Suresh Siddha <[email protected]>
> Date: Tue Jul 29 10:29:18 2008 -0700
>
> x86, xsave: xsave cpuid feature bits
>
> Add xsave CPU feature bits.
>
> and
>
> commit 32e1d0a0651004f5fe47f85a2a5c725ad579a90c
> Author: Suresh Siddha <[email protected]>
> Date: Thu Jul 10 11:16:50 2008 -0700
>
> x64, x2apic/intr-remap: cpuid bits for x2apic feature
>
> cpuid feature for x2apic.
>
> Both of these patches are straight forward, simple and can be moved
> to x86/core(?) now, if that helps.

This is true. Cherry-picking those to the x86/core branch is probably
the easiest, even if that means they're duplicated in the x2apic branch
(I presume we don't want to rebase x2apic).

-hpa

2008-07-30 18:26:42

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support


* Ingo Molnar <[email protected]> wrote:

> Well, in this case the conflicts seem to be quite heavy, so i'd
> suggest to use the method we have used for x86/x2apic and for
> xen-64bit:
>
> Merge the affected topics into tip/x86/core. Then merge x86/core into
> x86/xsave, and put the xsave patches ontop of that base.
>
> This way x86/xsave is a 'derived' topic and optional until it's
> proven, but one that is still mergable once all the dependent topics
> go upstream. We'd only have to rebase it in the (unlikely) event of
> there being some major problem with any of the topics merged into
> x86/core.

I've restructured tip/x86/xsave to be like this, and it can now be
merged into tip/master without conficts.

I briefly merged it into tip/master to test it, but after about 10
randconfig iterations it ran into a spontaneous reboot crash. There's
nothing in the log, the box (an Athlon64 X2) just reboots spontaneously.
Removing the x86/xsave changes makes the system boot up fine.

I've pushed out the tree i tested into the tip/tmp.x86/xsave.broken
branch. The bad config is:

http://redhat.com/~mingo/misc/config-Wed_Jul_30_20_09_12_CEST_2008.bad

Two crashlogs are here:

http://redhat.com/~mingo/misc/crash-Wed_Jul_30_20_09_12_CEST_2008.log

the serial log just stops at a certain point. It's not at a specific
point, but the crash happened both times i tried to boot it.

Ingo

2008-07-30 21:46:54

by Suresh Siddha

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

On Wed, Jul 30, 2008 at 11:25:39AM -0700, Ingo Molnar wrote:
>
> I've restructured tip/x86/xsave to be like this, and it can now be
> merged into tip/master without conficts.

thanks.

> I briefly merged it into tip/master to test it, but after about 10
> randconfig iterations it ran into a spontaneous reboot crash. There's
> nothing in the log, the box (an Athlon64 X2) just reboots spontaneously.
> Removing the x86/xsave changes makes the system boot up fine.

I am also seeing this reboot issue and am hitting this both on tip/master
aswell as the tip/tmp.x86/xsave.broken

It seems to be timing related. Will look further.

thanks,
suresh

2008-07-30 23:41:53

by Suresh Siddha

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

On Wed, Jul 30, 2008 at 11:25:39AM -0700, Ingo Molnar wrote:
> I briefly merged it into tip/master to test it, but after about 10
> randconfig iterations it ran into a spontaneous reboot crash. There's
> nothing in the log, the box (an Athlon64 X2) just reboots spontaneously.
> Removing the x86/xsave changes makes the system boot up fine.
>
> I've pushed out the tree i tested into the tip/tmp.x86/xsave.broken
> branch. The bad config is:
>
> http://redhat.com/~mingo/misc/config-Wed_Jul_30_20_09_12_CEST_2008.bad

Ingo, 2.6.25, 2.6.26 also reboots randomly with this config on my dual-socket
system.

2008-07-31 21:29:58

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support


* Suresh Siddha <[email protected]> wrote:

> On Wed, Jul 30, 2008 at 11:25:39AM -0700, Ingo Molnar wrote:
> > I briefly merged it into tip/master to test it, but after about 10
> > randconfig iterations it ran into a spontaneous reboot crash. There's
> > nothing in the log, the box (an Athlon64 X2) just reboots spontaneously.
> > Removing the x86/xsave changes makes the system boot up fine.
> >
> > I've pushed out the tree i tested into the tip/tmp.x86/xsave.broken
> > branch. The bad config is:
> >
> > http://redhat.com/~mingo/misc/config-Wed_Jul_30_20_09_12_CEST_2008.bad
>
> Ingo, 2.6.25, 2.6.26 also reboots randomly with this config on my
> dual-socket system.

hmmm ....

a few guesses: perhaps I2C related (the randconfig turned on I2C bits)?
Do you have any way (hw test-kit) to figure out where the reboot comes
from - is it a triple fault caused by the kernel? Or some hardware
event?

Ingo

2008-07-31 21:58:27

by Suresh Siddha

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

On Thu, Jul 31, 2008 at 02:29:15PM -0700, Ingo Molnar wrote:
> > Ingo, 2.6.25, 2.6.26 also reboots randomly with this config on my
> > dual-socket system.
>
> hmmm ....
>
> a few guesses: perhaps I2C related (the randconfig turned on I2C bits)?
> Do you have any way (hw test-kit) to figure out where the reboot comes
> from - is it a triple fault caused by the kernel? Or some hardware
> event?

Doh! I knew something was wrong with your randconfig :)

#
# Watchdog Device Drivers
#
....

CONFIG_PC87413_WDT=y
CONFIG_60XX_WDT=y
# CONFIG_SBC8360_WDT is not set
CONFIG_CPU5_WDT=y
CONFIG_SMSC37B787_WDT=y
CONFIG_W83627HF_WDT=y
CONFIG_W83697HF_WDT=y
CONFIG_W83877F_WDT=y
CONFIG_W83977F_WDT=y
CONFIG_MACHZ_WDT=y
CONFIG_SBC_EPX_C3_WATCHDOG=y

Kernel was enabling watchdog, with out the userspace having the
heartbeat daemon....

randconfig is really not a good way to test! I can't even ssh to the box
with this config (something else is missing in the config!)

Anyhow, with watchdog removed, it works just fine. (both xsave and non-xsave
kernels)

thanks,
suresh

2008-07-31 22:14:46

by Andi Kleen

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

> Kernel was enabling watchdog, with out the userspace having the
> heartbeat daemon....

Watchdog are normally only armed when someone opens the device
first.

-Andi

2008-07-31 22:18:38

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support


* Suresh Siddha <[email protected]> wrote:

> On Thu, Jul 31, 2008 at 02:29:15PM -0700, Ingo Molnar wrote:
> > > Ingo, 2.6.25, 2.6.26 also reboots randomly with this config on my
> > > dual-socket system.
> >
> > hmmm ....
> >
> > a few guesses: perhaps I2C related (the randconfig turned on I2C bits)?
> > Do you have any way (hw test-kit) to figure out where the reboot comes
> > from - is it a triple fault caused by the kernel? Or some hardware
> > event?
>
> Doh! I knew something was wrong with your randconfig :)
>
> #
> # Watchdog Device Drivers
> #
> ....
>
> CONFIG_PC87413_WDT=y
> CONFIG_60XX_WDT=y
> # CONFIG_SBC8360_WDT is not set
> CONFIG_CPU5_WDT=y
> CONFIG_SMSC37B787_WDT=y
> CONFIG_W83627HF_WDT=y
> CONFIG_W83697HF_WDT=y
> CONFIG_W83877F_WDT=y
> CONFIG_W83977F_WDT=y
> CONFIG_MACHZ_WDT=y
> CONFIG_SBC_EPX_C3_WATCHDOG=y
>
> Kernel was enabling watchdog, with out the userspace having the
> heartbeat daemon....

I never had function hw watchdog support on this box - so i guess the
2.6.27-rc1 kernel started supporting it - cool.

This was unfortunate timing i guess - when i removed xsave i didnt get
the reboot :-/

> randconfig is really not a good way to test! I can't even ssh to the
> box with this config (something else is missing in the config!)

it is a randconfig tailored to my testboxes, it might not work out of
box on yours :-)

randconfig is an excellent way of testing kernels, that testbox can run
strings of hundreds (sometimes thousands) of random kernels without
hitting any kernel bug. I had to resolve a large number of real kernel
bugs (and a handful of glitches like this hardware watchdog thing) to
get so far though.

> Anyhow, with watchdog removed, it works just fine. (both xsave and
> non-xsave kernels)

Great, thanks. I've re-integrated tip/x86/xsave into x86/master and have
pushed out the result.

Ingo

2008-07-31 22:19:38

by Suresh Siddha

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

On Thu, Jul 31, 2008 at 03:14:35PM -0700, Andi Kleen wrote:
> > Kernel was enabling watchdog, with out the userspace having the
> > heartbeat daemon....
>
> Watchdog are normally only armed when someone opens the device
> first.

But thats not what I see with for ex with, w83627hf_wdt.c

Its done at the module_init time. (while I thought it should be
really done when some user level app opens the device, probably
its poorly written to take care if the kernel panics before starting userland.
but kernel can even die before the watchdog driver load aswell ;-)

2008-07-31 22:36:34

by Andi Kleen

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

On Thu, Jul 31, 2008 at 03:19:24PM -0700, Suresh Siddha wrote:
> On Thu, Jul 31, 2008 at 03:14:35PM -0700, Andi Kleen wrote:
> > > Kernel was enabling watchdog, with out the userspace having the
> > > heartbeat daemon....
> >
> > Watchdog are normally only armed when someone opens the device
> > first.
>
> But thats not what I see with for ex with, w83627hf_wdt.c
>
> Its done at the module_init time. (while I thought it should be
> really done when some user level app opens the device, probably
> its poorly written to take care if the kernel panics before starting userland.
> but kernel can even die before the watchdog driver load aswell ;-)

Ok that watchdog driver is just broken then. Putting the watchdog maintainer
into cc.

-Andi
>

2008-07-31 22:43:00

by Linus Torvalds

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support



On Thu, 31 Jul 2008, Suresh Siddha wrote:
> On Thu, Jul 31, 2008 at 03:14:35PM -0700, Andi Kleen wrote:
> > > Kernel was enabling watchdog, with out the userspace having the
> > > heartbeat daemon....
> >
> > Watchdog are normally only armed when someone opens the device
> > first.
>
> But thats not what I see with for ex with, w83627hf_wdt.c
>
> Its done at the module_init time. (while I thought it should be
> really done when some user level app opens the device, probably
> its poorly written to take care if the kernel panics before starting userland.
> but kernel can even die before the watchdog driver load aswell ;-)

Yeah, that's a _really_ broken watchdog timer driver. There's no way that
it's correct to start the watchdog at init time, at least when compiled
in.

It also looks to me like it's not even probing for the hardware - it's
just assuming it's there. That's scary. Am I missing something?

It really shouldn't be activated until it's opened. And it really
shouldn't just write to ports randomly without checking that they make
sense...

Linus

2008-07-31 22:52:21

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support


* Linus Torvalds <[email protected]> wrote:

> > But thats not what I see with for ex with, w83627hf_wdt.c
> >
> > Its done at the module_init time. (while I thought it should be
> > really done when some user level app opens the device, probably its
> > poorly written to take care if the kernel panics before starting
> > userland. but kernel can even die before the watchdog driver load
> > aswell ;-)
>
> Yeah, that's a _really_ broken watchdog timer driver. There's no way
> that it's correct to start the watchdog at init time, at least when
> compiled in.
>
> It also looks to me like it's not even probing for the hardware - it's
> just assuming it's there. That's scary. Am I missing something?
>
> It really shouldn't be activated until it's opened. And it really
> shouldn't just write to ports randomly without checking that they make
> sense...

there are a handful of old ISA-ish drivers that can crash randconfig
kernels in various ways. [indefinite lockups, crashes, stomped-over
hardware, non-working keyboard, etc.]

I mapped most of them out via many months of trial-and-error - but it
would still be nice to have some separate config option to disable the
known ones. CONFIG_ALLOW_NON_GENERIC or something like that - which i
would unset in the randconfig runs.

( They are not CONFIG_BROKEN per se, because often it's hardware that
cannot be probed in any reliable way - the driver just assumes it's
there. )

Ingo

2008-07-31 23:07:29

by Alan

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

> But thats not what I see with for ex with, w83627hf_wdt.c
>
> Its done at the module_init time. (while I thought it should be
> really done when some user level app opens the device, probably
> its poorly written to take care if the kernel panics before starting userland.
> but kernel can even die before the watchdog driver load aswell ;-)

Various watchdogs fail to conform in assorted differing ways. Wim did
some patches to build a common watchdog library and I sent him a rework
of it a while ago. Once that is adopted this should all get cleaned up.
Right now Wim is a bit busy however.

Alan

2008-08-01 02:05:30

by Rene Herman

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

On 01-08-08 00:50, Ingo Molnar wrote:

> there are a handful of old ISA-ish drivers that can crash randconfig
> kernels in various ways. [indefinite lockups, crashes, stomped-over
> hardware, non-working keyboard, etc.]
>
> I mapped most of them out via many months of trial-and-error - but it
> would still be nice to have some separate config option to disable the
> known ones. CONFIG_ALLOW_NON_GENERIC or something like that - which i
> would unset in the randconfig runs.
>
> ( They are not CONFIG_BROKEN per se, because often it's hardware that
> cannot be probed in any reliable way - the driver just assumes it's
> there. )

If you have a list, I might be able to do something about some of them.

Rene.

2008-08-01 09:52:08

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support


* Rene Herman <[email protected]> wrote:

> On 01-08-08 00:50, Ingo Molnar wrote:
>
>> there are a handful of old ISA-ish drivers that can crash randconfig
>> kernels in various ways. [indefinite lockups, crashes, stomped-over
>> hardware, non-working keyboard, etc.]
>>
>> I mapped most of them out via many months of trial-and-error - but it
>> would still be nice to have some separate config option to disable the
>> known ones. CONFIG_ALLOW_NON_GENERIC or something like that - which i
>> would unset in the randconfig runs.
>>
>> ( They are not CONFIG_BROKEN per se, because often it's hardware that
>> cannot be probed in any reliable way - the driver just assumes it's
>> there. )
>
> If you have a list, I might be able to do something about some of
> them.

find attached below a newer version of the original list i published
half a year ago:

http://people.redhat.com/mingo/auto-qa-patches/Kconfig-qa.patch

these are just pragmatic local hacks to get things going. (There are
more per machine quirks as well.)

i have not used a BROKEN annotation because CONFIG_BROKEN is
impractical: it just kills code altogether, indiscriminately. There's no
way for users to enable CONFIG_BROKEN in the upstream kernel - nothing
selects it and it's not an interactive option either.

So by all means if we mark a driver or a kernel feature as
CONFIG_BROKEN, it's killed altogether for all practical purposes.

What we'd need is some more gradual approach: for example a way to mark
"drivers that are not expected to boot on a whitebox PC", without
removing them altogether via a CONFIG_BROKEN dependency - often it's
hardware that cannot be probed safely.

Ingo

------------------>
Index: linux/security/smack/Kconfig
===================================================================
--- linux.orig/security/smack/Kconfig
+++ linux/security/smack/Kconfig
@@ -1,6 +1,9 @@
config SECURITY_SMACK
bool "Simplified Mandatory Access Control Kernel Support"
depends on NETLABEL && SECURITY_NETWORK
+ # breaks networking (TCP connections)
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT
default n
help
This selects the Simplified Mandatory Access Control Kernel.
Index: linux/drivers/block/Kconfig
===================================================================
--- linux.orig/drivers/block/Kconfig
+++ linux/drivers/block/Kconfig
@@ -71,6 +71,11 @@ config BLK_DEV_XD
config PARIDE
tristate "Parallel port IDE device support"
depends on PARPORT_PC
+
+ # the probe can hang during bootup on non-PARIDE boxes
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if PARIDE = y
+
---help---
There are many external CD-ROM and disk devices that connect through
your computer's parallel port. Most of them are actually IDE devices
Index: linux/drivers/i2c/busses/Kconfig
===================================================================
--- linux.orig/drivers/i2c/busses/Kconfig
+++ linux/drivers/i2c/busses/Kconfig
@@ -610,6 +610,11 @@ config I2C_ELEKTOR
config I2C_PCA_ISA
tristate "PCA9564 on an ISA bus"
depends on ISA
+
+ # takes away IRQ10 on venus and thus breaks e1000
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT
+
select I2C_ALGOPCA
default n
help
Index: linux/drivers/ide/Kconfig
===================================================================
--- linux.orig/drivers/ide/Kconfig
+++ linux/drivers/ide/Kconfig
@@ -9,6 +9,11 @@ config HAVE_IDE
menuconfig IDE
tristate "ATA/ATAPI/MFM/RLL support"
depends on HAVE_IDE
+
+ # my test box expects /dev/sda, not /dev/hda
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if IDE = y
+
depends on BLOCK
---help---
If you say Y here, your kernel will be able to manage low cost mass
Index: linux/drivers/isdn/icn/Kconfig
===================================================================
--- linux.orig/drivers/isdn/icn/Kconfig
+++ linux/drivers/isdn/icn/Kconfig
@@ -4,6 +4,11 @@
config ISDN_DRV_ICN
tristate "ICN 2B and 4B support"
depends on ISA
+
+ # crashed on venus, see config-Sun_May_25_11_00_41_CEST_2008.bad
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT
+
help
This enables support for two kinds of ISDN-cards made by a German
company called ICN. 2B is the standard version for a single ISDN
Index: linux/drivers/media/video/Kconfig
===================================================================
--- linux.orig/drivers/media/video/Kconfig
+++ linux/drivers/media/video/Kconfig
@@ -481,6 +481,9 @@ config VIDEO_SAA6588
config VIDEO_PMS
tristate "Mediavision Pro Movie Studio Video For Linux"
depends on ISA && VIDEO_V4L1
+ # hung on bootup on mars, see config-Wed_Jun__4_14_33_56_CEST_2008.bad
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT
help
Say Y if you have such a thing.

Index: linux/drivers/mtd/Kconfig
===================================================================
--- linux.orig/drivers/mtd/Kconfig
+++ linux/drivers/mtd/Kconfig
@@ -1,6 +1,11 @@
menuconfig MTD
tristate "Memory Technology Device (MTD) support"
depends on HAS_IOMEM
+
+ # dangerous to enable - sometimes hangs on probe
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if MTD = y
+
help
Memory Technology Devices are flash, RAM and similar chips, often
used for solid state file systems on embedded devices. This option
Index: linux/drivers/net/appletalk/Kconfig
===================================================================
--- linux.orig/drivers/net/appletalk/Kconfig
+++ linux/drivers/net/appletalk/Kconfig
@@ -52,6 +52,11 @@ config LTPC
config COPS
tristate "COPS LocalTalk PC support"
depends on DEV_APPLETALK && (ISA || EISA)
+ #
+ # Can hang
+ #
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if COPS = y
help
This allows you to use COPS AppleTalk cards to connect to LocalTalk
networks. You also need version 1.3.3 or later of the netatalk
Index: linux/drivers/scsi/Kconfig
===================================================================
--- linux.orig/drivers/scsi/Kconfig
+++ linux/drivers/scsi/Kconfig
@@ -1520,6 +1520,11 @@ config SCSI_NSP32
config SCSI_DEBUG
tristate "SCSI debugging host simulator"
depends on SCSI
+
+ # this creates a fake /dev/sda which confuses the bzImage bootup
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if SCSI_DEBUG = y
+
help
This is a host adapter simulator that can simulate multiple hosts
each with multiple dummy SCSI devices (disks). It defaults to one
Index: linux/drivers/video/Kconfig
===================================================================
--- linux.orig/drivers/video/Kconfig
+++ linux/drivers/video/Kconfig
@@ -236,6 +236,11 @@ comment "Frame buffer hardware drivers"

config FB_CIRRUS
tristate "Cirrus Logic support"
+
+ # can hang on a box without this hardware
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if FB_CIRRUS = y
+
depends on FB && (ZORRO || PCI)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -546,6 +551,11 @@ config FB_CT65550

config FB_ASILIANT
bool "Asiliant (Chips) 69000 display support"
+
+ # can hang on a box without this hardware
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if FB_ASILIANT = y
+
depends on (FB = y) && PCI
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -564,6 +574,11 @@ config FB_IMSTT

config FB_VGA16
tristate "VGA 16-color graphics support"
+
+ # can hang on a box without this hardware
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if FB_VGA16 = y
+
depends on FB && (X86 || PPC)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -674,6 +689,11 @@ config FB_UVESA

config FB_VESA
bool "VESA VGA graphics support"
+
+ # can hang on a box without this hardware
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if FB_VESA = y
+
depends on (FB = y) && X86
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -1299,6 +1319,11 @@ config FB_MATROX_MULTIHEAD

config FB_RADEON
tristate "ATI Radeon display support"
+
+ # can hang on a box without this hardware
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if FB_RADEON = y
+
depends on FB && PCI
select FB_BACKLIGHT if FB_RADEON_BACKLIGHT
select FB_MODE_HELPERS
@@ -1581,6 +1606,11 @@ config FB_VT8623

config FB_CYBLA
tristate "Cyberblade/i1 support"
+
+ # can hang on a box without this hardware
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if FB_CYBLA = y
+
depends on FB && PCI && X86_32 && !64BIT
select FB_CFB_IMAGEBLIT
select VIDEO_SELECT
@@ -2006,6 +2036,11 @@ config FB_SH7760

config FB_VIRTUAL
tristate "Virtual Frame Buffer support (ONLY FOR TESTING!)"
+
+ # can hang on a box without this hardware
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if FB_VIRTUAL = y
+
depends on FB
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
Index: linux/drivers/video/console/Kconfig
===================================================================
--- linux.orig/drivers/video/console/Kconfig
+++ linux/drivers/video/console/Kconfig
@@ -61,6 +61,11 @@ config VIDEO_SELECT

config MDA_CONSOLE
depends on !M68K && !PARISC && ISA
+
+ # can hang on a box without this hardware
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if MDA_CONSOLE = y
+
tristate "MDA text console (dual-headed) (EXPERIMENTAL)"
---help---
Say Y here if you have an old MDA or monochrome Hercules graphics
@@ -113,6 +118,10 @@ config DUMMY_CONSOLE_ROWS

config FRAMEBUFFER_CONSOLE
tristate "Framebuffer Console support"
+
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if FRAMEBUFFER_CONSOLE = y
+
depends on FB
select CRC32
help
Index: linux/drivers/watchdog/Kconfig
===================================================================
--- linux.orig/drivers/watchdog/Kconfig
+++ linux/drivers/watchdog/Kconfig
@@ -324,6 +324,12 @@ config SC520_WDT
config EUROTECH_WDT
tristate "Eurotech CPU-1220/1410 Watchdog Timer"
depends on X86
+
+ # this ISA driver puts itself on IRQ10 - if IRQ10 happens to
+ # trigger then that will cause a watchdog-initiated reboot
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if EUROTECH_WDT = y
+
help
Enable support for the watchdog timer on the Eurotech CPU-1220 and
CPU-1410 cards. These are PC/104 SBCs. Spec sheets and product
@@ -830,6 +836,11 @@ config MIXCOMWD
config WDT
tristate "WDT Watchdog timer"
depends on ISA
+
+ # this ISA driver sits on IRQ11 by default - blocking forcedeth
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT if WDT = y
+
---help---
If you have a WDT500P or WDT501P watchdog board, say Y here,
otherwise N. It is not possible to probe for this board, which means
Index: linux/fs/Kconfig
===================================================================
--- linux.orig/fs/Kconfig
+++ linux/fs/Kconfig
@@ -1636,6 +1636,11 @@ config NFS_V4
config ROOT_NFS
bool "Root file system on NFS"
depends on NFS_FS=y && IP_PNP
+
+ # hangs a non-root-NFS box during bootup
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT
+
help
If you want your system to mount its root file system via NFS,
choose Y here. This is common practice for managing systems
Index: linux/security/Kconfig
===================================================================
--- linux.orig/security/Kconfig
+++ linux/security/Kconfig
@@ -85,6 +85,11 @@ config SECURITY_FILE_CAPABILITIES
config SECURITY_ROOTPLUG
bool "Root Plug Support"
depends on USB=y && SECURITY
+
+ # fails with hard-to-debug "could not find init" boot failure
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT
+
help
This is a sample LSM module that should only be used as such.
It prevents any programs running with egid == 0 if a specific
Index: linux/security/selinux/Kconfig
===================================================================
--- linux.orig/security/selinux/Kconfig
+++ linux/security/selinux/Kconfig
@@ -100,6 +100,11 @@ config SECURITY_SELINUX_CHECKREQPROT_VAL
config SECURITY_SELINUX_ENABLE_SECMARK_DEFAULT
bool "NSA SELinux enable new secmark network controls by default"
depends on SECURITY_SELINUX
+
+ # old system booted up with this cannot ssh out
+ depends on BROKEN_BOOT_ALLOWED
+ select BROKEN_BOOT
+
default n
help
This option determines whether the new secmark-based network

2008-08-01 14:26:31

by Rene Herman

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

>From 58f25a608b827426bf5c110795dd7e917e9cc568 Mon Sep 17 00:00:00 2001
From: Rene Herman <[email protected]>
Date: Fri, 1 Aug 2008 15:35:31 +0200
Subject: [PATCH] i2c: don't autograb i2c-pca-isa

Grabbing resources without anything telling us we should can break
randconfig builds by keeping them busy. Insist that when hardware
is not capable of telling us, the user does.

Signed-off-by: Rene Herman <[email protected]>
---
drivers/i2c/busses/i2c-pca-isa.c | 20 +++++++++++++++++---
1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pca-isa.c b/drivers/i2c/busses/i2c-pca-isa.c
index a119784..2579169 100644
--- a/drivers/i2c/busses/i2c-pca-isa.c
+++ b/drivers/i2c/busses/i2c-pca-isa.c
@@ -36,8 +36,8 @@
#define DRIVER "i2c-pca-isa"
#define IO_SIZE 4

-static unsigned long base = 0x330;
-static int irq = 10;
+static unsigned long base;
+static int irq = -1;

/* Data sheet recommends 59kHz for 100kHz operation due to variation
* in the actual clock rate */
@@ -107,6 +107,19 @@ static struct i2c_adapter pca_isa_ops = {
.timeout = 100,
};

+static int __devinit pca_isa_match(struct device *dev, unsigned int id)
+{
+ int match = base != 0;
+
+ if (match) {
+ if (irq == -1)
+ dev_warn(dev, "using poling mode (specify irq)\n");
+ } else
+ dev_err(dev, "please specify base\n");
+
+ return match;
+}
+
static int __devinit pca_isa_probe(struct device *dev, unsigned int id)
{
init_waitqueue_head(&pca_wait);
@@ -153,7 +166,7 @@ static int __devexit pca_isa_remove(struct device *dev, unsigned int id)
{
i2c_del_adapter(&pca_isa_ops);

- if (irq > 0) {
+ if (irq > -1) {
disable_irq(irq);
free_irq(irq, &pca_isa_ops);
}
@@ -163,6 +176,7 @@ static int __devexit pca_isa_remove(struct device *dev, unsigned int id)
}

static struct isa_driver pca_isa_driver = {
+ .match = pca_isa_match,
.probe = pca_isa_probe,
.remove = __devexit_p(pca_isa_remove),
.driver = {
--
1.5.5


Attachments:
0001-i2c-don-t-autograb-i2c-pca-isa.patch (1.93 kB)

2008-08-01 14:49:19

by Andi Kleen

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

On Fri, Aug 01, 2008 at 04:27:53PM +0200, Rene Herman wrote:
> On 01-08-08 11:51, Ingo Molnar wrote:
>
> >find attached below a newer version of the original list i published
> >half a year ago:
> >
> > http://people.redhat.com/mingo/auto-qa-patches/Kconfig-qa.patch
> >
> >these are just pragmatic local hacks to get things going. (There are
> >more per machine quirks as well.)
> >
> >i have not used a BROKEN annotation because CONFIG_BROKEN is
> >impractical: it just kills code altogether, indiscriminately. There's no
> >way for users to enable CONFIG_BROKEN in the upstream kernel - nothing
> >selects it and it's not an interactive option either.
> >
> >So by all means if we mark a driver or a kernel feature as
> >CONFIG_BROKEN, it's killed altogether for all practical purposes.
> >
> >What we'd need is some more gradual approach: for example a way to mark
> >"drivers that are not expected to boot on a whitebox PC", without
> >removing them altogether via a CONFIG_BROKEN dependency - often it's
> >hardware that cannot be probed safely.
>
> For real ISA at the least, the best fix I feel is just not go grabbing
> resources without anything (ie, ISAPnP) or anyone (ie, the user) telling

Really old systems don't have isapnp. Neither have smbus devices.

> us that's where the hardware's at.

That would mean the users on these systems would need to add tons
of obscure command line options.

-Andi

2008-08-01 15:17:19

by Rene Herman

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

On 01-08-08 16:49, Andi Kleen wrote:

>> For real ISA at the least, the best fix I feel is just not go grabbing
>> resources without anything (ie, ISAPnP) or anyone (ie, the user) telling
>
> Really old systems don't have isapnp. Neither have smbus devices.
>
>> us that's where the hardware's at.
>
> That would mean the users on these systems would need to add tons
> of obscure command line options.

No, it means users on those 1% of systems that are using drivers that
break the other 99% would have to for those one or two drivers that do.

And as said, we don't care.

Rene.

2008-08-01 15:44:43

by Andi Kleen

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

On Fri, Aug 01, 2008 at 05:19:17PM +0200, Rene Herman wrote:
> On 01-08-08 16:49, Andi Kleen wrote:
>
> >>For real ISA at the least, the best fix I feel is just not go grabbing
> >>resources without anything (ie, ISAPnP) or anyone (ie, the user) telling
> >
> >Really old systems don't have isapnp. Neither have smbus devices.
> >
> >>us that's where the hardware's at.
> >
> >That would mean the users on these systems would need to add tons
> >of obscure command line options.
>
> No, it means users on those 1% of systems that are using drivers that
> break the other 99% would have to for those one or two drivers that do.
>
> And as said, we don't care.

Not sure who "we" is in this context.

Especially dropping easy support for all smbus devices (which are often like
this) would seem quite wrong to me. And for real ISA devices there tends to
be a surprisingly large user base left for those.

The config option Ingo proposed is a good idea though.

Or they can just use 64bit which never defines CONFIG_ISA, but of course
still has to support all the similar smbus drivers.
-Andi

2008-08-01 16:01:19

by Rene Herman

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support

On 01-08-08 17:44, Andi Kleen wrote:
> On Fri, Aug 01, 2008 at 05:19:17PM +0200, Rene Herman wrote:
>> On 01-08-08 16:49, Andi Kleen wrote:
>>
>>>> For real ISA at the least, the best fix I feel is just not go grabbing
>>>> resources without anything (ie, ISAPnP) or anyone (ie, the user) telling
>>> Really old systems don't have isapnp. Neither have smbus devices.
>>>
>>>> us that's where the hardware's at.
>>> That would mean the users on these systems would need to add tons
>>> of obscure command line options.
>> No, it means users on those 1% of systems that are using drivers that
>> break the other 99% would have to for those one or two drivers that do.
>>
>> And as said, we don't care.
>
> Not sure who "we" is in this context.

We, the old crap hardware users the drivers for which break sensible
systems by default.

Really, look at the patch I posted -- it's a patch to make passing in a
base (and irq if you want one) to i2c-pca-isa needed. Right now, the
thing just grabs those resources and breaks the boot on systems that
don't have it as per Ingo's testing.

On plain technical grounds mucking with resources you don't own is bad
(on those old systems, 0x330 might even be a NE2000 which gets confused
really quickly) and even more importantly; let's take a wild guess and
assume that that specific ISA driver has, oh let's say, less than 3
users in the world all of whom wouldn't care one bit about sticking in
an options line in their modprobe.conf.

There is no reason drivers like that should be allowed to break all the
_other_ systems instead of making those users do that. Not a single one.

> Especially dropping easy support for all smbus devices (which are often like
> this) would seem quite wrong to me. And for real ISA devices there tends to
> be a surprisingly large user base left for those.

Fortunately, smbus devices tend to not grab IRQs and go poking around
the general I/O space willy-nilly (they go poking around the smbus I/O
space willy-nilly). Note again, this specific case is a _bus_ driver for
a bus on an ISA card.

Rene

2008-08-13 11:01:29

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch 0/9] x86, xsave: xsave/xrstor support


* Ingo Molnar <[email protected]> wrote:

> > Anyhow, with watchdog removed, it works just fine. (both xsave and
> > non-xsave kernels)
>
> Great, thanks. I've re-integrated tip/x86/xsave into x86/master and
> have pushed out the result.

FYI, small tip/x86/xsave build fixlet below - user-space exposed
interfaces need to use __u64 not u64.

Ingo

-------------->
>From 26d809af6397ce5c37f5c44d89734d19cce1ad25 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Wed, 13 Aug 2008 12:46:26 +0200
Subject: [PATCH] x86: fix xsave build error
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

fix this build failure with certain glibc versions:

In file included from /usr/include/bits/sigcontext.h:28,
from /usr/include/signal.h:333,
from Documentation/accounting/getdelays.c:24:
/home/mingo/tip/usr/include/asm/sigcontext.h:191: error: expected specifier-qualifier-list before ‘u64’

Signed-off-by: Ingo Molnar <[email protected]>
---
include/asm-x86/sigcontext.h | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/asm-x86/sigcontext.h b/include/asm-x86/sigcontext.h
index 899fe2f..ee813f4 100644
--- a/include/asm-x86/sigcontext.h
+++ b/include/asm-x86/sigcontext.h
@@ -264,9 +264,9 @@ struct sigcontext {
#endif /* !__i386__ */

struct _xsave_hdr {
- u64 xstate_bv;
- u64 reserved1[2];
- u64 reserved2[5];
+ __u64 xstate_bv;
+ __u64 reserved1[2];
+ __u64 reserved2[5];
};

/*