Re: nmi is broken?

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

> I noticed recently NMI on guest kernel is not working well.  host/guest
> kernel is 2.6.39-rc4, and using vmx.
>
> And test code is something like the following:
>
> 	local_irq_disable();
> 	for (i = 0; i < 10; i++) {
> 		int cpu = get_cpu();
> 		printk("%s: nmi %u, lapic %u\n", __FUNCTION__,
> 			nmi_count(cpu), irq_stat[cpu].apic_timer_irqs);
> 		mdelay(1000);
> 		put_cpu();
> 	}
>
> the result is both of nmi and lapic are not increased. If I used
> -no-kvm-irqchip, it works fine (increase nmi only). So, it seems to be
> the bug of kvm driver side.

With some debug, the cause seems to be in pit_do_work(). With the
following patch, NMI watchdog seems to be working correctly (if irq
disabled for long time, NMI watchdog can detect it).

Is the following patch right?
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>


[PATCH] kvm: Fix NMI on irq disabled state

On irq disabled state, there is no ->irq_ask from quest kernel. But
NMI shouldn't be affected by it.

This fixes inject NMI unconditionally.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 arch/x86/kvm/i8254.c |   26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff -puN arch/x86/kvm/i8254.c~kvm-nmi-fix arch/x86/kvm/i8254.c
--- linux-2.6/arch/x86/kvm/i8254.c~kvm-nmi-fix	2011-04-24 07:42:23.000000000 +0900
+++ linux-2.6-hirofumi/arch/x86/kvm/i8254.c	2011-04-24 07:42:23.000000000 +0900
@@ -305,20 +305,20 @@ static void pit_do_work(struct work_stru
 	if (inject) {
 		kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1);
 		kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0);
-
-		/*
-		 * Provides NMI watchdog support via Virtual Wire mode.
-		 * The route is: PIT -> PIC -> LVT0 in NMI mode.
-		 *
-		 * Note: Our Virtual Wire implementation is simplified, only
-		 * propagating PIT interrupts to all VCPUs when they have set
-		 * LVT0 to NMI delivery. Other PIC interrupts are just sent to
-		 * VCPU0, and only if its LVT0 is in EXTINT mode.
-		 */
-		if (kvm->arch.vapics_in_nmi_mode > 0)
-			kvm_for_each_vcpu(i, vcpu, kvm)
-				kvm_apic_nmi_wd_deliver(vcpu);
 	}
+
+	/*
+	 * Provides NMI watchdog support via Virtual Wire mode.
+	 * The route is: PIT -> PIC -> LVT0 in NMI mode.
+	 *
+	 * Note: Our Virtual Wire implementation is simplified, only
+	 * propagating PIT interrupts to all VCPUs when they have set
+	 * LVT0 to NMI delivery. Other PIC interrupts are just sent to
+	 * VCPU0, and only if its LVT0 is in EXTINT mode.
+	 */
+	if (kvm->arch.vapics_in_nmi_mode > 0)
+		kvm_for_each_vcpu(i, vcpu, kvm)
+			kvm_apic_nmi_wd_deliver(vcpu);
 }
 
 static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
_