Re: [PATCH v2 0/3] Fix task switches into/out of VM86

[Gleb Natapov <gleb@redhat.com>]

On Mon, Jan 30, 2012 at 11:35:00AM +0100, Kevin Wolf wrote:
> Am 30.01.2012 09:55, schrieb Gleb Natapov:
> > On Mon, Jan 30, 2012 at 09:48:33AM +0100, Kevin Wolf wrote:
> >> Am 27.01.2012 20:52, schrieb Gleb Natapov:
> >>> On Fri, Jan 27, 2012 at 08:23:33PM +0100, Kevin Wolf wrote:
> >>>> I believe this should work with both VMX and SVM now. Gleb, Jörg, can one of
> >>>> you test this with SVM? I did some testing on my buggy processor and it looks
> >>>> as good as it gets, but it would be better if you could confirm.
> >>>>
> >>> You forgot to set cpl to 3 in vmcb in svm_set_rflags() when vm86 is enabled, no?
> >>
> >> SVM updates the CPL when the segment selector for CS is loaded. From a
> >> svm.c POV, segment selectors are updated immediately after set_rflags,
> >> so it wouldn't really make a difference to do it twice.
> >>
> > It is too subtle to rely on that. The fact is that checking cpl after
> > set_rflags provides incorrect value. This better be fixed.
> 
> Depends on what value you consider to be correct between reloading
> eflags and reloading cs. I think it's logical and more consistent to say
> that CPL only changes when cs is reloaded, but you could argue that it's
> effective with the reload of rflags. It doesn't make a difference to
> guests, so we can decide to choose whatever we like.
> 
> Depending on what we decide on (Gleb and I disagree on this, so more
> input would be helpful), either VMX or SVM need a cleanup. I think it
> can be done independent from and on top of this fix.
> 
I think you made my point (that cpl in svm should be updated on rflags
update) by pointing me to this part of the spec:

The processor tests the VM flag under three general conditions:
• When loading segment registers, to determine whether to use 8086-style address translation.
• When decoding instructions, to determine which instructions are not supported in
   virtual-8086 mode and which instructions are sensitive to IOPL.
• When checking privileged instructions, on page accesses, or when performing
   other permission checks. (Virtual-8086 mode always executes at CPL 3.)

Bullet 3 clearly proves it.

Furthermore task switch loads eflags and segment selector at stage 12.
After that CPU runs on a new task, but since segment descriptors are
still not loaded CS dpl is not updated yet, but task is in CPL3 already.

> > BTW does load_state_from_tss16() need the same fix?
> 
> The manual says "Do not use a 16-bit TSS to implement a virtual-8086
> task." Actually, I don't think you could do that, even if you wanted,
> with a 16-bit flags field.
> 
Yes. May be there are other reasons to update flags earlier like spec
specifies, but I can think of any. Will fix them when we find them.

--
			Gleb.