From: cheloha <cheloha@openbsd.org>
Date: Wed, 2 Jun 2021 00:39:25 +0000 (+0000)
Subject: kernel: introduce per-CPU panic(9) message buffers
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=1a4a9ab2b0c2c1651f94c16723a3fbc048452fbf;p=openbsd

kernel: introduce per-CPU panic(9) message buffers

Add a 512-byte buffer (ci_panicbuf) to each cpu_info struct on each
platform for use by panic(9).  The first panic on a given CPU writes
its message to this buffer.  Subsequent panics on a given CPU print
the panic message to the console but do not modify the buffer.  This
aids debugging in two cases:

- If 2+ CPUs panic simultaneously there is no risk of garbled messages
  in the panic buffer.

- If a CPU panics and then the operator causes a second panic while
  using ddb(4), the operator can still recall the first failure on
  a particular CPU.

Misc. changes to support this bigger change:

- Set panicstr atomically to identify the first CPU to reach panic().

- Tweak db_show_panic_cmd() to print all panic messages across all
  CPUs.  Prefix the first panic with an asterisk ('*').

- Prefer db_printf() to printf() during a panic if we have it.
  Apparently it disturbs less global state.

- On amd64, tweak fault() to write the local panic buffer.  This needs
  more work.

Prompted by bluhm@ and deraadt@.  Mostly written by deraadt@.
Discussed with bluhm@, deraadt@ and kettenis@.

Borne from a discussion on tech@ about making panic(9) more MP-safe:

https://marc.info/?l=openbsd-tech&m=162086462316143&w=2

ok kettenis@, visa@, bluhm@, deraadt@
---

diff --git a/sys/arch/alpha/include/cpu.h b/sys/arch/alpha/include/cpu.h
index a3ebb8a1f6b..6dedb267e2a 100644
--- a/sys/arch/alpha/include/cpu.h
+++ b/sys/arch/alpha/include/cpu.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: cpu.h,v 1.63 2020/06/05 14:25:05 naddy Exp $ */
+/* $OpenBSD: cpu.h,v 1.64 2021/06/02 00:39:26 cheloha Exp $ */
 /* $NetBSD: cpu.h,v 1.45 2000/08/21 02:03:12 thorpej Exp $ */
 
 /*-
@@ -213,6 +213,7 @@ struct cpu_info {
 #ifdef GPROF
 	struct gmonparam *ci_gmon;
 #endif
+	char ci_panicbuf[512];
 };
 
 #define	CPUF_PRIMARY	0x01		/* CPU is primary CPU */
diff --git a/sys/arch/amd64/amd64/trap.c b/sys/arch/amd64/amd64/trap.c
index 2b5262bba46..87e65841638 100644
--- a/sys/arch/amd64/amd64/trap.c
+++ b/sys/arch/amd64/amd64/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.88 2021/05/05 07:29:00 mpi Exp $	*/
+/*	$OpenBSD: trap.c,v 1.89 2021/06/02 00:39:26 cheloha Exp $	*/
 /*	$NetBSD: trap.c,v 1.2 2003/05/04 23:51:56 fvdl Exp $	*/
 
 /*-
@@ -87,6 +87,7 @@
 #include <machine/psl.h>
 #include <machine/trap.h>
 #ifdef DDB
+#include <ddb/db_output.h>
 #include <machine/db_machdep.h>
 #endif
 
@@ -135,20 +136,21 @@ static inline void debug_trap(struct trapframe *_frame, struct proc *_p,
     long _type);
 
 static inline void
-fault(const char *format, ...)
+fault(const char *fmt, ...)
 {
-	static char faultbuf[512];
+	struct cpu_info *ci = curcpu();
 	va_list ap;
 
-	/*
-	 * Save the fault info for DDB.  Kernel lock protects
-	 * faultbuf from being overwritten by another CPU.
-	 */
-	va_start(ap, format);
-	vsnprintf(faultbuf, sizeof faultbuf, format, ap);
+	atomic_cas_ptr(&panicstr, NULL, ci->ci_panicbuf);
+
+	va_start(ap, fmt);
+	vsnprintf(ci->ci_panicbuf, sizeof(ci->ci_panicbuf), fmt, ap);
 	va_end(ap);
