From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 3/5] KVM: VMX: Ensure interruptibility when single-stepping Date: Sun, 07 Dec 2008 12:05:06 +0200 Message-ID: <493B9FD2.4080304@redhat.com> References: <20081127114342.10901.31992.stgit@mchn012c.ww002.siemens.net> <20081127114343.10901.62425.stgit@mchn012c.ww002.siemens.net> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Hollis Blanchard , Joerg Roedel To: Jan Kiszka Return-path: Received: from mx2.redhat.com ([66.187.237.31]:48007 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752143AbYLGKFL (ORCPT ); Sun, 7 Dec 2008 05:05:11 -0500 In-Reply-To: <20081127114343.10901.62425.stgit@mchn012c.ww002.siemens.net> Sender: kvm-owner@vger.kernel.org List-ID: 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. 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. -- error compiling committee.c: too many arguments to function