| Message ID | 505A1F1F.1010606@siemens.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/19/2012 10:38 PM, Jan Kiszka wrote: > If we reset a vcpu on INIT, make sure to not touch dr7 as stored in the > VMCS/VMCB and also switch_db_regs if guest debugging is using hardware > breakpoints. Otherwise, the vcpu will not trigger hardware breakpoints > until userspace issues another KVM_SET_GUEST_DEBUG IOCTL for it. > > Found while trying to stop on start_secondary. > > @@ -1146,7 +1146,8 @@ static void init_vmcb(struct vcpu_svm *svm) > > svm_set_efer(&svm->vcpu, 0); > save->dr6 = 0xffff0ff0; > - save->dr7 = 0x400; > + if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_USE_HW_BP)) > + save->dr7 = 0x400; Whenever we multiplex a resource foo for several users, we should have a function update_foo() that recalculates foo from all its sources: vcpu->arch.dr7 = 0x400; update_dr7(&svm->vcpu); (don't know if the first line really belongs here) > kvm_set_rflags(&svm->vcpu, 2); > save->rip = 0x0000fff0; > svm->vcpu.arch.regs[VCPU_REGS_RIP] = save->rip; > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index d62b413..37f68f7 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -3959,7 +3959,8 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu) > kvm_rip_write(vcpu, 0); > kvm_register_write(vcpu, VCPU_REGS_RSP, 0); > > - vmcs_writel(GUEST_DR7, 0x400); > + if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)) > + vmcs_writel(GUEST_DR7, 0x400); > Ditto. update_dr7() could even be generic and use ->set_dr7(). > vmcs_writel(GUEST_GDTR_BASE, 0); > vmcs_write32(GUEST_GDTR_LIMIT, 0xffff); > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index c4d451e..7c8c2b8 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6043,7 +6043,8 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu) > vcpu->arch.nmi_pending = 0; > vcpu->arch.nmi_injected = false; > > - vcpu->arch.switch_db_regs = 0; > + if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)) > + vcpu->arch.switch_db_regs = 0; > memset(vcpu->arch.db, 0, sizeof(vcpu->arch.db)); > vcpu->arch.dr6 = DR6_FIXED_1; > vcpu->arch.dr7 = DR7_FIXED_1; > You could move the switch_db_regs calculation into update_dr7(), and move the update_dr7() call after assigning it here. This fixes everything neatly (and obviates the need for the first line in the snippet above).
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 611c728..4163ce2 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1146,7 +1146,8 @@ static void init_vmcb(struct vcpu_svm *svm) svm_set_efer(&svm->vcpu, 0); save->dr6 = 0xffff0ff0; - save->dr7 = 0x400; + if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_USE_HW_BP)) + save->dr7 = 0x400; kvm_set_rflags(&svm->vcpu, 2); save->rip = 0x0000fff0; svm->vcpu.arch.regs[VCPU_REGS_RIP] = save->rip; diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index d62b413..37f68f7 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -3959,7 +3959,8 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu) kvm_rip_write(vcpu, 0); kvm_register_write(vcpu, VCPU_REGS_RSP, 0); - vmcs_writel(GUEST_DR7, 0x400); + if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)) + vmcs_writel(GUEST_DR7, 0x400); vmcs_writel(GUEST_GDTR_BASE, 0); vmcs_write32(GUEST_GDTR_LIMIT, 0xffff); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c4d451e..7c8c2b8 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -6043,7 +6043,8 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu) vcpu->arch.nmi_pending = 0; vcpu->arch.nmi_injected = false; - vcpu->arch.switch_db_regs = 0; + if (!(vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP)) + vcpu->arch.switch_db_regs = 0; memset(vcpu->arch.db, 0, sizeof(vcpu->arch.db)); vcpu->arch.dr6 = DR6_FIXED_1; vcpu->arch.dr7 = DR7_FIXED_1;
If we reset a vcpu on INIT, make sure to not touch dr7 as stored in the VMCS/VMCB and also switch_db_regs if guest debugging is using hardware breakpoints. Otherwise, the vcpu will not trigger hardware breakpoints until userspace issues another KVM_SET_GUEST_DEBUG IOCTL for it. Found while trying to stop on start_secondary. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- Nice to have for stable as well. arch/x86/kvm/svm.c | 3 ++- arch/x86/kvm/vmx.c | 3 ++- arch/x86/kvm/x86.c | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-)