[PATCH v2 0/3] xen/x86: Improvements to asm assertions

[PATCH v2 0/3] xen/x86: Improvements to asm assertions

[Andrew Cooper]

In XenServer, we have a single server which occasionally finds interrupts
unexpectedtly in the wrong state (I suspect the SMM handler).  However,
asserts from ASSERT_INTERRUPTS_{EN,DIS}ABLED are quite opaque, being an
unqualified fatal #UD.

Changes in v2:
 * Use mergable string sections
 * Use \@ rather than raw numbers
 * Style fixes

Andrew Cooper (3):
  xen/x86: Infrastructure to create BUG_FRAMES in asm code
  xen/x86: Use real assert frames for ASSERT_INTERRUPTS_{EN,DIS}ABLED
  DO NOT APPLY - test code for this series

 xen/arch/x86/traps.c            |   66 +++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/x86_64/entry.S     |   26 +++++++++++++++
 xen/include/asm-x86/asm_defns.h |   13 +++++---
 xen/include/asm-x86/bug.h       |   50 ++++++++++++++++++++++++++---
 4 files changed, 145 insertions(+), 10 deletions(-)

-- 
1.7.10.4

[PATCH v2 1/3] xen/x86: Infrastructure to create BUG_FRAMES in asm code

[Andrew Cooper]

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>

---
v2: Use mergable string sections, \@ instead of raw numbers, indentation fixes.
---
 xen/include/asm-x86/bug.h |   50 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/xen/include/asm-x86/bug.h b/xen/include/asm-x86/bug.h
index cd862e3..f1a8c9d 100644
--- a/xen/include/asm-x86/bug.h
+++ b/xen/include/asm-x86/bug.h
@@ -5,6 +5,13 @@
 #define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
 #define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
 
+#define BUGFRAME_run_fn 0
+#define BUGFRAME_warn   1
+#define BUGFRAME_bug    2
+#define BUGFRAME_assert 3
+
+#ifndef __ASSEMBLY__
+
 struct bug_frame {
     signed int loc_disp:BUG_DISP_WIDTH;
     unsigned int line_hi:BUG_LINE_HI_WIDTH;
@@ -22,11 +29,6 @@ struct bug_frame {
                       ((1 << BUG_LINE_LO_WIDTH) - 1)))
 #define bug_msg(b) ((const char *)(b) + (b)->msg_disp[1])
 
