pci/dmar: Combine the BIOS DMAR table warning messages

PATCH In review Ben Hutchings 7 participants 16 messages Mar 20, 2010 – Mar 24, 2010

Subsystems (7)

PowerPC PCI Library code s390 +3 more subsystems

lore

Share & tools

lore

Ben Hutchings

author

2010-03-20 23:01 UTC #0 lore raw

PATCH 1/4 pci/dmar: Combine the BIOS DMAR table warning messages

We have nearly the same code for warnings repeated four times. Move it into a separate function.

Signed-off-by Ben Hutchings <[email protected]>

---
 drivers/pci/dmar.c |   48 ++++++++++++++----------------------------------
 1 files changed, 14 insertions(+), 34 deletions(-)

drivers/pci/dmar.c +14 ~~-34~~

index ffe22bc..f101057 100644
@@ -613,7 +613,15 @@ int __init dmar_table_init(void)
 	return 0;
 }
 
-static int bios_warned;
+static void warn_invalid_dmar(u64 addr, const char *message)
+{
+	WARN_ONCE(1, "Your BIOS is broken; DMAR reported at address %llx%s!\n"
+		  "BIOS vendor: %s; Ver: %s; Product Version: %s\n",
+		  addr, message,
+		  dmi_get_system_info(DMI_BIOS_VENDOR),
+		  dmi_get_system_info(DMI_BIOS_VERSION),
+		  dmi_get_system_info(DMI_PRODUCT_VERSION));
+}
 
 int __init check_zero_address(void)
 {

@@ -639,13 +647,7 @@ int __init check_zero_address(void)
 
 			drhd = (void *)entry_header;
 			if (!drhd->address) {
-				/* Promote an attitude of violence to a BIOS engineer today */
-				WARN(1, "Your BIOS is broken; DMAR reported at address zero!\n"
-				     "BIOS vendor: %s; Ver: %s; Product Version: %s\n",
-				     dmi_get_system_info(DMI_BIOS_VENDOR),
-				     dmi_get_system_info(DMI_BIOS_VERSION),
-				     dmi_get_system_info(DMI_PRODUCT_VERSION));
-				bios_warned = 1;
+				warn_invalid_dmar(0, "");
 				goto failed;
 			}

@@ -658,14 +660,8 @@ int __init check_zero_address(void)
 			ecap = dmar_readq(addr + DMAR_ECAP_REG);
 			early_iounmap(addr, VTD_PAGE_SIZE);
 			if (cap == (uint64_t)-1 && ecap == (uint64_t)-1) {
-				/* Promote an attitude of violence to a BIOS engineer today */
-				WARN(1, "Your BIOS is broken; DMAR reported at address %llx returns all ones!\n"
-				     "BIOS vendor: %s; Ver: %s; Product Version: %s\n",
-				      drhd->address,
-				      dmi_get_system_info(DMI_BIOS_VENDOR),
-				      dmi_get_system_info(DMI_BIOS_VERSION),
-				      dmi_get_system_info(DMI_PRODUCT_VERSION));
-				bios_warned = 1;
+				warn_invalid_dmar(drhd->address,
+						  " returns all ones");
 				goto failed;
 			}
 		}

@@ -730,14 +726,7 @@ int alloc_iommu(struct dmar_drhd_unit *drhd)
 	int msagaw = 0;
 
 	if (!drhd->reg_base_addr) {
-		if (!bios_warned) {
-			WARN(1, "Your BIOS is broken; DMAR reported at address zero!\n"
-			     "BIOS vendor: %s; Ver: %s; Product Version: %s\n",
-			     dmi_get_system_info(DMI_BIOS_VENDOR),
-			     dmi_get_system_info(DMI_BIOS_VERSION),
-			     dmi_get_system_info(DMI_PRODUCT_VERSION));
-			bios_warned = 1;
-		}
+		warn_invalid_dmar(0, "");
 		return -EINVAL;
 	}

