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 <[email protected]>
> >>>
> >>> 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 <[email protected]>
> >>> CC: Jan Kiszka <[email protected]>
> >> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
