From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: KVM: x86: Push potential exception error code on task switches Date: Wed, 14 Apr 2010 17:09:13 +0300 Message-ID: <20100414140913.GB18132@redhat.com> References: <4BC5B0FB.8020700@siemens.com> <20100414123845.GA18132@redhat.com> <4BC5BC62.2090904@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , Marcelo Tosatti , kvm To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:22618 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755727Ab0DNOJR (ORCPT ); Wed, 14 Apr 2010 10:09:17 -0400 Content-Disposition: inline In-Reply-To: <4BC5BC62.2090904@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: 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.