[xen staging] x86/debugger: Remove debugger_trap_entry()

[patchbot@xxxxxxx]

commit af8b805c15199831aad50bc33066494fb2cab363
Author:     Bobby Eshleman <bobby.eshleman@xxxxxxxxx>
AuthorDate: Tue Sep 28 13:30:25 2021 -0700
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Fri Apr 22 20:39:34 2022 +0100

    x86/debugger: Remove debugger_trap_entry()
    
    debugger_trap_entry() is unrelated to the other contents of debugger.h.  It 
is
    a no-op for everything other than #DB/#BP, and for those it invokes guest
    debugging (CONFIG_GDBSX) not host debugging (CONFIG_CRASH_DEBUG).
    
    The reason it is a no-op for gdbstub is related to the fact that it's
    description is inappropriate for any kind of useful debugging.  In normal
    debugging, gdb only sees things which manifest as signals; it doesn't see
    things which the kernel resolves itself (some #PF, #NM, etc).  Furthermore,
    without a mechanism to invoke pv_inject_event(), the current infrastructure
    will livelock on faults from guest context.
    
    As such, there is no plausible future matching it's description.  Any work 
to
    do something better than the current nothing will have to design something
    more coherent.
    
    Therefore, simplify everything by expanding debugger_trap_entry() into its 
two
    non-empty locations, fixing bugs with their positioning (vs early exceptions
    and curr not being safe to deference) and for #DB, deferring the pause until
    the changes in %dr6 are saved to v->arch.dr6 so the debugger can actually 
see
    which condition triggered.  This also removes some logically dead code from
    do_trap(), where the compiler can't prove that #DB/#BP are handled by
    different codepaths.
    
    Signed-off-by: Bobby Eshleman <bobby.eshleman@xxxxxxxxx>
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/include/asm/debugger.h | 42 ++-----------------------------------
 xen/arch/x86/traps.c                | 34 +++++++++++++-----------------
 2 files changed, 16 insertions(+), 60 deletions(-)

diff --git a/xen/arch/x86/include/asm/debugger.h 
b/xen/arch/x86/include/asm/debugger.h
index 221bcde137..e83b346a21 100644
--- a/xen/arch/x86/include/asm/debugger.h
+++ b/xen/arch/x86/include/asm/debugger.h
@@ -5,19 +5,12 @@
  * 
  * Each debugger should define two functions here:
  * 
- * 1. debugger_trap_entry(): 
- *  Called at start of any synchronous fault or trap, before any other work
- *  is done. The idea is that if your debugger deliberately caused the trap
- *  (e.g. to implement breakpoints or data watchpoints) then you can take
- *  appropriate action and return a non-zero value to cause early exit from
- *  the trap function.
- * 
- * 2. debugger_trap_fatal():
+ * 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).
  *
