|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 07/11] x86/emul: Add pending_dbg field to x86_event
All #DB exceptions result in an update of %dr6, but this isn't captured in
Xen's handling.
PV guests generally work by modifying %dr6 before raising #DB, whereas HVM
guests do nothing and have a single-step special case in the lowest levels of
{vmx,svm}_inject_event(). All of this is buggy, but in particular, task
switches with the trace flag never end up signalling BT in %dr6.
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,hvm}_inject_debug_exn() helpers to replace the current callers using
{pv,hvm}_inject_hw_exception().
A key property is that pending_dbg is taken with positive polarity to deal
with RTM 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 the
polarity of RTM and reserved fields.
For PV guests, move the ad-hoc updating of %dr6 into pv_inject_event(). This
in principle breaks the handing of RTM in do_debug(), but PV guests can't
actually enable MSR_DEBUGCTL.RTM yet, so this doesn't matter in practice.
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
CC: Kevin Tian <kevin.tian@xxxxxxxxx>
CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
CC: Brian Woods <brian.woods@xxxxxxx>
CC: Tim Deegan <tim@xxxxxxx>
---
xen/arch/x86/hvm/emulate.c | 3 ++-
xen/arch/x86/hvm/hvm.c | 2 +-
xen/arch/x86/hvm/svm/svm.c | 9 ++++++---
xen/arch/x86/hvm/vmx/vmx.c | 13 ++++++++-----
xen/arch/x86/mm/shadow/multi.c | 5 +++--
xen/arch/x86/pv/emul-priv-op.c | 11 +++++------
xen/arch/x86/pv/emulate.c | 6 ++----
xen/arch/x86/pv/ro-page-fault.c | 3 ++-
xen/arch/x86/pv/traps.c | 16 ++++++++++++----
xen/arch/x86/traps.c | 7 +------
xen/arch/x86/x86_emulate/x86_emulate.h | 5 ++++-
xen/include/asm-x86/domain.h | 12 ++++++++++++
xen/include/asm-x86/hvm/hvm.h | 15 ++++++++++++++-
13 files changed, 72 insertions(+), 35 deletions(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index c9aa188..0292058 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -15,6 +15,7 @@
#include <xen/paging.h>
#include <xen/trace.h>
#include <xen/vm_event.h>
+#include <asm/debugreg.h>
#include <asm/event.h>
#include <asm/i387.h>
#include <asm/xstate.h>
@@ -2283,7 +2284,7 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt
*hvmemul_ctxt,
}
if ( hvmemul_ctxt->ctxt.retire.singlestep )
- hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ hvm_inject_debug_exn(X86_DR6_BS);
new_intr_shadow = hvmemul_ctxt->intr_shadow;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 4f825a2..b35cf54 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3115,7 +3115,7 @@ void hvm_task_switch(
}
if ( (tss.trace & 1) && !exn_raised )
- hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ hvm_inject_debug_exn(X86_DR6_BT);
out:
hvm_unmap_entry(optss_desc);
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index dabb96f..c06bd68 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -119,7 +119,7 @@ void __update_guest_eip(struct cpu_user_regs *regs,
unsigned int inst_len)
curr->arch.hvm_svm.vmcb->interrupt_shadow = 0;
if ( regs->eflags & X86_EFLAGS_TF )
- hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ hvm_inject_debug_exn(X86_DR6_BS);
}
static void svm_cpu_down(void)
@@ -2798,7 +2798,10 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
goto unexpected_exit_type;
if ( !rc )
hvm_inject_exception(TRAP_debug,
- trap_type, inst_len, X86_EVENT_NO_EC);
+ trap_type, inst_len, X86_EVENT_NO_EC,
+ exit_reason == VMEXIT_ICEBP ? 0 :
+ /* #DB - Hardware already updated dr6. */
+ vmcb_get_dr6(vmcb) ^ X86_DR6_DEFAULT);
}
else
domain_pause_for_debugger();
@@ -2830,7 +2833,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
if ( !rc )
hvm_inject_exception(TRAP_int3,
X86_EVENTTYPE_SW_EXCEPTION,
- inst_len, X86_EVENT_NO_EC);
+ inst_len, X86_EVENT_NO_EC, 0 /* N/A */);
}
break;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index bfa3a0d..39c9ddc 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2483,7 +2483,7 @@ void update_guest_eip(void)
}
if ( regs->eflags & X86_EFLAGS_TF )
- hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ hvm_inject_debug_exn(X86_DR6_BS);
}
static void vmx_fpu_dirty_intercept(void)
@@ -3382,7 +3382,7 @@ static int vmx_handle_eoi_write(void)
* It is the callers responsibility to ensure that this function is only used
* in the context of an appropriate vmexit.
*/
-static void vmx_propagate_intr(unsigned long intr)
+static void vmx_propagate_intr(unsigned long intr, unsigned long pending_dbg)
{
struct x86_event event = {
.vector = MASK_EXTR(intr, INTR_INFO_VECTOR_MASK),
@@ -3406,6 +3406,9 @@ static void vmx_propagate_intr(unsigned long intr)
else
event.insn_len = 0;
+ if ( event.vector == TRAP_debug )
+ event.pending_dbg = pending_dbg;
+
hvm_inject_event(&event);
}
@@ -3715,7 +3718,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
if ( rc < 0 )
goto exit_and_crash;
if ( !rc )
- vmx_propagate_intr(intr_info);
+ vmx_propagate_intr(intr_info, exit_qualification);
}
else
domain_pause_for_debugger();
@@ -3736,7 +3739,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
if ( rc < 0 )
goto exit_and_crash;
if ( !rc )
- vmx_propagate_intr(intr_info);
+ vmx_propagate_intr(intr_info, 0 /* N/A */);
}
else
{
@@ -3776,7 +3779,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
break;
case TRAP_alignment_check:
HVMTRACE_1D(TRAP, vector);
- vmx_propagate_intr(intr_info);
+ vmx_propagate_intr(intr_info, 0 /* N/A */);
break;
case TRAP_nmi:
if ( MASK_EXTR(intr_info, INTR_INFO_INTR_TYPE_MASK) !=
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index da586c2..57d24d9 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -30,6 +30,7 @@ asm(".file \"" __OBJECT_FILE__ "\"");
#include <xen/perfc.h>
#include <xen/domain_page.h>
#include <xen/iocap.h>
+#include <asm/debugreg.h>
#include <xsm/xsm.h>
#include <asm/page.h>
#include <asm/current.h>
@@ -3453,7 +3454,7 @@ static int sh_page_fault(struct vcpu *v,
#endif
if ( emul_ctxt.ctxt.retire.singlestep )
- hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ hvm_inject_debug_exn(X86_DR6_BS);
#if GUEST_PAGING_LEVELS == 3 /* PAE guest */
/*
@@ -3494,7 +3495,7 @@ static int sh_page_fault(struct vcpu *v,
TRACE_SHADOW_PATH_FLAG(TRCE_SFLAG_EMULATION_LAST_FAILED);
if ( emul_ctxt.ctxt.retire.singlestep )
- hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ hvm_inject_debug_exn(X86_DR6_BS);
break; /* Don't emulate again if we failed! */
}
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index dfde70e..45788b2 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -1366,12 +1366,11 @@ int pv_emulate_privileged_op(struct cpu_user_regs *regs)
if ( ctxt.ctxt.retire.singlestep )
ctxt.bpmatch |= DR_STEP;
- if ( ctxt.bpmatch )
- {
- curr->arch.dr6 |= ctxt.bpmatch | DR_STATUS_RESERVED_ONE;
- if ( !(curr->arch.pv_vcpu.trap_bounce.flags & TBF_EXCEPTION) )
- pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
- }
+
+ if ( ctxt.bpmatch &&
+ !(curr->arch.pv_vcpu.trap_bounce.flags & TBF_EXCEPTION) )
+ pv_inject_debug_exn(ctxt.bpmatch);
+
/* fall through */
case X86EMUL_RETRY:
return EXCRET_fault_fixed;
diff --git a/xen/arch/x86/pv/emulate.c b/xen/arch/x86/pv/emulate.c
index 757ffd1..542b6d0 100644
--- a/xen/arch/x86/pv/emulate.c
+++ b/xen/arch/x86/pv/emulate.c
@@ -76,11 +76,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(TRAP_debug, X86_EVENT_NO_EC);
- }
+ pv_inject_debug_exn(X86_DR6_BS);
}
/*
diff --git a/xen/arch/x86/pv/ro-page-fault.c b/xen/arch/x86/pv/ro-page-fault.c
index aa8d5a7..33873c9 100644
--- a/xen/arch/x86/pv/ro-page-fault.c
+++ b/xen/arch/x86/pv/ro-page-fault.c
@@ -25,6 +25,7 @@
#include <xen/sched.h>
#include <xen/trace.h>
+#include <asm/debugreg.h>
#include <asm/domain.h>
#include <asm/mm.h>
#include <asm/pci.h>
@@ -387,7 +388,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(TRAP_debug, 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 f48db92..7d48d83 100644
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -26,6 +26,7 @@
#include <xen/softirq.h>
#include <asm/apic.h>
+#include <asm/debugreg.h>
#include <asm/shared.h>
#include <asm/traps.h>
@@ -70,9 +71,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 == TRAP_page_fault )
+ switch ( vector | -(event->type == X86_EVENTTYPE_SW_INTERRUPT) )
{
+ case TRAP_page_fault:
curr->arch.pv_vcpu.ctrlreg[2] = event->cr2;
arch_set_cr2(curr, event->cr2);
@@ -82,9 +83,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 TRAP_debug:
+ 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 d0d9011..8ef22b4 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1768,7 +1768,6 @@ void do_device_not_available(struct cpu_user_regs *regs)
void do_debug(struct cpu_user_regs *regs)
{
unsigned long dr6;
- struct vcpu *v = current;
/* Stash dr6 as early as possible. */
dr6 = read_debugreg(6);
@@ -1860,11 +1859,7 @@ void do_debug(struct cpu_user_regs *regs)
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);
-
- pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
+ pv_inject_debug_exn(dr6 ^ X86_DR6_DEFAULT);
}
static void __init noinline __set_intr_gate(unsigned int n,
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h
b/xen/arch/x86/x86_emulate/x86_emulate.h
index c22e774..d85a84a 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -88,7 +88,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 TRAP_page_fault h/w exception */
+ union {
+ unsigned long cr2; /* #PF */
+ unsigned long pending_dbg; /* #DB (new DR6 bits, positive polarity) */
+ };
};
/*
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 59d5e4a..dfe995f 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -648,6 +648,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 long 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/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index ef5e198..65d512e 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -410,13 +410,14 @@ void hvm_inject_event(const struct x86_event *event);
static inline void hvm_inject_exception(
unsigned int vector, unsigned int type,
- unsigned int insn_len, int error_code)
+ unsigned int insn_len, int error_code, unsigned long extra)
{
struct x86_event event = {
.vector = vector,
.type = type,
.insn_len = insn_len,
.error_code = error_code,
+ .cr2 = extra, /* Any union field will do. */
};
hvm_inject_event(&event);
@@ -433,6 +434,18 @@ static inline void hvm_inject_hw_exception(unsigned int
vector, int errcode)
hvm_inject_event(&event);
}
+static inline void hvm_inject_debug_exn(unsigned long 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,
+ };
+
+ hvm_inject_event(&event);
+}
+
static inline void hvm_inject_page_fault(int errcode, unsigned long cr2)
{
struct x86_event event = {
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |