On 06/06/18 14:50, Jan Beulich wrote: >>>> On 04.06.18 at 15:59, <andrew.coop...@citrix.com> wrote: >> --- a/xen/arch/x86/hvm/vmx/vmx.c >> +++ b/xen/arch/x86/hvm/vmx/vmx.c >> @@ -3696,6 +3696,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) >> */ >> __vmread(EXIT_QUALIFICATION, &exit_qualification); >> HVMTRACE_1D(TRAP_DEBUG, exit_qualification); >> + __restore_debug_registers(v); >> write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE); > The change is certainly correct as is, but I'd still like to put out for > discussion the alternative option: > > if ( v->arch.hvm_vcpu.flag_dr_dirty ) > write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE); > else > v->arch.debugreg[6] = exit_qualification | DR_STATUS_RESERVED_ONE; > > After all the guest may know it's single stepping, and may not care to > read DR6 at all.
All of this code changes across the series (so this specific suggestion is incorrect), but to the recommendation in general... I considered this, particularly when looking at the AMD side where %dr6 lives in the VMCB, and is unrelated to the dirty state of the other The first thing a #DB handler will do is read %dr6 and reset it (as recommended by both manuals), so forcing the %dr state to dirty (and therefore clearing %dr interception) is a direct benefit to the guests subsequent performance. TBH, I'm still not terribly happy with these accessors, but I was planning to defer reworking how the lazy state handing works. We've got ad-hoc lazy logic for each type of state, and its about to get very very complicated with guest_{rd,wr}msr(). I'm planning to build a more generic piece of state-syncing logic where every piece of emulation / access (particularly remote access code) code starts with vcpu_sync_state(state_$X) which takes care of pulling that specific piece of state out of hardware (if necessary) and into struct vcpu, and vcpu_state_dirtied(state_$X) which identifies that state in struct vcpu has been modified. The idea is to make all the emulation and access code agnostic to how a certain piece of state is stored, whether that be in VMCS/VMCB, in hardware registers for current context, switched on entry/exit, or fully emulated. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel