From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: Single step in HVM domU on Intel machine may see wrong DB6 Date: Tue, 06 May 2014 07:17:14 +0200 Message-ID: <5368705A.4090309@ts.fujitsu.com> References: <5305BE9F.2090600@ts.fujitsu.com> <5306E5D3.6000302@ts.fujitsu.com> <530E1D9E020000780011F938@nat28.tlf.novell.com> <530F00C7020000780011FBA1@nat28.tlf.novell.com> <5316E8E5020000780012117B@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WhXkj-0002zs-FT for xen-devel@lists.xenproject.org; Tue, 06 May 2014 05:17:32 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Zhang, Yang Z" , Jan Beulich Cc: xen-devel , "Dong, Eddie" , "Nakajima, Jun" List-Id: xen-devel@lists.xenproject.org Hi folks, any chance to get this fixed? Juergen On 07.03.2014 06:10, Zhang, Yang Z wrote: > Jan Beulich wrote on 2014-03-05: >>>>> On 05.03.14 at 03:22, "Zhang, Yang Z" wrote: >>> Jan Beulich wrote on 2014-02-27: >>>>>>> On 27.02.14 at 02:31, "Zhang, Yang Z" >> wrote: >>>>> Jan Beulich wrote on 2014-02-27: >>>>>>>>> On 26.02.14 at 06:15, "Zhang, Yang Z" >>>> wrote: >>>>>>> @@ -2690,9 +2688,13 @@ void vmx_vmexit_handler(struct >>>> cpu_user_regs >>>>>> *regs) >>>>>>> __vmread(EXIT_QUALIFICATION, &exit_qualification); >>>>>>> HVMTRACE_1D(TRAP_DEBUG, exit_qualification); >>>>>>> write_debugreg(6, exit_qualification | 0xffff0ff0); >>>>>>> - if ( !v->domain->debugger_attached || >>>>>>> cpu_has_monitor_trap_flag ) - goto exit_and_crash; - >>>>>>> domain_pause_for_debugger(); + if ( >>>>>>> v->domain->debugger_attached ) + >>>>>>> domain_pause_for_debugger(); + else + { + >>>>>>> __restore_debug_registers(v); + >>>>>>> hvm_inject_hw_exception(TRAP_debug, >>>> HVM_DELIVER_NO_ERROR_CODE); + >>>>>>> } >>>>>> >>>>>> I suppose you need to set DR6.BS after restoring the reigsters? >>>>> >>>>> Right but is not enough. If flag_dr_dirty is set, we need to >>>>> restore register from hardware. Conversely, restore is from >>>>> debugreg and set >>>>> DR6 to exit_qualification. >>>> >>>> After some more thought, I in fact doubt that restoring the debug >>>> registers is in line with the current model: We should simply set >>>> DR6.BS in the in-memory copy when the debug registers aren't live >>>> yet (and it doesn't hurt to always do that). And since DR6 bits >>>> generally are sticky, I think exit_qualification actually needs to >>>> be or-ed into the >>> in-memory copy. >>> >>> Will guest be confused to see the DR6.BS always set? >> >> It certainly shouldn't when single stepping. And as long as the guest >> clears the bit while handling the single step trap, it won't see it set on other kinds of #DB. >> After all that's how hardware behaves. >> >>>> And presumably we would be better off if we dropped the >>>> interception of TRAP_debug once we restored the debug registers. >>> >>> Yes, it's unnecessary to trap into hypervisor always. Also, if we >>> set DR6.BS always, I guess there is no need to intercept the TRAP_debug. >> >> Oh, perhaps you misunderstood then: I didn't suggest to always set that flag. >> What I suggested is that during the intercepted TRAP_debug we should >> or exit_qualification (which ought to have BS set when single stepping >> with no other breakpoint enabled in DR7) into the in-memory copy of >> DR6. Once the intercept got dropped (as also suggested), hardware >> would again take care of setting DR6 correctly. > > Oh, I am sorry, I misunderstand you. > How about the following changes: Intercept the TRAP_debug when schedule out and drop it after restoring VCPU's DR register into hardware. > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index b128e81..5784dd1 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -372,6 +372,10 @@ static void vmx_save_dr(struct vcpu *v) > v->arch.hvm_vmx.exec_control |= CPU_BASED_MOV_DR_EXITING; > vmx_update_cpu_exec_control(v); > > + /* Trap debug for signle stepping. */ > + v->arch.hvm_vmx.exception_bitmap |= 1 << TRAP_debug; > + vmx_update_exception_bitmap(v); > + > v->arch.debugreg[0] = read_debugreg(0); > v->arch.debugreg[1] = read_debugreg(1); > v->arch.debugreg[2] = read_debugreg(2); > @@ -388,6 +392,13 @@ static void __restore_debug_registers(struct vcpu *v) > > v->arch.hvm_vcpu.flag_dr_dirty = 1; > > + /* > + * After restore, hardware has the right context. > + * So no need to trap debug anymore. > + */ > + v->arch.hvm_vmx.exception_bitmap |= ~(1 << TRAP_debug); > + vmx_update_exception_bitmap(v); > + > write_debugreg(0, v->arch.debugreg[0]); > write_debugreg(1, v->arch.debugreg[1]); > write_debugreg(2, v->arch.debugreg[2]); > @@ -1171,8 +1182,6 @@ void vmx_update_debug_state(struct vcpu *v) > unsigned long mask; > > mask = 1u << TRAP_int3; > - if ( !cpu_has_monitor_trap_flag ) > - mask |= 1u << TRAP_debug; > > if ( v->arch.hvm_vcpu.debug_state_latch ) > v->arch.hvm_vmx.exception_bitmap |= mask; > @@ -2689,10 +2698,18 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) > */ > __vmread(EXIT_QUALIFICATION, &exit_qualification); > HVMTRACE_1D(TRAP_DEBUG, exit_qualification); > - write_debugreg(6, exit_qualification | 0xffff0ff0); > - if ( !v->domain->debugger_attached || cpu_has_monitor_trap_flag ) > - goto exit_and_crash; > - domain_pause_for_debugger(); > + exit_qualification |= 0xffff0ff0; > + if ( v->domain->debugger_attached ) > + { > + write_debugreg(6, exit_qualification); > + domain_pause_for_debugger(); > + } > + else > + { > + __restore_debug_registers(v); > + write_debugreg(6, exit_qualification | read_debugreg(6)); > + hvm_inject_hw_exception(TRAP_debug, HVM_DELIVER_NO_ERROR_CODE); > + } > break; > case TRAP_int3: > { > diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h > index dcc3483..0d76d8c 100644 > --- a/xen/include/asm-x86/hvm/hvm.h > +++ b/xen/include/asm-x86/hvm/hvm.h > @@ -378,7 +378,8 @@ static inline int hvm_event_pending(struct vcpu *v) > (cpu_has_xsave ? X86_CR4_OSXSAVE : 0)))) > > /* These exceptions must always be intercepted. */ > -#define HVM_TRAP_MASK ((1U << TRAP_machine_check) | (1U << TRAP_invalid_op)) > +#define HVM_TRAP_MASK ((1U << TRAP_machine_check) | (1U << TRAP_invalid_op) |\ > + (1U << TRAP_debug)) > > /* > * x86 event types. This enumeration is valid for: > > > > Best regards, > Yang > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > > -- Juergen Gross Principal Developer Operating Systems PSO PM&D ES&S SWE OS6 Telephone: +49 (0) 89 62060 2932 Fujitsu e-mail: juergen.gross@ts.fujitsu.com Mies-van-der-Rohe-Str. 8 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html