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 <joerg.roedel@xxxxxxx> > >>> > >>> 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 <joerg.roedel@xxxxxxx> > >>> CC: Jan Kiszka <jan.kiszka@xxxxxx> > >> 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 > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
