* [PATCH v4 0/6] Remove unconditional arch dependency on asm/debugger.h
@ 2021-09-28 20:30 Bobby Eshleman
2021-09-28 20:30 ` [PATCH v4 1/6] arm/traps: remove debugger_trap_fatal() calls Bobby Eshleman
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Bobby Eshleman @ 2021-09-28 20:30 UTC (permalink / raw)
To: xen-devel
Cc: Bobby Eshleman, Stefano Stabellini, Julien Grall,
Volodymyr Babchuk, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
This series removes the unconditional requirement that all architectures
implement asm/debugger.h. It additionally removes arm's debugger.h and
disentangles some of the x86 gdbsx/gdbstub/generic debugger code.
Additionally, this series does the following:
- Provides generic stubs when !CONFIG_CRASH_DEBUG
- Adds stronger separation between gdbstub, gdbsx, and generic debugger
code.
v4 simply includes the review feedback from v3 with no other big changes
(as was the case for v3 in comparison to v2).
Bobby Eshleman (6):
arm/traps: remove debugger_trap_fatal() calls
x86/debugger: separate Xen and guest debugging debugger_trap_*
functions
arch/x86: rename debug.c to gdbsx.c
x86/gdbsx: expand dbg_rw_mem() inline
arch/x86: move domain_pause_for_debugger() to domain.h
x86: change asm/debugger.h to xen/debugger.h
xen/arch/arm/traps.c | 7 ----
xen/arch/x86/Makefile | 2 +-
xen/arch/x86/domain.c | 2 +-
xen/arch/x86/domctl.c | 12 +-----
xen/arch/x86/{debug.c => gdbsx.c} | 28 ++++++-------
xen/arch/x86/nmi.c | 1 -
xen/arch/x86/traps.c | 52 +++++++++++++++----------
xen/common/domain.c | 2 +-
xen/common/gdbstub.c | 2 +-
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 | 65 +------------------------------
xen/include/asm-x86/domain.h | 2 +
xen/include/asm-x86/gdbsx.h | 19 +++++++++
xen/include/xen/debugger.h | 51 ++++++++++++++++++++++++
17 files changed, 125 insertions(+), 141 deletions(-)
rename xen/arch/x86/{debug.c => gdbsx.c} (89%)
delete mode 100644 xen/include/asm-arm/debugger.h
create mode 100644 xen/include/asm-x86/gdbsx.h
create mode 100644 xen/include/xen/debugger.h
--
2.32.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 1/6] arm/traps: remove debugger_trap_fatal() calls
2021-09-28 20:30 [PATCH v4 0/6] Remove unconditional arch dependency on asm/debugger.h Bobby Eshleman
@ 2021-09-28 20:30 ` Bobby Eshleman
2021-09-28 21:01 ` Andrew Cooper
2021-09-28 20:30 ` [PATCH v4 2/6] x86/debugger: separate Xen and guest debugging debugger_trap_* functions Bobby Eshleman
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Bobby Eshleman @ 2021-09-28 20:30 UTC (permalink / raw)
To: xen-devel
Cc: Bobby Eshleman, Stefano Stabellini, Julien Grall,
Volodymyr Babchuk, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
ARM doesn't actually use debugger_trap_* anything, and is stubbed out.
This commit simply removes the unneeded calls.
Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
xen/arch/arm/traps.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 4ccb6e7d18..889650ba63 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -41,7 +41,6 @@
#include <asm/acpi.h>
#include <asm/cpuerrata.h>
#include <asm/cpufeature.h>
-#include <asm/debugger.h>
#include <asm/event.h>
#include <asm/hsr.h>
#include <asm/mmio.h>
@@ -1266,10 +1265,6 @@ int do_bug_frame(const struct cpu_user_regs *regs, vaddr_t pc)
case BUGFRAME_bug:
printk("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
-
- if ( debugger_trap_fatal(TRAP_invalid_op, regs) )
- return 0;
-
show_execution_state(regs);
panic("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
@@ -1281,8 +1276,6 @@ int do_bug_frame(const struct cpu_user_regs *regs, vaddr_t pc)
printk("Assertion '%s' failed at %s%s:%d\n",
predicate, prefix, filename, lineno);
- if ( debugger_trap_fatal(TRAP_invalid_op, regs) )
- return 0;
show_execution_state(regs);
panic("Assertion '%s' failed at %s%s:%d\n",
predicate, prefix, filename, lineno);
--
2.32.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 2/6] x86/debugger: separate Xen and guest debugging debugger_trap_* functions
2021-09-28 20:30 [PATCH v4 0/6] Remove unconditional arch dependency on asm/debugger.h Bobby Eshleman
2021-09-28 20:30 ` [PATCH v4 1/6] arm/traps: remove debugger_trap_fatal() calls Bobby Eshleman
@ 2021-09-28 20:30 ` Bobby Eshleman
2021-09-28 20:52 ` Andrew Cooper
2021-09-28 20:30 ` [PATCH v4 3/6] arch/x86: rename debug.c to gdbsx.c Bobby Eshleman
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Bobby Eshleman @ 2021-09-28 20:30 UTC (permalink / raw)
To: xen-devel
Cc: Bobby Eshleman, Stefano Stabellini, Julien Grall,
Volodymyr Babchuk, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
The functions debugger_trap_fatal(), debugger_trap_immediate(), and
debugger_trap_entry() are generic hook functions for debugger support.
In practice, debugger_trap_fatal() and debugger_trap_immediate() are
only used in the debugging of Xen itself and debugger_trap_entry() is
only used in the debugging of guests. That is, debugger_trap_entry() is
part of gdbsx functionality and not the Xen gdstub. This is evidenced by
debugger_trap_fatal()'s usage of domain_pause_for_debugger(). Because of
this, debugger_trap_entry() many not belong alongside the Xen debug
functions.
This commit fixes this by expanding inline debugger_trap_entry() into
its usage sites in x86/traps.c and stubbing out
domain_pause_for_debugger() when !CONFIG_GDBSX. Placing what
was debugger_trap_entry() under the scope of gdbsx instead of gdbstub.
The function calls that caused an effective no-op and early exit out of
debugger_trap_entry() are removed completely (when the trapnr is not
int3/debug).
This commit is one of a series geared towards removing the unnecessary
requirement that all architectures to implement <asm/debugger.h>.
Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
Changes in v4:
- Reword commit message for accuracy (make weaker claims)
- Fix "if { return } else if { return }" anti-pattern
xen/arch/x86/domain.c | 2 +-
xen/arch/x86/traps.c | 50 ++++++++++++++++++++--------------
xen/include/asm-x86/debugger.h | 42 ++--------------------------
3 files changed, 33 insertions(+), 61 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index ef1812dc14..70894ff826 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -2541,7 +2541,7 @@ __initcall(init_vcpu_kick_softirq);
void domain_pause_for_debugger(void)
{
-#ifdef CONFIG_CRASH_DEBUG
+#ifdef CONFIG_GDBSX
struct vcpu *curr = current;
struct domain *d = curr->domain;
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index e60af16ddd..772e2a5bfc 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -858,13 +858,20 @@ static 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) )
{
+ struct vcpu *curr = current;
+ if ( (trapnr == TRAP_debug || trapnr == TRAP_int3) &&
+ guest_kernel_mode(curr, regs) &&
+ curr->domain->debugger_attached )
+ {
+ if ( trapnr != TRAP_debug )
+ curr->arch.gdbsx_vcpu_event = trapnr;
+ domain_pause_for_debugger();
+ return;
+ }
pv_inject_hw_exception(trapnr,
(TRAP_HAVE_EC & (1u << trapnr))
? regs->error_code : X86_EVENT_NO_EC);
@@ -1094,9 +1101,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) )
@@ -1201,8 +1205,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) )
{
@@ -1216,6 +1219,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);
}
@@ -1492,9 +1502,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. */
@@ -1593,9 +1600,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;
@@ -1888,9 +1892,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:
*
@@ -1995,6 +1996,12 @@ void do_debug(struct cpu_user_regs *regs)
return;
}
+ if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached )
+ {
+ domain_pause_for_debugger();
+ return;
+ }
+
/* Save debug status register where guest OS can peek at it */
v->arch.dr6 |= (dr6 & ~X86_DR6_DEFAULT);
v->arch.dr6 &= (dr6 | ~X86_DR6_DEFAULT);
@@ -2014,9 +2021,6 @@ void do_entry_CP(struct cpu_user_regs *regs)
const char *err = "??";
unsigned int ec = regs->error_code;
- if ( debugger_trap_entry(TRAP_debug, regs) )
- return;
-
/* Decode ec if possible */
if ( ec < ARRAY_SIZE(errors) && errors[ec][0] )
err = errors[ec];
@@ -2028,6 +2032,12 @@ void do_entry_CP(struct cpu_user_regs *regs)
*/
if ( guest_mode(regs) )
{
+ struct vcpu *curr = current;
+ if ( guest_kernel_mode(curr, regs) && curr->domain->debugger_attached )
+ {
+ domain_pause_for_debugger();
+ return;
+ }
gprintk(XENLOG_ERR, "Hit #CP[%04x] in guest context %04x:%p\n",
ec, regs->cs, _p(regs->rip));
ASSERT_UNREACHABLE();
diff --git a/xen/include/asm-x86/debugger.h b/xen/include/asm-x86/debugger.h
index 99803bfd0c..cd6b9477f7 100644
--- a/xen/include/asm-x86/debugger.h
+++ b/xen/include/asm-x86/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():
+ * 1. 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():
+ * 2. 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
--
2.32.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 3/6] arch/x86: rename debug.c to gdbsx.c
2021-09-28 20:30 [PATCH v4 0/6] Remove unconditional arch dependency on asm/debugger.h Bobby Eshleman
2021-09-28 20:30 ` [PATCH v4 1/6] arm/traps: remove debugger_trap_fatal() calls Bobby Eshleman
2021-09-28 20:30 ` [PATCH v4 2/6] x86/debugger: separate Xen and guest debugging debugger_trap_* functions Bobby Eshleman
@ 2021-09-28 20:30 ` Bobby Eshleman
2021-09-28 21:09 ` Andrew Cooper
2021-09-28 20:30 ` [PATCH v4 4/6] x86/gdbsx: expand dbg_rw_mem() inline Bobby Eshleman
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Bobby Eshleman @ 2021-09-28 20:30 UTC (permalink / raw)
To: xen-devel
Cc: Bobby Eshleman, Stefano Stabellini, Julien Grall,
Volodymyr Babchuk, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
This commit renames debug.c to gdbsx.c to clarify its purpose.
The function gdbsx_guest_mem_io() is moved from domctl.c to gdbsx.c.
Although gdbsx_guest_mem_io() is conditionally removed from its single
call site in domctl.c upon !CONFIG_GDBSX and so no stub is technically
necessary, this commit adds a stub that would preserve the functioning
of that call site if the #ifdef CONFIG_GDBSX were to ever be removed or
the function were to ever be called outside of such an ifdef block.
Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
Changes in v4:
- Alphebetize Makefile addition
- Fix broken header guard
- Include errno.h in gdbsx.h
xen/arch/x86/Makefile | 2 +-
xen/arch/x86/domctl.c | 12 +-----------
xen/arch/x86/{debug.c => gdbsx.c} | 12 ++++++++++--
xen/include/asm-x86/debugger.h | 6 ------
xen/include/asm-x86/gdbsx.h | 19 +++++++++++++++++++
5 files changed, 31 insertions(+), 20 deletions(-)
rename xen/arch/x86/{debug.c => gdbsx.c} (93%)
create mode 100644 xen/include/asm-x86/gdbsx.h
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index fe38cfd544..9fa2ea9aa1 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -20,7 +20,6 @@ obj-y += cpuid.o
obj-$(CONFIG_PV) += compat.o
obj-$(CONFIG_PV32) += x86_64/compat.o
obj-$(CONFIG_KEXEC) += crash.o
-obj-$(CONFIG_GDBSX) += debug.o
obj-y += delay.o
obj-y += desc.o
obj-bin-y += dmi_scan.init.o
@@ -32,6 +31,7 @@ obj-y += emul-i8254.o
obj-y += extable.o
obj-y += flushtlb.o
obj-$(CONFIG_CRASH_DEBUG) += gdbstub.o
+obj-$(CONFIG_GDBSX) += gdbsx.o
obj-y += hypercall.o
obj-y += i387.o
obj-y += i8259.o
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 26a76d2be9..a492fe140e 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -20,6 +20,7 @@
#include <xen/console.h>
#include <xen/iocap.h>
#include <xen/paging.h>
+#include <asm/gdbsx.h>
#include <asm/irq.h>
#include <asm/hvm/emulate.h>
#include <asm/hvm/hvm.h>
@@ -33,20 +34,9 @@
#include <public/vm_event.h>
#include <asm/mem_sharing.h>
#include <asm/xstate.h>
-#include <asm/debugger.h>
#include <asm/psr.h>
#include <asm/cpuid.h>
-#ifdef CONFIG_GDBSX
-static int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
-{
- iop->remain = dbg_rw_mem(iop->gva, guest_handle_from_ptr(iop->uva, void),
- iop->len, domid, iop->gwr, iop->pgd3val);
-
- return iop->remain ? -EFAULT : 0;
-}
-#endif
-
static int update_domain_cpu_policy(struct domain *d,
xen_domctl_cpu_policy_t *xdpc)
{
diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/gdbsx.c
similarity index 93%
rename from xen/arch/x86/debug.c
rename to xen/arch/x86/gdbsx.c
index d90dc93056..adea0f017b 100644
--- a/xen/arch/x86/debug.c
+++ b/xen/arch/x86/gdbsx.c
@@ -19,7 +19,7 @@
#include <xen/mm.h>
#include <xen/domain_page.h>
#include <xen/guest_access.h>
-#include <asm/debugger.h>
+#include <asm/gdbsx.h>
#include <asm/p2m.h>
typedef unsigned long dbgva_t;
@@ -158,7 +158,7 @@ static unsigned int dbg_rw_guest_mem(struct domain *dp, unsigned long addr,
* pgd3: value of init_mm.pgd[3] in guest. see above.
* Returns: number of bytes remaining to be copied.
*/
-unsigned int dbg_rw_mem(unsigned long gva, XEN_GUEST_HANDLE_PARAM(void) buf,
+static 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)
{
@@ -174,6 +174,14 @@ unsigned int dbg_rw_mem(unsigned long gva, XEN_GUEST_HANDLE_PARAM(void) buf,
return len;
}
+int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
+{
+ iop->remain = dbg_rw_mem(iop->gva, guest_handle_from_ptr(iop->uva, void),
+ iop->len, domid, iop->gwr, iop->pgd3val);
+
+ return iop->remain ? -EFAULT : 0;
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/include/asm-x86/debugger.h b/xen/include/asm-x86/debugger.h
index cd6b9477f7..ed4d5c829b 100644
--- a/xen/include/asm-x86/debugger.h
+++ b/xen/include/asm-x86/debugger.h
@@ -54,10 +54,4 @@ static inline bool debugger_trap_fatal(
#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/asm-x86/gdbsx.h b/xen/include/asm-x86/gdbsx.h
new file mode 100644
index 0000000000..473229a7fb
--- /dev/null
+++ b/xen/include/asm-x86/gdbsx.h
@@ -0,0 +1,19 @@
+#ifndef __X86_GDBX_H__
+#define __X86_GDBX_H__
+
+#include <xen/errno.h>
+
+#ifdef CONFIG_GDBSX
+
+int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop);
+
+#else
+
+static inline int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
+{
+ return -EOPNOTSUPP;
+}
+
+#endif
+
+#endif
--
2.32.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 4/6] x86/gdbsx: expand dbg_rw_mem() inline
2021-09-28 20:30 [PATCH v4 0/6] Remove unconditional arch dependency on asm/debugger.h Bobby Eshleman
` (2 preceding siblings ...)
2021-09-28 20:30 ` [PATCH v4 3/6] arch/x86: rename debug.c to gdbsx.c Bobby Eshleman
@ 2021-09-28 20:30 ` Bobby Eshleman
2021-09-28 21:22 ` Andrew Cooper
2021-09-28 20:30 ` [PATCH v4 5/6] arch/x86: move domain_pause_for_debugger() to domain.h Bobby Eshleman
2021-09-28 20:30 ` [PATCH v4 6/6] x86: change asm/debugger.h to xen/debugger.h Bobby Eshleman
5 siblings, 1 reply; 14+ messages in thread
From: Bobby Eshleman @ 2021-09-28 20:30 UTC (permalink / raw)
To: xen-devel
Cc: Bobby Eshleman, Stefano Stabellini, Julien Grall,
Volodymyr Babchuk, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
Because dbg_rw_mem() has only a single call site, this commit
expands it inline.
Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
Changes in v4:
- Add DCO
xen/arch/x86/gdbsx.c | 30 +++++++++---------------------
1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/xen/arch/x86/gdbsx.c b/xen/arch/x86/gdbsx.c
index adea0f017b..9c8827c6c4 100644
--- a/xen/arch/x86/gdbsx.c
+++ b/xen/arch/x86/gdbsx.c
@@ -151,33 +151,21 @@ static unsigned int dbg_rw_guest_mem(struct domain *dp, unsigned long addr,
return len;
}
-/*
- * addr is guest addr
- * buf is debugger buffer.
- * if toaddr, then addr = buf (write to addr), else buf = addr (rd from guest)
- * pgd3: value of init_mm.pgd[3] in guest. see above.
- * Returns: number of bytes remaining to be copied.
- */
-static 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)
+int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
{
struct domain *d = rcu_lock_domain_by_id(domid);
- if ( d )
+ if ( d && !d->is_dying )
{
- if ( !d->is_dying )
- len = dbg_rw_guest_mem(d, gva, buf, len, toaddr, pgd3);
- rcu_unlock_domain(d);
+ iop->remain = dbg_rw_guest_mem(
+ d, iop->gva, guest_handle_from_ptr(iop->uva, void),
+ iop->len, domid, iop->pgd3val);
}
+ else
+ iop->remain = iop->len;
- return len;
-}
-
-int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
-{
- iop->remain = dbg_rw_mem(iop->gva, guest_handle_from_ptr(iop->uva, void),
- iop->len, domid, iop->gwr, iop->pgd3val);
+ if ( d )
+ rcu_unlock_domain(d);
return iop->remain ? -EFAULT : 0;
}
--
2.32.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 5/6] arch/x86: move domain_pause_for_debugger() to domain.h
2021-09-28 20:30 [PATCH v4 0/6] Remove unconditional arch dependency on asm/debugger.h Bobby Eshleman
` (3 preceding siblings ...)
2021-09-28 20:30 ` [PATCH v4 4/6] x86/gdbsx: expand dbg_rw_mem() inline Bobby Eshleman
@ 2021-09-28 20:30 ` Bobby Eshleman
2021-09-28 21:12 ` Andrew Cooper
2021-09-28 20:30 ` [PATCH v4 6/6] x86: change asm/debugger.h to xen/debugger.h Bobby Eshleman
5 siblings, 1 reply; 14+ messages in thread
From: Bobby Eshleman @ 2021-09-28 20:30 UTC (permalink / raw)
To: xen-devel
Cc: Bobby Eshleman, Stefano Stabellini, Julien Grall,
Volodymyr Babchuk, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
domain_pause_for_debugger() was previously in debugger.h. This commit
moves it to domain.h because its implementation is in domain.c.
Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
Changes in v3:
- domain_pause_for_debugger() is now moved into debugger.h, not a new
file debugger.c
Changes in v4:
- Don't unnecessarily include <asm/domain.h>
xen/arch/x86/nmi.c | 1 -
xen/arch/x86/traps.c | 1 -
xen/include/asm-x86/debugger.h | 2 --
xen/include/asm-x86/domain.h | 2 ++
4 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index ab94a96c4d..11d5f5a917 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -30,7 +30,6 @@
#include <asm/msr.h>
#include <asm/mpspec.h>
#include <asm/nmi.h>
-#include <asm/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 772e2a5bfc..742fa9e2ca 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -62,7 +62,6 @@
#include <asm/uaccess.h>
#include <asm/i387.h>
#include <asm/xstate.h>
-#include <asm/debugger.h>
#include <asm/msr.h>
#include <asm/nmi.h>
#include <asm/xenoprof.h>
diff --git a/xen/include/asm-x86/debugger.h b/xen/include/asm-x86/debugger.h
index ed4d5c829b..8f6222956e 100644
--- a/xen/include/asm-x86/debugger.h
+++ b/xen/include/asm-x86/debugger.h
@@ -26,8 +26,6 @@
#include <asm/regs.h>
#include <asm/processor.h>
-void domain_pause_for_debugger(void);
-
#ifdef CONFIG_CRASH_DEBUG
#include <xen/gdbstub.h>
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 92d54de0b9..de854b5bfa 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -672,6 +672,8 @@ void update_guest_memory_policy(struct vcpu *v,
void domain_cpu_policy_changed(struct domain *d);
+void domain_pause_for_debugger(void);
+
bool update_runstate_area(struct vcpu *);
bool update_secondary_system_time(struct vcpu *,
struct vcpu_time_info *);
--
2.32.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 6/6] x86: change asm/debugger.h to xen/debugger.h
2021-09-28 20:30 [PATCH v4 0/6] Remove unconditional arch dependency on asm/debugger.h Bobby Eshleman
` (4 preceding siblings ...)
2021-09-28 20:30 ` [PATCH v4 5/6] arch/x86: move domain_pause_for_debugger() to domain.h Bobby Eshleman
@ 2021-09-28 20:30 ` Bobby Eshleman
2021-09-28 21:25 ` Andrew Cooper
5 siblings, 1 reply; 14+ messages in thread
From: Bobby Eshleman @ 2021-09-28 20:30 UTC (permalink / raw)
To: xen-devel
Cc: Bobby Eshleman, Stefano Stabellini, Julien Grall,
Volodymyr Babchuk, Jan Beulich, Andrew Cooper,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
This commit allows non-x86 architecture to omit the file asm/debugger.h
if they do not require it. It changes debugger.h to be a general
xen/debugger.h which, if CONFIG_CRASH_DEBUG, resolves to include
asm/debugger.h.
It also changes all asm/debugger.h includes to xen/debugger.h.
Because it is no longer required, arm/debugger.h is removed.
Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
Changes in v4:
- Replace #include <asm/regs.h> with `struct cpu_user_regs`
xen/arch/x86/traps.c | 1 +
xen/common/domain.c | 2 +-
xen/common/gdbstub.c | 2 +-
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 | 15 ----------
xen/include/xen/debugger.h | 51 ++++++++++++++++++++++++++++++++++
9 files changed, 57 insertions(+), 35 deletions(-)
delete mode 100644 xen/include/asm-arm/debugger.h
create mode 100644 xen/include/xen/debugger.h
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 742fa9e2ca..36d7fc6238 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -35,6 +35,7 @@
#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>
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6b71c6d6a9..a87d814b38 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -11,6 +11,7 @@
#include <xen/err.h>
#include <xen/param.h>
#include <xen/sched.h>
+#include <xen/debugger.h>
#include <xen/domain.h>
#include <xen/mm.h>
#include <xen/event.h>
@@ -33,7 +34,6 @@
#include <xen/xenoprof.h>
#include <xen/irq.h>
#include <xen/argo.h>
-#include <asm/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..1d7b98cdac 100644
--- a/xen/common/gdbstub.c
+++ b/xen/common/gdbstub.c
@@ -38,12 +38,12 @@
#include <xen/serial.h>
#include <xen/irq.h>
#include <xen/watchdog.h>
-#include <asm/debugger.h>
#include <xen/init.h>
#include <xen/param.h>
#include <xen/smp.h>
#include <xen/console.h>
#include <xen/errno.h>
+#include <xen/debugger.h>
#include <xen/delay.h>
#include <asm/byteorder.h>
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 8b9f378371..1eafaef9b2 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -3,6 +3,7 @@
*/
#include <asm/regs.h>
+#include <xen/debugger.h>
#include <xen/delay.h>
#include <xen/keyhandler.h>
#include <xen/param.h>
@@ -20,7 +21,6 @@
#include <xen/mm.h>
#include <xen/watchdog.h>
#include <xen/init.h>
-#include <asm/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..a933ee001e 100644
--- a/xen/common/shutdown.c
+++ b/xen/common/shutdown.c
@@ -2,13 +2,13 @@
#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>
#include <xen/shutdown.h>
#include <xen/console.h>
#include <xen/kexec.h>
-#include <asm/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..3d1cdde821 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -15,6 +15,7 @@
#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>
@@ -26,7 +27,6 @@
#include <xen/kexec.h>
#include <xen/ctype.h>
#include <xen/warning.h>
-#include <asm/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 8f6222956e..b9eeed395c 100644
--- a/xen/include/asm-x86/debugger.h
+++ b/xen/include/asm-x86/debugger.h
@@ -25,9 +25,6 @@
#include <xen/sched.h>
#include <asm/regs.h>
#include <asm/processor.h>
-
-#ifdef CONFIG_CRASH_DEBUG
-
#include <xen/gdbstub.h>
static inline bool debugger_trap_fatal(
@@ -40,16 +37,4 @@ static inline bool debugger_trap_fatal(
/* Int3 is a trivial way to gather cpu_user_regs context. */
#define debugger_trap_immediate() __asm__ __volatile__ ( "int3" );
-#else
-
-static inline bool debugger_trap_fatal(
- unsigned int vector, struct cpu_user_regs *regs)
-{
- return false;
-}
-
-#define debugger_trap_immediate() ((void)0)
-
-#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..ddaa4a938b
--- /dev/null
+++ b/xen/include/xen/debugger.h
@@ -0,0 +1,51 @@
+/******************************************************************************
+ * 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 two functions here:
+ *
+ * 1. 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).
+ *
+ * 2. 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
+
+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__ */
--
2.32.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/6] x86/debugger: separate Xen and guest debugging debugger_trap_* functions
2021-09-28 20:30 ` [PATCH v4 2/6] x86/debugger: separate Xen and guest debugging debugger_trap_* functions Bobby Eshleman
@ 2021-09-28 20:52 ` Andrew Cooper
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2021-09-28 20:52 UTC (permalink / raw)
To: Bobby Eshleman, xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk, Jan Beulich,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
On 28/09/2021 21:30, Bobby Eshleman wrote:
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index e60af16ddd..772e2a5bfc 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -858,13 +858,20 @@ static 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) )
> {
> + struct vcpu *curr = current;
> + if ( (trapnr == TRAP_debug || trapnr == TRAP_int3) &&
> + guest_kernel_mode(curr, regs) &&
> + curr->domain->debugger_attached )
> + {
> + if ( trapnr != TRAP_debug )
> + curr->arch.gdbsx_vcpu_event = trapnr;
> + domain_pause_for_debugger();
> + return;
> + }
This is unreachable. do_trap() isn't used for TRAP_debug or TRAP_int3.
> @@ -2014,9 +2021,6 @@ void do_entry_CP(struct cpu_user_regs *regs)
> const char *err = "??";
> unsigned int ec = regs->error_code;
>
> - if ( debugger_trap_entry(TRAP_debug, regs) )
> - return;
> -
> /* Decode ec if possible */
> if ( ec < ARRAY_SIZE(errors) && errors[ec][0] )
> err = errors[ec];
> @@ -2028,6 +2032,12 @@ void do_entry_CP(struct cpu_user_regs *regs)
> */
> if ( guest_mode(regs) )
> {
> + struct vcpu *curr = current;
> + if ( guest_kernel_mode(curr, regs) && curr->domain->debugger_attached )
> + {
> + domain_pause_for_debugger();
> + return;
> + }
Urgh. The TRAP_debug above was a copy/paste error.
I'll submit a patch, as it wants backporting for a couple of releases,
after which there should be no additions in do_entry_CP().
Everything else looks good.
~Andrew
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/6] arm/traps: remove debugger_trap_fatal() calls
2021-09-28 20:30 ` [PATCH v4 1/6] arm/traps: remove debugger_trap_fatal() calls Bobby Eshleman
@ 2021-09-28 21:01 ` Andrew Cooper
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2021-09-28 21:01 UTC (permalink / raw)
To: Bobby Eshleman, xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk, Jan Beulich,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
On 28/09/2021 21:30, Bobby Eshleman wrote:
> ARM doesn't actually use debugger_trap_* anything, and is stubbed out.
>
> This commit simply removes the unneeded calls.
>
> Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
Julien already acked this patch on v3. You should carry the tag on
future revisions.
Acked-by: Julien Grall <jgrall@amazon.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/6] arch/x86: rename debug.c to gdbsx.c
2021-09-28 20:30 ` [PATCH v4 3/6] arch/x86: rename debug.c to gdbsx.c Bobby Eshleman
@ 2021-09-28 21:09 ` Andrew Cooper
2021-09-29 8:09 ` Jan Beulich
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2021-09-28 21:09 UTC (permalink / raw)
To: Bobby Eshleman, xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk, Jan Beulich,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
On 28/09/2021 21:30, Bobby Eshleman wrote:
> diff --git a/xen/include/asm-x86/gdbsx.h b/xen/include/asm-x86/gdbsx.h
> new file mode 100644
> index 0000000000..473229a7fb
> --- /dev/null
> +++ b/xen/include/asm-x86/gdbsx.h
> @@ -0,0 +1,19 @@
> +#ifndef __X86_GDBX_H__
> +#define __X86_GDBX_H__
> +
> +#include <xen/errno.h>
The errno include wants to move below....
However, you need to avoid latent build errors based on the order of
includes. I'd include public/domctl.h which will get you both domid_t
and struct xen_domctl_gdbsx_memio.
> +
> +#ifdef CONFIG_GDBSX
> +
> +int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop);
> +
> +#else
> +
... specifically here.
~Andrew
> +static inline int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +#endif
> +
> +#endif
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 5/6] arch/x86: move domain_pause_for_debugger() to domain.h
2021-09-28 20:30 ` [PATCH v4 5/6] arch/x86: move domain_pause_for_debugger() to domain.h Bobby Eshleman
@ 2021-09-28 21:12 ` Andrew Cooper
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2021-09-28 21:12 UTC (permalink / raw)
To: Bobby Eshleman, xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk, Jan Beulich,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
On 28/09/2021 21:30, Bobby Eshleman wrote:
> domain_pause_for_debugger() was previously in debugger.h. This commit
> moves it to domain.h because its implementation is in domain.c.
>
> Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 4/6] x86/gdbsx: expand dbg_rw_mem() inline
2021-09-28 20:30 ` [PATCH v4 4/6] x86/gdbsx: expand dbg_rw_mem() inline Bobby Eshleman
@ 2021-09-28 21:22 ` Andrew Cooper
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2021-09-28 21:22 UTC (permalink / raw)
To: Bobby Eshleman, xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk, Jan Beulich,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
On 28/09/2021 21:30, Bobby Eshleman wrote:
> Because dbg_rw_mem() has only a single call site, this commit
> expands it inline.
>
> Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 6/6] x86: change asm/debugger.h to xen/debugger.h
2021-09-28 20:30 ` [PATCH v4 6/6] x86: change asm/debugger.h to xen/debugger.h Bobby Eshleman
@ 2021-09-28 21:25 ` Andrew Cooper
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2021-09-28 21:25 UTC (permalink / raw)
To: Bobby Eshleman, xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk, Jan Beulich,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson
On 28/09/2021 21:30, Bobby Eshleman wrote:
> This commit allows non-x86 architecture to omit the file asm/debugger.h
> if they do not require it. It changes debugger.h to be a general
> xen/debugger.h which, if CONFIG_CRASH_DEBUG, resolves to include
> asm/debugger.h.
>
> It also changes all asm/debugger.h includes to xen/debugger.h.
>
> Because it is no longer required, arm/debugger.h is removed.
>
> Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
Julien also acked this patch.
> diff --git a/xen/include/xen/debugger.h b/xen/include/xen/debugger.h
> new file mode 100644
> index 0000000000..ddaa4a938b
> --- /dev/null
> +++ b/xen/include/xen/debugger.h
> @@ -0,0 +1,51 @@
> +/******************************************************************************
> + * 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 two functions here:
> + *
> + * 1. 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).
> + *
> + * 2. 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 comment is now duplicated in x86's asm/debugger.h. The x86 copy
wants deleting as part of this move.
> + */
> +
> +#ifndef __XEN_DEBUGGER_H__
> +#define __XEN_DEBUGGER_H__
> +
> +#ifdef CONFIG_CRASH_DEBUG
> +
> +#include <asm/debugger.h>
> +
> +#else
#include <xen/types.h> because you need bool and false to make this compile.
~Andrew
> +
> +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__ */
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/6] arch/x86: rename debug.c to gdbsx.c
2021-09-28 21:09 ` Andrew Cooper
@ 2021-09-29 8:09 ` Jan Beulich
0 siblings, 0 replies; 14+ messages in thread
From: Jan Beulich @ 2021-09-29 8:09 UTC (permalink / raw)
To: Andrew Cooper, Bobby Eshleman
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Roger Pau Monné,
Wei Liu, Elena Ufimtseva, George Dunlap, Ian Jackson, xen-devel
On 28.09.2021 23:09, Andrew Cooper wrote:
> On 28/09/2021 21:30, Bobby Eshleman wrote:
>> diff --git a/xen/include/asm-x86/gdbsx.h b/xen/include/asm-x86/gdbsx.h
>> new file mode 100644
>> index 0000000000..473229a7fb
>> --- /dev/null
>> +++ b/xen/include/asm-x86/gdbsx.h
>> @@ -0,0 +1,19 @@
>> +#ifndef __X86_GDBX_H__
>> +#define __X86_GDBX_H__
>> +
>> +#include <xen/errno.h>
>
> The errno include wants to move below....
>
> However, you need to avoid latent build errors based on the order of
> includes. I'd include public/domctl.h which will get you both domid_t
> and struct xen_domctl_gdbsx_memio.
public/xen.h should suffice as ...
>> +#ifdef CONFIG_GDBSX
>> +
>> +int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop);
>> +
>> +#else
>> +
>
> ... specifically here.
>
> ~Andrew
>
>> +static inline int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
>> +{
>> + return -EOPNOTSUPP;
>> +}
... for struct xen_domctl_gdbsx_memio a forward declaration is all that's
needed here.
Jan
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-09-29 8:10 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28 20:30 [PATCH v4 0/6] Remove unconditional arch dependency on asm/debugger.h Bobby Eshleman
2021-09-28 20:30 ` [PATCH v4 1/6] arm/traps: remove debugger_trap_fatal() calls Bobby Eshleman
2021-09-28 21:01 ` Andrew Cooper
2021-09-28 20:30 ` [PATCH v4 2/6] x86/debugger: separate Xen and guest debugging debugger_trap_* functions Bobby Eshleman
2021-09-28 20:52 ` Andrew Cooper
2021-09-28 20:30 ` [PATCH v4 3/6] arch/x86: rename debug.c to gdbsx.c Bobby Eshleman
2021-09-28 21:09 ` Andrew Cooper
2021-09-29 8:09 ` Jan Beulich
2021-09-28 20:30 ` [PATCH v4 4/6] x86/gdbsx: expand dbg_rw_mem() inline Bobby Eshleman
2021-09-28 21:22 ` Andrew Cooper
2021-09-28 20:30 ` [PATCH v4 5/6] arch/x86: move domain_pause_for_debugger() to domain.h Bobby Eshleman
2021-09-28 21:12 ` Andrew Cooper
2021-09-28 20:30 ` [PATCH v4 6/6] x86: change asm/debugger.h to xen/debugger.h Bobby Eshleman
2021-09-28 21:25 ` Andrew Cooper
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.