Re: [PATCH v2 3/3] KVM: x86 emulator: Allow PM/VM86 switch during task switch

[Avi Kivity <avi@redhat.com>]

On 01/30/2012 02:04 PM, Gleb Natapov wrote:
> On Mon, Jan 30, 2012 at 02:02:14PM +0200, Avi Kivity wrote:
> > On 01/30/2012 12:56 PM, Gleb Natapov wrote:
> > > On Mon, Jan 30, 2012 at 12:24:11PM +0200, Avi Kivity wrote:
> > > > > +
> > > > >  	ctxt->ops->set_segment(ctxt, selector, &desc, base3, seg);
> > > > >  }
> > > > >  
> > > > > @@ -2273,6 +2281,24 @@ static int load_state_from_tss32(struct x86_emulate_ctxt *ctxt,
> > > > >  		return emulate_gp(ctxt, 0);
> > > > >  	ctxt->_eip = tss->eip;
> > > > >  	ctxt->eflags = tss->eflags | 2;
> > > > > +
> > > > > +	/*
> > > > > +	 * If we're switching between Protected Mode and VM86, we need to make
> > > > > +	 * sure to update the mode before loading the segment descriptors so
> > > > > +	 * that the selectors are interpreted correctly.
> > > > > +	 *
> > > > > +	 * Need to get it to the vcpu struct immediately because it influences
> > > > > +	 * the CPL which is checked at least when loading the segment
> > > > > +	 * descriptors and when pushing an error code to the new kernel stack.
> > > > > +	 */
> > > > > +	if (ctxt->eflags & X86_EFLAGS_VM)
> > > > > +		ctxt->mode = X86EMUL_MODE_VM86;
> > > > > +	else
> > > > > +		ctxt->mode = X86EMUL_MODE_PROT32;
> > > > > +
> > > > 
> > > > Shouldn't this be done after the set_segment_selector() block?  My
> > > > interpretation of the SDM is that if a fault happens while loading
> > > > descriptors the fault happens with old segment cache, that is, it needs
> > > > to be interpreted according to the old mode.
> > > > 
> > > No, spec says:
> > > Any errors associated with this loading and qualification occur in the
> > > context of the new task.
> > 
> > There aren't any possible errors until that point.  But we don't want
> > set_segment_selector() to use the new mode, it just changes the segment
> > selectors, nothing else.
> > 
> I agree with that set_segment_selector() part, but
> load_segment_descriptor() happens on a new task.
>

Yes.  What I think is that the mode change should happen on the
boundary, after the set_segment_selector() block and before the
load_segment_descriptor() block.

-- 
error compiling committee.c: too many arguments to function