[Skiboot] [PATCH 2/4] trap based assertions

Nicholas Piggin npiggin at gmail.com
Wed Sep 25 11:01:34 AEST 2019


Using traps for assertions like Linux does gives a few advantages:
- The asm code leading to the failure condition is nicer.
- The interrupt gives a clean snapshot of machine state to dump.

The difficulty with using traps for this in OPAL is that the runtime
component will not deal well with the OS taking the 0x700 interrupt
caused by a trap in OPAL.

The long term goal is to improve the ability of the OS to inspect and
debug OPAL at runtime. For now though, the traps are patched out before
passing control to the OS, and the assert falls through to in-line
failure handling.

Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
---
 core/exceptions.c     | 40 ++++++++++++++++++++++++++++++++++-----
 core/fast-reboot.c    |  1 +
 core/init.c           | 29 +++++++++++++++++++++++++++-
 core/platform.c       |  6 +++++-
 core/utils.c          | 19 ++++---------------
 include/processor.h   |  3 +++
 include/skiboot.h     |  3 +++
 libc/include/assert.h | 44 +++++++++++++++++++++++++++++++++++--------
 libc/include/stdlib.h |  7 +------
 skiboot.lds.S         |  7 +++++++
 10 files changed, 123 insertions(+), 36 deletions(-)

diff --git a/core/exceptions.c b/core/exceptions.c
index f85327873..c50db822d 100644
--- a/core/exceptions.c
+++ b/core/exceptions.c
@@ -89,6 +89,35 @@ void exception_entry(struct stack_frame *stack)
 			"Fatal MCE at "REG"   ", nip);
 		break;
 
+	case 0x700: {
+		struct trap_table_entry *tte;
+
+		fatal = true;
+		prerror("***********************************************\n");
+		for (tte = __trap_table_start; tte < __trap_table_end; tte++) {
+			if (tte->address == nip) {
+				prerror("< %s >\n", tte->message);
+				prerror("    .\n");
+				prerror("     .\n");
+				prerror("      .\n");
+				prerror("        OO__)\n");
+				prerror("       <\"__/\n");
+				prerror("        ^ ^\n");
+				break;
+			}
+		}
+		l += snprintf(buf + l, EXCEPTION_MAX_STR - l,
+			"Fatal TRAP at "REG"   ", nip);
+		l += snprintf_symbol(buf + l, EXCEPTION_MAX_STR - l, nip);
+		l += snprintf(buf + l, EXCEPTION_MAX_STR - l, "  MSR "REG, msr);
+		prerror("%s\n", buf);
+		dump_regs(stack);
+		backtrace();
+		if (platform.terminate)
+			platform.terminate(buf);
+		for (;;) ;
+		break; }
+
 	default:
 		fatal = true;
 		prerror("***********************************************\n");
@@ -100,11 +129,12 @@ void exception_entry(struct stack_frame *stack)
 	l += snprintf(buf + l, EXCEPTION_MAX_STR - l, "  MSR "REG, msr);
 	prerror("%s\n", buf);
 	dump_regs(stack);
-
-	if (fatal)
-		abort();
-	else
-		backtrace();
+	backtrace();
+	if (fatal) {
+		if (platform.terminate)
+			platform.terminate(buf);
+		for (;;) ;
+	}
 
 	if (hv) {
 		/* Set up for SRR return */
diff --git a/core/fast-reboot.c b/core/fast-reboot.c
index 9631eb96d..a6c903f3c 100644
--- a/core/fast-reboot.c
+++ b/core/fast-reboot.c
@@ -183,6 +183,7 @@ void fast_reboot(void)
 
 	/* Restore skiboot vectors  */
 	copy_exception_vectors();
+	patch_traps(true);
 
 	/*
 	 * Secondaries may still have an issue with machine checks if they have
diff --git a/core/init.c b/core/init.c
index cd333dcbd..7dc061198 100644
--- a/core/init.c
+++ b/core/init.c
@@ -618,6 +618,8 @@ void __noreturn load_and_boot_kernel(bool is_reboot)
 	/* Disable machine checks on all */
 	cpu_disable_ME_RI_all();
 
+	patch_traps(false);
+
 	debug_descriptor.state_flags |= OPAL_BOOT_COMPLETE;
 
 	cpu_give_self_os();
@@ -758,7 +760,7 @@ static void __nomcount do_ctors(void)
 #ifndef PPC64_ELF_ABI_v2
 static void branch_null(void)
 {
-	assert_fail("Branch to NULL !");
+	assert(0);
 }
 
 
@@ -821,6 +823,28 @@ void copy_exception_vectors(void)
 	sync_icache();
 }
 
