Re: [PATCH 2/7] KVM: VMX: relax check for CS register in rmode_segment_valid()

[Marcelo Tosatti <mtosatti@redhat.com>]

On Sat, Dec 22, 2012 at 12:55:43PM -0200, Marcelo Tosatti wrote:
> On Sat, Dec 22, 2012 at 09:02:41AM +0200, Gleb Natapov wrote:
> > On Fri, Dec 21, 2012 at 09:17:16PM -0200, Marcelo Tosatti wrote:
> > > On Wed, Dec 12, 2012 at 07:10:50PM +0200, Gleb Natapov wrote:
> > > > rmode_segment_valid() checks if segment descriptor can be used to enter
> > > > vm86 mode. VMX spec mandates that in vm86 mode CS register will be of
> > > > type data, not code. Lets allow guest entry with vm86 mode if the only
> > > > problem with CS register is incorrect type. Otherwise entire real mode
> > > > will be emulated.
> > > > 
> > > > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > > > ---
> > > >  arch/x86/kvm/vmx.c |    2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > > > index 4df3991..acbe86f 100644
> > > > --- a/arch/x86/kvm/vmx.c
> > > > +++ b/arch/x86/kvm/vmx.c
> > > > @@ -3383,6 +3383,8 @@ static bool rmode_segment_valid(struct kvm_vcpu *vcpu, int seg)
> > > >  	var.dpl = 0x3;
> > > >  	var.g = 0;
> > > >  	var.db = 0;
> > > > +	if (seg == VCPU_SREG_CS)
> > > > +		var.type = 0x3;
> > > >  	ar = vmx_segment_access_rights(&var);
> > > >  
> > > >  	if (var.base != (var.selector << 4))
> > > > -- 
> > > > 1.7.10.4
> > > 
> > > But with emulate_invalid_guest_state=1, segments are not fixed on 
> > > transition to real mode. So this patch can result in 
> > > invalid guest state entry failure.
> > > 
> > At the point of this patch emulate_invalid_guest_state=1 is broken and
> > sate is fixed unconditionally during the call to vmx_set_segment().
> 
> Err, i meant:
> 
> "But with emulate_invalid_guest_state=1, CS segment is not fixed
> on transition to real mode. So this patch can result in
> invalid guest state entry failure."
> 
> Which is true, see enter_rmode. So unless i am missing something,
> the patch opens the possibility for an invalid vm-entry which was 
> covered before.
> 
> Should you force vmx_set_segment(CS) with emulate_invalid_guest_state=1?
> (then it would be fixed unconditionally).

Oh, patch 3 fixes that, sorry. ACK.