@@ -757,16 +746,7 @@ int alloc_iommu(struct dmar_drhd_unit *drhd)
 	iommu->ecap = dmar_readq(iommu->reg + DMAR_ECAP_REG);
 
 	if (iommu->cap == (uint64_t)-1 && iommu->ecap == (uint64_t)-1) {
-		if (!bios_warned) {
-			/* Promote an attitude of violence to a BIOS engineer today */
-			WARN(1, "Your BIOS is broken; DMAR reported at address %llx returns all ones!\n"
-			     "BIOS vendor: %s; Ver: %s; Product Version: %s\n",
-			     drhd->reg_base_addr,
-			     dmi_get_system_info(DMI_BIOS_VENDOR),
-			     dmi_get_system_info(DMI_BIOS_VERSION),
-			     dmi_get_system_info(DMI_PRODUCT_VERSION));
-			bios_warned = 1;
-		}
+		warn_invalid_dmar(drhd->reg_base_addr, " returns all ones");
 		goto err_unmap;
 	}

Signature · 1 line

1.7.0

Ben Hutchings

author

2010-03-20 23:05 UTC #1 lore raw

PATCH 2/4 panic: Allow taint flag for warnings to be changed from TAINT_WARN

WARN() is used in some places to report firmware or hardware bugs that are then worked-around. These bugs do not affect the stability of the kernel and should not set the usual TAINT_WARN flag. To allow for this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint flag as argument.

Architectures that implement warnings using trap instructions instead of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint) instead of __WARN().

Signed-off-by Ben Hutchings <[email protected]>

---
The architecture-specific changes here are untested and need to be reviewed by architecture maintainers.

Ben.

 arch/parisc/include/asm/bug.h  |    8 ++++----
 arch/powerpc/include/asm/bug.h |    6 +++---
 arch/s390/include/asm/bug.h    |    8 ++++----
 arch/sh/include/asm/bug.h      |    4 ++--
 include/asm-generic/bug.h      |   34 ++++++++++++++++++++++++++++++++--
 kernel/panic.c                 |   24 ++++++++++++++++++++----
 lib/bug.c                      |    2 +-
 7 files changed, 66 insertions(+), 20 deletions(-)

arch/parisc/include/asm/bug.h +4 -4

index 75e46c5..72cfdb0 100644
@@ -44,7 +44,7 @@
 #endif
 
 #ifdef CONFIG_DEBUG_BUGVERBOSE
-#define __WARN()							\
+#define __WARN_TAINT(taint)						\
 	do {								\
 		asm volatile("\n"					\
 			     "1:\t" PARISC_BUG_BREAK_ASM "\n"		\

@@ -54,11 +54,11 @@
 			     "\t.org 2b+%c3\n"				\
 			     "\t.popsection"				\
 			     : : "i" (__FILE__), "i" (__LINE__),	\
-			     "i" (BUGFLAG_WARNING),			\
+			     "i" (BUGFLAG_TAINT(taint)), 		\
 			     "i" (sizeof(struct bug_entry)) );		\
 	} while(0)
 #else
-#define __WARN()							\
+#define __WARN_TAINT(taint)						\
 	do {								\
 		asm volatile("\n"					\
 			     "1:\t" PARISC_BUG_BREAK_ASM "\n"		\

@@ -67,7 +67,7 @@
 			     "\t.short %c0\n"				\
 			     "\t.org 2b+%c1\n"				\
 			     "\t.popsection"				\
-			     : : "i" (BUGFLAG_WARNING),			\
+			     : : "i" (BUGFLAG_TAINT(taint)),		\
 			     "i" (sizeof(struct bug_entry)) );		\
 	} while(0)
 #endif

arch/powerpc/include/asm/bug.h +3 -3

index 2c15212..065c590 100644
@@ -85,12 +85,12 @@
 	}							\
 } while (0)
 
-#define __WARN() do {						\
+#define __WARN_TAINT(taint) do {				\
 	__asm__ __volatile__(					\
 		"1:	twi 31,0,0\n"				\
 		_EMIT_BUG_ENTRY					\
 		: : "i" (__FILE__), "i" (__LINE__),		\
-		  "i" (BUGFLAG_WARNING),			\
+		  "i" (BUGFLAG_TAINT(taint)),			\
 		  "i" (sizeof(struct bug_entry)));		\
 } while (0)

@@ -104,7 +104,7 @@
 		"1:	"PPC_TLNEI"	%4,0\n"			\
 		_EMIT_BUG_ENTRY					\
 		: : "i" (__FILE__), "i" (__LINE__),		\
-		  "i" (BUGFLAG_WARNING),			\
+		  "i" (BUGFLAG_TAINT(TAINT_WARN)),		\
 		  "i" (sizeof(struct bug_entry)),		\
 		  "r" (__ret_warn_on));				\
 	}							\

arch/s390/include/asm/bug.h +4 -4

index 9beeb9d..bf90d1f 100644
@@ -46,18 +46,18 @@
 	unreachable();					\
 } while (0)
 
-#define __WARN() do {					\
-	__EMIT_BUG(BUGFLAG_WARNING);			\
+#define __WARN_TAINT(taint) do {			\
+	__EMIT_BUG(BUGFLAG_TAINT(taint));		\
 } while (0)
 
 #define WARN_ON(x) ({					\
 	int __ret_warn_on = !!(x);			\
 	if (__builtin_constant_p(__ret_warn_on)) {	\
 		if (__ret_warn_on)			\
-			__EMIT_BUG(BUGFLAG_WARNING);	\
+			__WARN();			\
 	} else {					\
 		if (unlikely(__ret_warn_on))		\
-			__EMIT_BUG(BUGFLAG_WARNING);	\
+			__WARN();			\
 	}						\
 	unlikely(__ret_warn_on);			\
 })

arch/sh/include/asm/bug.h +2 -2

index d02c01b..6323f86 100644
@@ -48,7 +48,7 @@ do {							\
 		   "i" (sizeof(struct bug_entry)));	\
 } while (0)
 
-#define __WARN()					\
+#define __WARN_TAINT(taint)				\
 do {							\
 	__asm__ __volatile__ (				\
 		"1:\t.short %O0\n"			\

@@ -57,7 +57,7 @@ do {							\
 		 : "n" (TRAPA_BUG_OPCODE),		\
 		   "i" (__FILE__),			\
 		   "i" (__LINE__),			\
-		   "i" (BUGFLAG_WARNING),		\
+		   "i" (BUGFLAG_TAINT(taint)),		\
 		   "i" (sizeof(struct bug_entry)));	\
 } while (0)

include/asm-generic/bug.h +32 -2

index 18c435d..c2c9ba0 100644
@@ -25,7 +25,10 @@ struct bug_entry {
 };
 #endif		/* __ASSEMBLY__ */
 
-#define BUGFLAG_WARNING	(1<<0)
+#define BUGFLAG_WARNING		(1 << 0)
+#define BUGFLAG_TAINT(taint)	(BUGFLAG_WARNING | ((taint) << 8))
+#define BUG_GET_TAINT(bug)	((bug)->flags >> 8)
+
 #endif	/* CONFIG_GENERIC_BUG */
 
 /*

@@ -56,17 +59,25 @@ struct bug_entry {
  * appear at runtime.  Use the versions with printk format strings
  * to provide better diagnostics.
  */
-#ifndef __WARN
+#ifndef __WARN_TAINT
 #ifndef __ASSEMBLY__
 extern void warn_slowpath_fmt(const char *file, const int line,
 		const char *fmt, ...) __attribute__((format(printf, 3, 4)));
+extern void warn_slowpath_fmt_taint(const char *file, const int line,
+				    unsigned taint, const char *fmt, ...)
+	__attribute__((format(printf, 4, 5)));
 extern void warn_slowpath_null(const char *file, const int line);
 #define WANT_WARN_ON_SLOWPATH
 #endif
 #define __WARN()		warn_slowpath_null(__FILE__, __LINE__)
 #define __WARN_printf(arg...)	warn_slowpath_fmt(__FILE__, __LINE__, arg)
+#define __WARN_printf_taint(taint, arg...)				\
+	warn_slowpath_fmt_taint(__FILE__, __LINE__, taint, arg)
 #else
+#define __WARN()		__WARN_TAINT(TAINT_WARN)
 #define __WARN_printf(arg...)	do { printk(arg); __WARN(); } while (0)
+#define __WARN_printf_taint(taint, arg...)				\
+	do { printk(arg); __WARN_TAINT(taint); } while (0)
 #endif
 
 #ifndef WARN_ON

@@ -87,6 +98,13 @@ extern void warn_slowpath_null(const char *file, const int line);
 })
 #endif
 
