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

[Gleb Natapov <gleb@redhat.com>]

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(). See
big switch at the end of the function there. Note that here
rmode_segment_valid() does not check register properly anyway. It ignores
db value and allow guest entry with limit > 0xffff.  My patches fixes
this. After the patches are applied rmode_segment_valid() still relax
dpl and CS.type check:

	var.dpl = 0x3;
  	if (seg == VCPU_SREG_CS)
   		var.type = 0x3;

but fix_rmode_seg() unconditionally does exactly same:

        var.dpl = 0x3;
        if (seg == VCPU_SREG_CS)
                var.type = 0x3;

> Does this defeat the purpose of emulate_invalid_guest_state=1?
No since changing dpl to 3 and segment register type to 3 does not
affect instruction execution in vm86 mode, so entering vcpu in vm86
mode or fully emulate all instruction will yield exactly same result
no matter what instructions are executed. If we do not relax this check
in rmode_segment_valid() all real mode will be fully emulated because
no guest ever configure CS register to data type and set DPL of all
segments to 3 before entering real mode.

--
			Gleb.