diff mbox series

[2/2] xen: Remove debugger.h

Message ID 20240126205418.3587177-3-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series Remove gdbstub and debugger infrastructure | expand

Commit Message

Andrew Cooper Jan. 26, 2024, 8:54 p.m. UTC
With x86 having dropped gdbstub, Xen's only debugger has gone.

Drop xen/debugger.h and remove the hooks spread around the codebase.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>

I debated splitting this up, but decided it wasn't worth the effort. There's a
lot of further cleanup that can be done, including an include tangle it seems?
---
 xen/arch/x86/include/asm/bug.h      | 20 -------------
 xen/arch/x86/include/asm/debugger.h | 30 --------------------
 xen/arch/x86/traps.c                | 30 +++-----------------
 xen/common/bug.c                    | 27 ------------------
 xen/common/keyhandler.c             | 22 +--------------
 xen/common/shutdown.c               |  2 --
 xen/drivers/char/console.c          |  3 --
 xen/include/xen/bug.h               |  4 ---
 xen/include/xen/debugger.h          | 44 -----------------------------
 9 files changed, 5 insertions(+), 177 deletions(-)
 delete mode 100644 xen/arch/x86/include/asm/debugger.h
 delete mode 100644 xen/include/xen/debugger.h

Comments

Jan Beulich Jan. 30, 2024, 9:54 a.m. UTC | #1
On 26.01.2024 21:54, Andrew Cooper wrote:
> --- a/xen/arch/x86/include/asm/bug.h
> +++ b/xen/arch/x86/include/asm/bug.h
> @@ -1,28 +1,8 @@
>  #ifndef __X86_BUG_H__
>  #define __X86_BUG_H__
>  
> -/*
> - * Please do not include in the header any header that might
> - * use BUG/ASSERT/etc maros asthey will be defined later after
> - * the return to <xen/bug.h> from the current header:
> - * 
> - * <xen/bug.h>:
> - *  ...
> - *   <asm/bug.h>:
> - *     ...
> - *     <any_header_which_uses_BUG/ASSERT/etc macros.h>
> - *     ...
> - *  ...
> - *  #define BUG() ...
> - *  ...
> - *  #define ASSERT() ...
> - *  ...
> - */
> -

Leaving aside the clumsiness of the initial text, other than in common/bug.c
I don't see why this comment is to go away (and, if at all, in this very
patch). With it retained or the removal suitably explained
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

>  #ifndef __ASSEMBLY__
>  
> -#define BUG_DEBUGGER_TRAP_FATAL(regs) debugger_trap_fatal(X86_EXC_GP,regs)
> -
>  #define BUG_INSTR       "ud2"
>  #define BUG_ASM_CONST   "c"
>
diff mbox series

Patch

diff --git a/xen/arch/x86/include/asm/bug.h b/xen/arch/x86/include/asm/bug.h
index 7fe879a8ccb9..a9e99786a7bb 100644
--- a/xen/arch/x86/include/asm/bug.h
+++ b/xen/arch/x86/include/asm/bug.h
@@ -1,28 +1,8 @@ 
 #ifndef __X86_BUG_H__
 #define __X86_BUG_H__
 
-/*
- * Please do not include in the header any header that might
- * use BUG/ASSERT/etc maros asthey will be defined later after
- * the return to <xen/bug.h> from the current header:
- * 
- * <xen/bug.h>:
- *  ...
- *   <asm/bug.h>:
- *     ...
- *     <any_header_which_uses_BUG/ASSERT/etc macros.h>
- *     ...
- *  ...
- *  #define BUG() ...
- *  ...
- *  #define ASSERT() ...
- *  ...
- */
-
 #ifndef __ASSEMBLY__
 
-#define BUG_DEBUGGER_TRAP_FATAL(regs) debugger_trap_fatal(X86_EXC_GP,regs)
-
 #define BUG_INSTR       "ud2"
 #define BUG_ASM_CONST   "c"
 
