From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 4/4] Fix task switching. Date: Mon, 30 Mar 2009 18:04:45 +0200 Message-ID: <49D0ED9D.9080600@web.de> References: <20090329141202.30481.91797.stgit@trex.usersys.redhat.com> <20090329141218.30481.92675.stgit@trex.usersys.redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig5B5EC63F3351B10230157AAB" Cc: avi@redhat.com, kvm@vger.kernel.org, Bernhard Kohl , "Ostler, Thomas (NSN - DE/Munich)" , bliitz@users.sourceforge.net To: Gleb Natapov Return-path: Received: from fmmailgate02.web.de ([217.72.192.227]:48522 "EHLO fmmailgate02.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753250AbZC3QEu (ORCPT ); Mon, 30 Mar 2009 12:04:50 -0400 In-Reply-To: <20090329141218.30481.92675.stgit@trex.usersys.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig5B5EC63F3351B10230157AAB Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Gleb Natapov wrote: > The patch fixes two problems with task switching. > 1. Back link is written to a wrong TSS. > 2. Instruction emulation is not needed if the reason for task switch > is a task gate in IDT and access to it is caused by an external even= =2E >=20 > 2 is currently solved only for VMX since there is not reliable way to > skip an instruction in SVM. We should emulate it instead. Does this series fix all issues Bernhard, Thomas and Julian stumbled over= ? Jan >=20 > Signed-off-by: Gleb Natapov > --- >=20 > arch/x86/include/asm/svm.h | 1 + > arch/x86/kvm/svm.c | 25 ++++++++++++++++++------- > arch/x86/kvm/vmx.c | 40 +++++++++++++++++++++++++++++-------= ---- > arch/x86/kvm/x86.c | 40 +++++++++++++++++++++++++++++++-----= ---- > 4 files changed, 79 insertions(+), 27 deletions(-) >=20 > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h > index 82ada75..85574b7 100644 > --- a/arch/x86/include/asm/svm.h > +++ b/arch/x86/include/asm/svm.h > @@ -225,6 +225,7 @@ struct __attribute__ ((__packed__)) vmcb { > #define SVM_EVTINJ_VALID_ERR (1 << 11) > =20 > #define SVM_EXITINTINFO_VEC_MASK SVM_EVTINJ_VEC_MASK > +#define SVM_EXITINTINFO_TYPE_MASK SVM_EVTINJ_TYPE_MASK > =20 > #define SVM_EXITINTINFO_TYPE_INTR SVM_EVTINJ_TYPE_INTR > #define SVM_EXITINTINFO_TYPE_NMI SVM_EVTINJ_TYPE_NMI > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 1fcbc17..3ffb695 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -1823,17 +1823,28 @@ static int task_switch_interception(struct vcpu= _svm *svm, > struct kvm_run *kvm_run) > { > u16 tss_selector; > + int reason; > + int int_type =3D svm->vmcb->control.exit_int_info & > + SVM_EXITINTINFO_TYPE_MASK; > =20 > tss_selector =3D (u16)svm->vmcb->control.exit_info_1; > + > if (svm->vmcb->control.exit_info_2 & > (1ULL << SVM_EXITINFOSHIFT_TS_REASON_IRET)) > - return kvm_task_switch(&svm->vcpu, tss_selector, > - TASK_SWITCH_IRET); > - if (svm->vmcb->control.exit_info_2 & > - (1ULL << SVM_EXITINFOSHIFT_TS_REASON_JMP)) > - return kvm_task_switch(&svm->vcpu, tss_selector, > - TASK_SWITCH_JMP); > - return kvm_task_switch(&svm->vcpu, tss_selector, TASK_SWITCH_CALL); > + reason =3D TASK_SWITCH_IRET; > + else if (svm->vmcb->control.exit_info_2 & > + (1ULL << SVM_EXITINFOSHIFT_TS_REASON_JMP)) > + reason =3D TASK_SWITCH_JMP; > + else if (svm->vmcb->control.exit_int_info & SVM_EXITINTINFO_VALID) > + reason =3D TASK_SWITCH_GATE; > + else > + reason =3D TASK_SWITCH_CALL; > + > + > + if (reason !=3D TASK_SWITCH_GATE || int_type =3D=3D SVM_EXITINTINFO_T= YPE_SOFT) > + skip_emulated_instruction(&svm->vcpu); > + > + return kvm_task_switch(&svm->vcpu, tss_selector, reason); > } > =20 > static int cpuid_interception(struct vcpu_svm *svm, struct kvm_run *kv= m_run) > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 0da7a9e..01db958 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -3025,22 +3025,40 @@ static int handle_task_switch(struct kvm_vcpu *= vcpu, struct kvm_run *kvm_run) > struct vcpu_vmx *vmx =3D to_vmx(vcpu); > unsigned long exit_qualification; > u16 tss_selector; > - int reason; > + int reason, type, idt_v; > + > + idt_v =3D (vmx->idt_vectoring_info & VECTORING_INFO_VALID_MASK); > + type =3D (vmx->idt_vectoring_info & VECTORING_INFO_TYPE_MASK); > =20 > exit_qualification =3D vmcs_readl(EXIT_QUALIFICATION); > =20 > reason =3D (u32)exit_qualification >> 30; > - if (reason =3D=3D TASK_SWITCH_GATE && vmx->vcpu.arch.nmi_injected && > - (vmx->idt_vectoring_info & VECTORING_INFO_VALID_MASK) && > - (vmx->idt_vectoring_info & VECTORING_INFO_TYPE_MASK) > - =3D=3D INTR_TYPE_NMI_INTR) { > - vcpu->arch.nmi_injected =3D false; > - if (cpu_has_virtual_nmis()) > - vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO, > - GUEST_INTR_STATE_NMI); > + if (reason =3D=3D TASK_SWITCH_GATE && idt_v) { > + switch (type) { > + case INTR_TYPE_NMI_INTR: > + vcpu->arch.nmi_injected =3D false; > + if (cpu_has_virtual_nmis()) > + vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO, > + GUEST_INTR_STATE_NMI); > + break; > + case INTR_TYPE_EXT_INTR: > + kvm_clear_interrupt_queue(vcpu); > + break; > + case INTR_TYPE_HARD_EXCEPTION: > + case INTR_TYPE_SOFT_EXCEPTION: > + kvm_clear_exception_queue(vcpu); > + break; > + default: > + break; > + } > } > tss_selector =3D exit_qualification; > =20 > + if (!idt_v || (type !=3D INTR_TYPE_HARD_EXCEPTION && > + type !=3D INTR_TYPE_EXT_INTR && > + type !=3D INTR_TYPE_NMI_INTR)) > + skip_emulated_instruction(vcpu); > + > if (!kvm_task_switch(vcpu, tss_selector, reason)) > return 0; > =20 > @@ -3292,8 +3310,8 @@ static void vmx_complete_interrupts(struct vcpu_v= mx *vmx) > =20 > vector =3D idt_vectoring_info & VECTORING_INFO_VECTOR_MASK; > type =3D idt_vectoring_info & VECTORING_INFO_TYPE_MASK; > -=09 > - switch(type) { > + > + switch (type) { > case INTR_TYPE_NMI_INTR: > vmx->vcpu.arch.nmi_injected =3D true; > /* > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index ae4918c..573bb3f 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3697,7 +3697,6 @@ static void save_state_to_tss32(struct kvm_vcpu *= vcpu, > tss->fs =3D get_segment_selector(vcpu, VCPU_SREG_FS); > tss->gs =3D get_segment_selector(vcpu, VCPU_SREG_GS); > tss->ldt_selector =3D get_segment_selector(vcpu, VCPU_SREG_LDTR); > - tss->prev_task_link =3D get_segment_selector(vcpu, VCPU_SREG_TR); > } > =20 > static int load_state_from_tss32(struct kvm_vcpu *vcpu, > @@ -3794,8 +3793,8 @@ static int load_state_from_tss16(struct kvm_vcpu = *vcpu, > } > =20 > static int kvm_task_switch_16(struct kvm_vcpu *vcpu, u16 tss_selector,= > - u32 old_tss_base, > - struct desc_struct *nseg_desc) > + u16 old_tss_sel, u32 old_tss_base, > + struct desc_struct *nseg_desc) > { > struct tss_segment_16 tss_segment_16; > int ret =3D 0; > @@ -3814,6 +3813,16 @@ static int kvm_task_switch_16(struct kvm_vcpu *v= cpu, u16 tss_selector, > &tss_segment_16, sizeof tss_segment_16)) > goto out; > =20 > + if (old_tss_sel !=3D 0xffff) { > + tss_segment_16.prev_task_link =3D old_tss_sel; > + > + if (kvm_write_guest(vcpu->kvm, > + get_tss_base_addr(vcpu, nseg_desc), > + &tss_segment_16.prev_task_link, > + sizeof tss_segment_16.prev_task_link)) > + goto out; > + } > + > if (load_state_from_tss16(vcpu, &tss_segment_16)) > goto out; > =20 > @@ -3823,7 +3832,7 @@ out: > } > =20 > static int kvm_task_switch_32(struct kvm_vcpu *vcpu, u16 tss_selector,= > - u32 old_tss_base, > + u16 old_tss_sel, u32 old_tss_base, > struct desc_struct *nseg_desc) > { > struct tss_segment_32 tss_segment_32; > @@ -3843,6 +3852,16 @@ static int kvm_task_switch_32(struct kvm_vcpu *v= cpu, u16 tss_selector, > &tss_segment_32, sizeof tss_segment_32)) > goto out; > =20 > + if (old_tss_sel !=3D 0xffff) { > + tss_segment_32.prev_task_link =3D old_tss_sel; > + > + if (kvm_write_guest(vcpu->kvm, > + get_tss_base_addr(vcpu, nseg_desc), > + &tss_segment_32.prev_task_link, > + sizeof tss_segment_32.prev_task_link)) > + goto out; > + } > + > if (load_state_from_tss32(vcpu, &tss_segment_32)) > goto out; > =20 > @@ -3896,14 +3915,17 @@ int kvm_task_switch(struct kvm_vcpu *vcpu, u16 = tss_selector, int reason) > kvm_x86_ops->set_rflags(vcpu, eflags & ~X86_EFLAGS_NT); > } > =20 > - kvm_x86_ops->skip_emulated_instruction(vcpu); > + /* set back link to prev task only if NT bit is set in eflags > + note that old_tss_sel is not used afetr this point */ > + if (reason !=3D TASK_SWITCH_CALL && reason !=3D TASK_SWITCH_GATE) > + old_tss_sel =3D 0xffff; > =20 > if (nseg_desc.type & 8) > - ret =3D kvm_task_switch_32(vcpu, tss_selector, old_tss_base, > - &nseg_desc); > + ret =3D kvm_task_switch_32(vcpu, tss_selector, old_tss_sel, > + old_tss_base, &nseg_desc); > else > - ret =3D kvm_task_switch_16(vcpu, tss_selector, old_tss_base, > - &nseg_desc); > + ret =3D kvm_task_switch_16(vcpu, tss_selector, old_tss_sel, > + old_tss_base, &nseg_desc); > =20 > if (reason =3D=3D TASK_SWITCH_CALL || reason =3D=3D TASK_SWITCH_GATE)= { > u32 eflags =3D kvm_x86_ops->get_rflags(vcpu); >=20 > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >=20 --------------enig5B5EC63F3351B10230157AAB Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAknQ7Z0ACgkQniDOoMHTA+lorgCdGYDD+zLC19Zh9NfIoGkmmBwE qvkAn1DhuDRVEwNQ3tsTiCvXCk2vcFiS =CYMe -----END PGP SIGNATURE----- --------------enig5B5EC63F3351B10230157AAB--