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

[Gleb Natapov <gleb@redhat.com>]

On Mon, Jan 30, 2012 at 01:59:34PM +0200, Avi Kivity wrote:
> On 01/30/2012 12:50 PM, Gleb Natapov wrote:
> > On Mon, Jan 30, 2012 at 12:45:15PM +0200, Avi Kivity wrote:
> > > On 01/30/2012 12:35 PM, 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.
> > > 
> > > It's best to make it independent (like svm, and force vmx to emulate
> > > this behaviour).  Real mode forces cpl to 0, vm86 forces cpl to 3,
> > > protected mode (and I think long mode) uses cs.rpl.
> > This is what vmx does, not svm. 
> 
> That's the architectural definition, except for mode switch sequences. 
> vmx implements it directly which means that mode switch sequences
> sometimes fail, either in guest software (setting cr0.pe while cs & 3 !=
> 0) or in "microcode" (emulate.c).
> 
> > svm checks vmcb->cpl that can be
> > outdated during emulation.
> 
> This decoupling is actually helpful, since you can defer the cpl change
> until the end of the switch, and avoid inconsistencies like those
> checked by cs_ss_rpl_check().
> 
I am not saying it is not helpful. The fact that it exists tells us
that dpl and cpl are not always the same.  But cpl change should not be
delayed until the end of the switch! Mode switch happens in the middle of
a task switch. Task switch happens in 3 stages according to the spec. If
error happens during the first one (steps 1-11) it is handled by an old
task, if error happens during second stage (12 this is where mode change
happens) then anything can happen (we may kill vcpu till reset if we wish)
after that new task is running and all errors are handled by a new task.

To model this accurately we need to do task switch in this three stages
too and do a full register writeback after stage 2 before stage 3. Or
alternatively emulator should never access vcpu state during emulation.
Entire vcpu state should be in emulation ctx. But this is more
complicated and slow. 

--
			Gleb.