[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] x86: svm: Always clear event_inj on vmexit
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH v2] x86: svm: Always clear event_inj on vmexit |
Date: |
Wed, 30 Jun 2010 21:00:20 +0200 |
User-agent: |
Mutt/1.5.20 (2009-06-14) |
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 <address@hidden>
>
> 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 <address@hidden>
> Signed-off-by: Jan Kiszka <address@hidden>
> ---
> 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
address@hidden http://www.aurel32.net