> What I'll do is reorder SVM's intr.c core code to be the same
> as for VMX (it's currently bogusly different) but with the
> added constraint that we do not propagate exitintinfo.swint
> into eventinj. I'll add a comment to explain why.
Ok. Shoot me the patch first if you have time, I'll test with all the
HVM guests I have on hand.
Re: 15652
15658 looks good initially, appreciate the quick fixes.
tom
> -----Original Message-----
> From: Keir Fraser [mailto:Keir.Fraser@xxxxxxxxxxxx]
> Sent: Monday, July 30, 2007 4:17 PM
> To: Woller, Thomas; xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH][SVM] HVM fix for SWINT event
> injection
>
> Thanks for the detailed answer! It sounds like never
> injecting a SWINT is the right answer -- if we ever did, we
> could never tell whether exitintinfo==swint because of INTn
> execution in guest context (in which case RIP points at the
> SWINT instruction) or because of event injection (in which
> case RIP points past the SWINT instruction). Intel increments
> RIP on successful SWINT injection, which avoids this nasty edge case.
>
> What I'll do is reorder SVM's intr.c core code to be the same
> as for VMX (it's currently bogusly different) but with the
> added constraint that we do not propagate exitintinfo.swint
> into eventinj. I'll add a comment to explain why.
>
> -- Keir
>
> On 30/7/07 20:06, "Woller, Thomas" <thomas.woller@xxxxxxx> wrote:
>
> >> The right thing to do here is copy the
> exitintinfo->eventinj only if
> >> eventinj.fields.v==0. And to add a comment that in future the two
> >> events should be 'merged' in the architecturally correct manner
> >> (e.g., turned into #DF in some cases).
> > I agree with both in principle.
> >
> > Overall, if the eventinj field is filled when calling into
> > svm_intr_assist() then this event/exception should most likely take
> > precedence over exitintinfo. I did some profiling on some
> HVM guests
> > (not all), but in normal conditions. During these tests I only saw
> > both fields filled, *when* SWINT is in the exitintinfo
> field, and with
> > a #PF in the eventinj field. Not very elaborate testing
> though, just
> > booting HVM guests and running some rudimentary tests.
> >
> > What I did see though was that sometimes condition that
> > 1) exitintinfo field had SWINT information, and
> > 2) the eventinj.fields.v was not true (i.e. paging_fault()
> handled the
> > VMEXIT_EXECEPTION_PF).
> > And in this case, I discovered that we still do not want to
> move the
> > SWINT exitintinfo over to eventinj - just let the instruction start
> > re-execution.
> >
> >>
> >> At least I *think* this is the right thing to do -- but
> now I think
> >> about it, is it actually ever okay to copy a SWINT from
> exitintinfo
> >> into eventinj?
> > I believe the answer to this is no.
> >
> >> At the time the #PF vmexit happens, will EIP have been incremented
> >> across the SWINT instruction?
> > No, FAI can find out, but I was wondering this myself for
> all cases.
> > I did not observe any cases with SWINT in exitintinfo, and the EIP
> > *past* the "int NN"/SWINT instruction. So, just re-executing the
> > SWINT instruction seems to be acceptable here.
> >
> >> If not, does
> >> eventinj of a SWINT correctly cause EIP to be incremented?
> > Reinjecting the SWINT found in exitintinfo seems to cause
> the SWINT to
> > be executed twice (not good). So, just never injecting
> SWINT at this
> > time seems to be the best approach.
> >
> > Now, then, looking at the guest instruction in this case, might be
> > useful - and if the eip is not pointing to the "int nn"
> instruction in
> > question, then perhaps injecting the SWINT? I have not
> seen this case
> > in practice though.. If exitintinfo has SWINT, then the next
> > instruction is the unexecuted "int nn" instruction. but if
> there is a
> > series of SWINT instructions (can this happen?):
> > Int 10
> > Int 10
> > Int 10
> > Then, no telling how to tell if the instruction was
> executed or that
> > the next instruction is same as previous.
> > So, I don't think that looking at the next guest
> instruction is worth
> > any effort at this time.
> >
> > So, bottom line is that the code most likely needs:
> > 1) if eventinj is filled, then do not move over info from
> exitintinfo
> > 2) never inject SWINT from exitintinfo (allow instruction
> to start up
> > execution again)
> >
> > I will test more HVM guests with the instrumented the code to
> > determine if there is an occurances with both exitintinfo
> AND eventinj filled out.
> > I don't believe that we'll see any in usual testing, but do need to
> > handle "merging" at some point in the future.
> >
> >
> >>
> >>> Note that HVM guests will not boot on AMD-V with
> xen-staging.hg c/s
> >>> 15652.
> >>> Keir, do you want me to take a look at staging c/s 15652
> on AMD-V w/
> > HVM?
> >>
> >> Yes please!
> > I'll take a look at it at the guests that don't boot and
> see what's up.
> >
> > tom
> >
> >
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxxxxxxxx
> > http://lists.xensource.com/xen-devel
>
>
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|