-	printf("%s\n", faultbuf);
-	faultstr = faultbuf;
+#ifdef DDB
+	db_printf("%s\n", ci->ci_panicbuf);
+#else
+	printf("%s\n", ci->ci_panicbuf);
+#endif
 }
 
 static inline int
diff --git a/sys/arch/amd64/include/cpu.h b/sys/arch/amd64/include/cpu.h
index 5a76eb21e3c..4a53dd0724b 100644
--- a/sys/arch/amd64/include/cpu.h
+++ b/sys/arch/amd64/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.137 2020/06/03 06:54:04 dlg Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.138 2021/06/02 00:39:26 cheloha Exp $	*/
 /*	$NetBSD: cpu.h,v 1.1 2003/04/26 18:39:39 fvdl Exp $	*/
 
 /*-
@@ -208,6 +208,8 @@ struct cpu_info {
 	struct vmxon_region *ci_vmxon_region;
 
 	int64_t		ci_tsc_skew;		/* counter skew vs cpu0 */
+
+	char		ci_panicbuf[512];
 };
 
 #define CPUF_BSP	0x0001		/* CPU is the original BSP */
diff --git a/sys/arch/arm/include/cpu.h b/sys/arch/arm/include/cpu.h
index 118381e7fc7..28f1c9186ee 100644
--- a/sys/arch/arm/include/cpu.h
+++ b/sys/arch/arm/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.59 2020/05/31 06:23:57 dlg Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.60 2021/06/02 00:39:26 cheloha Exp $	*/
 /*	$NetBSD: cpu.h,v 1.34 2003/06/23 11:01:08 martin Exp $	*/
 
 /*
@@ -198,6 +198,8 @@ struct cpu_info {
 #ifdef GPROF
 	struct gmonparam *ci_gmon;
 #endif
+
+	char			ci_panicbuf[512];
 };
 
 #define CPUF_PRIMARY 		(1<<0)
diff --git a/sys/arch/arm64/include/cpu.h b/sys/arch/arm64/include/cpu.h
index 977a89e6e63..29f7d4d9a0e 100644
--- a/sys/arch/arm64/include/cpu.h
+++ b/sys/arch/arm64/include/cpu.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: cpu.h,v 1.20 2021/05/15 11:30:27 kettenis Exp $ */
+/* $OpenBSD: cpu.h,v 1.21 2021/06/02 00:39:26 cheloha Exp $ */
 /*
  * Copyright (c) 2016 Dale Rahn <drahn@dalerahn.com>
  *
@@ -135,6 +135,8 @@ struct cpu_info {
 #ifdef GPROF
 	struct gmonparam	*ci_gmon;
 #endif
+
+	char			ci_panicbuf[512];
 };
 
 #define CPUF_PRIMARY 		(1<<0)
diff --git a/sys/arch/hppa/include/cpu.h b/sys/arch/hppa/include/cpu.h
index dc32d050c00..b8dfc05117d 100644
--- a/sys/arch/hppa/include/cpu.h
+++ b/sys/arch/hppa/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.93 2020/06/14 20:29:13 naddy Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.94 2021/06/02 00:39:26 cheloha Exp $	*/
 
 /*
  * Copyright (c) 2000-2004 Michael Shalayeff
@@ -114,6 +114,8 @@ struct cpu_info {
 #ifdef GPROF
 	struct gmonparam *ci_gmon;
 #endif
+
+	char		ci_panicbuf[512];
 } __attribute__((__aligned__(64)));
 
 #define		CPUF_RUNNING	0x0001		/* CPU is running. */