+#define WARN_TAINT(condition, taint, format...) ({			\
+	int __ret_warn_on = !!(condition);				\
+	if (unlikely(__ret_warn_on))					\
+		__WARN_printf_taint(taint, format);			\
+	unlikely(__ret_warn_on);					\
+})
+
 #else /* !CONFIG_BUG */
 #ifndef HAVE_ARCH_BUG
 #define BUG() do {} while(0)

@@ -110,6 +128,8 @@ extern void warn_slowpath_null(const char *file, const int line);
 })
 #endif
 
+#define WARN_TAINT(condition, taint, format...) WARN_ON(condition)
+
 #endif
 
 #define WARN_ON_ONCE(condition)	({				\

@@ -132,6 +152,16 @@ extern void warn_slowpath_null(const char *file, const int line);
 	unlikely(__ret_warn_once);				\
 })
 
+#define WARN_TAINT_ONCE(condition, taint, format...)	({	\
+	static bool __warned;					\
+	int __ret_warn_once = !!(condition);			\
+								\
+	if (unlikely(__ret_warn_once))				\
+		if (WARN_TAINT(!__warned, taint, format))	\
+			__warned = true;			\
+	unlikely(__ret_warn_once);				\
+})
+
 #define WARN_ON_RATELIMIT(condition, state)			\
 		WARN_ON((condition) && __ratelimit(state))

kernel/panic.c +20 -4

index 13d966b..8b821bc 100644
@@ -365,7 +365,8 @@ struct slowpath_args {
 	va_list args;
 };
 
-static void warn_slowpath_common(const char *file, int line, void *caller, struct slowpath_args *args)
+static void warn_slowpath_common(const char *file, int line, void *caller,
+				 unsigned taint, struct slowpath_args *args)
 {
 	const char *board;

@@ -381,7 +382,7 @@ static void warn_slowpath_common(const char *file, int line, void *caller, struc
 	print_modules();
 	dump_stack();
 	print_oops_end_marker();
-	add_taint(TAINT_WARN);
+	add_taint(taint);
 }
 
 void warn_slowpath_fmt(const char *file, int line, const char *fmt, ...)

@@ -390,14 +391,29 @@ void warn_slowpath_fmt(const char *file, int line, const char *fmt, ...)
 
 	args.fmt = fmt;
 	va_start(args.args, fmt);
-	warn_slowpath_common(file, line, __builtin_return_address(0), &args);
+	warn_slowpath_common(file, line, __builtin_return_address(0),
+			     TAINT_WARN, &args);
 	va_end(args.args);
 }
 EXPORT_SYMBOL(warn_slowpath_fmt);
 
+void warn_slowpath_fmt_taint(const char *file, int line,
+			     unsigned taint, const char *fmt, ...)
+{
+	struct slowpath_args args;
+
+	args.fmt = fmt;
+	va_start(args.args, fmt);
+	warn_slowpath_common(file, line, __builtin_return_address(0),
+			     taint, &args);
+	va_end(args.args);
+}
+EXPORT_SYMBOL(warn_slowpath_fmt_taint);
+
 void warn_slowpath_null(const char *file, int line)
 {
-	warn_slowpath_common(file, line, __builtin_return_address(0), NULL);
+	warn_slowpath_common(file, line, __builtin_return_address(0),
+			     TAINT_WARN, NULL);
 }
 EXPORT_SYMBOL(warn_slowpath_null);
 #endif

lib/bug.c +1 -1

index 300e41a..f13daf4 100644
@@ -165,7 +165,7 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
 			       (void *)bugaddr);
 
 		show_regs(regs);
