RE: [Xen-devel] [PATCH][SVM] HVM fix for SWINT event injection

["Woller, Thomas" <thomas.woller@xxxxxxx>]

> 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