diff --git a/sys/arch/i386/include/cpu.h b/sys/arch/i386/include/cpu.h
index 4e0095ca0bb..97e5bc64816 100644
--- a/sys/arch/i386/include/cpu.h
+++ b/sys/arch/i386/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.173 2021/03/11 11:16:57 jsg Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.174 2021/06/02 00:39:26 cheloha Exp $	*/
 /*	$NetBSD: cpu.h,v 1.35 1996/05/05 19:29:26 christos Exp $	*/
 
 /*-
@@ -170,6 +170,7 @@ struct cpu_info {
 #if defined(GPROF) || defined(DDBPROF)
 	struct gmonparam	*ci_gmon;
 #endif
+	char			ci_panicbuf[512];
 };
 
 /*
diff --git a/sys/arch/m88k/include/cpu.h b/sys/arch/m88k/include/cpu.h
index 48e6d36e1d4..5b97b79bdef 100644
--- a/sys/arch/m88k/include/cpu.h
+++ b/sys/arch/m88k/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.68 2020/05/31 06:23:57 dlg Exp $ */
+/*	$OpenBSD: cpu.h,v 1.69 2021/06/02 00:39:26 cheloha Exp $ */
 /*
  * Copyright (c) 1996 Nivas Madhur
  * Copyright (c) 1992, 1993
@@ -177,6 +177,7 @@ struct cpu_info {
 #ifdef GPROF
 	struct gmonparam *ci_gmon;
 #endif
+	char		 ci_panicbuf[512];
 };
 
 extern cpuid_t master_cpu;
diff --git a/sys/arch/mips64/include/cpu.h b/sys/arch/mips64/include/cpu.h
index dd59c88161a..79b0a2484ca 100644
--- a/sys/arch/mips64/include/cpu.h
+++ b/sys/arch/mips64/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.133 2021/05/28 16:33:36 visa Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.134 2021/06/02 00:39:26 cheloha Exp $	*/
 
 /*-
  * Copyright (c) 1992, 1993
@@ -196,11 +196,12 @@ struct cpu_info {
 #define	CI_DDB_INDDB		4
 
 #ifdef DIAGNOSTIC
-	int	ci_mutex_level;
+	int		ci_mutex_level;
 #endif
 #ifdef GPROF
 	struct gmonparam *ci_gmon;
 #endif
+	char		ci_panicbuf[512];
 };
 
 #define	CPUF_PRIMARY	0x01		/* CPU is primary CPU */
diff --git a/sys/arch/powerpc/include/cpu.h b/sys/arch/powerpc/include/cpu.h
index d69fce02ee1..271169434de 100644
--- a/sys/arch/powerpc/include/cpu.h
+++ b/sys/arch/powerpc/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.68 2020/06/05 14:25:05 naddy Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.69 2021/06/02 00:39:26 cheloha Exp $	*/
 /*	$NetBSD: cpu.h,v 1.1 1996/09/30 16:34:21 ws Exp $	*/
 
 /*
@@ -92,6 +92,7 @@ struct cpu_info {
 #ifdef GPROF
 	struct gmonparam *ci_gmon;
 #endif
+	char ci_panicbuf[512];
 };
 
 static __inline struct cpu_info *
diff --git a/sys/arch/powerpc64/include/cpu.h b/sys/arch/powerpc64/include/cpu.h
index 0a1fc75a3f2..0bd905be3f8 100644
--- a/sys/arch/powerpc64/include/cpu.h
+++ b/sys/arch/powerpc64/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.29 2020/12/30 06:06:30 gkoehler Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.30 2021/06/02 00:39:27 cheloha Exp $	*/
 
 /*
  * Copyright (c) 2020 Mark Kettenis <kettenis@openbsd.org>
@@ -102,6 +102,7 @@ struct cpu_info {
 #define	CI_DDB_ENTERDDB		3
 #define	CI_DDB_INDDB		4
 #endif
+	char		ci_panicbuf[512];
 };
 
 #define CPUF_PRIMARY 		(1 << 0)
diff --git a/sys/arch/riscv64/include/cpu.h b/sys/arch/riscv64/include/cpu.h
index 91d4fad2f92..fb4846558de 100644
--- a/sys/arch/riscv64/include/cpu.h
+++ b/sys/arch/riscv64/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.6 2021/05/12 01:20:52 jsg Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.7 2021/06/02 00:39:27 cheloha Exp $	*/
 
 /*
  * Copyright (c) 2019 Mike Larkin <mlarkin@openbsd.org>
@@ -142,6 +142,7 @@ struct cpu_info {
 #ifdef GPROF
 	struct gmonparam	*ci_gmon;
 #endif
+	char			ci_panicbuf[512];
 };
 
 #define CPUF_PRIMARY		(1<<0)
diff --git a/sys/arch/sh/include/cpu.h b/sys/arch/sh/include/cpu.h
index 0d9e5ea0bfa..bf51581b84a 100644
--- a/sys/arch/sh/include/cpu.h
+++ b/sys/arch/sh/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.30 2020/09/25 14:42:39 deraadt Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.31 2021/06/02 00:39:27 cheloha Exp $	*/
 /*	$NetBSD: cpu.h,v 1.41 2006/01/21 04:24:12 uwe Exp $	*/
 
 /*-
@@ -70,6 +70,8 @@ struct cpu_info {
 #endif
 
 	int	ci_want_resched;
+
+	char	ci_panicbuf[512];
 };
 
 extern struct cpu_info cpu_info_store;
diff --git a/sys/arch/sparc64/include/cpu.h b/sys/arch/sparc64/include/cpu.h
index 267673a3adc..7c98e51432b 100644
--- a/sys/arch/sparc64/include/cpu.h
+++ b/sys/arch/sparc64/include/cpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cpu.h,v 1.96 2020/09/14 20:28:41 deraadt Exp $	*/
+/*	$OpenBSD: cpu.h,v 1.97 2021/06/02 00:39:27 cheloha Exp $	*/
 /*	$NetBSD: cpu.h,v 1.28 2001/06/14 22:56:58 thorpej Exp $ */
 
 /*
@@ -164,6 +164,7 @@ struct cpu_info {
 #ifdef GPROF
 	struct gmonparam *ci_gmon;
 #endif
+	char			ci_panicbuf[512];
 };
 
 #define CPUF_RUNNING	0x0001		/* CPU is running */