-		add_taint(TAINT_WARN);
+		add_taint(BUG_GET_TAINT(bug));
 		return BUG_TRAP_TYPE_WARN;
 	}

Signature · 1 line

1.7.0

Andi Kleen

2010-03-21 19:10 UTC #2 lore raw

Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

Quoted 9 lines · Ben Hutchings

WARN() is used in some places to report firmware or hardware bugs that are then worked-around. These bugs do not affect the stability of the kernel and should not set the usual TAINT_WARN flag. To allow for this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint flag as argument.
Architectures that implement warnings using trap instructions instead of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint) instead of __WARN().

I guess this should enforce that at least some taint flag is set? (e.g. with a BUILD_BUG_ON)

-Andi

Signature · 1 line

[email protected] -- Speaking for myself only.

Ben Hutchings

author

2010-03-21 19:25 UTC #3 lore raw

Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

Quoted 8 lines · Andi Kleen

Quoted 6 lines · Ben Hutchings
WARN() is used in some places to report firmware or hardware bugs that are then worked-around. These bugs do not affect the stability of the kernel and should not set the usual TAINT_WARN flag. To allow for this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint flag as argument.

Quoted 6 lines

Quoted 3 lines
Architectures that implement warnings using trap instructions instead of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint) instead of __WARN().
I guess this should enforce that at least some taint flag is set? (e.g. with a BUILD_BUG_ON)

I'm being a bit sloppy with the wording here. The TAINT_* macros are actually bit numbers, not flags. I could define a TAINT_MAX and add:

	BUILD_BUG_ON(taint < 0 || taint > TAINT_MAX);

Not sure that that's really worth doing though.

Ben.

Signature · 2 lines

Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.

Andrew Morton

maintainer

2010-03-23 02:47 UTC #4 lore raw

Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

In reply to Ben Hutchings

Quoted 9 lines · On Sat, 20 Mar 2010 23:05:40 +0000 Ben Hutchings <[email protected]> wrote:

WARN() is used in some places to report firmware or hardware bugs that are then worked-around. These bugs do not affect the stability of the kernel and should not set the usual TAINT_WARN flag. To allow for this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint flag as argument.
Architectures that implement warnings using trap instructions instead of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint) instead of __WARN().

When you say they "must now implement", I assume that you mean that they _do_ now implement, and that no additional architecture work is needed.

Quoted 2 lines

The architecture-specific changes here are untested and need to be reviewed by architecture maintainers.

That would be nice.

Ben Hutchings

author

2010-03-23 13:20 UTC #5 lore raw

Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

Quoted 8 lines · Andrew Morton

Quoted 6 lines · On Sat, 20 Mar 2010 23:05:40 +0000 Ben Hutchings <[email protected]> wrote:
WARN() is used in some places to report firmware or hardware bugs that are then worked-around. These bugs do not affect the stability of the kernel and should not set the usual TAINT_WARN flag. To allow for this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint flag as argument.

Quoted 7 lines

Quoted 3 lines
Architectures that implement warnings using trap instructions instead of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint) instead of __WARN().
When you say they "must now implement", I assume that you mean that they _do_ now implement, and that no additional architecture work is needed.

Right, I believe I fixed-up all the current architectures. There might be more architectures out there, unmerged as yet.

Ben.

Signature · 2 lines

Ben Hutchings
Any smoothly functioning technology is indistinguishable from a rigged demo.

Paul Mundt

Tested-by on v1 2/4

2010-03-23 07:45 UTC #6 lore raw

Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

In reply to Ben Hutchings

Quoted 6 lines · Ben Hutchings

WARN() is used in some places to report firmware or hardware bugs that are then worked-around. These bugs do not affect the stability of the kernel and should not set the usual TAINT_WARN flag. To allow for this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint flag as argument.

Quoted 9 lines

Architectures that implement warnings using trap instructions instead of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint) instead of __WARN().
Signed-off-by Ben Hutchings <[email protected]>
---
The architecture-specific changes here are untested and need to be reviewed by architecture maintainers.

