x86: svm: Always clear event_inj on vmexit

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.

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 <[email protected]>

Signed-off-by Jan Kiszka <[email protected]>

---

(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(-)

target-i386/op_helper.c +1 -7

index dcbdfe7..caabdb4 100644
@@ -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). */

Hi,

Quoted 4 lines

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:

Quoted 14 lines · Erik van der Kouwe

Hi,

Quoted 4 lines

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:

Quoted 7 lines

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 <[email protected]>

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 <[email protected]>

Signed-off-by Jan Kiszka <[email protected]>

---
 target-i386/op_helper.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

target-i386/op_helper.c +1 -0

index dcbdfe7..52e8910 100644
@@ -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). */

Signature · 1 line

1.6.0.2