This patch is similar to Theordore Ts'o's TAINT_USER patch,
linux-2.6 commit 34f5a39899f3f3e815da64f48ddb72942d86c366.
Individual distributions may enable "generic" features such as X86 support,
PPC support, and driver support.
Some of the features that are enabled by these "generic" feature flags may
not be considered supported by the individual distribution.
For example, a distribution may want to support PPC but not the Power5
chipset, or the e1000e driver but not a card with a specific DeviceID because
of known firmware issues.
Typically, one would push a config patch to enable and disable the feature and
patch the distribution. However, in some cases this is not feasible in order
to preserve kabi and at the same time maintain parity with the upstream kernel.
In some cases the distribution may want to allow booting of these features but
explicitly notify a user that they are not "officially" supported. It is also
possible that the hardware is fixed via a firmware update at a later date,
making it supported again.
It would be useful for a distribution to notify the installer and
bug reporting applications, and notify users that the hardware they are using
is unsupported during panic, oops, BUG(), and WARN().
This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions
to use.
Signed-off-by: Prarit Bhargava <[email protected]>
Signed-off-by: Don Zickus <[email protected]>
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 8317ec4..f722b0d 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -347,6 +347,7 @@ extern enum system_states {
#define TAINT_WARN 9
#define TAINT_CRAP 10
#define TAINT_FIRMWARE_WORKAROUND 11
+#define TAINT_HARDWARE_UNSUPPORTED 12
extern void dump_stack(void) __cold;
diff --git a/kernel/panic.c b/kernel/panic.c
index 3b16cd9..394a5bb 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -180,6 +180,7 @@ static const struct tnt tnts[] = {
{ TAINT_WARN, 'W', ' ' },
{ TAINT_CRAP, 'C', ' ' },
{ TAINT_FIRMWARE_WORKAROUND, 'I', ' ' },
+ { TAINT_HARDWARE_UNSUPPORTED, 'H', ' ' },
};
/**
@@ -197,6 +198,7 @@ static const struct tnt tnts[] = {
* 'W' - Taint on warning.
* 'C' - modules from drivers/staging are loaded.
* 'I' - Working around severe firmware bug.
+ * 'H' - Hardware is unsupported.
*
* The string is overwritten by the next call to print_tainted().
*/
@@ -243,6 +245,9 @@ void add_taint(unsigned flag)
*/
if (flag != TAINT_CRAP && flag != TAINT_WARN && __debug_locks_off())
printk(KERN_WARNING "Disabling lock debugging due to kernel taint\n");
+ if (flag == TAINT_HARDWARE_UNSUPPORTED)
+ printk(KERN_CRIT "WARNING: This system's hardware is "
+ "unsupported.\n");
set_bit(flag, &tainted_mask);
}
On Thu, 17 Jun 2010 09:54:45 -0400 Prarit Bhargava wrote:
> This patch is similar to Theordore Ts'o's TAINT_USER patch,
> linux-2.6 commit 34f5a39899f3f3e815da64f48ddb72942d86c366.
and augmented by 92946bc72f2e74c3281b7fc12be9704d455fb3ed, so please
add similar info to Documentation/oops-tracing.txt.
> This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions
> to use.
>
> Signed-off-by: Prarit Bhargava <[email protected]>
> Signed-off-by: Don Zickus <[email protected]>
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 8317ec4..f722b0d 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -347,6 +347,7 @@ extern enum system_states {
> #define TAINT_WARN 9
> #define TAINT_CRAP 10
> #define TAINT_FIRMWARE_WORKAROUND 11
> +#define TAINT_HARDWARE_UNSUPPORTED 12
>
> extern void dump_stack(void) __cold;
>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 3b16cd9..394a5bb 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -180,6 +180,7 @@ static const struct tnt tnts[] = {
> { TAINT_WARN, 'W', ' ' },
> { TAINT_CRAP, 'C', ' ' },
> { TAINT_FIRMWARE_WORKAROUND, 'I', ' ' },
> + { TAINT_HARDWARE_UNSUPPORTED, 'H', ' ' },
> };
>
> /**
> @@ -197,6 +198,7 @@ static const struct tnt tnts[] = {
> * 'W' - Taint on warning.
> * 'C' - modules from drivers/staging are loaded.
> * 'I' - Working around severe firmware bug.
> + * 'H' - Hardware is unsupported.
> *
> * The string is overwritten by the next call to print_tainted().
> */
> @@ -243,6 +245,9 @@ void add_taint(unsigned flag)
> */
> if (flag != TAINT_CRAP && flag != TAINT_WARN && __debug_locks_off())
> printk(KERN_WARNING "Disabling lock debugging due to kernel taint\n");
> + if (flag == TAINT_HARDWARE_UNSUPPORTED)
> + printk(KERN_CRIT "WARNING: This system's hardware is "
> + "unsupported.\n");
Preferable not to split the string, so more like:
printk(KERN_CRIT
"WARNING: This system's hardware is unsupported.\n");
>
> set_bit(flag, &tainted_mask);
> }
> --
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
On 06/17/2010 12:13 PM, Randy Dunlap wrote:
> On Thu, 17 Jun 2010 09:54:45 -0400 Prarit Bhargava wrote:
>
>> This patch is similar to Theordore Ts'o's TAINT_USER patch,
>> linux-2.6 commit 34f5a39899f3f3e815da64f48ddb72942d86c366.
>
> and augmented by 92946bc72f2e74c3281b7fc12be9704d455fb3ed, so please
> add similar info to Documentation/oops-tracing.txt.
>
>
>> This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions
>> to use.
<snip>
>> + "unsupported.\n");
>
> Preferable not to split the string, so more like:
>
> printk(KERN_CRIT
> "WARNING: This system's hardware is unsupported.\n");
>>
>> set_bit(flag, &tainted_mask);
Hey Randy, thanks for the input. New patch.
This patch is similar to Theordore Ts'o's TAINT_USER patch,
linux-2.6 commit 34f5a39899f3f3e815da64f48ddb72942d86c366.
Individual distributions may enable "generic" features such as X86 support,
PPC support, and driver support.
Some of the features that are enabled by these "generic" feature flags may
not be considered supported by the individual distribution.
For example, a distribution may want to support PPC but not the Power5
chipset, or the e1000e driver but not a card with a specific DeviceID because
of known firmware issues.
Typically, one would push a config patch to enable and disable the feature and
patch the distribution. However, in some cases this is not feasible in order
to preserve kabi and at the same time maintain parity with the upstream kernel.
In some cases the distribution may want to allow booting of these features but
explicitly notify a user that they are not "officially" supported. It is also
possible that the hardware is fixed via a firmware update at a later date,
making it supported again.
It would be useful for a distribution to notify the installer and
bug reporting applications, and notify users that the hardware they are using
is unsupported during panic, oops, BUG(), and WARN().
This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions
to use.
Signed-off-by: Prarit Bhargava <[email protected]>
Signed-off-by: Don Zickus <[email protected]>
diff --git a/Documentation/oops-tracing.txt b/Documentation/oops-tracing.txt
index 6fe9001..e337b0a 100644
--- a/Documentation/oops-tracing.txt
+++ b/Documentation/oops-tracing.txt
@@ -263,6 +263,8 @@ characters, each representing a particular tainted value.
12: 'I' if the kernel is working around a severe bug in the platform
firmware (BIOS or similar).
+ 13: 'H' if the hardware is unsupported by the distribution
+
The primary reason for the 'Tainted: ' string is to tell kernel
debuggers if this is a clean kernel or if anything unusual has
occurred. Tainting is permanent: even if an offending module is
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 8317ec4..f722b0d 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -347,6 +347,7 @@ extern enum system_states {
#define TAINT_WARN 9
#define TAINT_CRAP 10
#define TAINT_FIRMWARE_WORKAROUND 11
+#define TAINT_HARDWARE_UNSUPPORTED 12
extern void dump_stack(void) __cold;
diff --git a/kernel/panic.c b/kernel/panic.c
index 3b16cd9..8d081ff 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -180,6 +180,7 @@ static const struct tnt tnts[] = {
{ TAINT_WARN, 'W', ' ' },
{ TAINT_CRAP, 'C', ' ' },
{ TAINT_FIRMWARE_WORKAROUND, 'I', ' ' },
+ { TAINT_HARDWARE_UNSUPPORTED, 'H', ' ' },
};
/**
@@ -197,6 +198,7 @@ static const struct tnt tnts[] = {
* 'W' - Taint on warning.
* 'C' - modules from drivers/staging are loaded.
* 'I' - Working around severe firmware bug.
+ * 'H' - Hardware is unsupported.
*
* The string is overwritten by the next call to print_tainted().
*/
@@ -243,6 +245,9 @@ void add_taint(unsigned flag)
*/
if (flag != TAINT_CRAP && flag != TAINT_WARN && __debug_locks_off())
printk(KERN_WARNING "Disabling lock debugging due to kernel taint\n");
+ if (flag == TAINT_HARDWARE_UNSUPPORTED)
+ printk(KERN_CRIT
+ "WARNING: This system's hardware is unsupported.\n");
set_bit(flag, &tainted_mask);
}
On Thu, 17 Jun 2010 15:54:09 -0400 Prarit Bhargava wrote:
> This patch is similar to Theordore Ts'o's TAINT_USER patch,
> linux-2.6 commit 34f5a39899f3f3e815da64f48ddb72942d86c366.
>
> Individual distributions may enable "generic" features such as X86 support,
> PPC support, and driver support.
>
> Some of the features that are enabled by these "generic" feature flags may
> not be considered supported by the individual distribution.
>
> For example, a distribution may want to support PPC but not the Power5
> chipset, or the e1000e driver but not a card with a specific DeviceID because
> of known firmware issues.
>
> Typically, one would push a config patch to enable and disable the feature and
> patch the distribution. However, in some cases this is not feasible in order
> to preserve kabi and at the same time maintain parity with the upstream kernel.
> In some cases the distribution may want to allow booting of these features but
> explicitly notify a user that they are not "officially" supported. It is also
> possible that the hardware is fixed via a firmware update at a later date,
> making it supported again.
>
> It would be useful for a distribution to notify the installer and
> bug reporting applications, and notify users that the hardware they are using
> is unsupported during panic, oops, BUG(), and WARN().
>
> This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions
> to use.
>
> Signed-off-by: Prarit Bhargava <[email protected]>
> Signed-off-by: Don Zickus <[email protected]>
Acked-by: Randy Dunlap <[email protected]>
Thanks.
> diff --git a/Documentation/oops-tracing.txt b/Documentation/oops-tracing.txt
> index 6fe9001..e337b0a 100644
> --- a/Documentation/oops-tracing.txt
> +++ b/Documentation/oops-tracing.txt
> @@ -263,6 +263,8 @@ characters, each representing a particular tainted value.
> 12: 'I' if the kernel is working around a severe bug in the platform
> firmware (BIOS or similar).
>
> + 13: 'H' if the hardware is unsupported by the distribution
> +
> The primary reason for the 'Tainted: ' string is to tell kernel
> debuggers if this is a clean kernel or if anything unusual has
> occurred. Tainting is permanent: even if an offending module is
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 8317ec4..f722b0d 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -347,6 +347,7 @@ extern enum system_states {
> #define TAINT_WARN 9
> #define TAINT_CRAP 10
> #define TAINT_FIRMWARE_WORKAROUND 11
> +#define TAINT_HARDWARE_UNSUPPORTED 12
>
> extern void dump_stack(void) __cold;
>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 3b16cd9..8d081ff 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -180,6 +180,7 @@ static const struct tnt tnts[] = {
> { TAINT_WARN, 'W', ' ' },
> { TAINT_CRAP, 'C', ' ' },
> { TAINT_FIRMWARE_WORKAROUND, 'I', ' ' },
> + { TAINT_HARDWARE_UNSUPPORTED, 'H', ' ' },
> };
>
> /**
> @@ -197,6 +198,7 @@ static const struct tnt tnts[] = {
> * 'W' - Taint on warning.
> * 'C' - modules from drivers/staging are loaded.
> * 'I' - Working around severe firmware bug.
> + * 'H' - Hardware is unsupported.
> *
> * The string is overwritten by the next call to print_tainted().
> */
> @@ -243,6 +245,9 @@ void add_taint(unsigned flag)
> */
> if (flag != TAINT_CRAP && flag != TAINT_WARN && __debug_locks_off())
> printk(KERN_WARNING "Disabling lock debugging due to kernel taint\n");
> + if (flag == TAINT_HARDWARE_UNSUPPORTED)
> + printk(KERN_CRIT
> + "WARNING: This system's hardware is unsupported.\n");
>
> set_bit(flag, &tainted_mask);
> }
> --
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Prarit Bhargava <[email protected]> writes:
nt\n");
> + if (flag == TAINT_HARDWARE_UNSUPPORTED)
> + printk(KERN_CRIT "WARNING: This system's hardware is "
> + "unsupported.\n");
The problem with such a message in mainline is that it would
imply that other hardware is actually 'supported'
Which is not really true in most cases.
-Andi
--
[email protected] -- Speaking for myself only.
> Typically, one would push a config patch to enable and disable the feature and
> patch the distribution. However, in some cases this is not feasible in order
If you can push a patch to set the flag you can push a patch to panic or
reject that combination.
Devil's advocate time:
Also the fact some distributions chose a binary compatible interface for
their internal modules was their choice. It is one that has been
repeatedly rejected by upstream and at kernel summit.
So given we fundamnetally reject your approach why should we carry your
flag ?
> In some cases the distribution may want to allow booting of these features but
> explicitly notify a user that they are not "officially" supported. It is also
We have printk. You can add a module of your own which indicates
'support' status too.
> possible that the hardware is fixed via a firmware update at a later date,
> making it supported again.
IMHO it's not properly named in the first place. You are talking about
combinations of hardware/firmware and you actually mean 'configuration
not supported' ?
> This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions
> to use.
and why KERN_CRIT when the other printk's don't use that level ?
A suggestion: instead of all this push a single patch with a comment and
maybe defines indicating that taint flag bits 28-31 are 'reserved' for
experimental and out of tree applications.
That way anyone who has a requirement like yours can deal with it and
nobody has to worry about bit collisions, naming and the like. Nor if you
suddenely need an extra bit in 3 years time are you going to come unstuck
on your KABI. That will help other people doing experiments with taint or
with differing needs to the Red Hat one.
Alan
On Sat, Jun 19, 2010 at 10:30:56AM +0100, Alan Cox wrote:
> > Typically, one would push a config patch to enable and disable the feature and
> > patch the distribution. However, in some cases this is not feasible in order
>
> If you can push a patch to set the flag you can push a patch to panic or
> reject that combination.
We could, but we didn't think mainline would interested in restricting
what they support based on RHEL's needs. Also we still wanted to retain
the ability to use a piece of hardware even though we do not officially
support it, thus we were trying to avoid the panic and just set a flag
instead.
>
> Devil's advocate time:
>
> Also the fact some distributions chose a binary compatible interface for
> their internal modules was their choice. It is one that has been
> repeatedly rejected by upstream and at kernel summit.
>
> So given we fundamnetally reject your approach why should we carry your
> flag ?
I'm confused, this has nothing to do with KABI. It is just a flag that
something like the installer or a system report could look at to inform
users they are running on a system that may not be supported.
Again, we thought this might be useful for other distros as well who want
to make it easier to filter through bugzillas as close out bugs that has
this flag set.
>
> > In some cases the distribution may want to allow booting of these features but
> > explicitly notify a user that they are not "officially" supported. It is also
>
> We have printk. You can add a module of your own which indicates
> 'support' status too.
>
> > possible that the hardware is fixed via a firmware update at a later date,
> > making it supported again.
>
> IMHO it's not properly named in the first place. You are talking about
> combinations of hardware/firmware and you actually mean 'configuration
> not supported' ?
Mainly hardware platforms that do not necessarily have a config option
wrapped around them.
>
> > This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions
> > to use.
>
> and why KERN_CRIT when the other printk's don't use that level ?
The thinking was that we wanted to make sure the end user saw the
message, but we can set it to any level really.
>
> A suggestion: instead of all this push a single patch with a comment and
> maybe defines indicating that taint flag bits 28-31 are 'reserved' for
> experimental and out of tree applications.
>
> That way anyone who has a requirement like yours can deal with it and
> nobody has to worry about bit collisions, naming and the like. Nor if you
> suddenely need an extra bit in 3 years time are you going to come unstuck
> on your KABI. That will help other people doing experiments with taint or
> with differing needs to the Red Hat one.
That seems like a reasonable request and basically covers one of the
reasons for pushing this patch.
Thanks for the review.
Cheers,
Don
On Thu, 17 Jun 2010 15:54:09 -0400
Prarit Bhargava <[email protected]> wrote:
>
> This patch is similar to Theordore Ts'o's TAINT_USER patch,
> linux-2.6 commit 34f5a39899f3f3e815da64f48ddb72942d86c366.
>
> Individual distributions may enable "generic" features such as X86 support,
> PPC support, and driver support.
>
> Some of the features that are enabled by these "generic" feature flags may
> not be considered supported by the individual distribution.
>
> For example, a distribution may want to support PPC but not the Power5
> chipset, or the e1000e driver but not a card with a specific DeviceID because
> of known firmware issues.
>
> Typically, one would push a config patch to enable and disable the feature and
> patch the distribution. However, in some cases this is not feasible in order
> to preserve kabi and at the same time maintain parity with the upstream kernel.
> In some cases the distribution may want to allow booting of these features but
> explicitly notify a user that they are not "officially" supported. It is also
> possible that the hardware is fixed via a firmware update at a later date,
> making it supported again.
>
> It would be useful for a distribution to notify the installer and
> bug reporting applications, and notify users that the hardware they are using
> is unsupported during panic, oops, BUG(), and WARN().
>
> This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions
> to use.
>
> Signed-off-by: Prarit Bhargava <[email protected]>
> Signed-off-by: Don Zickus <[email protected]>
>
> diff --git a/Documentation/oops-tracing.txt b/Documentation/oops-tracing.txt
> index 6fe9001..e337b0a 100644
> --- a/Documentation/oops-tracing.txt
> +++ b/Documentation/oops-tracing.txt
> @@ -263,6 +263,8 @@ characters, each representing a particular tainted value.
> 12: 'I' if the kernel is working around a severe bug in the platform
> firmware (BIOS or similar).
>
> + 13: 'H' if the hardware is unsupported by the distribution
> +
> The primary reason for the 'Tainted: ' string is to tell kernel
> debuggers if this is a clean kernel or if anything unusual has
> occurred. Tainting is permanent: even if an offending module is
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 8317ec4..f722b0d 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -347,6 +347,7 @@ extern enum system_states {
> #define TAINT_WARN 9
> #define TAINT_CRAP 10
> #define TAINT_FIRMWARE_WORKAROUND 11
> +#define TAINT_HARDWARE_UNSUPPORTED 12
>
> extern void dump_stack(void) __cold;
>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 3b16cd9..8d081ff 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -180,6 +180,7 @@ static const struct tnt tnts[] = {
> { TAINT_WARN, 'W', ' ' },
> { TAINT_CRAP, 'C', ' ' },
> { TAINT_FIRMWARE_WORKAROUND, 'I', ' ' },
> + { TAINT_HARDWARE_UNSUPPORTED, 'H', ' ' },
> };
>
> /**
> @@ -197,6 +198,7 @@ static const struct tnt tnts[] = {
> * 'W' - Taint on warning.
> * 'C' - modules from drivers/staging are loaded.
> * 'I' - Working around severe firmware bug.
> + * 'H' - Hardware is unsupported.
> *
> * The string is overwritten by the next call to print_tainted().
> */
> @@ -243,6 +245,9 @@ void add_taint(unsigned flag)
> */
> if (flag != TAINT_CRAP && flag != TAINT_WARN && __debug_locks_off())
> printk(KERN_WARNING "Disabling lock debugging due to kernel taint\n");
> + if (flag == TAINT_HARDWARE_UNSUPPORTED)
> + printk(KERN_CRIT
> + "WARNING: This system's hardware is unsupported.\n");
>
> set_bit(flag, &tainted_mask);
> }
That's pretty user-hostile. What are they to do - throw the entire
computer away because it has the wrong type of mouse?
How about
void add_hardware_unsupported_taint(const char *hardware)
{
printk(KERN_CRIT
"Hardware device %s is unsupported by this kernel's vendor\n",
hardware);
add_taint(TAINT_HARDWARE_UNSUPPORTED);
}
and
/*
* Don't call add_taint(TAINT_HARDWARE_UNSUPPORTED) directly - use
* add_hardware_unsupported_taint()
*/
#define TAINT_HARDWARE_UNSUPPORTED 12
On Mon, Jun 21, 2010 at 12:21:45PM -0700, Andrew Morton wrote:
> > @@ -243,6 +245,9 @@ void add_taint(unsigned flag)
> > */
> > if (flag != TAINT_CRAP && flag != TAINT_WARN && __debug_locks_off())
> > printk(KERN_WARNING "Disabling lock debugging due to kernel taint\n");
> > + if (flag == TAINT_HARDWARE_UNSUPPORTED)
> > + printk(KERN_CRIT
> > + "WARNING: This system's hardware is unsupported.\n");
> >
> > set_bit(flag, &tainted_mask);
> > }
>
> That's pretty user-hostile. What are they to do - throw the entire
> computer away because it has the wrong type of mouse?
>
> How about
>
> void add_hardware_unsupported_taint(const char *hardware)
> {
> printk(KERN_CRIT
> "Hardware device %s is unsupported by this kernel's vendor\n",
> hardware);
> add_taint(TAINT_HARDWARE_UNSUPPORTED);
> }
>
> and
>
> /*
> * Don't call add_taint(TAINT_HARDWARE_UNSUPPORTED) directly - use
> * add_hardware_unsupported_taint()
> */
> #define TAINT_HARDWARE_UNSUPPORTED 12
Internally here that is how we planned on using the flag. We weren't sure
if upstream would accept that piece or not, so we started with the flag
for now to at least reserve the bit.
But I can see your point about the other printk being a little
user-hostile if someone forgot to call the wrapper function.
I can respin with the wrapper function (Prarit is on vacation) or go with
Alan's idea of just reserving the upper few bits.
Thoughts?
Cheers,
Don
On Mon, 21 Jun 2010 15:45:59 -0400
Don Zickus <[email protected]> wrote:
> On Mon, Jun 21, 2010 at 12:21:45PM -0700, Andrew Morton wrote:
> > > @@ -243,6 +245,9 @@ void add_taint(unsigned flag)
> > > */
> > > if (flag != TAINT_CRAP && flag != TAINT_WARN && __debug_locks_off())
> > > printk(KERN_WARNING "Disabling lock debugging due to kernel taint\n");
> > > + if (flag == TAINT_HARDWARE_UNSUPPORTED)
> > > + printk(KERN_CRIT
> > > + "WARNING: This system's hardware is unsupported.\n");
> > >
> > > set_bit(flag, &tainted_mask);
> > > }
> >
> > That's pretty user-hostile. What are they to do - throw the entire
> > computer away because it has the wrong type of mouse?
> >
> > How about
> >
> > void add_hardware_unsupported_taint(const char *hardware)
> > {
> > printk(KERN_CRIT
> > "Hardware device %s is unsupported by this kernel's vendor\n",
> > hardware);
> > add_taint(TAINT_HARDWARE_UNSUPPORTED);
> > }
> >
> > and
> >
> > /*
> > * Don't call add_taint(TAINT_HARDWARE_UNSUPPORTED) directly - use
> > * add_hardware_unsupported_taint()
> > */
> > #define TAINT_HARDWARE_UNSUPPORTED 12
>
> Internally here that is how we planned on using the flag. We weren't sure
> if upstream would accept that piece or not, so we started with the flag
> for now to at least reserve the bit.
>
> But I can see your point about the other printk being a little
> user-hostile if someone forgot to call the wrapper function.
>
> I can respin with the wrapper function (Prarit is on vacation)
Not telling the user _which_ hardware is causing the problem is daft.
> or go with
> Alan's idea of just reserving the upper few bits.
um. Given that the vendor needs to patch the individual drivers to
indicate ther unsupported status, why not just patch in the taint
changes as well?
On Mon, Jun 21, 2010 at 01:00:08PM -0700, Andrew Morton wrote:
> > Internally here that is how we planned on using the flag. We weren't sure
> > if upstream would accept that piece or not, so we started with the flag
> > for now to at least reserve the bit.
> >
> > But I can see your point about the other printk being a little
> > user-hostile if someone forgot to call the wrapper function.
> >
> > I can respin with the wrapper function (Prarit is on vacation)
>
> Not telling the user _which_ hardware is causing the problem is daft.
>
> > or go with
> > Alan's idea of just reserving the upper few bits.
>
> um. Given that the vendor needs to patch the individual drivers to
> indicate ther unsupported status, why not just patch in the taint
> changes as well?
Well we wanted to secure a bit flag first, so we don't bite ourselves later and
have to change the userspace tools.
Cheers,
Don
This patch is similar to Theordore Ts'o's TAINT_USER patch,
linux-2.6 commit 34f5a39899f3f3e815da64f48ddb72942d86c366.
Individual distributions may enable "generic" features such as X86 support,
PPC support, and driver support.
Some of the features that are enabled by these "generic" feature flags may
not be considered supported by the individual distribution.
For example, a distribution may want to support PPC but not the Power5
chipset, or the e1000e driver but not a card with a specific DeviceID because
of known firmware issues.
Typically, one would push a config patch to enable and disable the feature and
patch the distribution. However, in some cases this is not feasible in order
to preserve kabi and at the same time maintain parity with the upstream kernel.
In some cases the distribution may want to allow booting of these features but
explicitly notify a user that they are not "officially" supported. It is also
possible that the hardware is fixed via a firmware update at a later date,
making it supported again.
It would be useful for a distribution to notify the installer and
bug reporting applications, and notify users that the hardware they are using
is unsupported during panic, oops, BUG(), and WARN().
This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions
to use.
V2: clean up documentation
V3: add wrapper function around tainting unsupported function
Signed-off-by: Prarit Bhargava <[email protected]>
Signed-off-by: Don Zickus <[email protected]>
---
This includes a wrapper function we were preparing to use internally. We
weren't sure if people would frown upon adding a function with no callers
upstream, so we initially held off on it and just submitted the flag.
Another alternative is I can just submit a patch that just reserves
bits for vendor specific uses, if that is more favorable.
Cheers,
Don
diff --git a/Documentation/oops-tracing.txt b/Documentation/oops-tracing.txt
index 6fe9001..e337b0a 100644
--- a/Documentation/oops-tracing.txt
+++ b/Documentation/oops-tracing.txt
@@ -263,6 +263,8 @@ characters, each representing a particular tainted value.
12: 'I' if the kernel is working around a severe bug in the platform
firmware (BIOS or similar).
+ 13: 'H' if the hardware is unsupported by the distribution
+
The primary reason for the 'Tainted: ' string is to tell kernel
debuggers if this is a clean kernel or if anything unusual has
occurred. Tainting is permanent: even if an offending module is
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index cc5e3ff..422b3f4 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -347,6 +347,11 @@ extern enum system_states {
#define TAINT_WARN 9
#define TAINT_CRAP 10
#define TAINT_FIRMWARE_WORKAROUND 11
+/*
+ * Don't call add_taint(TAINT_HARDWARE_UNSUPPORTED) directly - use
+ * mark_hardware_unsupported()
+ */
+#define TAINT_HARDWARE_UNSUPPORTED 12
extern void dump_stack(void) __cold;
@@ -781,4 +786,5 @@ struct sysinfo {
# define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
#endif
+extern void mark_hardware_unsupported(char *msg);
#endif
diff --git a/kernel/panic.c b/kernel/panic.c
index dbe13db..90f87c7 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -179,6 +179,7 @@ static const struct tnt tnts[] = {
{ TAINT_WARN, 'W', ' ' },
{ TAINT_CRAP, 'C', ' ' },
{ TAINT_FIRMWARE_WORKAROUND, 'I', ' ' },
+ { TAINT_HARDWARE_UNSUPPORTED, 'H', ' ' },
};
/**
@@ -196,6 +197,7 @@ static const struct tnt tnts[] = {
* 'W' - Taint on warning.
* 'C' - modules from drivers/staging are loaded.
* 'I' - Working around severe firmware bug.
+ * 'H' - Hardware is unsupported.
*
* The string is overwritten by the next call to print_tainted().
*/
@@ -247,6 +249,15 @@ void add_taint(unsigned flag)
}
EXPORT_SYMBOL(add_taint);
+void mark_hardware_unsupported(char *msg)
+{
+ printk(KERN_CRIT "UNSUPPORTED HARDWARE DEVICE: %s\n", msg);
+ WARN_TAINT(1, TAINT_HARDWARE_UNSUPPORTED,
+ "Your hardware is unsupported. Please do not report "
+ "bugs, panics, oopses, etc., on this hardware.\n");
+}
+EXPORT_SYMBOL(mark_hardware_unsupported);
+
static void spin_msec(int msecs)
{
int i;
On Tue, 22 Jun 2010 11:34:57 -0400
Don Zickus <[email protected]> wrote:
> This patch is similar to Theordore Ts'o's TAINT_USER patch,
> linux-2.6 commit 34f5a39899f3f3e815da64f48ddb72942d86c366.
This seems to have gone backwards
> This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions
> to use.
As I said before - just mark some as 'reserved'
Giving it a specific meaning means it can't freely be used for many
things by many distributions
> + printk(KERN_CRIT "UNSUPPORTED HARDWARE DEVICE: %s\n", msg);
> + WARN_TAINT(1, TAINT_HARDWARE_UNSUPPORTED,
> + "Your hardware is unsupported. Please do not report "
> + "bugs, panics, oopses, etc., on this hardware.\n");
And as already pointed out it's not unsupported hardware even in the case
you described
NAK
On Tue, Jun 22, 2010 at 04:48:35PM +0100, Alan Cox wrote:
> > + printk(KERN_CRIT "UNSUPPORTED HARDWARE DEVICE: %s\n", msg);
> > + WARN_TAINT(1, TAINT_HARDWARE_UNSUPPORTED,
> > + "Your hardware is unsupported. Please do not report "
> > + "bugs, panics, oopses, etc., on this hardware.\n");
>
> And as already pointed out it's not unsupported hardware even in the case
> you described
It's hardware that isn't supported by the vendor. How is it not
unsupported hardware in that context?
--
Matthew Garrett | [email protected]
On Tue, 22 Jun 2010 17:38:17 +0100
Matthew Garrett <[email protected]> wrote:
> On Tue, Jun 22, 2010 at 04:48:35PM +0100, Alan Cox wrote:
> > > + printk(KERN_CRIT "UNSUPPORTED HARDWARE DEVICE: %s\n", msg);
> > > + WARN_TAINT(1, TAINT_HARDWARE_UNSUPPORTED,
> > > + "Your hardware is unsupported. Please do not report "
> > > + "bugs, panics, oopses, etc., on this hardware.\n");
> >
> > And as already pointed out it's not unsupported hardware even in the case
> > you described
>
> It's hardware that isn't supported by the vendor. How is it not
> unsupported hardware in that context?
Which vendor ? What do you mean by support ? Even in Red Hat you will
I suspect need to change that message because after the first 500 calls
received by Dell or HP or whoever about it they'll demand you add
"unsupported by Red Hat" or similar to avoid them getting support calls !
And then as I said originally the example given was not even
"unsupported hardware" for an obvious Red Hat definition of the two
because it was actually about firmware combinations on specific boards -
ie it was an unsupported configuration.
The trouble in part is that the moment you borrow a bit for Red Hat
private use and put that use into the kernel you've made it unusuable for
anyone else. If you simply mark a few bits 'private/experimental use'
then you can use it for "unsupported" but others can use it for things
like 'uncertified configuration' or 'test' or 'non vendor signed module
loaded' and so on. All valid uses in specific vendor customisations.
The other giggle of course is that the WARN says do not report, but the
existing oops capturing software will capture the WARN so auto-report it.
Alan
On Tue, Jun 22, 2010 at 05:57:41PM +0100, Alan Cox wrote:
> And then as I said originally the example given was not even
> "unsupported hardware" for an obvious Red Hat definition of the two
> because it was actually about firmware combinations on specific boards -
> ie it was an unsupported configuration.
The two examples given were:
"a distribution may want to support PPC but not the Power5 chipset, or
the e1000e driver but not a card with a specific DeviceID because of
known firmware issues."
In both those cases it's specific hardware that's unsupported, not the
configuration.
--
Matthew Garrett | [email protected]
This patch is similar to Theordore Ts'o's TAINT_USER patch,
linux-2.6 commit 34f5a39899f3f3e815da64f48ddb72942d86c366.
Individual distributions may enable "generic" features such as X86 support,
PPC support, and driver support.
Some of the features that are enabled by these "generic" feature flags may
not be considered supported by the individual distribution.
For example, a distribution may want to support PPC but not the Power5
chipset, or the e1000e driver but not a card with a specific DeviceID because
of known firmware issues.
Typically, one would push a config patch to enable and disable the feature and
patch the distribution. However, in some cases this is not feasible in order
to preserve kabi and at the same time maintain parity with the upstream kernel.
In some cases the distribution may want to allow booting of these features but
explicitly notify a user that they are not "officially" supported. It is also
possible that the hardware is fixed via a firmware update at a later date,
making it supported again.
It would be useful for a distribution to notify the installer and
bug reporting applications, and notify users that the hardware they are using
is unsupported during panic, oops, BUG(), and WARN().
This patch reserves taint bits for distributions to use as they seem fit
without the fear of them being stepped up in future kernels.
V4: just reserve the bits
Signed-off-by: Don Zickus <[email protected]>
---
Not sure if this is sufficient for reserving or if there is some other code
I have to add elsewhere too.
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index cc5e3ff..c7b9e2c 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -347,6 +347,11 @@ extern enum system_states {
#define TAINT_WARN 9
#define TAINT_CRAP 10
#define TAINT_FIRMWARE_WORKAROUND 11
+/* Reserving bits for vendor specific uses */
+#define TAINT_RESERVED28 28
+#define TAINT_RESERVED29 29
+#define TAINT_RESERVED30 30
+#define TAINT_RESERVED31 31
extern void dump_stack(void) __cold;
On Tue, Jun 22, 2010 at 06:04:10PM +0100, Matthew Garrett wrote:
> On Tue, Jun 22, 2010 at 05:57:41PM +0100, Alan Cox wrote:
>
> > And then as I said originally the example given was not even
> > "unsupported hardware" for an obvious Red Hat definition of the two
> > because it was actually about firmware combinations on specific boards -
> > ie it was an unsupported configuration.
>
> The two examples given were:
>
> "a distribution may want to support PPC but not the Power5 chipset, or
> the e1000e driver but not a card with a specific DeviceID because of
> known firmware issues."
>
> In both those cases it's specific hardware that's unsupported, not the
> configuration.
>
What exactly is the use case supposed to be? If drivers are supposed to
call in to it for specific devices then they already have all of the
information they need for constructing a device blacklist and providing
more detailed information. If it's a configuration issue then we have
device quirks, which could also be extended to other busses as needed. In
either case, the context ought to be fairly explicit. I would much rather
see a message from the bus code stating that a specific device has been
disabled and skip the probe path entirely rather than trying to bolt on a
system-wide unsupported hardware state.
On Wed, Jun 23, 2010 at 12:06:38PM +0900, Paul Mundt wrote:
> What exactly is the use case supposed to be? If drivers are supposed to
> call in to it for specific devices then they already have all of the
> information they need for constructing a device blacklist and providing
> more detailed information. If it's a configuration issue then we have
> device quirks, which could also be extended to other busses as needed. In
> either case, the context ought to be fairly explicit. I would much rather
> see a message from the bus code stating that a specific device has been
> disabled and skip the probe path entirely rather than trying to bolt on a
> system-wide unsupported hardware state.
Hardware may work, it may just not work well enough that a software
vendor (eg, Red Hat) wants to deal with problem reports (eg, oopses
caused by a network card DMAing to the wrong place) from systems with
specific bits of hardware (eg, network cards that enjoy DMAing to the
wrong place occasionally). It may not even be down to technical issues -
the vendor may just have chosen to refuse to support systems with old
CPU families. It'd be straightforward to make the kernel simply refuse
to boot on them, but it seems more elegant to let it boot and alert the
user to the situation.
I don't think it's a flag that would ever be used in mainline, and
Alan's suggestion to just keep a range of taint flags as vendor-specific
would avoid the risk of collisions in future. But there's a minor
incentive to maintain standardisation over these things in order to
encourage commonality of report code.
--
Matthew Garrett | [email protected]
On Wed, Jun 23, 2010 at 04:30:47AM +0100, Matthew Garrett wrote:
> I don't think it's a flag that would ever be used in mainline, and
> Alan's suggestion to just keep a range of taint flags as vendor-specific
> would avoid the risk of collisions in future. But there's a minor
> incentive to maintain standardisation over these things in order to
> encourage commonality of report code.
Exactly. This was why we posted this upstream to see if there were other
distros interested in a mechanism like this.
Cheers,
Don
On Tue, Jun 22, 2010 at 02:58:12PM -0400, Don Zickus wrote:
> This patch reserves taint bits for distributions to use as they seem fit
> without the fear of them being stepped up in future kernels.
>
> V4: just reserve the bits
Any acks or naks on this version of the patch?
Cheers,
Don
>
> Signed-off-by: Don Zickus <[email protected]>
>
> ---
>
> Not sure if this is sufficient for reserving or if there is some other code
> I have to add elsewhere too.
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index cc5e3ff..c7b9e2c 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -347,6 +347,11 @@ extern enum system_states {
> #define TAINT_WARN 9
> #define TAINT_CRAP 10
> #define TAINT_FIRMWARE_WORKAROUND 11
> +/* Reserving bits for vendor specific uses */
> +#define TAINT_RESERVED28 28
> +#define TAINT_RESERVED29 29
> +#define TAINT_RESERVED30 30
> +#define TAINT_RESERVED31 31
>
> extern void dump_stack(void) __cold;
>
On Tue, 6 Jul 2010 16:33:09 -0400
Don Zickus <[email protected]> wrote:
> On Tue, Jun 22, 2010 at 02:58:12PM -0400, Don Zickus wrote:
> > This patch reserves taint bits for distributions to use as they seem fit
> > without the fear of them being stepped up in future kernels.
> >
> > V4: just reserve the bits
>
> Any acks or naks on this version of the patch?
You can have my ack for it.