All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bobby Eshleman <bobby.eshleman@gmail.com>
To: xen-devel@lists.xenproject.org
Cc: "Bobby Eshleman" <bobby.eshleman@gmail.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Ian Jackson" <iwj@xenproject.org>,
	"Jan Beulich" <jbeulich@suse.com>, "Wei Liu" <wl@xen.org>,
	"Elena Ufimtseva" <elena.ufimtseva@oracle.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Jun Nakajima" <jun.nakajima@intel.com>,
	"Kevin Tian" <kevin.tian@intel.com>
Subject: [PATCH 1/4] build: use common stubs for debugger_trap_* functions if !CONFIG_CRASH_DEBUG
Date: Mon, 12 Jul 2021 18:59:53 -0700	[thread overview]
Message-ID: <2fc1a1416d37b5eed0ebfdeff8bb9dd61bc7acc7.1626136452.git.bobby.eshleman@gmail.com> (raw)
In-Reply-To: <cover.1626136452.git.bobby.eshleman@gmail.com>

Previously Xen required all architectures implement the debugger_trap_*
functions whether or not it actually needs them.

This commit makes debugger_trap* functions resolve to arch-specific
function definitions if CONFIG_CRASH_DEBUG=y, but resolves to a set of
common no-op stubs if !CONFIG_CRASH_DEBUG, which avoids requiring every
arch to carry its own stubs.  This means asm/debugger.h may also be
dropped for architectures that do not need this functionality.

Inside xen/debugger.h:
    * If !CONFIG_CRASH_DEBUG, use stubs.
    * Otherwise, include arch-specific <asm/debugger.h>

Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
 xen/arch/arm/traps.c            |  2 +-
 xen/arch/x86/debug.c            |  2 +-
 xen/arch/x86/domain.c           |  5 +-
 xen/arch/x86/domctl.c           |  2 +-
 xen/arch/x86/gdbstub.c          |  4 +-
 xen/arch/x86/hvm/svm/svm.c      |  2 +-
 xen/arch/x86/hvm/vmx/realmode.c |  2 +-
 xen/arch/x86/hvm/vmx/vmx.c      |  2 +-
 xen/arch/x86/nmi.c              |  2 +-
 xen/arch/x86/traps.c            |  2 +-
 xen/arch/x86/x86_64/gdbstub.c   |  3 +-
 xen/common/domain.c             |  2 +-
 xen/common/gdbstub.c            |  3 +-
 xen/common/keyhandler.c         |  2 +-
 xen/common/shutdown.c           |  2 +-
 xen/drivers/char/console.c      |  2 +-
 xen/include/asm-arm/debugger.h  | 15 ------
 xen/include/asm-x86/debugger.h  | 66 +++++----------------------
 xen/include/xen/debugger.h      | 81 +++++++++++++++++++++++++++++++++
 19 files changed, 115 insertions(+), 86 deletions(-)
 delete mode 100644 xen/include/asm-arm/debugger.h
 create mode 100644 xen/include/xen/debugger.h

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 4ccb6e7d18..5a0a5eff1d 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -41,7 +41,7 @@
 #include <asm/acpi.h>
 #include <asm/cpuerrata.h>
 #include <asm/cpufeature.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
 #include <asm/event.h>
 #include <asm/hsr.h>
 #include <asm/mmio.h>
diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c
index d90dc93056..4386e8d1b1 100644
--- a/xen/arch/x86/debug.c
+++ b/xen/arch/x86/debug.c
@@ -19,7 +19,7 @@
 #include <xen/mm.h>
 #include <xen/domain_page.h>
 #include <xen/guest_access.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
 #include <asm/p2m.h>
 
 typedef unsigned long dbgva_t;
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index ef1812dc14..47448f2f8c 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -16,6 +16,7 @@
 #include <xen/errno.h>
 #include <xen/sched.h>
 #include <xen/domain.h>
+#include <xen/debugger.h>
 #include <xen/smp.h>
 #include <xen/delay.h>
 #include <xen/softirq.h>
@@ -2539,9 +2540,9 @@ static int __init init_vcpu_kick_softirq(void)
 }
 __initcall(init_vcpu_kick_softirq);
 
+#ifdef CONFIG_CRASH_DEBUG
 void domain_pause_for_debugger(void)
 {
-#ifdef CONFIG_CRASH_DEBUG
     struct vcpu *curr = current;
     struct domain *d = curr->domain;
 
@@ -2550,8 +2551,8 @@ void domain_pause_for_debugger(void)
     /* if gdbsx active, we just need to pause the domain */
     if ( curr->arch.gdbsx_vcpu_event == 0 )
         send_global_virq(VIRQ_DEBUGGER);
-#endif
 }
+#endif
 
 /*
  * Local variables:
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 26a76d2be9..1bc8ba7251 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -33,7 +33,7 @@
 #include <public/vm_event.h>
 #include <asm/mem_sharing.h>
 #include <asm/xstate.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
 #include <asm/psr.h>
 #include <asm/cpuid.h>
 
diff --git a/xen/arch/x86/gdbstub.c b/xen/arch/x86/gdbstub.c
index 8f4f49fd3b..f20cbf1f45 100644
--- a/xen/arch/x86/gdbstub.c
+++ b/xen/arch/x86/gdbstub.c
@@ -18,7 +18,9 @@
  * You should have received a copy of the GNU General Public License
  * along with this program; If not, see <http://www.gnu.org/licenses/>.
  */