diff --git a/xen/arch/x86/include/asm/debugger.h b/xen/arch/x86/include/asm/debugger.h
deleted file mode 100644
index a5c299c6c34d..000000000000
--- a/xen/arch/x86/include/asm/debugger.h
+++ /dev/null
@@ -1,30 +0,0 @@ 
-/* SPDX-License-Identifier: GPL-2.0 */
-/******************************************************************************
- * xen/arch/x86/include/asm/debugger.h
- *
- * x86-specific debugger hooks.
- */
-#ifndef __X86_DEBUGGER_H__
-#define __X86_DEBUGGER_H__
-
-#include <xen/gdbstub.h>
-#include <xen/stdbool.h>
-
-#include <asm/x86-defns.h>
-
-/* Returns true if GDB handled the trap, or it is surviveable. */
-static inline bool debugger_trap_fatal(
-    unsigned int vector, struct cpu_user_regs *regs)
-{
-    int rc = __trap_to_gdb(regs, vector);
-
-    if ( rc == 0 )
-        return true;
-
-    return vector == X86_EXC_BP;
-}
-
-/* Int3 is a trivial way to gather cpu_user_regs context. */
-#define debugger_trap_immediate() __asm__ __volatile__ ( "int3" )
-
-#endif /* __X86_DEBUGGER_H__ */
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 7724306116e9..5f4bcc0537f7 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -25,7 +25,6 @@ 
 #include <xen/shutdown.h>
 #include <xen/guest_access.h>
 #include <asm/regs.h>
-#include <xen/debugger.h>
 #include <xen/delay.h>
 #include <xen/event.h>
 #include <xen/spinlock.h>
@@ -835,11 +834,6 @@  void fatal_trap(const struct cpu_user_regs *regs, bool show_remote)
 
 void asmlinkage do_unhandled_trap(struct cpu_user_regs *regs)
 {
-    unsigned int trapnr = regs->entry_vector;
-
-    if ( debugger_trap_fatal(trapnr, regs) )
-        return;
-
     fatal_trap(regs, false);
 }
 
@@ -941,9 +935,6 @@  void asmlinkage do_trap(struct cpu_user_regs *regs)
         return;
 
  hardware_trap:
-    if ( debugger_trap_fatal(trapnr, regs) )
-        return;
-
     fatal_trap(regs, false);
 }
 
@@ -1190,9 +1181,6 @@  void asmlinkage do_invalid_op(struct cpu_user_regs *regs)
     if ( likely(extable_fixup(regs, true)) )
         return;
 
-    if ( debugger_trap_fatal(X86_EXC_UD, regs) )
-        return;
-
     show_execution_state(regs);
     panic("FATAL TRAP: vector = %d (invalid opcode)\n", X86_EXC_UD);
 }
@@ -1206,9 +1194,8 @@  void asmlinkage do_int3(struct cpu_user_regs *regs)
         if ( likely(extable_fixup(regs, true)) )
             return;
 
-        if ( !debugger_trap_fatal(X86_EXC_BP, regs) )
-            printk(XENLOG_DEBUG "Hit embedded breakpoint at %p [%ps]\n",
-                   _p(regs->rip), _p(regs->rip));
+        printk(XENLOG_DEBUG "Hit embedded breakpoint at %p [%ps]\n",
+               _p(regs->rip), _p(regs->rip));
 
         return;
     }
@@ -1294,9 +1281,6 @@  void do_general_protection(struct cpu_user_regs *regs)
         return;
 
  hardware_gp:
-    if ( debugger_trap_fatal(X86_EXC_GP, regs) )
-        return;
-
     show_execution_state(regs);
     panic("GENERAL PROTECTION FAULT\n[error_code=%04x]\n", regs->error_code);
 }
@@ -1620,9 +1604,6 @@  void asmlinkage do_page_fault(struct cpu_user_regs *regs)
         }
 
     fatal:
-        if ( debugger_trap_fatal(X86_EXC_PF, regs) )
-            return;
-
         show_execution_state(regs);
         show_page_walk(addr);
         panic("FATAL PAGE FAULT\n"
@@ -1933,11 +1914,8 @@  void asmlinkage do_debug(struct cpu_user_regs *regs)
                 return;
             }
 #endif
-            if ( !debugger_trap_fatal(X86_EXC_DB, regs) )
-            {
-                WARN();
-                regs->eflags &= ~X86_EFLAGS_TF;
-            }
+            WARN();
+            regs->eflags &= ~X86_EFLAGS_TF;
         }
 
         /*
diff --git a/xen/common/bug.c b/xen/common/bug.c
index ca166e102b4b..5fbe703a81b3 100644
--- a/xen/common/bug.c
+++ b/xen/common/bug.c
@@ -1,23 +1,4 @@ 
 #include <xen/bug.h>
-/*
- * Ideally <xen/debugger.h> should be included in <asm/bug.h>
- * but an issue with compilation can occur as <xen/debugger.h> uses
- * BUG/ASSERT/etc macros inside but they will be defined later in
- * <xen/bug.h> after return from inclusion of <asm/bug.h>:
- * 
- * <xen/bug.h>:
- *  ...
- *   <asm/bug.h>:
- *     ...
- *     <xen/debugger.h> -> some of included header in it uses BUG/ASSERT/etc
- *     ...
- *  ...
- *  #define BUG() ...
- *  ...
- *  #define ASSERT() ...
- *  ...
- */
-#include <xen/debugger.h>
 #include <xen/errno.h>
 #include <xen/kernel.h>
 #include <xen/livepatch.h>
