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

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

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 3487 bytes --]

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

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/pci/dmar.c |   48 ++++++++++++++----------------------------------
 1 files changed, 14 insertions(+), 34 deletions(-)

diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
index ffe22bc..f101057 100644
--- a/drivers/pci/dmar.c
+++ b/drivers/pci/dmar.c
@@ -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;
 	}
 
-- 
1.7.0




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 9299 bytes --]

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 <ben@decadent.org.uk>
---
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(-)

diff --git a/arch/parisc/include/asm/bug.h b/arch/parisc/include/asm/bug.h
index 75e46c5..72cfdb0 100644
--- a/arch/parisc/include/asm/bug.h
+++ b/arch/parisc/include/asm/bug.h
@@ -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
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 2c15212..065c590 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -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));				\
 	}							\
diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index 9beeb9d..bf90d1f 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -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);			\
 })
diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h
index d02c01b..6323f86 100644
--- a/arch/sh/include/asm/bug.h
+++ b/arch/sh/include/asm/bug.h
@@ -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)
 
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 18c435d..c2c9ba0 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -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))
 
diff --git a/kernel/panic.c b/kernel/panic.c
index 13d966b..8b821bc 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -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
diff --git a/lib/bug.c b/lib/bug.c
index 300e41a..f13daf4 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -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;
 	}
 
-- 
1.7.0




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[Andi Kleen]

Ben Hutchings <ben@decadent.org.uk> writes:

> 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

-- 
ak@linux.intel.com -- Speaking for myself only.

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

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1054 bytes --]

On Sun, 2010-03-21 at 20:10 +0100, Andi Kleen wrote:
> Ben Hutchings <ben@decadent.org.uk> writes:
> 
> > 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)

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.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[Andrew Morton]

On Sat, 20 Mar 2010 23:05:40 +0000 Ben Hutchings <ben@decadent.org.uk> 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.

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

That would be nice.

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

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

On Mon, 2010-03-22 at 22:47 -0400, Andrew Morton wrote:
> On Sat, 20 Mar 2010 23:05:40 +0000 Ben Hutchings <ben@decadent.org.uk> 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.

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

Ben.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[Paul Mundt]

On Sat, Mar 20, 2010 at 11:05:40PM +0000, Ben Hutchings 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().
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> 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 <lethal@linux-sh.org>

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

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2177 bytes --]

On Tue, 2010-03-23 at 16:45 +0900, Paul Mundt wrote:
> On Sat, Mar 20, 2010 at 11:05:40PM +0000, Ben Hutchings 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().
> > 
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > ---
> > 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).

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

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

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

> Tested-by: Paul Mundt <lethal@linux-sh.org>
> 

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 560 bytes --]

On Sat, 2010-03-20 at 23:05 +0000, Ben Hutchings 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.
..
> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]

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

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 include/linux/kernel.h |    1 +
 kernel/panic.c         |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index eaf7221..ab8072b 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -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;
 
diff --git a/kernel/panic.c b/kernel/panic.c
index 8b821bc..e9fa224 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -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().
  */
-- 
1.7.0




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[Randy Dunlap]

On 03/20/10 16:06, Ben Hutchings wrote:
> This taint flag will initially be used when warning about invalid ACPI
> DMAR tables.
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
>  include/linux/kernel.h |    1 +
>  kernel/panic.c         |    2 ++
>  2 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index eaf7221..ab8072b 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -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;
>  
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 8b821bc..e9fa224 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -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().
>   */

Please update Documentation/oops-tracing.txt also.

thanks,
-- 
~Randy

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

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

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 <ben@decadent.org.uk>
---
 drivers/pci/dmar.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
index f101057..8aa31c2 100644
--- a/drivers/pci/dmar.c
+++ b/drivers/pci/dmar.c
@@ -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)
-- 
1.7.0



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[David Woodhouse]

On Sat, 2010-03-20 at 23:07 +0000, Ben Hutchings wrote:
> 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?

-- 
dwmw2

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

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

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 956 bytes --]

On Sun, 2010-03-21 at 08:43 +0000, David Woodhouse wrote:
> On Sat, 2010-03-20 at 23:07 +0000, Ben Hutchings wrote:
> > 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.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[David Woodhouse]

On Sun, 21 Mar 2010, Ben Hutchings wrote:

> 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).

-- 
dwmw2

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

[Andrew Morton]

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.