-#include <asm/debugger.h>
+#include <asm/uaccess.h>
+#include <xen/debugger.h>
+#include <xen/gdbstub.h>
 
 u16
 gdb_arch_signal_num(struct cpu_user_regs *regs, unsigned long cookie)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 642a64b747..d7ec7c15f9 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -58,7 +58,7 @@
 #include <asm/hvm/trace.h>
 #include <asm/hap.h>
 #include <asm/apic.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
 #include <asm/hvm/monitor.h>
 #include <asm/monitor.h>
 #include <asm/xstate.h>
diff --git a/xen/arch/x86/hvm/vmx/realmode.c b/xen/arch/x86/hvm/vmx/realmode.c
index cc23afa788..1f8513c132 100644
--- a/xen/arch/x86/hvm/vmx/realmode.c
+++ b/xen/arch/x86/hvm/vmx/realmode.c
@@ -14,7 +14,7 @@
 #include <xen/sched.h>
 #include <xen/paging.h>
 #include <xen/softirq.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
 #include <asm/event.h>
 #include <asm/hvm/emulate.h>
 #include <asm/hvm/hvm.h>
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index e09b7e3af9..1820e4be0c 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -51,7 +51,7 @@
 #include <asm/hvm/trace.h>
 #include <asm/hvm/monitor.h>
 #include <asm/xenoprof.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
 #include <asm/apic.h>
 #include <asm/hvm/nestedhvm.h>
 #include <asm/altp2m.h>
diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index ab94a96c4d..eaf404402d 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -30,7 +30,7 @@
 #include <asm/msr.h>
 #include <asm/mpspec.h>
 #include <asm/nmi.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
 #include <asm/div64.h>
 #include <asm/apic.h>
 
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index e60af16ddd..44811c9599 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -62,7 +62,7 @@
 #include <asm/uaccess.h>
 #include <asm/i387.h>
 #include <asm/xstate.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
 #include <asm/msr.h>
 #include <asm/nmi.h>
 #include <asm/xenoprof.h>
diff --git a/xen/arch/x86/x86_64/gdbstub.c b/xen/arch/x86/x86_64/gdbstub.c
index 2626519c89..126af03f50 100644
--- a/xen/arch/x86/x86_64/gdbstub.c
+++ b/xen/arch/x86/x86_64/gdbstub.c
@@ -17,7 +17,8 @@
  * along with this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <asm/debugger.h>
+#include <xen/debugger.h>
+#include <xen/gdbstub.h>
 
 #define GDB_REG64(r) gdb_write_to_packet_hex(r, sizeof(u64), ctx)
 #define GDB_REG32(r)  gdb_write_to_packet_hex(r, sizeof(u32), ctx)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6b71c6d6a9..88ba680fe7 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -33,7 +33,7 @@
 #include <xen/xenoprof.h>
 #include <xen/irq.h>
 #include <xen/argo.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
 #include <asm/p2m.h>
 #include <asm/processor.h>
 #include <public/sched.h>
diff --git a/xen/common/gdbstub.c b/xen/common/gdbstub.c
index 848c1f4327..6f3d7ca72f 100644
--- a/xen/common/gdbstub.c
+++ b/xen/common/gdbstub.c
@@ -38,7 +38,8 @@
 #include <xen/serial.h>
 #include <xen/irq.h>
 #include <xen/watchdog.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
+#include <xen/gdbstub.h>
 #include <xen/init.h>
 #include <xen/param.h>
 #include <xen/smp.h>
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 8b9f378371..5c66ca0056 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -20,7 +20,7 @@
 #include <xen/mm.h>
 #include <xen/watchdog.h>
 #include <xen/init.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
 #include <asm/div64.h>
 
 static unsigned char keypress_key;
diff --git a/xen/common/shutdown.c b/xen/common/shutdown.c
index abde48aa4c..c82a4999d9 100644
--- a/xen/common/shutdown.c
+++ b/xen/common/shutdown.c
@@ -8,7 +8,7 @@
 #include <xen/shutdown.h>
 #include <xen/console.h>
 #include <xen/kexec.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
 #include <public/sched.h>
 
 /* opt_noreboot: If true, machine will need manual reset on error. */
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 7d0a603d03..060d32757f 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -26,7 +26,7 @@
 #include <xen/kexec.h>
 #include <xen/ctype.h>
 #include <xen/warning.h>
-#include <asm/debugger.h>
+#include <xen/debugger.h>
 #include <asm/div64.h>
 #include <xen/hypercall.h> /* for do_console_io */
 #include <xen/early_printk.h>
diff --git a/xen/include/asm-arm/debugger.h b/xen/include/asm-arm/debugger.h
deleted file mode 100644
index ac776efa78..0000000000
--- a/xen/include/asm-arm/debugger.h
+++ /dev/null
@@ -1,15 +0,0 @@
-#ifndef __ARM_DEBUGGER_H__
-#define __ARM_DEBUGGER_H__
-
-#define debugger_trap_fatal(v, r) (0)
-#define debugger_trap_immediate() ((void) 0)
-
-#endif /* __ARM_DEBUGGER_H__ */
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/include/asm-x86/debugger.h b/xen/include/asm-x86/debugger.h
index 99803bfd0c..38359da0a1 100644
--- a/xen/include/asm-x86/debugger.h
+++ b/xen/include/asm-x86/debugger.h
@@ -1,44 +1,26 @@
 /******************************************************************************
- * asm/debugger.h
- * 
- * Generic hooks into arch-dependent Xen.
- * 
- * 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():
- *  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).
+ * x86 Debugger Hooks
  *
- * 3. 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().
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; If not, see <http://www.gnu.org/licenses/>.
  */
-
 #ifndef __X86_DEBUGGER_H__
 #define __X86_DEBUGGER_H__
 
-#include <xen/sched.h>
 #include <asm/regs.h>
 #include <asm/processor.h>
+#include <xen/gdbstub.h>
+#include <xen/domain.h>
+#include <xen/event.h>
+#include <xen/sched.h>
 
 void domain_pause_for_debugger(void);
 
-#ifdef CONFIG_CRASH_DEBUG
-
-#include <xen/gdbstub.h>
-
 static inline bool debugger_trap_fatal(
     unsigned int vector, struct cpu_user_regs *regs)
 {
@@ -74,28 +56,4 @@ static inline bool debugger_trap_entry(
     return false;
 }
 
-#else
-
-static inline bool debugger_trap_fatal(
-    unsigned int vector, struct cpu_user_regs *regs)
-{
-    return false;
-}
-
-#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
-unsigned int dbg_rw_mem(unsigned long gva, XEN_GUEST_HANDLE_PARAM(void) buf,
-                        unsigned int len, domid_t domid, bool toaddr,
-                        uint64_t pgd3);
-#endif
-
 #endif /* __X86_DEBUGGER_H__ */
diff --git a/xen/include/xen/debugger.h b/xen/include/xen/debugger.h
new file mode 100644
index 0000000000..d6d820f4e5
--- /dev/null
+++ b/xen/include/xen/debugger.h
@@ -0,0 +1,81 @@
+/******************************************************************************
+ * Generic hooks into arch-dependent Xen.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; If not, see <http://www.gnu.org/licenses/>.
+ *
+ *
+ * Each debugger should define three 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():
+ *  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():
+ *  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__
+
+/* Dummy value used by ARM stubs. */
+#ifndef TRAP_invalid_op
+# define TRAP_invalid_op 6
+#endif
+
+#ifdef CONFIG_CRASH_DEBUG
+
+#include <asm/debugger.h>
+
+#else
+
+#include <asm/regs.h>
+#include <asm/processor.h>
+
+static inline void domain_pause_for_debugger(void)
+{
+}
+
+static inline bool debugger_trap_fatal(
+    unsigned int vector, const struct cpu_user_regs *regs)
+{
+    return false;
+}
+
+static inline void debugger_trap_immediate(void)
+{
+}
+
+static inline bool debugger_trap_entry(
+    unsigned int vector, const struct cpu_user_regs *regs)
+{
+    return false;
+}
+
+#endif /* CONFIG_CRASH_DEBUG */
+
+#ifdef CONFIG_GDBSX
+unsigned int dbg_rw_mem(unsigned long gva, XEN_GUEST_HANDLE_PARAM(void) buf,
+                        unsigned int len, domid_t domid, bool toaddr,
+                        uint64_t pgd3);
+#endif /* CONFIG_GDBSX */
+
+#endif /* __XEN_DEBUGGER_H__ */
-- 
2.30.0



       reply	other threads:[~2021-07-13  2:01 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1626136452.git.bobby.eshleman@gmail.com>
2021-07-13  1:59 ` Bobby Eshleman [this message]
2021-07-14  9:34   ` [PATCH 1/4] build: use common stubs for debugger_trap_* functions if !CONFIG_CRASH_DEBUG Jan Beulich
2021-07-14 21:03     ` Bob Eshleman
2021-07-15  6:45       ` Jan Beulich
2021-07-13  1:59 ` [PATCH 2/4] arm/traps: remove debugger_trap_fatal() calls Bobby Eshleman
2021-07-13  1:59 ` [PATCH 3/4] x86/debug: move debugger_trap_entry into debugger.c not inlined Bobby Eshleman
2021-07-14  9:52   ` Jan Beulich
2021-07-13  1:59 ` [PATCH 4/4] x86/debug: move domain_pause_for_debugger to debugger.c Bobby Eshleman
2021-07-14  9:55   ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2fc1a1416d37b5eed0ebfdeff8bb9dd61bc7acc7.1626136452.git.bobby.eshleman@gmail.com \
    --to=bobby.eshleman@gmail.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=elena.ufimtseva@oracle.com \
    --cc=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=jun.nakajima@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.