All #DB exceptions result in an update of %dr6, but this isn't captured in
Xen's handling, and is buggy everywhere.
To begin resolving this issue, add a new pending_dbg field to x86_event
(unioned with cr2 to avoid taking any extra space), and introduce
pv_inject_debug_exn() helpers to replace the current callers using
pv_inject_hw_exception().
Push the adjustment of v->arch.dr6 into pv_inject_event(), and use the new
x86_merge_dr6() rather than the current incorrect logic.
A key property is that pending_dbg is taken with positive polarity to deal
with RTM/BLD sensibly. Most callers pass in a constant, but callers passing
in a hardware %dr6 value need to XOR the value with X86_DR6_DEFAULT to flip to
positive polarity.
This fixes the behaviour of the breakpoint status bits; specifically that any
left pending are discarded when a new #DB is raised. In principle it would
fix RTM/BLD too, except PV guests can't turn these capabilities on to start
with.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
v2:
* Rebase (~6y worth)
* Split PV changes out of joint HVM patch.
---
xen/arch/x86/include/asm/domain.h | 12 ++++++++++++
xen/arch/x86/pv/emul-priv-op.c | 5 +----
xen/arch/x86/pv/emulate.c | 6 ++----
xen/arch/x86/pv/ro-page-fault.c | 2 +-
xen/arch/x86/pv/traps.c | 16 ++++++++++++----
xen/arch/x86/traps.c | 2 +-
xen/arch/x86/x86_emulate/x86_emulate.h | 5 ++++-
7 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index bca3258d69ac..90c959996914 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -731,6 +731,18 @@ static inline void pv_inject_hw_exception(unsigned int vector, int errcode)
pv_inject_event(&event);
}
+static inline void pv_inject_debug_exn(unsigned int pending_dbg)
+{
+ struct x86_event event = {
+ .vector = X86_EXC_DB,
+ .type = X86_EVENTTYPE_HW_EXCEPTION,
+ .error_code = X86_EVENT_NO_EC,
+ .pending_dbg = pending_dbg,
+ };
+
+ pv_inject_event(&event);
+}
+
static inline void pv_inject_page_fault(int errcode, unsigned long cr2)
{
const struct x86_event event = {
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index aa11ecadaac0..3be02d85f2fe 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -1366,10 +1366,7 @@ int pv_emulate_privileged_op(struct cpu_user_regs *regs)
ctxt.bpmatch |= DR_STEP;
if ( ctxt.bpmatch )
- {
- curr->arch.dr6 |= ctxt.bpmatch | DR_STATUS_RESERVED_ONE;
- pv_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC);
- }
+ pv_inject_debug_exn(ctxt.bpmatch);
/* fall through */
case X86EMUL_RETRY:
diff --git a/xen/arch/x86/pv/emulate.c b/xen/arch/x86/pv/emulate.c
index e7a1c0a2cc4f..aa8af96c30f3 100644
--- a/xen/arch/x86/pv/emulate.c
+++ b/xen/arch/x86/pv/emulate.c
@@ -71,11 +71,9 @@ void pv_emul_instruction_done(struct cpu_user_regs *regs, unsigned long rip)
{
regs->rip = rip;
regs->eflags &= ~X86_EFLAGS_RF;
+
if ( regs->eflags & X86_EFLAGS_TF )
- {
- current->arch.dr6 |= DR_STEP | DR_STATUS_RESERVED_ONE;
- pv_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC);
- }
+ pv_inject_debug_exn(X86_DR6_BS);
}
uint64_t pv_get_reg(struct vcpu *v, unsigned int reg)
diff --git a/xen/arch/x86/pv/ro-page-fault.c b/xen/arch/x86/pv/ro-page-fault.c
index cad28ef928ad..73c9f7578a87 100644
--- a/xen/arch/x86/pv/ro-page-fault.c
+++ b/xen/arch/x86/pv/ro-page-fault.c
@@ -390,7 +390,7 @@ int pv_ro_page_fault(unsigned long addr, struct cpu_user_regs *regs)
/* Fallthrough */
case X86EMUL_OKAY:
if ( ctxt.retire.singlestep )
- pv_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC);
+ pv_inject_debug_exn(X86_DR6_BS);
/* Fallthrough */
case X86EMUL_RETRY:
diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
index 83e84e276233..5a7341abf068 100644
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -12,6 +12,7 @@
#include <xen/lib.h>
#include <xen/softirq.h>
+#include <asm/debugreg.h>
#include <asm/pv/trace.h>
#include <asm/shared.h>
#include <asm/traps.h>
@@ -50,9 +51,9 @@ void pv_inject_event(const struct x86_event *event)
tb->cs = ti->cs;
tb->eip = ti->address;
- if ( event->type == X86_EVENTTYPE_HW_EXCEPTION &&
- vector == X86_EXC_PF )
+ switch ( vector | -(event->type == X86_EVENTTYPE_SW_INTERRUPT) )
{
+ case X86_EXC_PF:
curr->arch.pv.ctrlreg[2] = event->cr2;
arch_set_cr2(curr, event->cr2);
@@ -62,9 +63,16 @@ void pv_inject_event(const struct x86_event *event)
error_code |= PFEC_user_mode;
trace_pv_page_fault(event->cr2, error_code);
- }
- else
+ break;
+
+ case X86_EXC_DB:
+ curr->arch.dr6 = x86_merge_dr6(curr->domain->arch.cpu_policy,
+ curr->arch.dr6, event->pending_dbg);
+ fallthrough;
+ default:
trace_pv_trap(vector, regs->rip, use_error_code, error_code);
+ break;
+ }
if ( use_error_code )
{
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 521ed4dd816d..06e4e3e9af90 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2030,7 +2030,7 @@ void asmlinkage do_debug(struct cpu_user_regs *regs)
return;
}
- pv_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC);
+ pv_inject_debug_exn(0 /* N/A, already merged */);
}
void asmlinkage do_entry_CP(struct cpu_user_regs *regs)
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index d92be69d84d9..e8a0e572284c 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -78,7 +78,10 @@ struct x86_event {
uint8_t type; /* X86_EVENTTYPE_* */
uint8_t insn_len; /* Instruction length */
int32_t error_code; /* X86_EVENT_NO_EC if n/a */
- unsigned long cr2; /* Only for X86_EXC_PF h/w exception */
+ union {
+ unsigned long cr2; /* #PF */
+ unsigned long pending_dbg; /* #DB (new DR6 bits, positive polarity) */
+ };
};
/*
--
2.39.2
© 2016 - 2024 Red Hat, Inc.