[Skiboot] [RFC PATCH 2/2] trap based assertions
Nicholas Piggin
npiggin at gmail.com
Sun Sep 15 17:16:47 AEST 2019
Using traps for assertions gives you a few advantages, the asm code is
nicer, and the interrupt gives you a clean break to snapshot all state
and dump it all out. This is impossible if you try to handle the
assertion in the failing context, because printing things out alters
what you are trying to collect.
The problem with doing this in OPAL of course is that 0x700 goes to the
OS after it boots.
Linux can be taught about handling interrupts that occur in OPAL context,
and the dump / xmon code can make an OPAL call to get assertion messages
and unwind the OPAL stack. This is really the best way of doing things
because you unwind out the OPAL call and back up the Linux stack as well.
Problem of course is coping with OS that doesn't do this. Currently I
think Linux would just crap out pretty badly.
We really should be handling this better *anyway* because of 0x100 and
0x200 interrupts that we can take in OPAL context at any time.
Anyway, early in boot the OS could make an OPAL call to say it can deal
with these traps, but problem is they are statically compiled. So I guess
we'd have to compile both code paths and select between them at runtime.
That'll make the asm not quite so clean, but I think the advantage will
still be worth it.
I haven't implemented that in this patch. We have to think about what
kind of OPAL interrupt handling / debugging facilities to add to the
OPAL API.
Comments?
---
core/exceptions.c | 44 ++++++++++++++++++++++++++++++++++++++-----
core/init.c | 2 +-
core/platform.c | 6 +++++-
core/utils.c | 33 ++++++--------------------------
libc/include/assert.h | 33 ++++++++++++++++++++++++--------
libc/include/stdlib.h | 7 +------
skiboot.lds.S | 7 +++++++
7 files changed, 84 insertions(+), 48 deletions(-)
diff --git a/core/exceptions.c b/core/exceptions.c
index f85327873..c2ce12f9d 100644
--- a/core/exceptions.c
+++ b/core/exceptions.c
@@ -32,6 +32,10 @@ static void dump_regs(struct stack_frame *stack)
#define EXCEPTION_MAX_STR 320
+extern struct trap_table_entry __trap_table_start[];
+extern struct trap_table_entry __trap_table_end[];
+extern char _start[];
+
void exception_entry(struct stack_frame *stack)
{
bool fatal = false;
@@ -89,6 +93,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 +133,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/init.c b/core/init.c
index cd333dcbd..99e58638c 100644
--- a/core/init.c
+++ b/core/init.c
@@ -758,7 +758,7 @@ static void __nomcount do_ctors(void)
#ifndef PPC64_ELF_ABI_v2
static void branch_null(void)
{
- assert_fail("Branch to NULL !");
+ assert(0);
}
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..456beef6d 100644
--- a/core/utils.c
+++ b/core/utils.c
@@ -13,34 +13,13 @@
#include <cpu.h>
#include <stack.h>
-void __noreturn assert_fail(const char *msg)
+void __attribute__((noreturn)) assert_fail(const char *msg,
+ const char *file,
+ unsigned int line,
+ const char *function)
{
- /**
- * @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)
-{
- static bool in_abort = false;
-
- if (in_abort)
- for (;;) ;
- in_abort = true;
-
- prlog(PR_EMERG, "Aborting!\n");
- backtrace();
-
- if (platform.terminate)
- platform.terminate(msg);
-
- for (;;) ;
+ prlog(PR_EMERG, "%s %s:%u (%s)\n", msg, file, line, function);
+ assert(0);
}
char __attrconst tohex(uint8_t nibble)
diff --git a/libc/include/assert.h b/libc/include/assert.h
index 2c49fd791..422dc0258 100644
--- a/libc/include/assert.h
+++ b/libc/include/assert.h
@@ -13,17 +13,34 @@
#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
+#define assert(cond) \
+do { \
+ asm volatile( \
+ "1: tdeqi %0,0 # assert" "\n\t" \
+ ".section .rodata" "\n\t" \
+ "2: .string \"assert failed at " __FILE__ ":" stringify(__LINE__) "\"" "\n\t" \
+ ".previous" "\n\t" \
+ ".section .trap_table,\"aw\"" "\n\t" \
+ ".llong 1b" "\n\t" \
+ ".llong 2b" "\n\t" \
+ ".previous" "\n\t" \
+ : : "r"((unsigned long)!!(cond))); \
+ if (__builtin_constant_p(cond) && !(cond)) \
+ for (;;); \
+} while (0)
+
+void __attribute__((noreturn)) assert_fail(const char *msg,
+ const char *file,
+ unsigned int line,
+ const char *function);
+
#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