From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=49646 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OJYBn-0002Ot-PK for qemu-devel@nongnu.org; Tue, 01 Jun 2010 16:35:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OJYBm-0001eN-BB for qemu-devel@nongnu.org; Tue, 01 Jun 2010 16:35:55 -0400 Received: from hera.amn.nl ([213.189.22.5]:3627 helo=AMN.nl) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OJYBm-0001dU-6D for qemu-devel@nongnu.org; Tue, 01 Jun 2010 16:35:54 -0400 Message-ID: <4C056F16.8000906@cs.vu.nl> Date: Tue, 01 Jun 2010 22:35:34 +0200 From: Erik van der Kouwe MIME-Version: 1.0 References: <4C05479E.3010705@siemens.com> In-Reply-To: <4C05479E.3010705@siemens.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH] x86: svm: Always clear event_inj on vmexit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Joerg Roedel , qemu-devel , Gleb Natapov Hi, > We currently only clear SVM_EVTINJ_VALID after successful interrupt > delivery. This apparently does not match real hardware which clears the > whole event_inj field on every vmexit, including unsuccessful interrupt > delivery. Thanks for the patch. It is a bit hard for me to test right now as I messed up my test setup, but I will do so ASAP and let you know. However, I'm worried that this patch may introduce a new problem (I may be mistaken though). There is still this code to load the exit interrupt info: stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_int_info_err), ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj_err))); Now that event_inj is no longer loaded, won't this mean that exit_int_info and exit_int_info_err also won't be loaded? With kind regards, Erik Jan Kiszka wrote: > We currently only clear SVM_EVTINJ_VALID after successful interrupt > delivery. This apparently does not match real hardware which clears the > whole event_inj field on every vmexit, including unsuccessful interrupt > delivery. > > Reported-by: Erik van der Kouwe > Signed-off-by: Jan Kiszka > --- > > (before it gets lost) > Erik, please confirm that this works for you. > > target-i386/op_helper.c | 8 +------- > 1 files changed, 1 insertions(+), 7 deletions(-) > > diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c > index dcbdfe7..caabdb4 100644 > --- a/target-i386/op_helper.c > +++ b/target-i386/op_helper.c > @@ -1263,13 +1263,6 @@ void do_interrupt(int intno, int is_int, int error_code, > #endif > do_interrupt_real(intno, is_int, error_code, next_eip); > } > - > -#if !defined(CONFIG_USER_ONLY) > - if (env->hflags & HF_SVMI_MASK) { > - uint32_t event_inj = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj)); > - stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj), event_inj & ~SVM_EVTINJ_VALID); > - } > -#endif > } > > /* This should come from sysemu.h - if we could include it here... */ > @@ -5388,6 +5381,7 @@ void helper_vmexit(uint32_t exit_code, uint64_t exit_info_1) > ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj))); > stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_int_info_err), > ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj_err))); > + stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj), 0); > > env->hflags2 &= ~HF2_GIF_MASK; > /* FIXME: Resets the current ASID register to zero (host ASID). */