[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: "Woller, Thomas" <thomas.woller@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • From: Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>
  • Date: Mon, 30 Jul 2007 22:17:08 +0100
  • Delivery-date: Mon, 30 Jul 2007 14:11:17 -0700
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: AcfQYzPKvOx6lNgtT0G0uq8JV8aZRwAA5lXuAJrKr0AAB0EbsQ==
  • Thread-topic: [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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.