From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 3/5] KVM: VMX: Ensure interruptibility when single-stepping Date: Mon, 08 Dec 2008 10:17:09 +0100 Message-ID: <493CE615.1090401@siemens.com> References: <20081127114342.10901.31992.stgit@mchn012c.ww002.siemens.net> <20081127114343.10901.62425.stgit@mchn012c.ww002.siemens.net> <493B9FD2.4080304@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Hollis Blanchard , Joerg Roedel To: Avi Kivity Return-path: Received: from lizzard.sbs.de ([194.138.37.39]:17833 "EHLO lizzard.sbs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751298AbYLHJRh (ORCPT ); Mon, 8 Dec 2008 04:17:37 -0500 In-Reply-To: <493B9FD2.4080304@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Avi Kivity wrote: > Jan Kiszka wrote: >> When single-stepping, we have to ensure that the INT1 can make it >> through even if the guest itself is uninterruptible due to MOV SS or >> STI. VMENTRY will fail otherwise. >> >> Signed-off-by: Jan Kiszka >> --- >> >> arch/x86/kvm/vmx.c | 10 ++++++++-- >> 1 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 3a422dc..8e83102 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -1010,6 +1010,7 @@ static void vmx_cache_reg(struct kvm_vcpu *vcpu, >> enum kvm_reg reg) >> static int set_guest_debug(struct kvm_vcpu *vcpu, struct >> kvm_guest_debug *dbg) >> { >> int old_debug = vcpu->guest_debug; >> + u32 interruptibility; >> unsigned long flags; >> >> vcpu->guest_debug = dbg->control; >> @@ -1017,9 +1018,14 @@ static int set_guest_debug(struct kvm_vcpu >> *vcpu, struct kvm_guest_debug *dbg) >> vcpu->guest_debug = 0; >> >> flags = vmcs_readl(GUEST_RFLAGS); >> - if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) >> + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) { >> flags |= X86_EFLAGS_TF | X86_EFLAGS_RF; >> - else if (old_debug & KVM_GUESTDBG_SINGLESTEP) >> + /* We must be interruptible when single-stepping */ >> + interruptibility = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO); >> + if (interruptibility & 3) >> + vmcs_write32(GUEST_INTERRUPTIBILITY_INFO, >> + interruptibility & ~3); >> > > Could just write unconditionally - it's not like the write has any > effect on speed. vmcs_clear_bits() will do it cleanly. OK. > > But I'm worried about correctness. Suppose we're singlestepping a sti; > hlt sequence. While we'll clear interruptibility, probably receive the > debug trap (since that's a high priority exception), but then inject the > interrupt before the hlt, hanging the guest. So we probably need to > restore interruptibility on exit. > > This looks like a good candidate for a test case. You're concern might be valid. Will dig into the test infrastructure. Maybe it is a good idea to have something like the kgdb testsuite as part of the kvm tests to check debugger regressions, not just this special case. Jan -- Siemens AG, Corporate Technology, CT SE 26 Corporate Competence Center Embedded Linux