- * 3. debugger_trap_immediate():
+ * 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.
@@ -49,31 +42,6 @@ static inline bool debugger_trap_fatal(
 /* Int3 is a trivial way to gather cpu_user_regs context. */
 #define debugger_trap_immediate() __asm__ __volatile__ ( "int3" );
 
-static inline bool debugger_trap_entry(
-    unsigned int vector, struct cpu_user_regs *regs)
-{
-    /*
-     * This function is called before any checks are made.  Amongst other
-     * things, be aware that during early boot, current is not a safe pointer
-     * to follow.
-     */
-    struct vcpu *v = current;
-
-    if ( vector != TRAP_int3 && vector != TRAP_debug )
-        return false;
-
-    if ( guest_mode(regs) && guest_kernel_mode(v, regs) &&
-         v->domain->debugger_attached  )
-    {
-        if ( vector != TRAP_debug ) /* domain pause is good enough */
-            current->arch.gdbsx_vcpu_event = vector;
-        domain_pause_for_debugger();
-        return true;
-    }
-
-    return false;
-}
-
 #else
 
 static inline bool debugger_trap_fatal(
@@ -84,12 +52,6 @@ static inline bool debugger_trap_fatal(
 
 #define debugger_trap_immediate() ((void)0)
 
-static inline bool debugger_trap_entry(
-    unsigned int vector, struct cpu_user_regs *regs)
-{
-    return false;
-}
-
 #endif
 
 #ifdef CONFIG_GDBSX
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 4c38f6c015..84cd038dc3 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -946,9 +946,6 @@ void do_trap(struct cpu_user_regs *regs)
     if ( regs->error_code & X86_XEC_EXT )
         goto hardware_trap;
 
-    if ( debugger_trap_entry(trapnr, regs) )
-        return;
-
     ASSERT(trapnr < 32);
 
     if ( guest_mode(regs) )
@@ -1177,9 +1174,6 @@ void do_invalid_op(struct cpu_user_regs *regs)
     int id = -1, lineno;
     const struct virtual_region *region;
 
-    if ( debugger_trap_entry(TRAP_invalid_op, regs) )
-        return;
-
     if ( likely(guest_mode(regs)) )
     {
         if ( pv_emulate_invalid_op(regs) )
@@ -1284,8 +1278,7 @@ void do_invalid_op(struct cpu_user_regs *regs)
 
 void do_int3(struct cpu_user_regs *regs)
 {
-    if ( debugger_trap_entry(TRAP_int3, regs) )
-        return;
+    struct vcpu *curr = current;
 
     if ( !guest_mode(regs) )
     {
@@ -1299,6 +1292,13 @@ void do_int3(struct cpu_user_regs *regs)
         return;
     }
 
+    if ( guest_kernel_mode(curr, regs) && curr->domain->debugger_attached )
+    {
+        curr->arch.gdbsx_vcpu_event = TRAP_int3;
+        domain_pause_for_debugger();
+        return;
+    }
+
     pv_inject_hw_exception(TRAP_int3, X86_EVENT_NO_EC);
 }
 
@@ -1575,9 +1575,6 @@ void do_page_fault(struct cpu_user_regs *regs)
     /* fixup_page_fault() might change regs->error_code, so cache it here. */
     error_code = regs->error_code;
 
-    if ( debugger_trap_entry(TRAP_page_fault, regs) )
-        return;
-
     perfc_incr(page_faults);
 
     /* Any shadow stack access fault is a bug in Xen. */
@@ -1676,9 +1673,6 @@ void do_general_protection(struct cpu_user_regs *regs)
     struct vcpu *v = current;
 #endif
 
-    if ( debugger_trap_entry(TRAP_gp_fault, regs) )
-        return;
-
     if ( regs->error_code & X86_XEC_EXT )
         goto hardware_gp;
 
@@ -1971,9 +1965,6 @@ void do_debug(struct cpu_user_regs *regs)
     /* Stash dr6 as early as possible. */
     dr6 = read_debugreg(6);
 
-    if ( debugger_trap_entry(TRAP_debug, regs) )
-        return;
-
     /*
      * At the time of writing (March 2018), on the subject of %dr6:
      *
@@ -2082,6 +2073,12 @@ void do_debug(struct cpu_user_regs *regs)
     v->arch.dr6 |= (dr6 & ~X86_DR6_DEFAULT);
     v->arch.dr6 &= (dr6 | ~X86_DR6_DEFAULT);
 
+    if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached )
+    {
+        domain_pause_for_debugger();
+        return;
+    }
+
     pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
 }
 
@@ -2097,9 +2094,6 @@ void do_entry_CP(struct cpu_user_regs *regs)
     const char *err = "??";
     unsigned int ec = regs->error_code;
 
-    if ( debugger_trap_entry(X86_EXC_CP, regs) )
-        return;
-
     /* Decode ec if possible */
     if ( ec < ARRAY_SIZE(errors) && errors[ec][0] )
         err = errors[ec];
--
generated by git-patchbot for /home/xen/git/xen.git#staging