Fix&Enhance NMI support for KVM - v3

This patch adds the required bits to the VMX side for user space injected NMIs. As with the preexisting in-kernel irqchip support, the CPU must provide the "virtual NMI" feature for proper tracking of the NMI blocking state.

Based on the original patch by Sheng Yang.

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

---
 arch/x86/kvm/vmx.c |   33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

Index: b/arch/x86/kvm/vmx.c
===================================================================

arch/x86/kvm/vmx.c +22 -0

@@ -2356,6 +2356,7 @@ static void vmx_inject_nmi(struct kvm_vc
 {
        struct vcpu_vmx *vmx = to_vmx(vcpu);
 
+       ++vcpu->stat.nmi_injections;
        if (vcpu->arch.rmode.active) {
                vmx->rmode.irq.pending = true;
                vmx->rmode.irq.vector = NMI_VECTOR;

@@ -2424,6 +2425,30 @@ static void do_interrupt_requests(struct
 {
        vmx_update_window_states(vcpu);
 
+       if (cpu_has_virtual_nmis()) {
+               if (vcpu->arch.nmi_pending && !vcpu->arch.nmi_injected) {
+                       if (vcpu->arch.nmi_window_open) {
+                               vcpu->arch.nmi_pending = false;
+                               vcpu->arch.nmi_injected = true;
+                       } else {
+                               enable_nmi_window(vcpu);
+                               return;
+                       }
+               }
+               if (vcpu->arch.nmi_injected) {
+                       vmx_inject_nmi(vcpu);
+                       if (vcpu->arch.nmi_pending
+                           || kvm_run->request_nmi_window)
+                               enable_nmi_window(vcpu);
+                       else if (vcpu->arch.irq_summary
+                                || kvm_run->request_interrupt_window)
+                               enable_irq_window(vcpu);
+                       return;
+               }
+               if (!vcpu->arch.nmi_window_open ||

kvm_run->request_nmi_window) +
enable_nmi_window(vcpu);

arch/x86/kvm/vmx.c +2 -0

+       }
+
        if (vcpu->arch.interrupt_window_open) {
                if (vcpu->arch.irq_summary &&

!vcpu->arch.interrupt.pending) kvm_do_inject_irq(vcpu);

arch/x86/kvm/vmx.c +2 -0

@@ -2936,6 +2961,14 @@ static int handle_nmi_window(struct kvm_
        vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control);
        ++vcpu->stat.nmi_window_exits;
 
+       /*
+        * If the user space waits to inject a NNI, exit as soon as

possible + */

arch/x86/kvm/vmx.c +5 -0

+       if (kvm_run->request_nmi_window && !vcpu->arch.nmi_pending) {
+               kvm_run->exit_reason = KVM_EXIT_NMI_WINDOW_OPEN;
+               return 0;
+       }
+
        return 1;
 }

Quoted 60 lines · On Monday 22 September 2008 15:59:02 Jan Kiszka wrote:

This patch adds the required bits to the VMX side for user space injected NMIs. As with the preexisting in-kernel irqchip support, the CPU must provide the "virtual NMI" feature for proper tracking of the NMI blocking state.

Based on the original patch by Sheng Yang.

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

---
 arch/x86/kvm/vmx.c |   33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

Index: b/arch/x86/kvm/vmx.c
===================================================================

arch/x86/kvm/vmx.c +22 -0

@@ -2356,6 +2356,7 @@ static void vmx_inject_nmi(struct kvm_vc
 {
        struct vcpu_vmx *vmx = to_vmx(vcpu);
 
+       ++vcpu->stat.nmi_injections;
        if (vcpu->arch.rmode.active) {
                vmx->rmode.irq.pending = true;
                vmx->rmode.irq.vector = NMI_VECTOR;

@@ -2424,6 +2425,30 @@ static void do_interrupt_requests(struct
 {
        vmx_update_window_states(vcpu);
 
+       if (cpu_has_virtual_nmis()) {
+               if (vcpu->arch.nmi_pending && !vcpu->arch.nmi_injected) {
+                       if (vcpu->arch.nmi_window_open) {
+                               vcpu->arch.nmi_pending = false;
+                               vcpu->arch.nmi_injected = true;
+                       } else {
+                               enable_nmi_window(vcpu);
+                               return;
+                       }
+               }
+               if (vcpu->arch.nmi_injected) {
+                       vmx_inject_nmi(vcpu);
+                       if (vcpu->arch.nmi_pending
+                           || kvm_run->request_nmi_window)
+                               enable_nmi_window(vcpu);
+                       else if (vcpu->arch.irq_summary
+                                || kvm_run->request_interrupt_window)
+                               enable_irq_window(vcpu);
+                       return;
+               }
+               if (!vcpu->arch.nmi_window_open ||

kvm_run->request_nmi_window) +
enable_nmi_window(vcpu);

arch/x86/kvm/vmx.c +2 -0

+       }
+
        if (vcpu->arch.interrupt_window_open) {
                if (vcpu->arch.irq_summary &&

!vcpu->arch.interrupt.pending) kvm_do_inject_irq(vcpu);

arch/x86/kvm/vmx.c +0 -0

@@ -2936,6 +2961,14 @@ static int handle_nmi_window(struct kvm_
        vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control);
        ++vcpu->stat.nmi_window_exits;
 

Quoted 3 lines

arch/x86/kvm/vmx.c +2 -0

+       /*
+        * If the user space waits to inject a NNI, exit as soon as

possible + */

o... found a typo :)

arch/x86/kvm/vmx.c +3 -0

@@ -2356,6 +2384,19 @@ static void vmx_inject_nmi(struct kvm_vc
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 
+	if (!cpu_has_virtual_nmis()) {
+		/*
+		 * Tracking the NMI-blocked state in software is built upon

Quoted 4 lines · Jan Kiszka

arch/x86/kvm/vmx.c +0 -0

@@ -2356,6 +2384,19 @@ static void vmx_inject_nmi(struct kvm_vc
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 

arch/x86/kvm/vmx.c static void vmx_inject_nmi(struct kvm_vc · line 2399

Quoted 13 lines

arch/x86/kvm/vmx.c +13 -0

+	if (!cpu_has_virtual_nmis()) {
+		/*
+		 * Tracking the NMI-blocked state in software is built upon
+		 * finding the next open IRQ window. This, in turn, depends on
+		 * well-behaving guests: They have to keep IRQs disabled at
+		 * least as long as the NMI handler runs. Otherwise we may
+		 * cause NMI nesting, maybe breaking the guest. But as this is
+		 * highly unlikely, we can live with the residual risk.
+		 */
+		vmx->soft_vnmi_blocked = 1;
+		vmx->vnmi_blocked_time = 0;
+	}
+

We still get here with vmx->soft_vnmi_blocked = 1. Trying to find out how.

Quoted 5 lines · Gleb Natapov

Quoted 4 lines · Jan Kiszka

arch/x86/kvm/vmx.c +0 -0

@@ -2356,6 +2384,19 @@ static void vmx_inject_nmi(struct kvm_vc
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 

Quoted 15 lines

arch/x86/kvm/vmx.c static void vmx_inject_nmi(struct kvm_vc · line 2399

Quoted 13 lines

arch/x86/kvm/vmx.c +13 -0

+	if (!cpu_has_virtual_nmis()) {
+		/*
+		 * Tracking the NMI-blocked state in software is built upon
+		 * finding the next open IRQ window. This, in turn, depends on
+		 * well-behaving guests: They have to keep IRQs disabled at
+		 * least as long as the NMI handler runs. Otherwise we may
+		 * cause NMI nesting, maybe breaking the guest. But as this is
+		 * highly unlikely, we can live with the residual risk.
+		 */
+		vmx->soft_vnmi_blocked = 1;
+		vmx->vnmi_blocked_time = 0;
+	}
+

We still get here with vmx->soft_vnmi_blocked = 1. Trying to find out how.

We should only come along here with vnmi blocked on reinjection (after a fault on calling the handler).

Quoted 21 lines · Jan Kiszka

Quoted 20 lines · Gleb Natapov

Quoted 7 lines · Jan Kiszka

arch/x86/kvm/vmx.c +3 -0

@@ -2356,6 +2384,19 @@ static void vmx_inject_nmi(struct kvm_vc
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 
+	if (!cpu_has_virtual_nmis()) {
+		/*
+		 * Tracking the NMI-blocked state in software is built upon

arch/x86/kvm/vmx.c static void vmx_inject_nmi(struct kvm_vc · line 2399

Quoted 10 lines

arch/x86/kvm/vmx.c +10 -0

+		 * finding the next open IRQ window. This, in turn, depends on
+		 * well-behaving guests: They have to keep IRQs disabled at
+		 * least as long as the NMI handler runs. Otherwise we may
+		 * cause NMI nesting, maybe breaking the guest. But as this is
+		 * highly unlikely, we can live with the residual risk.
+		 */
+		vmx->soft_vnmi_blocked = 1;
+		vmx->vnmi_blocked_time = 0;
+	}
+

We still get here with vmx->soft_vnmi_blocked = 1. Trying to find out how.

Quoted 3 lines

We should only come along here with vnmi blocked on reinjection (after a fault on calling the handler).

I see that nmi_injected is never cleared and it is check before calling vmx_inject_nmi();

Quoted 25 lines · Gleb Natapov

Quoted 24 lines · Jan Kiszka

Quoted 5 lines · Gleb Natapov

Quoted 4 lines · Jan Kiszka

arch/x86/kvm/vmx.c +0 -0

@@ -2356,6 +2384,19 @@ static void vmx_inject_nmi(struct kvm_vc
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 

Quoted 15 lines

arch/x86/kvm/vmx.c static void vmx_inject_nmi(struct kvm_vc · line 2399

Quoted 13 lines

arch/x86/kvm/vmx.c +13 -0

+	if (!cpu_has_virtual_nmis()) {
+		/*
+		 * Tracking the NMI-blocked state in software is built upon
+		 * finding the next open IRQ window. This, in turn, depends on
+		 * well-behaving guests: They have to keep IRQs disabled at
+		 * least as long as the NMI handler runs. Otherwise we may
+		 * cause NMI nesting, maybe breaking the guest. But as this is
+		 * highly unlikely, we can live with the residual risk.
+		 */
+		vmx->soft_vnmi_blocked = 1;
+		vmx->vnmi_blocked_time = 0;
+	}
+

We still get here with vmx->soft_vnmi_blocked = 1. Trying to find out how.

We should only come along here with vnmi blocked on reinjection (after a fault on calling the handler).

Quoted 2 lines

I see that nmi_injected is never cleared and it is check before calling vmx_inject_nmi();

That should happen in vmx_complete_interrupts, but only if the exit takes place after the NMI has been successfully delivered to the guest (which is not the case if invoking the handler raises an exception). So far for the theory...

Quoted 32 lines · Jan Kiszka

Quoted 30 lines · Gleb Natapov

Quoted 24 lines · Jan Kiszka

Quoted 14 lines · Gleb Natapov

Quoted 13 lines · Jan Kiszka

arch/x86/kvm/vmx.c +3 -0

@@ -2356,6 +2384,19 @@ static void vmx_inject_nmi(struct kvm_vc
 {
         struct vcpu_vmx *vmx = to_vmx(vcpu);
 
+        if (!cpu_has_virtual_nmis()) {
+                /*
+                 * Tracking the NMI-blocked state in software is

built upon + * finding the next open IRQ window.
This, in turn, depends on + * well-behaving guests:
They have to keep IRQs disabled at + * least as long
as the NMI handler runs. Otherwise we may + * cause
NMI nesting, maybe breaking the guest. But as this is +
* highly unlikely, we can live with the residual risk. +

Quoted 8 lines

arch/x86/kvm/vmx.c static void vmx_inject_nmi(struct kvm_vc · line 2399

Quoted 5 lines

       */

arch/x86/kvm/vmx.c +4 -0

+                vmx->soft_vnmi_blocked = 1;
+                vmx->vnmi_blocked_time = 0;
+        }
+

We still get here with vmx->soft_vnmi_blocked = 1. Trying to find out how.

Quoted 2 lines

We should only come along here with vnmi blocked on reinjection (after a fault on calling the handler).

I see that nmi_injected is never cleared and it is check before calling vmx_inject_nmi();

Quoted 4 lines

That should happen in vmx_complete_interrupts, but only if the exit takes place after the NMI has been successfully delivered to the guest (which is not the case if invoking the handler raises an exception). So far for the theory...

Okey, I have this one in dmesg:
kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

We still get here with vmx->soft_vnmi_blocked = 1. Trying to find out how.

We should only come along here with vnmi blocked on reinjection (after a fault on calling the handler).

I see that nmi_injected is never cleared and it is check before calling vmx_inject_nmi();

Quoted 8 lines

Quoted 4 lines

That should happen in vmx_complete_interrupts, but only if the exit takes place after the NMI has been successfully delivered to the guest (which is not the case if invoking the handler raises an exception). So far for the theory...

Okey, I have this one in dmesg:
kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9

Oh... Another task switch issue...

I think it's may not be a issue import by this patchset? Seems need more debug...

The patchset is OK for me, except I don't know when we would need that timeout one (buggy guest?...), and we may also root cause this issue or ensure that it's not a regression.

Quoted 23 lines · Sheng

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

We still get here with vmx->soft_vnmi_blocked = 1. Trying to find out how.

We should only come along here with vnmi blocked on reinjection (after a fault on calling the handler).

I see that nmi_injected is never cleared and it is check before calling vmx_inject_nmi();

Quoted 8 lines

Quoted 4 lines

That should happen in vmx_complete_interrupts, but only if the exit takes place after the NMI has been successfully delivered to the guest (which is not the case if invoking the handler raises an exception). So far for the theory...

Okey, I have this one in dmesg:
kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9

Oh... Another task switch issue...

I think it's may not be a issue import by this patchset? Seems need more debug...

Quoted 3 lines

The patchset is OK for me, except I don't know when we would need that timeout one (buggy guest?...), and we may also root cause this issue or ensure that it's not a regression.

Without the patch series kvm doesn't inject NMIs on this machine, so guest hangs. It's hard to tell if this message is caused by these patches or not.

Quoted 28 lines · On Tuesday 23 September 2008 17:15:09 Gleb Natapov wrote:

Quoted 26 lines · Sheng

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

We still get here with vmx->soft_vnmi_blocked = 1. Trying to find out how.

We should only come along here with vnmi blocked on reinjection (after a fault on calling the handler).

I see that nmi_injected is never cleared and it is check before calling vmx_inject_nmi();

Quoted 8 lines

Quoted 4 lines

That should happen in vmx_complete_interrupts, but only if the exit takes place after the NMI has been successfully delivered to the guest (which is not the case if invoking the handler raises an exception). So far for the theory...

Okey, I have this one in dmesg:
kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9

Oh... Another task switch issue...

I think it's may not be a issue import by this patchset? Seems need more debug...

The patchset is OK for me, except I don't know when we would need that timeout one (buggy guest?...), and we may also root cause this issue or ensure that it's not a regression.

Quoted 3 lines

Without the patch series kvm doesn't inject NMIs on this machine, so guest hangs. It's hard to tell if this message is caused by these patches or not.

Maybe try to reproduce it on virtual_nmi support machine is OK. But I only got Windows 2003 server edition by the hand. Does other Windows behaviour the same?

Quoted 33 lines · Sheng

Quoted 31 lines · On Tuesday 23 September 2008 17:15:09 Gleb Natapov wrote:

Quoted 18 lines · Sheng

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

We still get here with vmx->soft_vnmi_blocked = 1. Trying to find out how.

We should only come along here with vnmi blocked on reinjection (after a fault on calling the handler).

I see that nmi_injected is never cleared and it is check before calling vmx_inject_nmi();

Quoted 8 lines

Quoted 4 lines

That should happen in vmx_complete_interrupts, but only if the exit takes place after the NMI has been successfully delivered to the guest (which is not the case if invoking the handler raises an exception). So far for the theory...

Okey, I have this one in dmesg:
kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9

Quoted 8 lines

Oh... Another task switch issue...

I think it's may not be a issue import by this patchset? Seems need more debug...

The patchset is OK for me, except I don't know when we would need that timeout one (buggy guest?...), and we may also root cause this issue or ensure that it's not a regression.

Without the patch series kvm doesn't inject NMIs on this machine, so guest hangs. It's hard to tell if this message is caused by these patches or not.

Quoted 3 lines

Maybe try to reproduce it on virtual_nmi support machine is OK. But I only got Windows 2003 server edition by the hand. Does other Windows behaviour the same?

That is exactly what I am using. Run it with SMP hal and do hibernate.

Quoted 38 lines · On Tuesday 23 September 2008 17:26:55 Gleb Natapov wrote:

Quoted 36 lines · Sheng

Quoted 24 lines · On Tuesday 23 September 2008 17:15:09 Gleb Natapov wrote:

Quoted 23 lines · Sheng

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

We still get here with vmx->soft_vnmi_blocked = 1. Trying to find out how.

We should only come along here with vnmi blocked on reinjection (after a fault on calling the handler).

I see that nmi_injected is never cleared and it is check before calling vmx_inject_nmi();

Quoted 8 lines

Quoted 4 lines

That should happen in vmx_complete_interrupts, but only if the exit takes place after the NMI has been successfully delivered to the guest (which is not the case if invoking the handler raises an exception). So far for the theory...

Okey, I have this one in dmesg:
kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9

Oh... Another task switch issue...

I think it's may not be a issue import by this patchset? Seems need more debug...

Quoted 7 lines

Quoted 3 lines

The patchset is OK for me, except I don't know when we would need that timeout one (buggy guest?...), and we may also root cause this issue or ensure that it's not a regression.

Without the patch series kvm doesn't inject NMIs on this machine, so guest hangs. It's hard to tell if this message is caused by these patches or not.

Maybe try to reproduce it on virtual_nmi support machine is OK. But I only got Windows 2003 server edition by the hand. Does other Windows behaviour the same?

Quoted 1 line

That is exactly what I am using. Run it with SMP hal and do hibernate.

Oh... Finally found how to enable that hibernate option....

And this hibernate works on my virtual_nmi supported box, with smp hal and 2 cpus.

Quoted 6 lines

Quoted 1 line

That is exactly what I am using. Run it with SMP hal and do hibernate.

Oh... Finally found how to enable that hibernate option....

And this hibernate works on my virtual_nmi supported box, with smp hal and 2 cpus.

However, for this hibernate won't success if there is no NMI support, maybe we can say it's not a "regression"...

Quoted 5 lines · Sheng

Quoted 5 lines

Quoted 2 lines

That is exactly what I am using. Run it with SMP hal and do hibernate.

Oh... Finally found how to enable that hibernate option....

Quoted 5 lines

Quoted 2 lines

And this hibernate works on my virtual_nmi supported box, with smp hal and 2 cpus.

However, for this hibernate won't success if there is no NMI support, maybe we can say it's not a "regression"...

I am not saying it's a regression, but it would be nice to have it working :)

Quoted 5 lines · On Tuesday 23 September 2008 17:45:44 Gleb Natapov wrote:

Quoted 4 lines · Sheng

Quoted 4 lines

Quoted 2 lines

That is exactly what I am using. Run it with SMP hal and do hibernate.

Oh... Finally found how to enable that hibernate option....

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

And this hibernate works on my virtual_nmi supported box, with smp hal and 2 cpus.

However, for this hibernate won't success if there is no NMI support, maybe we can say it's not a "regression"...

I am not saying it's a regression, but it would be nice to have it working :)

Yeah, of course. :)

After Windows' graphical installation phase I had a hanging guest. At the same time I got

kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9
kvm_handle_exit: Breaking out of NMI-blocked state on VCPU 0 after 1 s timeout

in the kernel log as well. Something is borken. Will retest the installation with vanilla KVM. Anyone any ideas on the task switch thing? Just a false positive or an indication for the real problem in that domain?

Quoted 13 lines · Sheng

Quoted 12 lines · On Tuesday 23 September 2008 17:45:44 Gleb Natapov wrote:

Quoted 8 lines · Sheng

Quoted 6 lines

Quoted 2 lines

That is exactly what I am using. Run it with SMP hal and do hibernate.

Oh... Finally found how to enable that hibernate option....

And this hibernate works on my virtual_nmi supported box, with smp hal and 2 cpus.

However, for this hibernate won't success if there is no NMI support, maybe we can say it's not a "regression"...

I am not saying it's a regression, but it would be nice to have it working :)

Quoted 1 line

Yeah, of course. :)

OK, I've a 2003 server up and running now, I'm able to reproduce

kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9

but not via hibernate (it suspends and powers off normally, but then hangs after resume), rather by manually injecting an NMI on CPU0.

After Windows' graphical installation phase I had a hanging guest. At the same time I got

kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9
kvm_handle_exit: Breaking out of NMI-blocked state on VCPU 0 after 1 s timeout

in the kernel log as well. Something is borken. Will retest the installation with vanilla KVM. Anyone any ideas on the task switch thing? Just a false positive or an indication for the real problem in that domain?

Quoted 19 lines · Jan Kiszka

Quoted 6 lines · Sheng

Quoted 5 lines · On Tuesday 23 September 2008 17:45:44 Gleb Natapov wrote:

Quoted 4 lines · Sheng

Quoted 4 lines

Quoted 2 lines

That is exactly what I am using. Run it with SMP hal and do hibernate.

Oh... Finally found how to enable that hibernate option....

Quoted 8 lines

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

And this hibernate works on my virtual_nmi supported box, with smp hal and 2 cpus.

However, for this hibernate won't success if there is no NMI support, maybe we can say it's not a "regression"...

I am not saying it's a regression, but it would be nice to have it working :)

Yeah, of course. :)

OK, I've a 2003 server up and running now, I'm able to reproduce

kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9

Quoted 3 lines

but not via hibernate (it suspends and powers off normally, but then hangs after resume), rather by manually injecting an NMI on CPU0.

I found out today that on regular windows 2003 the problem does not exist (on hibernate at least). The image I have was used to run WLK tests (windows logo kit) and this kit changes something in windows kernel to do additional stuff after hibernation and that is where we crash.

Quoted 8 lines

After Windows' graphical installation phase I had a hanging guest. At the same time I got

kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9
kvm_handle_exit: Breaking out of NMI-blocked state on VCPU 0 after 1 s timeout

Quoted 5 lines

in the kernel log as well. Something is borken. Will retest the installation with vanilla KVM. Anyone any ideas on the task switch thing? Just a false positive or an indication for the real problem in that domain?

Nothing is broken IMO. The IDT entry for NMI is set up as task gate so we get a task switch exit after NMI injection.

We should do something like this:

arch/x86/kvm/vmx.c +1 -0

index 046a91b..860e66d 100644
@@ -2826,10 +2826,20 @@ static int handle_task_switch(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	unsigned long exit_qualification;
 	u16 tss_selector;
 	int reason;
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
 
 	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
 

arch/x86/kvm/vmx.c +0 -1

 	if (exit_reason < kvm_vmx_max_exit_handlers
 
--

Gleb.

Quoted 14 lines

We should do something like this:

arch/x86/kvm/vmx.c +1 -0

index 046a91b..860e66d 100644
@@ -2826,10 +2826,20 @@ static int handle_task_switch(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	unsigned long exit_qualification;
 	u16 tss_selector;
 	int reason;
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
 
 	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
 

arch/x86/kvm/vmx.c static int handle_task_switch(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) · line 2842

Quoted 10 lines

arch/x86/kvm/vmx.c +9 -0

 	reason = (u32)exit_qualification >> 30;
+
+	if (reason == TASK_SWITCH_GATE && vmx->vcpu.arch.nmi_injected &&
+		(vmx->idt_vectoring_info & VECTORING_INFO_VALID_MASK) &&
+		(vmx->idt_vectoring_info & VECTORING_INFO_TYPE_MASK) == INTR_TYPE_NMI_INTR) {
+		vcpu->arch.nmi_injected = false;
+		vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
+				      GUEST_INTR_STATE_NMI);
+		printk(KERN_DEBUG"NMI cause task switch. No need to reinject\n");
+	}

OK, I just think we are not supposed to set GUEST_INTR_STATE_NMI without cpu_has_virtual_nmis(). Otherwise looks reasonable. Have you tested this? Does it make your 2003 power-off?

Quoted 14 lines · Jan Kiszka

Quoted 14 lines

We should do something like this:

arch/x86/kvm/vmx.c +1 -0

index 046a91b..860e66d 100644
@@ -2826,10 +2826,20 @@ static int handle_task_switch(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	unsigned long exit_qualification;
 	u16 tss_selector;
 	int reason;
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
 
 	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
 

Quoted 14 lines

arch/x86/kvm/vmx.c static int handle_task_switch(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) · line 2842

Quoted 10 lines

arch/x86/kvm/vmx.c +9 -0

 	reason = (u32)exit_qualification >> 30;
+
+	if (reason == TASK_SWITCH_GATE && vmx->vcpu.arch.nmi_injected &&
+		(vmx->idt_vectoring_info & VECTORING_INFO_VALID_MASK) &&
+		(vmx->idt_vectoring_info & VECTORING_INFO_TYPE_MASK) == INTR_TYPE_NMI_INTR) {
+		vcpu->arch.nmi_injected = false;
+		vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
+				      GUEST_INTR_STATE_NMI);
+		printk(KERN_DEBUG"NMI cause task switch. No need to reinject\n");
+	}

OK, I just think we are not supposed to set GUEST_INTR_STATE_NMI without cpu_has_virtual_nmis(). Otherwise looks reasonable. Have you tested this? Does it make your 2003 power-off?

It does power-off, but hands during reboot. Looking at it right now.

Quoted 1 line

It does power-off, but hands during reboot. Looking at it right now.

After completing the RAM read-back from disk? This is where it hangs

Quoted 2 lines · Jan Kiszka

Quoted 1 line

It does power-off, but hands during reboot. Looking at it right now.

After completing the RAM read-back from disk? This is where it hangs

Much earlier. BIOS hangs because CPU1 ignores SIPI.

Quoted 4 lines · Gleb Natapov

Quoted 2 lines · Jan Kiszka

Quoted 1 line

It does power-off, but hands during reboot. Looking at it right now.

After completing the RAM read-back from disk? This is where it hangs

Much earlier. BIOS hangs because CPU1 ignores SIPI.

That sounds like an APIC state reset issue.

BTW, I'm getting tons of

Ignoring de-assert INIT to vcpu 0 Ignoring de-assert INIT to vcpu 1 Ignoring de-assert INIT to vcpu 0 Ignoring de-assert INIT to vcpu 1 Ignoring de-assert INIT to vcpu 1 SIPI to vcpu 1 vector 0x24
vcpu 1 received sipi with vector # 24 SIPI to vcpu 1 vector 0x24
SIPI to vcpu 1 vector 0x24 Ignoring de-assert INIT to vcpu 0 Ignoring de-assert INIT to vcpu 1 SIPI to vcpu 1 vector 0x24

on my box with SMP guests (Linux and Windows). Do they all point to yet incomplete emulations, or are they just far too verbose?

Quoted 8 lines · Gleb Natapov

Quoted 4 lines · Jan Kiszka

Quoted 2 lines

It does power-off, but hands during reboot. Looking at it right now.

After completing the RAM read-back from disk? This is where it hangs

Much earlier. BIOS hangs because CPU1 ignores SIPI.

That sounds like an APIC state reset issue.

BTW, I'm getting tons of

Ignoring de-assert INIT to vcpu 0 Ignoring de-assert INIT to vcpu 1 Ignoring de-assert INIT to vcpu 0 Ignoring de-assert INIT to vcpu 1 Ignoring de-assert INIT to vcpu 1 SIPI to vcpu 1 vector 0x24
vcpu 1 received sipi with vector # 24 SIPI to vcpu 1 vector 0x24
SIPI to vcpu 1 vector 0x24 Ignoring de-assert INIT to vcpu 0 Ignoring de-assert INIT to vcpu 1 SIPI to vcpu 1 vector 0x24

on my box with SMP guests (Linux and Windows). Do they all point to yet incomplete emulations, or are they just far too verbose?

Quoted 2 lines · Jan Kiszka

Quoted 1 line

It does power-off, but hands during reboot. Looking at it right now.

After completing the RAM read-back from disk? This is where it hangs

Much earlier. BIOS hangs because CPU1 ignores SIPI.

Quoted 4 lines · Gleb Natapov

Quoted 2 lines · Jan Kiszka

Quoted 2 lines

It does power-off, but hands during reboot. Looking at it right now.

Quoted 4 lines

Quoted 2 lines

After completing the RAM read-back from disk? This is where it hangs

Much earlier. BIOS hangs because CPU1 ignores SIPI.

Apropos APIC: I also have this with Win2003 in my kernel log:

apic write: bad size=1 fee00030

Quoted 14 lines · Jan Kiszka

Quoted 14 lines

We should do something like this:

arch/x86/kvm/vmx.c +1 -0

index 046a91b..860e66d 100644
@@ -2826,10 +2826,20 @@ static int handle_task_switch(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	unsigned long exit_qualification;
 	u16 tss_selector;
 	int reason;
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
 
 	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
 

Quoted 14 lines

arch/x86/kvm/vmx.c static int handle_task_switch(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) · line 2842

Quoted 10 lines

arch/x86/kvm/vmx.c +9 -0

 	reason = (u32)exit_qualification >> 30;
+
+	if (reason == TASK_SWITCH_GATE && vmx->vcpu.arch.nmi_injected &&
+		(vmx->idt_vectoring_info & VECTORING_INFO_VALID_MASK) &&
+		(vmx->idt_vectoring_info & VECTORING_INFO_TYPE_MASK) == INTR_TYPE_NMI_INTR) {
+		vcpu->arch.nmi_injected = false;
+		vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
+				      GUEST_INTR_STATE_NMI);
+		printk(KERN_DEBUG"NMI cause task switch. No need to reinject\n");
+	}

OK, I just think we are not supposed to set GUEST_INTR_STATE_NMI without cpu_has_virtual_nmis(). Otherwise looks reasonable. Have you tested this? Does it make your 2003 power-off?

It does power-off, but hands during reboot. Looking at it right now.

Quoted 5 lines

Quoted 4 lines

OK, I just think we are not supposed to set GUEST_INTR_STATE_NMI without cpu_has_virtual_nmis(). Otherwise looks reasonable. Have you tested this? Does it make your 2003 power-off?

It does power-off, but hands during reboot. Looking at it right now.

Yeah, with your patch I'm getting a totally different Blue Screen on spurious manual NMI injection:

"Hardware Malfunction

Call your hardware vendor for support"

8)

Quoted 9 lines · Jan Kiszka

Quoted 5 lines

Quoted 4 lines

OK, I just think we are not supposed to set GUEST_INTR_STATE_NMI without cpu_has_virtual_nmis(). Otherwise looks reasonable. Have you tested this? Does it make your 2003 power-off?

It does power-off, but hands during reboot. Looking at it right now.

Yeah, with your patch I'm getting a totally different Blue Screen on spurious manual NMI injection:

Quoted 6 lines

"Hardware Malfunction

Call your hardware vendor for support"

8)

Cool!

Quoted 9 lines · Gleb Natapov

Quoted 8 lines · Jan Kiszka

Quoted 5 lines

Quoted 4 lines

OK, I just think we are not supposed to set GUEST_INTR_STATE_NMI without cpu_has_virtual_nmis(). Otherwise looks reasonable. Have you tested this? Does it make your 2003 power-off?

It does power-off, but hands during reboot. Looking at it right now.

Yeah, with your patch I'm getting a totally different Blue Screen on spurious manual NMI injection:

Quoted 8 lines

Quoted 6 lines

"Hardware Malfunction

Call your hardware vendor for support"

8)

Cool!

BTW I have this is also needed in case CPU was reseted before it had a chance to handle NMI:

arch/x86/kvm/vmx.c +0 -0

index 5927b79..106e16d 100644

Quoted 14 lines · Jan Kiszka

Quoted 14 lines

We should do something like this:

arch/x86/kvm/vmx.c +1 -0

index 046a91b..860e66d 100644
@@ -2826,10 +2826,20 @@ static int handle_task_switch(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	unsigned long exit_qualification;
 	u16 tss_selector;
 	int reason;
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
 
 	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
 

Quoted 15 lines

arch/x86/kvm/vmx.c static int handle_task_switch(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) · line 2842

Quoted 10 lines

arch/x86/kvm/vmx.c +9 -0

 	reason = (u32)exit_qualification >> 30;
+
+	if (reason == TASK_SWITCH_GATE && vmx->vcpu.arch.nmi_injected &&
+		(vmx->idt_vectoring_info & VECTORING_INFO_VALID_MASK) &&
+		(vmx->idt_vectoring_info & VECTORING_INFO_TYPE_MASK) == INTR_TYPE_NMI_INTR) {
+		vcpu->arch.nmi_injected = false;
+		vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
+				      GUEST_INTR_STATE_NMI);
+		printk(KERN_DEBUG"NMI cause task switch. No need to reinject\n");
+	}

OK, I just think we are not supposed to set GUEST_INTR_STATE_NMI without cpu_has_virtual_nmis(). Otherwise looks reasonable. Have you tested this? Does it make your 2003 power-off?

It does power-off, but hands during reboot. Looking at it right now.

Quoted 23 lines · Sheng

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

We still get here with vmx->soft_vnmi_blocked = 1. Trying to find out how.

We should only come along here with vnmi blocked on reinjection (after a fault on calling the handler).

I see that nmi_injected is never cleared and it is check before calling vmx_inject_nmi();

Quoted 8 lines

Quoted 4 lines

That should happen in vmx_complete_interrupts, but only if the exit takes place after the NMI has been successfully delivered to the guest (which is not the case if invoking the handler raises an exception). So far for the theory...

Okey, I have this one in dmesg:
kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9

Oh... Another task switch issue...

I think it's may not be a issue import by this patchset? Seems need more debug...

Quoted 3 lines

The patchset is OK for me, except I don't know when we would need that timeout one (buggy guest?...), and we may also root cause this issue or ensure that it's not a regression.

Without the patch series kvm doesn't inject NMIs on this machine, so guest hangs. It's hard to tell if this message is caused by these patches or not.

--

Quoted 29 lines · On Tuesday 23 September 2008 17:00:21 Gleb Natapov wrote:

Quoted 28 lines · Jan Kiszka

Quoted 27 lines · Gleb Natapov

Quoted 22 lines · Jan Kiszka

Quoted 5 lines · Gleb Natapov

Quoted 4 lines · Jan Kiszka

(quoted diff) +0 -0

@@ -2356,6 +2384,19 @@ static void vmx_inject_nmi(struct kvm_vc
 {
         struct vcpu_vmx *vmx = to_vmx(vcpu);
 

Quoted 16 lines

Quoted 10 lines

(quoted diff) +3 -0

+        if (!cpu_has_virtual_nmis()) {
+                /*
+                 * Tracking the NMI-blocked state in software is

built upon + * finding the next open IRQ window.
This, in turn, depends on + * well-behaving guests:
They have to keep IRQs disabled at + * least as long
as the NMI handler runs. Otherwise we may + * cause
NMI nesting, maybe breaking the guest. But as this is +
* highly unlikely, we can live with the residual risk. +

       */

Review

Quoted 4 lines

(quoted diff) +4 -0

+                vmx->soft_vnmi_blocked = 1;
+                vmx->vnmi_blocked_time = 0;
+        }
+

We still get here with vmx->soft_vnmi_blocked = 1. Trying to find out how.

Quoted 2 lines

We should only come along here with vnmi blocked on reinjection (after a fault on calling the handler).

I see that nmi_injected is never cleared and it is check before calling vmx_inject_nmi();

Quoted 7 lines

Quoted 4 lines

That should happen in vmx_complete_interrupts, but only if the exit takes place after the NMI has been successfully delivered to the guest (which is not the case if invoking the handler raises an exception). So far for the theory...

Okey, I have this one in dmesg:
kvm_handle_exit: unexpected, valid vectoring info and exit reason is 0x9

Oh... Another task switch issue...

I think it's may not be a issue import by this patchset? Seems need more debug...

The patchset is OK for me, except I don't know when we would need that timeout one (buggy guest?...), and we may also root cause this issue or ensure that it's not a regression.

Jan Kiszka wrote:
..

Quoted 11 lines

Index: b/arch/x86/kvm/vmx.c
===================================================================

arch/x86/kvm/vmx.c +3 -0

@@ -90,6 +90,11 @@ struct vcpu_vmx {
 	} rmode;
 	int vpid;
 	bool emulation_required;
+
+	/* Support for vnmi-less CPUs */
+	int soft_vnmi_blocked;

arch/x86/kvm/vmx.c struct vcpu_vmx { · line 99

Quoted 3 lines

arch/x86/kvm/vmx.c +2 -0

+	ktime_t entry_time;
+	s64 vnmi_blocked_time;
 

I meanwhile realized that these states (except entry_time) and probably also arch.nmi_pending/injected are things that should be considered when the vcpu state is saved and restored, right? What is the right interface for this? An extension of kvm_sregs?

BTW, via which channel is GUEST_INTERRUPTIBILITY_INFO from the vmcs saved/restored? I'm currently not seeing any related, CPU-specific code.

Quoted 14 lines · Jan Kiszka

Jan Kiszka wrote:
..

Quoted 11 lines

Index: b/arch/x86/kvm/vmx.c
===================================================================

arch/x86/kvm/vmx.c +3 -0

@@ -90,6 +90,11 @@ struct vcpu_vmx {
     } rmode;
     int vpid;
     bool emulation_required;
+
+    /* Support for vnmi-less CPUs */
+    int soft_vnmi_blocked;

Quoted 10 lines

arch/x86/kvm/vmx.c struct vcpu_vmx { · line 99

Quoted 3 lines

arch/x86/kvm/vmx.c +2 -0

+    ktime_t entry_time;
+    s64 vnmi_blocked_time;
 

I meanwhile realized that these states (except entry_time) and probably also arch.nmi_pending/injected are things that should be considered when the vcpu state is saved and restored, right? What is the right interface for this? An extension of kvm_sregs?

kvm_sregs can't be extended because that would break the ABI, so we have to add a new ioctl.

I have some patches that allow ioctls to be extended, so if that's accepted, we can avoid the new ioctl.

Quoted 4 lines

BTW, via which channel is GUEST_INTERRUPTIBILITY_INFO from the vmcs saved/restored? I'm currently not seeing any related, CPU-specific code.

Looks like it's missing.

As a workaround (or safety bag), is it imaginable to delay or deny VCPU snapshots at not yet fully restorable points (like GUEST_INTERRUPTIBILITY_INFO != 0)? Or stick-your-head-into-the-sand for now?