I'm a bit confused about how this is supposed to work, the TAINT_xxx values are bit positions presently from 0 to 10, while BUGFLAG_xxx are ranged from 0 up. You've set up BUGFLAG_TAINT() to that the TAINT_xxx value is shifted up 8 bits but neglected the fact that the trap type is 16-bits on most (all?) of the platforms using trap-based BUG handling.

If the 'taint' in question is just the TAINT_xxx value by itself and will never be a bitmap then that's fine, but there's certainly not enough room to pass the bitmap in on top of the bugflag otherwise (I don't know if this is your intention or not though).

Also note that some platforms (like SH) implement additional bugflags, so we at least want to keep the lower byte available for architecture private use.

Having said that, the current patch does work for me, although I'm a bit nervous about someone thinking it's ok to pass in a taint bitmap here.

Tested-by Paul Mundt <[email protected]>

Ben Hutchings

author

2010-03-23 13:23 UTC #7 lore raw

Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

Quoted 22 lines · Paul Mundt

Quoted 6 lines · Ben Hutchings
WARN() is used in some places to report firmware or hardware bugs that are then worked-around. These bugs do not affect the stability of the kernel and should not set the usual TAINT_WARN flag. To allow for this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint flag as argument.
Quoted 9 lines
Architectures that implement warnings using trap instructions instead of calls to warn_slowpath_*() must now implement __WARN_TAINT(taint) instead of __WARN().
Signed-off-by Ben Hutchings <[email protected]>
---
The architecture-specific changes here are untested and need to be reviewed by architecture maintainers.
I'm a bit confused about how this is supposed to work, the TAINT_xxx values are bit positions presently from 0 to 10, while BUGFLAG_xxx are ranged from 0 up. You've set up BUGFLAG_TAINT() to that the TAINT_xxx value is shifted up 8 bits but neglected the fact that the trap type is 16-bits on most (all?) of the platforms using trap-based BUG handling.

Quoted 4 lines

If the 'taint' in question is just the TAINT_xxx value by itself and will never be a bitmap then that's fine, but there's certainly not enough room to pass the bitmap in on top of the bugflag otherwise (I don't know if this is your intention or not though).

Yes, the taint value must be a bit number not a flag. Sloppy wording on my part.

Quoted 3 lines

Also note that some platforms (like SH) implement additional bugflags, so we at least want to keep the lower byte available for architecture private use.

I noticed, that's why I started at 8 not 1.

Quoted 2 lines

Having said that, the current patch does work for me, although I'm a bit nervous about someone thinking it's ok to pass in a taint bitmap here.

We can maybe use BUILD_BUG_ON() here as the taint bit is already required to be a compile-time constant.

Ben.

Quoted 2 lines

Tested-by Paul Mundt <[email protected]>

Signature · 2 lines

Ben Hutchings
Any smoothly functioning technology is indistinguishable from a rigged demo.

Michael Ellerman

2010-03-24 11:11 UTC #8 lore raw

Re: [PATCH 2/4] panic: Allow taint flag for warnings to be changed from TAINT_WARN

In reply to Ben Hutchings

Quoted 5 lines · Ben Hutchings

WARN() is used in some places to report firmware or hardware bugs that are then worked-around. These bugs do not affect the stability of the kernel and should not set the usual TAINT_WARN flag. To allow for this, add WARN_TAINT() and WARN_TAINT_ONCE() macros that take a taint flag as argument.

Quoted 2 lines

The architecture-specific changes here are untested and need to be reviewed by architecture maintainers.

I'm not one of them, but this at least builds on powerpc FWIW.

cheers

Ben Hutchings

author

2010-03-20 23:06 UTC #9 lore raw

PATCH 3/4 panic: Add taint flag TAINT_FIRMWARE_WORKAROUND ('I')

In reply to Ben Hutchings

This taint flag will initially be used when warning about invalid ACPI DMAR tables.

Signed-off-by Ben Hutchings <[email protected]>

---
 include/linux/kernel.h |    1 +
 kernel/panic.c         |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

include/linux/kernel.h +1 -0

index eaf7221..ab8072b 100644
@@ -344,6 +344,7 @@ extern enum system_states {
 #define TAINT_OVERRIDDEN_ACPI_TABLE	8
 #define TAINT_WARN			9
 #define TAINT_CRAP			10
+#define TAINT_FIRMWARE_WORKAROUND	11
 
 extern void dump_stack(void) __cold;

kernel/panic.c +2 -0

index 8b821bc..e9fa224 100644
@@ -178,6 +178,7 @@ static const struct tnt tnts[] = {
 	{ TAINT_OVERRIDDEN_ACPI_TABLE,	'A', ' ' },
 	{ TAINT_WARN,			'W', ' ' },
 	{ TAINT_CRAP,			'C', ' ' },
+	{ TAINT_FIRMWARE_WORKAROUND,	'I', ' ' },
 };
 
 /**

@@ -194,6 +195,7 @@ static const struct tnt tnts[] = {
  *  'A' - ACPI table overridden.
  *  'W' - Taint on warning.
  *  'C' - modules from drivers/staging are loaded.
+ *  'I' - Firmware workaround was required.
  *
  *	The string is overwritten by the next call to print_tainted().
  */

Signature · 1 line

1.7.0

Randy Dunlap

2010-03-21 01:56 UTC #10 lore raw

Re: [PATCH 3/4] panic: Add taint flag TAINT_FIRMWARE_WORKAROUND ('I')

Quoted 34 lines · Ben Hutchings

This taint flag will initially be used when warning about invalid ACPI DMAR tables.

Signed-off-by Ben Hutchings <[email protected]>

---
 include/linux/kernel.h |    1 +
 kernel/panic.c         |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

include/linux/kernel.h +1 -0

index eaf7221..ab8072b 100644
@@ -344,6 +344,7 @@ extern enum system_states {
 #define TAINT_OVERRIDDEN_ACPI_TABLE	8
 #define TAINT_WARN			9
 #define TAINT_CRAP			10
+#define TAINT_FIRMWARE_WORKAROUND	11
 
 extern void dump_stack(void) __cold;

kernel/panic.c +1 -0

index 8b821bc..e9fa224 100644
@@ -178,6 +178,7 @@ static const struct tnt tnts[] = {
 	{ TAINT_OVERRIDDEN_ACPI_TABLE,	'A', ' ' },
 	{ TAINT_WARN,			'W', ' ' },
 	{ TAINT_CRAP,			'C', ' ' },
+	{ TAINT_FIRMWARE_WORKAROUND,	'I', ' ' },
 };
 
 /**

kernel/panic.c static const struct tnt tnts[] = { · line 201

Quoted 7 lines

kernel/panic.c +1 -0

@@ -194,6 +195,7 @@ static const struct tnt tnts[] = {
  *  'A' - ACPI table overridden.
  *  'W' - Taint on warning.
  *  'C' - modules from drivers/staging are loaded.
+ *  'I' - Firmware workaround was required.
  *
  *	The string is overwritten by the next call to print_tainted().
  */

Please update Documentation/oops-tracing.txt also.

thanks,

Signature · 1 line

~Randy

Ben Hutchings

author

2010-03-20 23:07 UTC #11 lore raw

PATCH 4/4 pci/dmar: Tone down warnings about invalid BIOS DMAR tables

In reply to Ben Hutchings

We now know how to deal with these tables so that they are harmless. Use the TAINT_FIRMWARE_WORKAROUND flag and don't say the BIOS is 'broken' as this makes users think of hardware damage.

Signed-off-by Ben Hutchings <[email protected]>

---
 drivers/pci/dmar.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

drivers/pci/dmar.c +7 -6

index f101057..8aa31c2 100644
@@ -615,12 +615,13 @@ int __init dmar_table_init(void)
 
 static void warn_invalid_dmar(u64 addr, const char *message)
 {
-	WARN_ONCE(1, "Your BIOS is broken; DMAR reported at address %llx%s!\n"
-		  "BIOS vendor: %s; Ver: %s; Product Version: %s\n",
-		  addr, message,
-		  dmi_get_system_info(DMI_BIOS_VENDOR),
-		  dmi_get_system_info(DMI_BIOS_VERSION),
-		  dmi_get_system_info(DMI_PRODUCT_VERSION));
+	WARN_TAINT_ONCE(1, TAINT_FIRMWARE_WORKAROUND,
+			"BIOS bug: DMAR reported at address %llx%s!\n"
+			"BIOS vendor: %s; Ver: %s; Product Version: %s\n",
+			addr, message,
+			dmi_get_system_info(DMI_BIOS_VENDOR),
+			dmi_get_system_info(DMI_BIOS_VERSION),
+			dmi_get_system_info(DMI_PRODUCT_VERSION));
 }
 
 int __init check_zero_address(void)

Signature · 1 line

1.7.0

David Woodhouse

2010-03-21 08:43 UTC #12 lore raw

Re: [PATCH 4/4] pci/dmar: Tone down warnings about invalid BIOS DMAR tables

Quoted 3 lines · Ben Hutchings

We now know how to deal with these tables so that they are harmless. Use the TAINT_FIRMWARE_WORKAROUND flag and don't say the BIOS is 'broken' as this makes users think of hardware damage.

Nack to the string change. If you see this message, it's because your BIOS is BROKEN, and this brokenness has caused us to have to disable the VT-d feature completely. The fuckwits obviously never tested it even as far as booting a VT-d enabled OS on it even once¹.

We _want_ people to think of it as hardware damage and return the board to the vendor, as unfit for the purpose for which it was sold.

The rest of the series looks good though; thanks.

I haven't applied the first patch, although it stands alone -- your other patches will depend on it and cause merge pain. Or do you want me to take the whole lot through the intel-iommu tree?

Signature · 4 lines

dwmw2

¹ Or maybe they did, but the OS didn't tell them clearly enough that it
  was broken.

Ben Hutchings

author

2010-03-21 14:57 UTC #13 lore raw

Re: [PATCH 4/4] pci/dmar: Tone down warnings about invalid BIOS DMAR tables

Quoted 9 lines · David Woodhouse

Quoted 3 lines · Ben Hutchings
We now know how to deal with these tables so that they are harmless. Use the TAINT_FIRMWARE_WORKAROUND flag and don't say the BIOS is 'broken' as this makes users think of hardware damage.
Nack to the string change. If you see this message, it's because your BIOS is BROKEN, and this brokenness has caused us to have to disable the VT-d feature completely. The fuckwits obviously never tested it even as far as booting a VT-d enabled OS on it even once¹.

[...]

Are you saying that these bogus tables are found on boards that do have VT-d hardware? I was working on the assumption that these tables result from the OEM building a BIOS with VT-d support for a board where the hardware is not present.

Ben.

Signature · 2 lines

Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.

David Woodhouse

2010-03-21 15:58 UTC #14 lore raw

Re: [PATCH 4/4] pci/dmar: Tone down warnings about invalid BIOS DMAR tables

Quoted 4 lines · Ben Hutchings

Are you saying that these bogus tables are found on boards that do have VT-d hardware? I was working on the assumption that these tables result from the OEM building a BIOS with VT-d support for a board where the hardware is not present.

Yes. Most of these boards have VT-d but can't use it because the BIOS vendors fucked up and never once booted a VT-d-capable OS to test it. (And because the spec is bloody stupid and relies on BIOS tables rather than enumerating through PCI or something like that).

Signature · 1 line

dwmw2

Andrew Morton

maintainer

2010-03-23 05:47 UTC #15 lore raw

In reply to Ben Hutchings

The patches seem reasonable. I reordered them thusly:

panic-allow-taint-flag-for-warnings-to-be-changed-from-taint_warn.patch panic-add-taint-flag-taint_firmware_workaround-i.patch pci-dmar-combine-the-bios-dmar-table-warning-messages.patch pci-dmar-tone-down-warnings-about-invalid-bios-dmar-tables.patch

so I can merge the first two and then I can feed the PCI patches into the PCI cabal. Alternatively they can ack them and I'll merge them all. Or they can snarf all four and I'll go away.