-#define BUGFRAME_run_fn 0
-#define BUGFRAME_warn   1
-#define BUGFRAME_bug    2
-#define BUGFRAME_assert 3
-
 #define BUG_FRAME(type, line, ptr, second_frame, msg) do {                   \
     BUILD_BUG_ON((line) >> (BUG_LINE_LO_WIDTH + BUG_LINE_HI_WIDTH));         \
     asm volatile ( ".Lbug%=: ud2\n"                                          \
@@ -66,4 +68,42 @@ struct bug_frame {
                               __stop_bug_frames_2[],
                               __stop_bug_frames_3[];
 
+#else  /* !__ASSEMBLY__ */
+
+/*
+ * Construct a bugframe, suitable for using in assembly code.  Should always
+ * match the C version above.  One complication is having to stash the strings
+ * in .rodata
+ */
+    .macro BUG_FRAME type, line, file_str, second_frame, msg
+    .L\@ud: ud2a
+
+    .pushsection .rodata.str1, "aMS", @progbits, 1
+         .L\@s1: .string8 "\file_str"
+    .popsection
+
+    .pushsection .bug_frames.\type, "a", @progbits
+        .L\@bf:
+        .long (.L\@ud - .L\@bf) + \
+               ((\line >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)
+        .long (.L\@s1 - .L\@bf) + \
+               ((\line & ((1 << BUG_LINE_LO_WIDTH) - 1)) << BUG_DISP_WIDTH)
+
+        .if \second_frame
+            .pushsection .rodata.str1, "aMS", @progbits, 1
+                .L\@s2: .string8 "\msg"
+            .popsection
+            .long 0, (.L\@s2 - .L\@bf)
+        .endif
+    .popsection
+    .endm
+
+#define WARN BUG_FRAME BUGFRAME_warn, __LINE__, __FILE__, 0, 0
+#define BUG  BUG_FRAME BUGFRAME_bug,  __LINE__, __FILE__, 0, 0
+
+#define ASSERT_FAILED(msg)                                      \
+     BUG_FRAME BUGFRAME_assert, __LINE__, __FILE__, 1, msg
+
+#endif /* !__ASSEMBLY__ */
+
 #endif /* __X86_BUG_H__ */
-- 
1.7.10.4

[PATCH v2 2/3] xen/x86: Use real assert frames for ASSERT_INTERRUPTS_{EN, DIS}ABLED

[Andrew Cooper]

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>

---
v2: Pass msg as a second parameter rather than duplicating ASSERT_INTERRUPT_STATUS
---
 xen/include/asm-x86/asm_defns.h |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index 1674c7c..7c8c2c0 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -6,6 +6,7 @@
 /* NB. Auto-generated from arch/.../asm-offsets.c */
 #include <asm/asm-offsets.h>
 #endif
+#include <asm/bug.h>
 #include <asm/processor.h>
 #include <asm/percpu.h>
 #include <xen/stringify.h>
@@ -26,18 +27,20 @@
 #endif
 
 #ifndef NDEBUG
-#define ASSERT_INTERRUPT_STATUS(x)              \
+#define ASSERT_INTERRUPT_STATUS(x, msg)         \
         pushf;                                  \
         testb $X86_EFLAGS_IF>>8,1(%rsp);        \
         j##x  1f;                               \
-        ud2a;                                   \
+        ASSERT_FAILED(msg);                     \
 1:      addq  $8,%rsp;
 #else
-#define ASSERT_INTERRUPT_STATUS(x)
+#define ASSERT_INTERRUPT_STATUS(x, msg)
 #endif
 
-#define ASSERT_INTERRUPTS_ENABLED  ASSERT_INTERRUPT_STATUS(nz)
-#define ASSERT_INTERRUPTS_DISABLED ASSERT_INTERRUPT_STATUS(z)
+#define ASSERT_INTERRUPTS_ENABLED \
+    ASSERT_INTERRUPT_STATUS(nz, "INTERRUPTS ENABLED")
+#define ASSERT_INTERRUPTS_DISABLED \
+    ASSERT_INTERRUPT_STATUS(z, "INTERRUPTS DISABLED")
 
 /*
  * This flag is set in an exception frame when registers R12-R15 did not get
-- 
1.7.10.4

[PATCH v2 3/3] DO NOT APPLY - test code for this series

[Andrew Cooper]

---
 xen/arch/x86/traps.c        |   66 +++++++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/x86_64/entry.S |   26 +++++++++++++++++
 2 files changed, 92 insertions(+)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 22cdfc4..c562842 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3981,6 +3981,72 @@ void asm_domain_crash_synchronous(unsigned long addr)
     __domain_crash_synchronous();
 }
 
+#include <xen/keyhandler.h>
+
+void asm_test_warnframe(void);
+void asm_test_bugframe(void);
+void asm_test_assertframe(void);
+void asm_test_assert_enabled(void);
+void asm_test_assert_disabled(void);
+
+static void do_extreme_debug(unsigned char key, struct cpu_user_regs *regs)
+{
+    printk("'%c' pressed -> Extreme debugging in progress...\n", key);
+
+    switch ( key )
+    {
+    case '1':
+        printk("WARN frame:\n");
+        asm_test_warnframe();
+        printk("Done\n");
+        break;
+
+    case '2':
+        printk("BUG frame:\n");
+        asm_test_bugframe();
+        printk("Done\n");
+        break;
+
+    case '3':
+        printk("ASSERT frame:\n");
+        asm_test_assertframe();
+        printk("Done\n");
+        break;
+
+    case '4':
+        printk("Trip ASSERT_IRQ_ENABLED:\n");
+        asm_test_assert_enabled();
+        printk("Done\n");
+        break;
+
+    case '5':
+        printk("Trip ASSERT_IRQ_DISABLED:\n");
+        asm_test_assert_disabled();
+        printk("Done\n");
+        break;
+
+    default:
+        printk("Nothing\n");
+    }
+}
+
+static struct keyhandler extreme_debug_keyhandler = {
+    .irq_callback = 1,
+    .u.irq_fn = do_extreme_debug,
+    .desc = "Extreme debugging"
+};
+
+static int __init extreme_debug_keyhandler_init(void)
+{
+    register_keyhandler('1', &extreme_debug_keyhandler);
+    register_keyhandler('2', &extreme_debug_keyhandler);
+    register_keyhandler('3', &extreme_debug_keyhandler);
+    register_keyhandler('4', &extreme_debug_keyhandler);
+    register_keyhandler('5', &extreme_debug_keyhandler);
+    return 0;
+}
+__initcall(extreme_debug_keyhandler_init);
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 2d25d57..08a2eb8 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -13,6 +13,32 @@
 #include <public/xen.h>
 #include <irq_vectors.h>
 
+ENTRY(asm_test_warnframe)
+        WARN
+        ret
+
+ENTRY(asm_test_bugframe)
+        BUG
+        ret
+
+ENTRY(asm_test_assertframe)
+	ASSERT_FAILED("asm assert frame")
+	ret
+
+ENTRY(asm_test_assert_enabled)
+        pushfq
+        cli
+	ASSERT_INTERRUPTS_ENABLED
+        popfq
+	ret
+
+ENTRY(asm_test_assert_disabled)
+        pushfq
+        sti
+	ASSERT_INTERRUPTS_DISABLED
+        popfq
+	ret
+
         ALIGN
 /* %rbx: struct vcpu */
 switch_to_kernel:
-- 
1.7.10.4