From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60169 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OU4W6-0001LC-9W for qemu-devel@nongnu.org; Wed, 30 Jun 2010 17:08:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OU4W4-0005Fl-Dc for qemu-devel@nongnu.org; Wed, 30 Jun 2010 17:08:22 -0400 Received: from hall.aurel32.net ([88.191.82.174]:49507) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OU4W4-0005Fe-7u for qemu-devel@nongnu.org; Wed, 30 Jun 2010 17:08:20 -0400 Date: Wed, 30 Jun 2010 21:00:20 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH v2] x86: svm: Always clear event_inj on vmexit Message-ID: <20100630190020.GF24593@ohm.aurel32.net> References: <4C05479E.3010705@siemens.com> <4C056F16.8000906@cs.vu.nl> <4C05FEEA.20909@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <4C05FEEA.20909@web.de> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Joerg Roedel , Gleb Natapov , qemu-devel , Erik van der Kouwe On Wed, Jun 02, 2010 at 08:49:14AM +0200, Jan Kiszka wrote: > Erik van der Kouwe wrote: > > 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? > > Sorry, can't follow this ATM. But maybe you mean this: there is indeed a > problem with removing the clearance of event_inj.invalid as it may be > later on transferred into exit_int_info. And if we succeed with > injecting the event, that field must not remaind valid. > > OK, here is v2: > > -------> > > From: Jan Kiszka > > 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 > --- > target-i386/op_helper.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) Thanks, applied. > diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c > index dcbdfe7..52e8910 100644 > --- a/target-i386/op_helper.c > +++ b/target-i386/op_helper.c > @@ -5388,6 +5388,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). */ > -- > 1.6.0.2 > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net