From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: KVM: x86: Push potential exception error code on task switches Date: Wed, 14 Apr 2010 14:43:00 +0200 Message-ID: <4BC5B854.1020809@siemens.com> References: <4BC5B0FB.8020700@siemens.com> <20100414123845.GA18132@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Marcelo Tosatti , kvm To: Gleb Natapov Return-path: Received: from goliath.siemens.de ([192.35.17.28]:20011 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755201Ab0DNMnP (ORCPT ); Wed, 14 Apr 2010 08:43:15 -0400 In-Reply-To: <20100414123845.GA18132@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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... Will do this in a preparational patch. > >> @@ -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. I should dramatically increase the overhead for the common case. :) Yeah, can do so if preferred. Thanks, Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux