|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH 3/5] x86/hvm: RFC - PROBABLY BROKEN - Defer all debugging/monitor actions to {svm,vmx}_inject_event()
From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Currently, there is a lot of functionality in the #DB intercepts, and some
repeated functionality in the *_inject_event() logic.
The gdbsx code is implemented at both levels (albeit differently for #BP,
which is presumably due to the fact that the old emulator behaviour used to be
to move %rip forwards for traps), while the monitor behaviour only exists at
the intercept level.
Updating of %dr6 is implemented (buggily) at both levels, but having it at
both levels is problematic to implement correctly.
Rearrange the logic to have nothing interesting at the intercept level, and
everything implemented at the injection level. Amongst other things, this
means that the monitor subsystem will pick up debug actions from emulated
events.
Signed-off-by: Jinoh Kang <jinoh.kang.kr@xxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
CC: Kevin Tian <kevin.tian@xxxxxxxxx>
CC: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
CC: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
This is RFC because it probably breaks introspection, as injection replies
from the introspection engine will (probably, but I haven't confirmed) trigger
new monitor events.
First of all, monitoring emulated debug events is a change in behaviour,
although IMO it is more of a bugfix than a new feature. Also, similar changes
will happen to other monitored events as we try to unify the
intercept/emulation paths.
As for the recursive triggering of monitor events, I was considering extending
the monitor infrastructure to have a "in monitor reply" boolean which causes
hvm_monitor_debug() to ignore the recursive request.
Does this plan sound ok, or have I missed something more subtle with monitor
handling?
---
xen/arch/x86/hvm/svm/svm.c | 126 ++++++++++++++++++++-----------------
xen/arch/x86/hvm/vmx/vmx.c | 102 +++++++++++++-----------------
2 files changed, 110 insertions(+), 118 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index ac3123c56f..bd3adde0ec 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1328,19 +1328,50 @@ static void cf_check svm_inject_event(const struct
x86_event *event)
switch ( _event.vector | -(_event.type == X86_EVENTTYPE_SW_INTERRUPT) )
{
case X86_EXC_DB:
- if ( regs->eflags & X86_EFLAGS_TF )
- {
- __restore_debug_registers(vmcb, curr);
- vmcb_set_dr6(vmcb, vmcb_get_dr6(vmcb) | DR_STEP);
- }
+ /*
+ * On AMD hardware, a #DB exception:
+ * 1) Merges new status bits into %dr6
+ * 2) Clears %dr7.gd and MSR_DEBUGCTL.{LBR,BTF}
+ *
+ * Item 1 is done by hardware before a #DB intercepted vmexit, but we
+ * may end up here from emulation so have to repeat it ourselves.
+ * Item 2 is done by hardware when injecting a #DB exception.
+ */
+ __restore_debug_registers(vmcb, curr);
+ vmcb_set_dr6(vmcb, vmcb_get_dr6(vmcb) | _event.pending_dbg);
+
/* fall through */
case X86_EXC_BP:
if ( curr->domain->debugger_attached )
{
/* Debug/Int3: Trap to debugger. */
+ if ( _event.vector == X86_EXC_BP )
+ {
+ /* N.B. Can't use __update_guest_eip() for risk of recusion. */
+ regs->rip += _event.insn_len;
+ regs->eflags &= ~X86_EFLAGS_RF;
+ curr->arch.gdbsx_vcpu_event = X86_EXC_BP;
+ }
+
domain_pause_for_debugger();
return;
}
+ else
+ {
+ int rc = hvm_monitor_debug(regs->rip,
+ _event.vector == X86_EXC_DB
+ ? HVM_MONITOR_DEBUG_EXCEPTION
+ : HVM_MONITOR_SOFTWARE_BREAKPOINT,
+ _event.type, _event.insn_len,
+ _event.pending_dbg);
+ if ( rc < 0 )
+ {
+ gprintk(XENLOG_ERR, "Monitor debug error %d\n", rc);
+ return svm_crash_or_fault(curr);
+ }
+ if ( rc )
+ return; /* VCPU paused. Wait for monitor. */
+ }
break;
case X86_EXC_PF:
@@ -2730,67 +2761,46 @@ void svm_vmexit_handler(void)
case VMEXIT_ICEBP:
case VMEXIT_EXCEPTION_DB:
- if ( !v->domain->debugger_attached )
+ case VMEXIT_EXCEPTION_BP:
+ {
+ unsigned int vec, type, len, extra;
+
+ switch ( exit_reason )
{
- unsigned int trap_type;
+ case VMEXIT_ICEBP:
+ vec = X86_EXC_DB;
+ type = X86_EVENTTYPE_PRI_SW_EXCEPTION;
+ len = svm_get_insn_len(v, INSTR_ICEBP);
+ extra = 0;
+ break;
- if ( likely(exit_reason != VMEXIT_ICEBP) )
- {
- trap_type = X86_EVENTTYPE_HW_EXCEPTION;
- insn_len = 0;
- }
- else
- {
- trap_type = X86_EVENTTYPE_PRI_SW_EXCEPTION;
- insn_len = svm_get_insn_len(v, INSTR_ICEBP);
+ case VMEXIT_EXCEPTION_DB:
+ vec = X86_EXC_DB;
+ type = X86_EVENTTYPE_HW_EXCEPTION;
+ len = 0;
+ /* #DB - Hardware has already updated %dr6 for us. */
+ extra = vmcb_get_dr6(vmcb) ^ X86_DR6_DEFAULT;
+ break;
- if ( !insn_len )
- break;
- }
+ case VMEXIT_EXCEPTION_BP:
+ vec = X86_EXC_BP;
+ type = X86_EVENTTYPE_SW_EXCEPTION;
+ len = svm_get_insn_len(v, INSTR_INT3);
+ extra = 0; /* N/A */
+ break;
- rc = hvm_monitor_debug(regs->rip,
- HVM_MONITOR_DEBUG_EXCEPTION,
- trap_type, insn_len, 0);
- if ( rc < 0 )
- goto unexpected_exit_type;
- if ( !rc )
- hvm_inject_exception(X86_EXC_DB,
- trap_type, insn_len, X86_EVENT_NO_EC,
- exit_reason == VMEXIT_ICEBP ? 0 :
- /* #DB - Hardware already updated dr6. */
- vmcb_get_dr6(vmcb) ^ X86_DR6_DEFAULT);
+ default:
+ ASSERT_UNREACHABLE();
+ goto unexpected_exit_type;
}
- else
- domain_pause_for_debugger();
- break;
- case VMEXIT_EXCEPTION_BP:
- insn_len = svm_get_insn_len(v, INSTR_INT3);
-
- if ( insn_len == 0 )
- break;
+ /* svm_get_insn_len() failure. #GP queued up. */
+ if ( type >= X86_EVENTTYPE_SW_INTERRUPT && !len )
+ break;
- if ( v->domain->debugger_attached )
- {
- /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update
RIP. */
- __update_guest_eip(regs, insn_len);
- current->arch.gdbsx_vcpu_event = X86_EXC_BP;
- domain_pause_for_debugger();
- }
- else
- {
- rc = hvm_monitor_debug(regs->rip,
- HVM_MONITOR_SOFTWARE_BREAKPOINT,
- X86_EVENTTYPE_SW_EXCEPTION,
- insn_len, 0);
- if ( rc < 0 )
- goto unexpected_exit_type;
- if ( !rc )
- hvm_inject_exception(X86_EXC_BP,
- X86_EVENTTYPE_SW_EXCEPTION,
- insn_len, X86_EVENT_NO_EC, 0 /* N/A */);
- }
+ hvm_inject_exception(vec, type, len, X86_EVENT_NO_EC, extra);
break;
+ }
case VMEXIT_EXCEPTION_NM:
svm_fpu_dirty_intercept();
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 716115332b..65166348f1 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2018,15 +2018,21 @@ static void cf_check vmx_inject_event(const struct
x86_event *event)
unsigned long intr_info;
struct vcpu *curr = current;
struct x86_event _event = *event;
+ struct cpu_user_regs *regs = guest_cpu_user_regs();
switch ( _event.vector | -(_event.type == X86_EVENTTYPE_SW_INTERRUPT) )
{
case X86_EXC_DB:
- if ( guest_cpu_user_regs()->eflags & X86_EFLAGS_TF )
- {
- __restore_debug_registers(curr);
- write_debugreg(6, read_debugreg(6) | DR_STEP);
- }
+ /*
+ * On Intel hardware, a #DB exception:
+ * 1) Merges new status bits into %dr6
+ * 2) Clears %dr7.gd and MSR_DEBUGCTL.LBR
+ *
+ * All actions are left up to the hypervisor to perform.
+ */
+ __restore_debug_registers(curr);
+ write_debugreg(6, read_debugreg(6) | event->pending_dbg);
+
if ( !nestedhvm_vcpu_in_guestmode(curr) ||
!nvmx_intercepts_exception(curr, X86_EXC_DB, _event.error_code) )
{
@@ -2037,16 +2043,39 @@ static void cf_check vmx_inject_event(const struct
x86_event *event)
__vmread(GUEST_IA32_DEBUGCTL, &val);
__vmwrite(GUEST_IA32_DEBUGCTL, val & ~IA32_DEBUGCTLMSR_LBR);
}
- if ( cpu_has_monitor_trap_flag )
- break;
+
/* fall through */
case X86_EXC_BP:
if ( curr->domain->debugger_attached )
{
/* Debug/Int3: Trap to debugger. */
+ if ( _event.vector == X86_EXC_BP )
+ {
+ /* N.B. Can't use __update_guest_eip() for risk of recusion. */
+ regs->rip += _event.insn_len;
+ regs->eflags &= ~X86_EFLAGS_RF;
+ curr->arch.gdbsx_vcpu_event = X86_EXC_BP;
+ }
+
domain_pause_for_debugger();
return;
}
+ else
+ {
+ int rc = hvm_monitor_debug(regs->rip,
+ _event.vector == X86_EXC_DB
+ ? HVM_MONITOR_DEBUG_EXCEPTION
+ : HVM_MONITOR_SOFTWARE_BREAKPOINT,
+ _event.type, _event.insn_len, 0);
+ if ( rc < 0 )
+ {
+ gprintk(XENLOG_ERR, "Monitor debug error %d\n", rc);
+ domain_crash(curr->domain);
+ return;
+ }
+ if ( rc )
+ return; /* VCPU paused. Wait for monitor. */
+ }
break;
case X86_EXC_PF:
@@ -4242,14 +4271,8 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
switch ( vector )
{
case X86_EXC_DB:
- /*
- * Updates DR6 where debugger can peek (See 3B 23.2.1,
- * Table 23-1, "Exit Qualification for Debug Exceptions").
- */
__vmread(EXIT_QUALIFICATION, &exit_qualification);
HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
- __restore_debug_registers(v);
- write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE);
/*
* Work around SingleStep + STI/MovSS VMEntry failures.
@@ -4286,53 +4309,15 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
}
}
- if ( !v->domain->debugger_attached )
- {
- unsigned long insn_len = 0;
- int rc;
- unsigned long trap_type = MASK_EXTR(intr_info,
- INTR_INFO_INTR_TYPE_MASK);
-
- if ( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
- __vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len);
-
- rc = hvm_monitor_debug(regs->rip,
- HVM_MONITOR_DEBUG_EXCEPTION,
- trap_type, insn_len, 0);
-
- if ( rc < 0 )
- goto exit_and_crash;
- if ( !rc )
- vmx_propagate_intr(intr_info, exit_qualification);
- }
- else
- domain_pause_for_debugger();
+ vmx_propagate_intr(intr_info, exit_qualification);
break;
+
case X86_EXC_BP:
+ case X86_EXC_AC:
HVMTRACE_1D(TRAP, vector);
- if ( !v->domain->debugger_attached )
- {
- unsigned long insn_len;
- int rc;
-
- __vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len);
- rc = hvm_monitor_debug(regs->rip,
- HVM_MONITOR_SOFTWARE_BREAKPOINT,
- X86_EVENTTYPE_SW_EXCEPTION,
- insn_len, 0);
-
- if ( rc < 0 )
- goto exit_and_crash;
- if ( !rc )
- vmx_propagate_intr(intr_info, 0 /* N/A */);
- }
- else
- {
- update_guest_eip(); /* Safe: INT3 */
- v->arch.gdbsx_vcpu_event = X86_EXC_BP;
- domain_pause_for_debugger();
- }
+ vmx_propagate_intr(intr_info, 0 /* N/A */);
break;
+
case X86_EXC_NM:
HVMTRACE_1D(TRAP, vector);
vmx_fpu_dirty_intercept();
@@ -4362,10 +4347,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
hvm_inject_page_fault(regs->error_code, exit_qualification);
break;
- case X86_EXC_AC:
- HVMTRACE_1D(TRAP, vector);
- vmx_propagate_intr(intr_info, 0 /* N/A */);
- break;
+
case X86_EXC_NMI:
if ( MASK_EXTR(intr_info, INTR_INFO_INTR_TYPE_MASK) !=
X86_EVENTTYPE_NMI )
--
2.41.0
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |