Re: KVM: VMX: fix incorrect cached cpl value with real/v8086 modes

[Gleb Natapov <gleb@redhat.com>]

On Thu, Jan 03, 2013 at 11:19:18AM -0200, Marcelo Tosatti wrote:
> On Thu, Jan 03, 2013 at 10:11:53AM +0200, Gleb Natapov wrote:
> > > FreeBSD 9.1 with -smp 2.
> > I cannot reproduce. I do see boot failure on the next branch with 9.[01]
> > 64 bit -smp 2 here, but it is caused but segment registers been all
> > incorrect on a secondary vcpu and this patch does not help. Applying
> > http://comments.gmane.org/gmane.comp.emulators.kvm.devel/102593 fixes
> > it.
> > 
> > > > 
> > > > > > The question is how does it happen that we enter real mode while cache
> > > > > > is set to 3. It should never be 3 during boot since boot process never
> > > > > > enters the userspace.
> > > > > 
> > > > > Its transition _to_ real mode (from protected).
> > > > But in protected mode CPL should be 0 during boot.
> > > 
> > > BTX (FreeBSD's bootloader) does:
> > > 
> > > http://people.freebsd.org/~jhb/docs/loader.txt. 
> > Crazy. Regardless, I think your patch is correct and the current code
> > that uses cpl cache during emulation is wrong and it remains wrong even
> > after your patch. What if last time cpl cache was updated was while vcpu
> > ran in cpl 3? Commit message says that it tries to fix the case when
> > CS.selector&3 != 0 during transition to protected mode, but this can be
> > fixed by setting cpl cache to 0 in vmx_set_cr0() instead of clearing it.
> 
> Yes, i suppose so, can be done by a separatch patch, though.
> 
Yes. I will send the patch.

> > Two things about the patch itself. Get rid of __vmx_get_cpl() and call
> > to vmx_read_guest_seg_selector() directly from vmx_get_cpl() and drop
> > __clear_bit(VCPU_EXREG_CPL) from vmx_set_rflags() and vmx_set_cr0()
> > since vmx->cpl no longer depends on rflags and cr0.
> 
> You still want to invalidate vmx->cpl cache on cr0 writes:
> think protected -> real -> protected transition.
If CS.selector didn't change during the transition vmx->cpl will be set
to the same value and if it changes cpl cache will be cleared either by
vmx_set_segment() or vmexit.

> And as for EFLAGS, agreed. Sending new patch.

--
			Gleb.