Add TAINT_HARDWARE_UNSUPPORTED flag

This patch is similar to Theordore Ts'o's TAINT_USER patch, linux-2.6 commit 34f5a39899f3f3e815da64f48ddb72942d86c366.

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]>

include/linux/kernel.h +1 -0

index 8317ec4..f722b0d 100644
@@ -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;
 

kernel/panic.c +2 -0

index 3b16cd9..394a5bb 100644
@@ -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().
  */

Quoted 2 lines · 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.

Quoted 2 lines

This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions to use.

Review

Quoted 1 line

(quoted diff) +1 -0

+		       "unsupported.\n");

Preferable not to split the string, so more like:

		printk(KERN_CRIT
			"WARNING: This system's hardware is unsupported.\n");

Quoted 2 lines

set_bit(flag, &tainted_mask);

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]>

Documentation/oops-tracing.txt +2 -0

index 6fe9001..e337b0a 100644
@@ -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

include/linux/kernel.h +1 -0

index 8317ec4..f722b0d 100644
@@ -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;
 

kernel/panic.c +2 -0

index 3b16cd9..8d081ff 100644
@@ -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().
  */

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]>

Documentation/oops-tracing.txt +2 -0

index 6fe9001..e337b0a 100644
@@ -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

include/linux/kernel.h +1 -0

index 8317ec4..f722b0d 100644
@@ -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;
 

kernel/panic.c +2 -0

index 3b16cd9..8d081ff 100644
@@ -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().
  */

kernel/panic.c void add_taint(unsigned flag) · line 253

Quoted 10 lines

kernel/panic.c +3 -0

@@ -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

Quoted 24 lines · Andrew Morton

kernel/panic.c void add_taint(unsigned flag) · line 253

Quoted 10 lines

kernel/panic.c +3 -0

@@ -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);
}

Quoted 7 lines

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.

Quoted 8 lines

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.

Quoted 2 lines

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?

This patch is similar to Theordore Ts'o's TAINT_USER patch, linux-2.6 commit 34f5a39899f3f3e815da64f48ddb72942d86c366.

This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions to use.

kernel/panic.c void add_taint(unsigned flag) · line 257

Quoted 4 lines

kernel/panic.c +4 -0

+	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

Quoted 7 lines · Alan Cox

kernel/panic.c void add_taint(unsigned flag) · line 257

Quoted 4 lines

kernel/panic.c +4 -0

+	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?

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.

Quoted 4 lines · Alan Cox

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.

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.

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.

include/linux/kernel.h +5 -0

index cc5e3ff..c7b9e2c 100644
@@ -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;
 

Quoted 4 lines · Don Zickus

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?

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

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.

This patch introduces the TAINT_HARDWARE_UNSUPPORTED flag for distributions to use.

Quoted 2 lines

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 ?

Quoted 3 lines

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.

Quoted 2 lines

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' ?

Quoted 3 lines

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.