From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gregory Haskins Subject: Re: [PATCH 5/9] [VMX] Do not re-execute INTn instruction. Date: Wed, 06 May 2009 06:59:05 -0400 Message-ID: <4A016D79.9040905@gmail.com> References: <1241511275-2261-1-git-send-email-gleb@redhat.com> <1241511275-2261-5-git-send-email-gleb@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig8E00A56884E37EC46429B4B5" Cc: avi@redhat.com, kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from qw-out-2122.google.com ([74.125.92.25]:26231 "EHLO qw-out-2122.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758922AbZEFK7J (ORCPT ); Wed, 6 May 2009 06:59:09 -0400 Received: by qw-out-2122.google.com with SMTP id 5so12893qwd.37 for ; Wed, 06 May 2009 03:59:09 -0700 (PDT) In-Reply-To: <1241511275-2261-5-git-send-email-gleb@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig8E00A56884E37EC46429B4B5 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Hi Gleb, Gleb Natapov wrote: > Re-inject event instead. This is what Intel suggest. Also use correct > instruction length when re-injecting soft fault/interrupt. > > Signed-off-by: Gleb Natapov > --- > arch/x86/include/asm/kvm_host.h | 5 ++++- > arch/x86/kvm/svm.c | 6 +++--- > arch/x86/kvm/vmx.c | 29 ++++++++++++++++++++++------- > arch/x86/kvm/x86.c | 13 ++++++++----- > arch/x86/kvm/x86.h | 9 ++++++++- > 5 files changed, 45 insertions(+), 17 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm= _host.h > index cc892f5..fea0429 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -319,6 +319,8 @@ struct kvm_vcpu_arch { > struct kvm_pio_request pio; > void *pio_data; > =20 > + u8 event_exit_inst_len; > + > struct kvm_queued_exception { > bool pending; > bool has_error_code; > @@ -328,6 +330,7 @@ struct kvm_vcpu_arch { > =20 > struct kvm_queued_interrupt { > bool pending; > + bool soft; > u8 nr; > } interrupt; > =20 > @@ -510,7 +513,7 @@ struct kvm_x86_ops { > void (*skip_emulated_instruction)(struct kvm_vcpu *vcpu); > void (*patch_hypercall)(struct kvm_vcpu *vcpu, > unsigned char *hypercall_addr); > - void (*set_irq)(struct kvm_vcpu *vcpu, int vec); > + void (*set_irq)(struct kvm_vcpu *vcpu, int vec, bool soft); > void (*set_nmi)(struct kvm_vcpu *vcpu); > void (*queue_exception)(struct kvm_vcpu *vcpu, unsigned nr, > bool has_error_code, u32 error_code); > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 14cdfce..d5173a2 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -2284,7 +2284,7 @@ static void svm_queue_irq(struct kvm_vcpu *vcpu, = unsigned nr) > SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_INTR; > } > =20 > -static void svm_set_irq(struct kvm_vcpu *vcpu, int irq) > +static void svm_set_irq(struct kvm_vcpu *vcpu, int irq, bool soft) > { > struct vcpu_svm *svm =3D to_svm(vcpu); > =20 > @@ -2392,7 +2392,7 @@ static void svm_complete_interrupts(struct vcpu_s= vm *svm) > case SVM_EXITINTINFO_TYPE_EXEPT: > /* In case of software exception do not reinject an exception > vector, but re-execute and instruction instead */ > - if (vector =3D=3D BP_VECTOR || vector =3D=3D OF_VECTOR) > + if (kvm_exception_is_soft(vector)) > break; > if (exitintinfo & SVM_EXITINTINFO_VALID_ERR) { > u32 err =3D svm->vmcb->control.exit_int_info_err; > @@ -2402,7 +2402,7 @@ static void svm_complete_interrupts(struct vcpu_s= vm *svm) > kvm_queue_exception(&svm->vcpu, vector); > break; > case SVM_EXITINTINFO_TYPE_INTR: > - kvm_queue_interrupt(&svm->vcpu, vector); > + kvm_queue_interrupt(&svm->vcpu, vector, false); > break; > default: > break; > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index a9b30e6..092a3ee 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -779,8 +779,9 @@ static void vmx_queue_exception(struct kvm_vcpu *vc= pu, unsigned nr, > return; > } > =20 > - if (nr =3D=3D BP_VECTOR || nr =3D=3D OF_VECTOR) { > - vmcs_write32(VM_ENTRY_INSTRUCTION_LEN, 1); > + if (kvm_exception_is_soft(nr)) { > + vmcs_write32(VM_ENTRY_INSTRUCTION_LEN, > + vmx->vcpu.arch.event_exit_inst_len); > intr_info |=3D INTR_TYPE_SOFT_EXCEPTION; > } else > intr_info |=3D INTR_TYPE_HARD_EXCEPTION; > @@ -2429,9 +2430,10 @@ static void enable_nmi_window(struct kvm_vcpu *v= cpu) > vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control); > } > =20 > -static void vmx_inject_irq(struct kvm_vcpu *vcpu, int irq) > +static void vmx_inject_irq(struct kvm_vcpu *vcpu, int irq, bool soft) > { > struct vcpu_vmx *vmx =3D to_vmx(vcpu); > + uint32_t intr; > =20 > KVMTRACE_1D(INJ_VIRQ, vcpu, (u32)irq, handler); > =20 > @@ -2446,8 +2448,14 @@ static void vmx_inject_irq(struct kvm_vcpu *vcpu= , int irq) > kvm_rip_write(vcpu, vmx->rmode.irq.rip - 1); > return; > } > - vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, > - irq | INTR_TYPE_EXT_INTR | INTR_INFO_VALID_MASK); > + intr =3D irq | INTR_INFO_VALID_MASK; > + if (soft) { > + intr |=3D INTR_TYPE_SOFT_INTR; > + vmcs_write32(VM_ENTRY_INSTRUCTION_LEN, > + vmx->vcpu.arch.event_exit_inst_len); > + } else > + intr |=3D INTR_TYPE_EXT_INTR; > + vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, intr); > } > =20 > static void vmx_inject_nmi(struct kvm_vcpu *vcpu) > @@ -3008,6 +3016,7 @@ static int handle_task_switch(struct kvm_vcpu *vc= pu, struct kvm_run *kvm_run) > GUEST_INTR_STATE_NMI); > break; > case INTR_TYPE_EXT_INTR: > + case INTR_TYPE_SOFT_INTR: > kvm_clear_interrupt_queue(vcpu); > break; > case INTR_TYPE_HARD_EXCEPTION: > @@ -3279,16 +3288,22 @@ static void vmx_complete_interrupts(struct vcpu= _vmx *vmx) > vmcs_clear_bits(GUEST_INTERRUPTIBILITY_INFO, > GUEST_INTR_STATE_NMI); > break; > - case INTR_TYPE_HARD_EXCEPTION: > case INTR_TYPE_SOFT_EXCEPTION: > + vmx->vcpu.arch.event_exit_inst_len =3D > + vmcs_read32(VM_EXIT_INSTRUCTION_LEN); > =20 Minor nit: Would suggest a "/* fall through */" comment here to denote the break was intentionally omitted (assuming it was, but I think so IIUC= ). > + case INTR_TYPE_HARD_EXCEPTION: > if (idt_vectoring_info & VECTORING_INFO_DELIVER_CODE_MASK) { > u32 err =3D vmcs_read32(IDT_VECTORING_ERROR_CODE); > kvm_queue_exception_e(&vmx->vcpu, vector, err); > } else > kvm_queue_exception(&vmx->vcpu, vector); > break; > + case INTR_TYPE_SOFT_INTR: > + vmx->vcpu.arch.event_exit_inst_len =3D > + vmcs_read32(VM_EXIT_INSTRUCTION_LEN); > case INTR_TYPE_EXT_INTR: > - kvm_queue_interrupt(&vmx->vcpu, vector); > + kvm_queue_interrupt(&vmx->vcpu, vector, > + type =3D=3D INTR_TYPE_SOFT_INTR); > break; > default: > break; > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 4596927..023842b 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -1424,7 +1424,7 @@ static int kvm_vcpu_ioctl_interrupt(struct kvm_vc= pu *vcpu, > return -ENXIO; > vcpu_load(vcpu); > =20 > - kvm_queue_interrupt(vcpu, irq->irq); > + kvm_queue_interrupt(vcpu, irq->irq, false); > =20 > vcpu_put(vcpu); > =20 > @@ -3136,7 +3136,8 @@ static void inject_irq(struct kvm_vcpu *vcpu) > } > =20 > if (vcpu->arch.interrupt.pending) { > - kvm_x86_ops->set_irq(vcpu, vcpu->arch.interrupt.nr); > + kvm_x86_ops->set_irq(vcpu, vcpu->arch.interrupt.nr, > + vcpu->arch.interrupt.soft); > return; > } > =20 > @@ -3149,8 +3150,10 @@ static void inject_irq(struct kvm_vcpu *vcpu) > } > } else if (kvm_cpu_has_interrupt(vcpu)) { > if (kvm_x86_ops->interrupt_allowed(vcpu)) { > - kvm_queue_interrupt(vcpu, kvm_cpu_get_interrupt(vcpu)); > - kvm_x86_ops->set_irq(vcpu, vcpu->arch.interrupt.nr); > + kvm_queue_interrupt(vcpu, kvm_cpu_get_interrupt(vcpu), > + false); > + kvm_x86_ops->set_irq(vcpu, vcpu->arch.interrupt.nr, > + false); > } > } > } > @@ -4077,7 +4080,7 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu= *vcpu, > pending_vec =3D find_first_bit( > (const unsigned long *)sregs->interrupt_bitmap, max_bits); > if (pending_vec < max_bits) { > - kvm_queue_interrupt(vcpu, pending_vec); > + kvm_queue_interrupt(vcpu, pending_vec, false); > pr_debug("Set back pending irq %d\n", pending_vec); > if (irqchip_in_kernel(vcpu->kvm)) > kvm_pic_clear_isr_ack(vcpu->kvm); > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h > index c1f1a8c..2817c86 100644 > --- a/arch/x86/kvm/x86.h > +++ b/arch/x86/kvm/x86.h > @@ -8,9 +8,11 @@ static inline void kvm_clear_exception_queue(struct kv= m_vcpu *vcpu) > vcpu->arch.exception.pending =3D false; > } > =20 > -static inline void kvm_queue_interrupt(struct kvm_vcpu *vcpu, u8 vecto= r) > +static inline void kvm_queue_interrupt(struct kvm_vcpu *vcpu, u8 vecto= r, > + bool soft) > { > vcpu->arch.interrupt.pending =3D true; > + vcpu->arch.interrupt.soft =3D soft; > vcpu->arch.interrupt.nr =3D vector; > } > =20 > @@ -24,4 +26,9 @@ static inline bool kvm_event_needs_reinjection(struct= kvm_vcpu *vcpu) > return vcpu->arch.exception.pending || vcpu->arch.interrupt.pending |= | > vcpu->arch.nmi_injected; > } > + > +static inline bool kvm_exception_is_soft(unsigned int nr) > +{ > + return (nr =3D=3D BP_VECTOR) || (nr =3D=3D OF_VECTOR) || (nr =3D=3D D= B_VECTOR); > +} > #endif > =20 --------------enig8E00A56884E37EC46429B4B5 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.11 (Darwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkoBbXoACgkQP5K2CMvXmqF4IQCfSUlTQVFkybOyRRXBw91U7NnH x5MAni9IA6DvnU04GsMmlYKVYSIj9QbJ =ftF/ -----END PGP SIGNATURE----- --------------enig8E00A56884E37EC46429B4B5--