diff --git a/sys/ddb/db_command.c b/sys/ddb/db_command.c
index 04356436b5c..57f0b7c1257 100644
--- a/sys/ddb/db_command.c
+++ b/sys/ddb/db_command.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: db_command.c,v 1.90 2020/10/26 18:53:20 deraadt Exp $	*/
+/*	$OpenBSD: db_command.c,v 1.91 2021/06/02 00:39:25 cheloha Exp $	*/
 /*	$NetBSD: db_command.c,v 1.20 1996/03/30 22:30:05 christos Exp $	*/
 
 /*
@@ -468,14 +468,20 @@ db_nfsnode_print_cmd(db_expr_t addr, int have_addr, db_expr_t count,
 void
 db_show_panic_cmd(db_expr_t addr, int have_addr, db_expr_t count, char *modif)
 {
-	if (panicstr)
-		db_printf("%s\n", panicstr);
-	else if (faultstr) {
-		db_printf("kernel page fault\n");
-		db_printf("%s\n", faultstr);
-		db_stack_trace_print(addr, have_addr, 1, modif, db_printf);
+	struct cpu_info *ci;
+	char *prefix;
+	CPU_INFO_ITERATOR cii;
+	int panicked = 0;
+
+	CPU_INFO_FOREACH(cii, ci) {
+		if (ci->ci_panicbuf[0] != '\0') {
+			prefix = (panicstr == ci->ci_panicbuf) ? "*" : " ";
+			db_printf("%scpu%d: %s\n",
+			    prefix, CPU_INFO_UNIT(ci), ci->ci_panicbuf);
+			panicked = 1;
+		}
 	}
-	else
+	if (!panicked)
 		db_printf("the kernel did not panic\n");	/* yet */
 }
 
diff --git a/sys/kern/subr_prf.c b/sys/kern/subr_prf.c
index b1cff96396b..e2ad6cd97b3 100644
--- a/sys/kern/subr_prf.c
+++ b/sys/kern/subr_prf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: subr_prf.c,v 1.103 2021/05/16 15:10:20 deraadt Exp $	*/
+/*	$OpenBSD: subr_prf.c,v 1.104 2021/06/02 00:39:25 cheloha Exp $	*/
 /*	$NetBSD: subr_prf.c,v 1.45 1997/10/24 18:14:25 chuck Exp $	*/
 
 /*-
@@ -99,7 +99,6 @@ struct mutex kprintf_mutex =
 extern	int log_open;	/* subr_log: is /dev/klog open? */
 const	char *panicstr; /* arg to first call to panic (used as a flag
 			   to indicate that panic has already been called). */
-const	char *faultstr; /* page fault string */
 #ifdef DDB
 /*
  * Enter ddb on panic.
@@ -172,6 +171,18 @@ tablefull(const char *tab)
 	log(LOG_ERR, "%s: table is full\n", tab);
 }
 
+/*
+ * If we have panicked, prefer db_printf() and db_vprintf() where
+ * available.
+ */
+#ifdef DDB
+#define panic_printf(...)	db_printf(__VA_ARGS__)
+#define panic_vprintf(...)	db_vprintf(__VA_ARGS__)
+#else
+#define panic_printf(...)	printf(__VA_ARGS__)
+#define panic_vprintf(...)	vprintf(__VA_ARGS__)
+#endif
+
 /*
  * panic: handle an unresolvable fatal error
  *
@@ -183,31 +194,37 @@ tablefull(const char *tab)
 void
 panic(const char *fmt, ...)
 {
-	static char panicbuf[512];
+	struct cpu_info *ci = curcpu();
 	int bootopt;
 	va_list ap;
 
+	bootopt = RB_AUTOBOOT | RB_DUMP;
+	if (atomic_cas_ptr(&panicstr, NULL, ci->ci_panicbuf) != NULL)
+		bootopt |= RB_NOSYNC;
+
 	/* do not trigger assertions, we know that we are inconsistent */
 	splassert_ctl = 0;
 
-	/* make sure we see kernel printf output */
-	printf_flags |= TOCONS;
+#ifdef BOOT_QUIET
+	printf_flags |= TOCONS;	/* make sure we see kernel printf output */
+#endif
 
-	bootopt = RB_AUTOBOOT | RB_DUMP;
-	va_start(ap, fmt);
-	if (panicstr)
-		bootopt |= RB_NOSYNC;
-	else {
-		vsnprintf(panicbuf, sizeof panicbuf, fmt, ap);
-		panicstr = panicbuf;
+	/*
+	 * All panic messages are printed, but only the first panic on a
+	 * given CPU is written to its panicbuf.
+	 */
+	if (ci->ci_panicbuf[0] == '\0') {
+		va_start(ap, fmt);
+		vsnprintf(ci->ci_panicbuf, sizeof(ci->ci_panicbuf), fmt, ap);
+		va_end(ap);
+		panic_printf("panic: %s\n", ci->ci_panicbuf);
+	} else {
+		panic_printf("panic: ");
+		va_start(ap, fmt);
+		panic_vprintf(fmt, ap);
+		va_end(ap);
+		panic_printf("\n");
 	}
-	va_end(ap);
-
-	printf("panic: ");
-	va_start(ap, fmt);
-	vprintf(fmt, ap);
-	printf("\n");
-	va_end(ap);
 
 #ifdef DDB
 	if (db_panic)
diff --git a/sys/sys/systm.h b/sys/sys/systm.h
index d4d005a1681..a0ef9a3dd7f 100644
--- a/sys/sys/systm.h
+++ b/sys/sys/systm.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: systm.h,v 1.153 2021/04/28 09:42:04 sashan Exp $	*/
+/*	$OpenBSD: systm.h,v 1.154 2021/06/02 00:39:25 cheloha Exp $	*/
 /*	$NetBSD: systm.h,v 1.50 1996/06/09 04:55:09 briggs Exp $	*/
 
 /*-
@@ -72,7 +72,6 @@
  */
 extern int securelevel;		/* system security level */
 extern const char *panicstr;	/* panic message */
-extern const char *faultstr;	/* fault message */
 extern const char version[];		/* system version */
 extern const char copyright[];	/* system copyright */
 extern const char ostype[];