+/*
+ * When skiboot owns the exception vectors, patch in 'trap' for assert fails.
+ * Otherwise use assert_fail()
+ */
+void patch_traps(bool enable)
+{
+	struct trap_table_entry *tte;
+
+	for (tte = __trap_table_start; tte < __trap_table_end; tte++) {
+		uint32_t *insn;
+
+		insn = (uint32_t *)tte->address;
+		if (enable) {
+			*insn = PPC_INST_TRAP;
+		} else {
+			*insn = PPC_INST_NOP;
+		}
+	}
+
+	sync_icache();
+}
+
 static void per_thread_sanity_checks(void)
 {
 	struct cpu_thread *cpu = this_cpu();
@@ -950,6 +974,9 @@ void __noreturn __nomcount main_cpu_entry(const void *fdt)
 	/* Copy all vectors down to 0 */
 	copy_exception_vectors();
 
+	/* Enable trap based asserts */
+	patch_traps(true);
+
 	/*
 	 * Enable MSR[ME] bit so we can take MCEs. We don't currently
 	 * recover, but we print some useful information.
diff --git a/core/platform.c b/core/platform.c
index 307fe671a..a6b9d5649 100644
--- a/core/platform.c
+++ b/core/platform.c
@@ -108,7 +108,11 @@ static int64_t opal_cec_reboot2(uint32_t reboot_type, char *diag)
 	case OPAL_REBOOT_MPIPL:
 		prlog(PR_NOTICE, "Reboot: OS reported error. Performing MPIPL\n");
 		console_complete_flush();
-		_abort("Reboot: OS reported error. Performing MPIPL\n");
+		if (platform.terminate)
+			platform.terminate("OS reported error. Performing MPIPL\n");
+		else
+			opal_cec_reboot();
+		for (;;);
 		break;
 	default:
 		prlog(PR_NOTICE, "OPAL: Unsupported reboot request %d\n", reboot_type);
diff --git a/core/utils.c b/core/utils.c
index 728cea407..9fb27ed03 100644
--- a/core/utils.c
+++ b/core/utils.c
@@ -13,28 +13,17 @@
 #include <cpu.h>
 #include <stack.h>
 
-void __noreturn assert_fail(const char *msg)
-{
-	/**
-	 * @fwts-label FailedAssert
-	 * @fwts-advice OPAL hit an assert(). During normal usage (even
-	 * testing) we should never hit an assert. There are other code
-	 * paths for controlled shutdown/panic in the event of catastrophic
-	 * errors.
-	 */
-	prlog(PR_EMERG, "Assert fail: %s\n", msg);
-	_abort(msg);
-}
-
-void __noreturn _abort(const char *msg)
+void __noreturn assert_fail(const char *msg, const char *file,
+				unsigned int line, const char *function)
 {
 	static bool in_abort = false;
 
+	(void)function;
 	if (in_abort)
 		for (;;) ;
 	in_abort = true;
 
-	prlog(PR_EMERG, "Aborting!\n");
+	prlog(PR_EMERG, "assert failed at %s:%u: %s\n", file, line, msg);
 	backtrace();
 
 	if (platform.terminate)
diff --git a/include/processor.h b/include/processor.h
index 352fd1ec4..a0c2864a8 100644
--- a/include/processor.h
+++ b/include/processor.h
@@ -206,6 +206,9 @@
 #include <stdbool.h>
 #include <stdint.h>
 
+#define PPC_INST_NOP	0x60000000UL
+#define PPC_INST_TRAP	0x7fe00008UL
+
 #define RB(b)		(((b) & 0x1f) << 11)
 #define MSGSND(b)	stringify(.long 0x7c00019c | RB(b))
 #define MSGCLR(b)	stringify(.long 0x7c0001dc | RB(b))
diff --git a/include/skiboot.h b/include/skiboot.h
index 96d25b83d..55e1e99c0 100644
--- a/include/skiboot.h
+++ b/include/skiboot.h
@@ -41,6 +41,8 @@ extern char _stext[];
 extern char _etext[];
 extern char __sym_map_end[];
 extern char _romem_end[];
+extern struct trap_table_entry __trap_table_start[];
+extern struct trap_table_entry __trap_table_end[];
 
 #ifndef __TESTING__
 /* Readonly section start and end. */
@@ -192,6 +194,7 @@ extern bool start_preload_kernel(void);
 extern void copy_exception_vectors(void);
 extern void copy_sreset_vector(void);
 extern void copy_sreset_vector_fast_reboot(void);
+extern void patch_traps(bool enable);
 
 /* Various probe routines, to replace with an initcall system */
 extern void probe_phb3(void);
diff --git a/libc/include/assert.h b/libc/include/assert.h
index 2c49fd791..0fae0fe94 100644
--- a/libc/include/assert.h
+++ b/libc/include/assert.h
@@ -13,17 +13,45 @@
 #ifndef _ASSERT_H
 #define _ASSERT_H
 
-#define assert(cond)						\
-	do { if (!(cond)) {					\
-		     assert_fail(__FILE__			\
-				 ":" stringify(__LINE__)	\
-				 ":" stringify(cond));	}	\
-	} while(0)
-
-void __attribute__((noreturn)) assert_fail(const char *msg);
+struct trap_table_entry {
+	unsigned long address;
+	const char *message;
+};
 
 #define stringify(expr)		stringify_1(expr)
 /* Double-indirection required to stringify expansions */
 #define stringify_1(expr)	#expr
 
+void __attribute__((noreturn)) assert_fail(const char *msg,
+						const char *file,
+						unsigned int line,
+						const char *function);
+
+/*
+ * The 'nop' gets patched to 'trap' after skiboot takes over the exception
+ * vectors, then patched to 'nop' before booting the OS (see patch_traps).
+ * This makes assert fall through to assert_fail when we can't use the 0x700
+ * interrupt.
+ */
+#define assert(cond)							\
+do {									\
+	/* evaluate cond exactly once */				\
+	const unsigned long __cond = (unsigned long)(cond);		\
+	asm volatile(							\
+		"	cmpdi	%0,0"				"\n\t"	\
+		"	bne	2f"				"\n\t"	\
+		"1:	nop		# assert"		"\n\t"	\
+		"2:"						"\n\t"	\
+		".section .rodata"				"\n\t"	\
+		"3:	.string	\"assert failed at " __FILE__ ":" stringify(__LINE__) "\""	"\n\t" \
+		".previous"					"\n\t"	\
+		".section .trap_table,\"aw\""			"\n\t"	\
+		".llong	1b"					"\n\t"	\
+		".llong	3b"					"\n\t"	\
+		".previous"					"\n\t"	\
+			: : "r"(__cond) : "cr0");			\
+	if (!__cond)							\
+		assert_fail(stringify(cond), __FILE__, __LINE__, __FUNCTION__); \
+} while (0)
+
 #endif
diff --git a/libc/include/stdlib.h b/libc/include/stdlib.h
index 234bd11a1..43d5c24dd 100644
--- a/libc/include/stdlib.h
+++ b/libc/include/stdlib.h
@@ -26,11 +26,6 @@ long int strtol(const char *nptr, char **endptr, int base);
 
 int rand(void);
 long int __attribute__((const)) labs(long int n);
-void __attribute__((noreturn)) _abort(const char *msg);
-#define abort() do {					\
-               _abort("abort():" __FILE__		\
-		      ":" stringify(__LINE__));		\
-       } while(0)
-
+#define abort() assert(0)
 
 #endif
diff --git a/skiboot.lds.S b/skiboot.lds.S
index 5b4bb41a2..8890d69aa 100644
--- a/skiboot.lds.S
+++ b/skiboot.lds.S
@@ -108,6 +108,13 @@ SECTIONS
 		__rodata_end = .;
 	}
 
+	. = ALIGN(0x10);
+	.trap_table : {
+		__trap_table_start = .;
+		KEEP(*(.trap_table))
+		__trap_table_end = .;
+	}
+
 	. = ALIGN(0x10);
 	.init : {
 		__ctors_start = .;
-- 
2.23.0



More information about the Skiboot mailing list