From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [patch 1/2] KVM: x86: handle double and triple faults for every exception Date: Wed, 11 Nov 2009 19:40:29 -0200 Message-ID: <20091111214029.GA14787@amt.cnet> References: <20091111192947.348198723@localhost.localdomain> <20091111193837.115825934@localhost.localdomain> <4AFB196C.7010302@web.de> <20091111204107.GA14011@amt.cnet> <4AFB265B.9030506@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, gleb@redhat.com, jan.kiszka@siemens.com, joerg.roedel@amd.com To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:32052 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759271AbZKKVk4 (ORCPT ); Wed, 11 Nov 2009 16:40:56 -0500 Content-Disposition: inline In-Reply-To: <4AFB265B.9030506@web.de> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Nov 11, 2009 at 10:02:19PM +0100, Jan Kiszka wrote: > Marcelo Tosatti wrote: > > On Wed, Nov 11, 2009 at 09:07:08PM +0100, Jan Kiszka wrote: > >> Marcelo Tosatti wrote: > >>> From: Joerg Roedel > >>> > >>> The current KVM x86 exception code handles double and triple faults only for > >>> page fault exceptions. This patch extends this detection for every exception > >>> that gets queued for the guest. > >>> > >>> Signed-off-by: Joerg Roedel > >>> CC: Jan Kiszka > >> For a moment I felt like I was time traveling - back in '08. :) > >> > >> Reading the archive I noticed that someone posted a fix-up for this patch: > >> > >> http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/16931 > >> > >> Why don't we need this anymore? > > > > I suppose qemu-kvm's call to set_sregs (via system_reset) will end up > > clearing pending exception? > > Right, forgot for the moment that triple fault implies user space. > > > > >> Moreover, are we sure to not regress /wrt to the cases that shall be > >> handled serially? So far they should have triggered the WARN_ON, right? > > > > Right. > > > > How can it regress though, given that serially handled exceptions are > > not supported at the moment (you get a WARN_ON and lose the previously > > queued anyway). > > The guest so far sees the second exception as the result, now it sees > DF. So the behavior changes from broken to broken, but I wondered if the > current state is already so broken that this change doesn't matter. I see your point. I suppose the WARN_ON is there to catch any code paths that could trigger (unsupported) multiple exceptions, and apparently no path does that now (other than pagefault which is handled separately) ? > Another micro difference is this: > > > @@ -184,24 +196,6 @@ void kvm_inject_page_fault(struct kvm_vc > > { > > ++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); > > } > > So far cr2 was not touched on DF, now it is. Yep. The PF was overwritten with DF, which means the cr2 value will not be interpreted by the guest? > > Jan >