Lots of this is very very broken, but we need to start somewhere. First, the bugfix. Hooks which use X86EMUL_DONE to skip the general emulation still need to evaluate singlestep as part of completing the instruction. Defer the logic until X86EMUL_DONE has been converted to X86EMUL_OKAY.
Second, the improvement. PENDING_DBG, INTERRUPTIBILITY and ACTIVITY are internal pipeline state which Intel exposed to software in the VMCS, and AMD exposed a subset of in the VMCB. Importantly, bits set in PENDING_DBG can survive across multiple instruction boundaries if e.g. delivery of #DB is delayed by a MovSS. For now, introduce a full pending_dbg field into the retire union. This keeps the sh_page_fault() and init_context() paths working but in due course the field will want to lose the "retire" infix. In addition, set singlestep into pending_dbg as appropriate. Leave the old singlestep bitfield in place until we can adjust the callers to the new scheme. Signed-off-by: Andrew Cooper <[email protected]> --- CC: Jan Beulich <[email protected]> CC: Roger Pau Monné <[email protected]> CC: Wei Liu <[email protected]> CC: Jinoh Kang <[email protected]> v2: * Only evaluate singlestep on X86EMUL_OKAY, but do so after X86EMUL_DONE has been adjusted to X86EMUL_OKAY. * Adjust comments in light of X86EMUL_DONE not getting back to callers. --- xen/arch/x86/x86_emulate/x86_emulate.c | 20 +++++++++++++------- xen/arch/x86/x86_emulate/x86_emulate.h | 14 +++++++++++--- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c index 94caec1d142c..de7f99500e3f 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -8379,13 +8379,6 @@ x86_emulate( if ( !mode_64bit() ) _regs.r(ip) = (uint32_t)_regs.r(ip); - /* Should a singlestep #DB be raised? */ - if ( rc == X86EMUL_OKAY && singlestep && !ctxt->retire.mov_ss ) - { - ctxt->retire.singlestep = true; - ctxt->retire.sti = false; - } - if ( rc != X86EMUL_DONE ) *ctxt->regs = _regs; else @@ -8394,6 +8387,19 @@ x86_emulate( rc = X86EMUL_OKAY; } + /* Should a singlestep #DB be raised? */ + if ( rc == X86EMUL_OKAY && singlestep ) + { + ctxt->retire.pending_dbg |= X86_DR6_BS; + + /* BROKEN - TODO, merge into pending_dbg. */ + if ( !ctxt->retire.mov_ss ) + { + ctxt->retire.singlestep = true; + ctxt->retire.sti = false; + } + } + ctxt->regs->eflags &= ~X86_EFLAGS_RF; done: diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h index 698750267a90..fbc023c37e34 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.h +++ b/xen/arch/x86/x86_emulate/x86_emulate.h @@ -588,15 +588,23 @@ struct x86_emulate_ctxt /* Canonical opcode (see below) (valid only on X86EMUL_OKAY). */ unsigned int opcode; - /* Retirement state, set by the emulator (valid only on X86EMUL_OKAY). */ + /* + * Retirement state, set by the emulator (valid only on X86EMUL_OKAY). + * + * TODO: all this state should be input/output from the VMCS PENDING_DBG, + * INTERRUPTIBILITY and ACTIVITIY fields. + */ union { - uint8_t raw; + unsigned long raw; struct { + /* Accumulated %dr6 trap bits, positive polarity. */ + unsigned int pending_dbg; + bool hlt:1; /* Instruction HLTed. */ bool mov_ss:1; /* Instruction sets MOV-SS irq shadow. */ bool sti:1; /* Instruction sets STI irq shadow. */ bool unblock_nmi:1; /* Instruction clears NMI blocking. */ - bool singlestep:1; /* Singlestepping was active. */ + bool singlestep:1; /* Singlestepping was active. (TODO, merge into pending_dbg) */ }; } retire; -- 2.30.2