@@ -96,10 +77,6 @@  int do_bug_frame(struct cpu_user_regs *regs, unsigned long pc)
 
     case BUGFRAME_bug:
         printk("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
-
-        if ( BUG_DEBUGGER_TRAP_FATAL(regs) )
-            break;
-
         show_execution_state(regs);
         panic("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
 
@@ -111,10 +88,6 @@  int do_bug_frame(struct cpu_user_regs *regs, unsigned long pc)
 
         printk("Assertion '%s' failed at %s%s:%d\n",
                predicate, prefix, filename, lineno);
-
-        if ( BUG_DEBUGGER_TRAP_FATAL(regs) )
-            break;
-
         show_execution_state(regs);
         panic("Assertion '%s' failed at %s%s:%d\n",
               predicate, prefix, filename, lineno);
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 99a2d72a0202..f131034d33ee 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -3,7 +3,6 @@ 
  */
 
 #include <asm/regs.h>
-#include <xen/debugger.h>
 #include <xen/delay.h>
 #include <xen/keyhandler.h>
 #include <xen/param.h>
@@ -29,8 +28,7 @@  static bool alt_key_handling;
 static keyhandler_fn_t cf_check show_handlers, cf_check dump_hwdom_registers,
     cf_check dump_domains, cf_check read_clocks;
 static irq_keyhandler_fn_t cf_check do_toggle_alt_key, cf_check dump_registers,
-    cf_check reboot_machine, cf_check run_all_keyhandlers,
-    cf_check do_debug_key;
+    cf_check reboot_machine, cf_check run_all_keyhandlers;
 
 static struct keyhandler {
     union {
@@ -57,7 +55,6 @@  static struct keyhandler {
     IRQ_KEYHANDLER('R', reboot_machine, "reboot machine", 0),
         KEYHANDLER('t', read_clocks, "display multi-cpu clock info", 1),
         KEYHANDLER('0', dump_hwdom_registers, "dump Dom0 registers", 1),
-    IRQ_KEYHANDLER('%', do_debug_key, "trap to xendbg", 0),
     IRQ_KEYHANDLER('*', run_all_keyhandlers, "print all diagnostics", 0),
 
 #ifdef CONFIG_PERF_COUNTERS
@@ -503,23 +500,6 @@  static void cf_check run_all_keyhandlers(
     tasklet_schedule(&run_all_keyhandlers_tasklet);
 }
 
-static void cf_check do_debugger_trap_fatal(struct cpu_user_regs *regs)
-{
-    (void)debugger_trap_fatal(0xf001, regs);
-
-    /* Prevent tail call optimisation, which confuses xendbg. */
-    barrier();
-}
-
-static void cf_check do_debug_key(unsigned char key, struct cpu_user_regs *regs)
-{
-    printk("'%c' pressed -> trapping into debugger\n", key);
-    if ( regs )
-        do_debugger_trap_fatal(regs);
-    else
-        run_in_exception_handler(do_debugger_trap_fatal);
-}
-
 static void cf_check do_toggle_alt_key(
     unsigned char key, struct cpu_user_regs *regs)
 {
diff --git a/xen/common/shutdown.c b/xen/common/shutdown.c
index dfd5e41097e9..5f8141edc6b2 100644
--- a/xen/common/shutdown.c
+++ b/xen/common/shutdown.c
@@ -2,7 +2,6 @@ 
 #include <xen/lib.h>
 #include <xen/param.h>
 #include <xen/sched.h>
-#include <xen/debugger.h>
 #include <xen/domain.h>
 #include <xen/delay.h>
 #include <xen/watchdog.h>
@@ -40,7 +39,6 @@  void hwdom_shutdown(u8 reason)
         machine_halt();
 
     case SHUTDOWN_crash:
-        debugger_trap_immediate();
         printk("Hardware Dom%u crashed: ", hardware_domain->domain_id);
         kexec_crash(CRASHREASON_HWDOM);
         reboot_or_halt();
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 0666564ec92e..dce0226e8711 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -15,7 +15,6 @@ 
 #include <xen/init.h>
 #include <xen/event.h>
 #include <xen/console.h>
-#include <xen/debugger.h>
 #include <xen/param.h>
 #include <xen/serial.h>
 #include <xen/softirq.h>
@@ -1297,8 +1296,6 @@  void panic(const char *fmt, ...)
 
     spin_unlock_irqrestore(&lock, flags);
 
-    debugger_trap_immediate();
-
     kexec_crash(CRASHREASON_PANIC);
 
     if ( opt_noreboot )
diff --git a/xen/include/xen/bug.h b/xen/include/xen/bug.h
index 8cca4486a477..4bd0be508825 100644
--- a/xen/include/xen/bug.h
+++ b/xen/include/xen/bug.h
@@ -16,10 +16,6 @@ 
 
 #ifndef __ASSEMBLY__
 
-#ifndef BUG_DEBUGGER_TRAP_FATAL
-#define BUG_DEBUGGER_TRAP_FATAL(regs) 0
-#endif
-
 #include <xen/macros.h>
 #include <xen/types.h>
 
diff --git a/xen/include/xen/debugger.h b/xen/include/xen/debugger.h
deleted file mode 100644
index 72684268aff7..000000000000
--- a/xen/include/xen/debugger.h
+++ /dev/null
@@ -1,44 +0,0 @@ 
-/* SPDX-License-Identifier: GPL-2.0 */
-/******************************************************************************
- * Arch specific debuggers should implement:
- *
- * debugger_trap_fatal():
- *  Called when Xen is about to give up and crash. Typically you will use this
- *  hook to drop into a debug session. It can also be used to hook off
- *  deliberately caused traps (which you then handle and return non-zero).
- *
- * debugger_trap_immediate():
- *  Called if we want to drop into a debugger now.  This is essentially the
- *  same as debugger_trap_fatal, except that we use the current register state
- *  rather than the state which was in effect when we took the trap.
- *  For example: if we're dying because of an unhandled exception, we call
- *  debugger_trap_fatal; if we're dying because of a panic() we call
- *  debugger_trap_immediate().
- */
-
-#ifndef __XEN_DEBUGGER_H__
-#define __XEN_DEBUGGER_H__
-
-#ifdef CONFIG_CRASH_DEBUG
-
-#include <asm/debugger.h>
-
-#else
-
-#include <xen/stdbool.h>
-
-struct cpu_user_regs;
-
-static inline bool debugger_trap_fatal(
-    unsigned int vector, const struct cpu_user_regs *regs)
-{
-    return false;
-}
-
-static inline void debugger_trap_immediate(void)
-{
-}
-
-#endif /* CONFIG_CRASH_DEBUG */
-
-#endif /* __XEN_DEBUGGER_H__ */