Re: KVM: x86: Push potential exception error code on task switches

[Gleb Natapov <gleb@redhat.com>]

On Wed, Apr 14, 2010 at 03:00:18PM +0200, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Wed, Apr 14, 2010 at 02:11:39PM +0200, Jan Kiszka wrote:
> >>  static int emulator_do_task_switch(struct x86_emulate_ctxt *ctxt,
> >> -				    struct x86_emulate_ops *ops,
> >> -				    u16 tss_selector, int reason)
> >> +				   struct x86_emulate_ops *ops,
> >> +				   u16 tss_selector, int reason,
> >> +				   bool has_error_code, u32 error_code)
> >>  {
> >>  	struct desc_struct curr_tss_desc, next_tss_desc;
> >>  	int ret;
> >> @@ -2416,12 +2417,23 @@ static int emulator_do_task_switch(struct x86_emulate_ctxt *ctxt,
> >>  	ops->set_cached_descriptor(&next_tss_desc, VCPU_SREG_TR, ctxt->vcpu);
> >>  	ops->set_segment_selector(tss_selector, VCPU_SREG_TR, ctxt->vcpu);
> >>  
> >> +	if (ret == X86EMUL_CONTINUE && has_error_code) {
> > It looks like we shouldn't get here if ret != X86EMUL_CONTINUE in the
> > first place. This check should be done just after call to
> > task_switch_16/32. Not directly related to your patch, but still...
> > 
> >> @@ -2416,12 +2417,23 @@ static int emulator_do_task_switch(struct
> >> x86_emulate_ctxt *ctxt,
> >>       ops->set_cached_descriptor(&next_tss_desc, VCPU_SREG_TR,
> >>       ctxt->vcpu);
> >>       ops->set_segment_selector(tss_selector, VCPU_SREG_TR,
> >>       ctxt->vcpu);
> >>  
> >> +     if (ret == X86EMUL_CONTINUE && has_error_code) {
> >> +             struct decode_cache *c = &ctxt->decode;
> >> +
> >> +             c->op_bytes = c->ad_bytes = (next_tss_desc.type & 8) ? 4
> >> : 2;
> >> +             c->lock_prefix = 0;
> >> +             c->src.val = (unsigned long) error_code;
> >> +             emulate_push(ctxt);
> >> +             ret = writeback(ctxt, ops);
> >> +     }
> > I would move writeback() to emulator_task_switch(). Just make
> > c->dst.type = OP_NONE if writeback is not needed.
> 
> BTW, how is state rollback realized if one of the steps raises an
> exception? Or where are the new state bits saved until the whole
> operation has succeeded so that they can be applied?
> 
Currently task switch code doesn't handle exception during task switch
absolutely correct. Task switch has three distinct steps. During first one
exceptions are delivered in a context of an old task, if exception happens
during second step arch state is undefined and finally if exception
happens during third step they are delivered in the context of a new
task.

--
			Gleb.