On 09/01/2009 01:42 PM, Jan Kiszka wrote:

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 891234b..9e3acbd 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3627,14 +3627,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)

        kvm_guest_enter();

-       get_debugreg(vcpu->arch.host_dr6, 6);
-       get_debugreg(vcpu->arch.host_dr7, 7);
        if (unlikely(vcpu->arch.switch_db_regs)) {
-               get_debugreg(vcpu->arch.host_db[0], 0);
-               get_debugreg(vcpu->arch.host_db[1], 1);
-               get_debugreg(vcpu->arch.host_db[2], 2);
-               get_debugreg(vcpu->arch.host_db[3], 3);
-
I'm fine with this for the common case, but we should switch to the safe
pattern on test_thread_flag(TIF_DEBUG), ie. if someone debugs qemu using
hardware break/watchpoints.

IIUC, thread.debugregs should be synced with the hardware registers. The kernel itself only writes to the debug registers, so we're safe doing the same.

                set_debugreg(0, 7);
                set_debugreg(vcpu->arch.eff_db[0], 0);
                set_debugreg(vcpu->arch.eff_db[1], 1);
@@ -3645,15 +3638,14 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
        trace_kvm_entry(vcpu->vcpu_id);
        kvm_x86_ops->run(vcpu);

-       if (unlikely(vcpu->arch.switch_db_regs)) {
-               set_debugreg(0, 7);
-               set_debugreg(vcpu->arch.host_db[0], 0);
-               set_debugreg(vcpu->arch.host_db[1], 1);
-               set_debugreg(vcpu->arch.host_db[2], 2);
-               set_debugreg(vcpu->arch.host_db[3], 3);
+       if (unlikely(vcpu->arch.switch_db_regs || test_thread_flag(TIF_DEBUG))) 
{
IIRC, you must clear dr7 before loading dr0..3 to avoid spurious effects.

Yeah, I thought of it but forgot to implement it.

+               set_debugreg(current->thread.debugreg0, 0);
+               set_debugreg(current->thread.debugreg1, 1);
+               set_debugreg(current->thread.debugreg2, 2);
+               set_debugreg(current->thread.debugreg3, 3);
+               set_debugreg(current->thread.debugreg6, 6);
+               set_debugreg(current->thread.debugreg7, 7);
        }
-       set_debugreg(vcpu->arch.host_dr6, 6);
-       set_debugreg(vcpu->arch.host_dr7, 7);
Unless I miss something ATM, moving this into the if-clause should cause
troubles if the guest leaves dr7 armed behind. But I need to refresh my
memory on this.

The hardware will clear dr7 on exit.

Updated patch to follow.

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to