From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [patch 0/2] Handle multiple exceptions (fixes Win2003 reboot by triple fault) Date: Mon, 23 Nov 2009 14:52:35 -0200 Message-ID: <20091123165235.GA9472@amt.cnet> References: <20091111192947.348198723@localhost.localdomain> <20091112122124.GB7392@redhat.com> <4AFC027B.1090101@siemens.com> <20091112130542.GD7392@redhat.com> <4AFFFA10.9020407@redhat.com> <20091119155407.GE3193@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , Jan Kiszka , kvm@vger.kernel.org, joerg.roedel@amd.com To: Gleb Natapov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:6197 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754721AbZKWSPn (ORCPT ); Mon, 23 Nov 2009 13:15:43 -0500 Content-Disposition: inline In-Reply-To: <20091119155407.GE3193@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Nov 19, 2009 at 05:54:07PM +0200, Gleb Natapov wrote: > > It's actually less readable. I know 11 is between 10 and 13, but is > > NP_VECTOR between TS_VECTOR and GP_VECTOR? > > > > This is better as a switch, or even: > > > > u8 exception_class[] = { > > [PF_VECTOR] EXPT_PF, > > > > etc. > OK what about this then: Looks good. > > From: Eddie Dong > > Move Double-Fault generation logic out of page fault > exception generating function to cover more generic case. > > Signed-off-by: Eddie Dong > Signed-off-by: Gleb Natapov > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 9b0602a..5e95ff9 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -248,12 +248,68 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data) > } > EXPORT_SYMBOL_GPL(kvm_set_apic_base); > > +#define EXCPT_BENIGN 0 > +#define EXCPT_CONTRIBUTORY 1 > +#define EXCPT_PF 2 > + > +static int exception_class(int vector) > +{ > + switch (vector) { > + case PF_VECTOR: > + return EXCPT_PF; > + case DE_VECTOR: > + case TS_VECTOR: > + case NP_VECTOR: > + case SS_VECTOR: > + case GP_VECTOR: > + return EXCPT_CONTRIBUTORY; > + default: > + break; > + } > + return EXCPT_BENIGN; > +} > + > +static void kvm_multiple_exception(struct kvm_vcpu *vcpu, > + unsigned nr, bool has_error, u32 error_code) > +{ > + u32 prev_nr; > + int class1, class2; > + > + if (!vcpu->arch.exception.pending) { > + queue: > + vcpu->arch.exception.pending = true; > + vcpu->arch.exception.has_error_code = has_error; > + vcpu->arch.exception.nr = nr; > + vcpu->arch.exception.error_code = error_code; > + return; > + } > + > + /* to check exception */ > + prev_nr = vcpu->arch.exception.nr; > + if (prev_nr == DF_VECTOR) { > + /* triple fault -> shutdown */ > + set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); > + return; > + } > + class1 = exception_class(prev_nr); > + class2 = exception_class(nr); > + if ((class1 == EXCPT_CONTRIBUTORY && class2 == EXCPT_CONTRIBUTORY) > + || (class1 == EXCPT_PF && class2 != EXCPT_BENIGN)) { > + /* generate double fault per SDM Table 5-5 */ > + vcpu->arch.exception.pending = true; > + vcpu->arch.exception.has_error_code = true; > + vcpu->arch.exception.nr = DF_VECTOR; > + vcpu->arch.exception.error_code = 0; > + } else > + /* replace previous exception with a new one in a hope > + that instruction re-execution will regenerate lost > + exception */ > + goto queue; > +} > + > void kvm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr) > { > - WARN_ON(vcpu->arch.exception.pending); > - vcpu->arch.exception.pending = true; > - vcpu->arch.exception.has_error_code = false; > - vcpu->arch.exception.nr = nr; > + kvm_multiple_exception(vcpu, nr, false, 0); > } > EXPORT_SYMBOL_GPL(kvm_queue_exception); > > @@ -261,25 +317,6 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr, > u32 error_code) > { > ++vcpu->stat.pf_guest; > - > - if (vcpu->arch.exception.pending) { > - switch(vcpu->arch.exception.nr) { > - case DF_VECTOR: > - /* triple fault -> shutdown */ > - set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); > - return; > - case PF_VECTOR: > - vcpu->arch.exception.nr = DF_VECTOR; > - vcpu->arch.exception.error_code = 0; > - return; > - default: > - /* replace previous exception with a new one in a hope > - that instruction re-execution will regenerate lost > - exception */ > - vcpu->arch.exception.pending = false; > - break; > - } > - } > vcpu->arch.cr2 = addr; > kvm_queue_exception_e(vcpu, PF_VECTOR, error_code); > } > @@ -292,11 +329,7 @@ EXPORT_SYMBOL_GPL(kvm_inject_nmi); > > void kvm_queue_exception_e(struct kvm_vcpu *vcpu, unsigned nr, u32 error_code) > { > - WARN_ON(vcpu->arch.exception.pending); > - vcpu->arch.exception.pending = true; > - vcpu->arch.exception.has_error_code = true; > - vcpu->arch.exception.nr = nr; > - vcpu->arch.exception.error_code = error_code; > + kvm_multiple_exception(vcpu, nr, true, error_code); > } > EXPORT_SYMBOL_GPL(kvm_queue_exception_e); > > -- > Gleb. > -- > 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