qemu: Fix inject-nmi

From: KAMEZAWA Hiroyuki <[email protected]>
Subject: [PATCH] Fix inject-nmi

Now, inject-nmi sends NMI to all cpus...but this doesn't emulate pc hardware 'NMI button', which triggers LINT1.

So, now, LINT1 mask is ignored by inject-nmi and NMIs are sent to all cpus without checking LINT1 mask.

Because Linux masks LINT1 of cpus other than 0, this makes trouble. For example, kdump cannot run sometimes.

---
 hw/apic.c |    7 +++++++
 hw/apic.h |    1 +
 monitor.c |    4 ++--
 3 files changed, 10 insertions(+), 2 deletions(-)

hw/apic.c +0 -0

index 69d6ac5..020305b 100644

Quoted 26 lines · Lai Jiangshan

From: KAMEZAWA Hiroyuki <[email protected]>
Subject: [PATCH] Fix inject-nmi

Now, inject-nmi sends NMI to all cpus...but this doesn't emulate pc hardware 'NMI button', which triggers LINT1.

So, now, LINT1 mask is ignored by inject-nmi and NMIs are sent to all cpus without checking LINT1 mask.

Because Linux masks LINT1 of cpus other than 0, this makes trouble. For example, kdump cannot run sometimes.

---
 hw/apic.c |    7 +++++++
 hw/apic.h |    1 +
 monitor.c |    4 ++--
 3 files changed, 10 insertions(+), 2 deletions(-)

hw/apic.c +0 -0

index 69d6ac5..020305b 100644
@@ -205,6 +205,13 @@ void apic_deliver_pic_intr(DeviceState *d, int level)
     }
 }
 

hw/apic.c void apic_deliver_pic_intr(DeviceState *d, int level) · line 212

Quoted 5 lines

hw/apic.c +5 -0

+void apic_deliver_lint1_intr(DeviceState *d)
+{
+    APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+   apic_local_deliver(s, APIC_LVT_LINT1);

This will cause a qemu crash when apic_state is NULL (non-SMP 486 systems).

Moreover: wrong indention.

You know that this won't work for qemu-kvm with in-kernel irqchip? You may want to provide a patch for that tree, emulating the unavailable LINT1 injection via testing the APIC configration and then raising an NMI as before if it is accepted.

Quoted 27 lines · Jan Kiszka

Quoted 26 lines · Lai Jiangshan

From: KAMEZAWA Hiroyuki <[email protected]>
Subject: [PATCH] Fix inject-nmi

Now, inject-nmi sends NMI to all cpus...but this doesn't emulate pc hardware 'NMI button', which triggers LINT1.

So, now, LINT1 mask is ignored by inject-nmi and NMIs are sent to all cpus without checking LINT1 mask.

Because Linux masks LINT1 of cpus other than 0, this makes trouble. For example, kdump cannot run sometimes.

---
 hw/apic.c |    7 +++++++
 hw/apic.h |    1 +
 monitor.c |    4 ++--
 3 files changed, 10 insertions(+), 2 deletions(-)

hw/apic.c +0 -0

index 69d6ac5..020305b 100644
@@ -205,6 +205,13 @@ void apic_deliver_pic_intr(DeviceState *d, int level)
     }
 }
 

Quoted 8 lines

hw/apic.c void apic_deliver_pic_intr(DeviceState *d, int level) · line 212

Quoted 5 lines

hw/apic.c +5 -0

+void apic_deliver_lint1_intr(DeviceState *d)
+{
+    APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+   apic_local_deliver(s, APIC_LVT_LINT1);

This will cause a qemu crash when apic_state is NULL (non-SMP 486 systems).

Ouch, I see.
What are the interrupt mode used for non-SMP 486 systems?

Quoted 7 lines

Moreover: wrong indention.

You know that this won't work for qemu-kvm with in-kernel irqchip? You may want to provide a patch for that tree, emulating the unavailable LINT1 injection via testing the APIC configration and then raising an NMI as before if it is accepted.

It works in my box but the NMI is not injected through the in-kernel irqchip, I will implement it as you suggested.

Quoted 8 lines · Lai Jiangshan

Quoted 7 lines

Moreover: wrong indention.

You know that this won't work for qemu-kvm with in-kernel irqchip? You may want to provide a patch for that tree, emulating the unavailable LINT1 injection via testing the APIC configration and then raising an NMI as before if it is accepted.

Quoted 3 lines

It works in my box but the NMI is not injected through the in-kernel irqchip,
I will implement it as you suggested.

Somewhat hacky; isn't it better to test LINT1 in the kernel (and redefine the KVM_NMI ioctl as "toggle LINT1")?

Quoted 9 lines · Avi Kivity

Quoted 8 lines · Lai Jiangshan

Quoted 7 lines

Moreover: wrong indention.

You know that this won't work for qemu-kvm with in-kernel irqchip? You may want to provide a patch for that tree, emulating the unavailable LINT1 injection via testing the APIC configration and then raising an NMI as before if it is accepted.

Quoted 6 lines

Quoted 3 lines

It works in my box but the NMI is not injected through the in-kernel irqchip,
I will implement it as you suggested.

Somewhat hacky; isn't it better to test LINT1 in the kernel (and redefine the KVM_NMI ioctl as "toggle LINT1")?

KVM_NMI is required for user space IRQ chip as well.

Introducing some KVM_SET_LINT1 is an option though. But emulating it for the NMI button on older kernels sounds worthwhile nevertheless.

Quoted 10 lines · Jan Kiszka

Quoted 9 lines · Avi Kivity

Quoted 8 lines · Lai Jiangshan

Quoted 7 lines

Moreover: wrong indention.

You know that this won't work for qemu-kvm with in-kernel irqchip? You may want to provide a patch for that tree, emulating the unavailable LINT1 injection via testing the APIC configration and then raising an NMI as before if it is accepted.

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

It works in my box but the NMI is not injected through the in-kernel irqchip,
I will implement it as you suggested.

Somewhat hacky; isn't it better to test LINT1 in the kernel (and redefine the KVM_NMI ioctl as "toggle LINT1")?

KVM_NMI is required for user space IRQ chip as well.

We could define KVM_NMI as edging the core NMI input if !irqchip_in_kernel, and toggling LINT1 otherwise. Hardly nice though.

The current KVM_NMI with irqchip_in_kernel is not meaningful, since it doesn't obey the rules of any NMI source.

Quoted 3 lines

Introducing some KVM_SET_LINT1 is an option though. But emulating it for the NMI button on older kernels sounds worthwhile nevertheless.

Perhaps this is the best option to avoid confusion.

From: Kenji Kaneshige <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, KVM_NMI ioctl is handled as follows.

- When in-kernel irqchip is enabled, KVM_NMI ioctl is handled as a request of triggering LINT1 on the processor. LINT1 is emulated in in-kernel irqchip.

- When in-kernel irqchip is disabled, KVM_NMI ioctl is handled as a request of injecting NMI to the processor. This assumes LINT1 is already emulated in userland.

Signed-off-by Kenji Kaneshige <[email protected]>

Tested-by Lai Jiangshan <[email protected]>

---
 arch/x86/kvm/irq.h   |    1 +
 arch/x86/kvm/lapic.c |    8 ++++++++
 arch/x86/kvm/x86.c   |   14 ++++----------
 3 files changed, 13 insertions(+), 10 deletions(-)

Index: linux/arch/x86/kvm/irq.h
===================================================================

arch/x86/kvm/irq.h +1 -0

@@ -95,6 +95,7 @@ void kvm_pic_reset(struct kvm_kpic_state
 void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
 void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
 void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu);
+void kvm_apic_lint1_deliver(struct kvm_vcpu *vcpu);
 void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu);
 void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu);
 void __kvm_migrate_timers(struct kvm_vcpu *vcpu);

Index: linux/arch/x86/kvm/lapic.c
===================================================================

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

+		kvm_apic_local_deliver(apic, APIC_LVT1);
+}
+
 static struct kvm_timer_ops lapic_timer_ops = {
 	.is_periodic = lapic_is_periodic,
 };

Index: linux/arch/x86/kvm/x86.c
===================================================================

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

@@ -2729,13 +2729,6 @@ static int kvm_vcpu_ioctl_interrupt(stru
 	return 0;
 }
 
-static int kvm_vcpu_ioctl_nmi(struct kvm_vcpu *vcpu)
-{
-	kvm_inject_nmi(vcpu);
-
-	return 0;
-}
-
 static int vcpu_ioctl_tpr_access_reporting(struct kvm_vcpu *vcpu,
 					   struct kvm_tpr_access_ctl *tac)
 {

@@ -3038,9 +3031,10 @@ long kvm_arch_vcpu_ioctl(struct file *fi
 		break;
 	}
 	case KVM_NMI: {

From: Kenji Kaneshige<[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, KVM_NMI ioctl is handled as follows.

- When in-kernel irqchip is enabled, KVM_NMI ioctl is handled as a request of triggering LINT1 on the processor. LINT1 is emulated in in-kernel irqchip.

- When in-kernel irqchip is disabled, KVM_NMI ioctl is handled as a request of injecting NMI to the processor. This assumes LINT1 is already emulated in userland.

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

-static int kvm_vcpu_ioctl_nmi(struct kvm_vcpu *vcpu)
-{
-	kvm_inject_nmi(vcpu);
-
-	return 0;
-}
-
  static int vcpu_ioctl_tpr_access_reporting(struct kvm_vcpu *vcpu,
  					   struct kvm_tpr_access_ctl *tac)
  {

@@ -3038,9 +3031,10 @@ long kvm_arch_vcpu_ioctl(struct file *fi
  		break;
  	}
  	case KVM_NMI: {

From: Kenji Kaneshige <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, KVM_NMI ioctl is handled as follows.

- When in-kernel irqchip is enabled, KVM_NMI ioctl is handled as a request of triggering LINT1 on the processor. LINT1 is emulated in in-kernel irqchip.

- When in-kernel irqchip is disabled, KVM_NMI ioctl is handled as a request of injecting NMI to the processor. This assumes LINT1 is already emulated in userland.

(laijs) Changed from v1:
Add KVM_NMI API document
Add KVM_CAP_USER_NMI

Signed-off-by Kenji Kaneshige <[email protected]>

Tested-by Lai Jiangshan <[email protected]>

---
 Documentation/virtual/kvm/api.txt |   20 ++++++++++++++++++++
 arch/x86/kvm/irq.h                |    1 +
 arch/x86/kvm/lapic.c              |    7 +++++++
 arch/x86/kvm/x86.c                |   12 ++++++++++++
 include/linux/kvm.h               |    3 +++
 5 files changed, 43 insertions(+), 0 deletions(-)

Documentation/virtual/kvm/api.txt +0 -0

index b0e4b9c..5c24cc3 100644

Documentation/virtual/kvm/api.txt +6 -0

+Architectures: x86
+Type: vcpu ioctl
+Parameters: none
+Returns: 0 on success, -1 on error
+
+This ioctl injects NMI to the vcpu.

Documentation/virtual/kvm/api.txt +1 -0

+
 5. The kvm_run structure
 
 Application code obtains a pointer to the kvm_run structure by

arch/x86/kvm/irq.h +1 -0

index 53e2d08..0c96315 100644
@@ -95,6 +95,7 @@ void kvm_pic_reset(struct kvm_kpic_state *s);
 void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
 void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
 void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu);
+void kvm_apic_lint1_deliver(struct kvm_vcpu *vcpu);
 void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu);
 void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu);
 void __kvm_migrate_timers(struct kvm_vcpu *vcpu);

arch/x86/kvm/lapic.c +7 -0

index 57dcbd4..87fe36a 100644
@@ -1039,6 +1039,13 @@ void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu)
 		kvm_apic_local_deliver(apic, APIC_LVT0);
 }
 
+void kvm_apic_lint1_deliver(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;
+
+	kvm_apic_local_deliver(apic, APIC_LVT1);
+}
+
 static struct kvm_timer_ops lapic_timer_ops = {
 	.is_periodic = lapic_is_periodic,
 };

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

index 84a28ea..6862ef7 100644

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

+static int kvm_vcpu_ioctl_nmi(struct kvm_vcpu *vcpu)
+{
+	if (irqchip_in_kernel(vcpu->kvm))
+		kvm_apic_lint1_deliver(vcpu);
+	else
+		kvm_inject_nmi(vcpu);
+
+	return 0;
+}
+#else
 static int kvm_vcpu_ioctl_nmi(struct kvm_vcpu *vcpu)
 {
 	kvm_inject_nmi(vcpu);
 
 	return 0;
 }
+#endif
 
 static int vcpu_ioctl_tpr_access_reporting(struct kvm_vcpu *vcpu,
 					   struct kvm_tpr_access_ctl *tac)

include/linux/kvm.h +0 -0

index aace6b8..5253a5c 100644

From: Kenji Kaneshige <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, KVM_NMI ioctl is handled as follows.

- When in-kernel irqchip is enabled, KVM_NMI ioctl is handled as a request of triggering LINT1 on the processor. LINT1 is emulated in in-kernel irqchip.

- When in-kernel irqchip is disabled, KVM_NMI ioctl is handled as a request of injecting NMI to the processor. This assumes LINT1 is already emulated in userland.

(laijs) Add KVM_NMI API document

Signed-off-by Kenji Kaneshige <[email protected]>

Tested-by Lai Jiangshan <[email protected]>

Quoted 19 lines · Lai Jiangshan

From: Kenji Kaneshige <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, KVM_NMI ioctl is handled as follows.

- When in-kernel irqchip is enabled, KVM_NMI ioctl is handled as a request of triggering LINT1 on the processor. LINT1 is emulated in in-kernel irqchip.

- When in-kernel irqchip is disabled, KVM_NMI ioctl is handled as a request of injecting NMI to the processor. This assumes LINT1 is already emulated in userland.

Quoted 5 lines

(laijs) Add KVM_NMI API document

Signed-off-by Kenji Kaneshige <[email protected]>

Tested-by Lai Jiangshan <[email protected]>

Patch looks good, but please add your own sign-off (it needs to be added even if you just forward the patch, and even more if you modify it).

+ KVM_CAP_LAPIC_NMI
+ documentation of KVM_CAP_LAPIC_NMI vs. !KVM_CAP_LAPIC_NMI

So you prefer this approach over V5 / KVM_SET_LINT1? Just to make sure we are all looking at the full picture.

From: Kenji Kaneshige<[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, KVM_NMI ioctl is handled as follows.

- When in-kernel irqchip is enabled, KVM_NMI ioctl is handled as a request of triggering LINT1 on the processor. LINT1 is emulated in in-kernel irqchip.

- When in-kernel irqchip is disabled, KVM_NMI ioctl is handled as a request of injecting NMI to the processor. This assumes LINT1 is already emulated in userland.

(laijs) Changed from v1:
Add KVM_NMI API document
Add KVM_CAP_USER_NMI

Signed-off-by Kenji Kaneshige<[email protected]>

Tested-by Lai Jiangshan<[email protected]>

---
  Documentation/virtual/kvm/api.txt |   20 ++++++++++++++++++++
  arch/x86/kvm/irq.h                |    1 +
  arch/x86/kvm/lapic.c              |    7 +++++++
  arch/x86/kvm/x86.c                |   12 ++++++++++++
  include/linux/kvm.h               |    3 +++
  5 files changed, 43 insertions(+), 0 deletions(-)

Documentation/virtual/kvm/api.txt +20 -0

index b0e4b9c..5c24cc3 100644
@@ -1430,6 +1430,26 @@ is supported; 2 if the processor requires all virtual machines to have
  an RMA, or 1 if the processor can use an RMA but doesn't require it,
  because it supports the Virtual RMA (VRMA) facility.
 
+4.64 KVM_NMI
+
+Capability: KVM_CAP_USER_NMI
+Architectures: x86
+Type: vcpu ioctl
+Parameters: none
+Returns: 0 on success, -1 on error
+
+This ioctl injects NMI to the vcpu.
+
+If with capability KVM_CAP_LAPIC_NMI, KVM_NMI ioctl is handled as follows:
+
+	- When in-kernel irqchip is enabled, KVM_NMI ioctl is handled as a
+	  request of triggering LINT1 on the processor. LINT1 is emulated in
+	  in-kernel lapic irqchip.
+
+	- When in-kernel irqchip is disabled, KVM_NMI ioctl is handled as a
+	  request of injecting NMI to the processor. This assumes LINT1 is
+	  already emulated in userland lapic.
+
  5. The kvm_run structure
 
  Application code obtains a pointer to the kvm_run structure by

arch/x86/kvm/irq.h +1 -0

index 53e2d08..0c96315 100644
@@ -95,6 +95,7 @@ void kvm_pic_reset(struct kvm_kpic_state *s);
  void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
  void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
  void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu);
+void kvm_apic_lint1_deliver(struct kvm_vcpu *vcpu);
  void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu);
  void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu);
  void __kvm_migrate_timers(struct kvm_vcpu *vcpu);

arch/x86/kvm/lapic.c +7 -0

index 57dcbd4..87fe36a 100644
@@ -1039,6 +1039,13 @@ void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu)
  		kvm_apic_local_deliver(apic, APIC_LVT0);
  }
 
+void kvm_apic_lint1_deliver(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;
+
+	kvm_apic_local_deliver(apic, APIC_LVT1);
+}
+
  static struct kvm_timer_ops lapic_timer_ops = {
  	.is_periodic = lapic_is_periodic,
  };

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

index 84a28ea..6862ef7 100644
@@ -2729,12 +2729,24 @@ static int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu,
  	return 0;
  }
 
+#ifdef KVM_CAP_LAPIC_NMI
+static int kvm_vcpu_ioctl_nmi(struct kvm_vcpu *vcpu)
+{
+	if (irqchip_in_kernel(vcpu->kvm))
+		kvm_apic_lint1_deliver(vcpu);
+	else
+		kvm_inject_nmi(vcpu);
+

Quoted 11 lines · Lai Jiangshan

From: Kenji Kaneshige<[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, KVM_NMI ioctl is handled as follows.

Quoted 7 lines

- When in-kernel irqchip is enabled, KVM_NMI ioctl is handled as a request of triggering LINT1 on the processor. LINT1 is emulated in in-kernel irqchip.

- When in-kernel irqchip is disabled, KVM_NMI ioctl is handled as a request of injecting NMI to the processor. This assumes LINT1 is already emulated in userland.

Please add a KVM_NMI section to Documentation/virtual/kvm/api.txt.

Quoted 15 lines

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

-static int kvm_vcpu_ioctl_nmi(struct kvm_vcpu *vcpu)
-{
- kvm_inject_nmi(vcpu);
-
- return 0;
-}
-

static int vcpu_ioctl_tpr_access_reporting(struct kvm_vcpu *vcpu, struct kvm_tpr_access_ctl *tac)
{

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

@@ -3038,9 +3031,10 @@ long kvm_arch_vcpu_ioctl(struct file *fi
 break;
 }
 case KVM_NMI: {

arch/x86/kvm/x86.c long kvm_arch_vcpu_ioctl(struct file *fi · line 3040

Quoted 10 lines

arch/x86/kvm/x86.c +4 -3

- r = kvm_vcpu_ioctl_nmi(vcpu);
- if (r)
- goto out;
+ if (irqchip_in_kernel(vcpu->kvm))
+ kvm_apic_lint1_deliver(vcpu);
+ else
+ kvm_inject_nmi(vcpu);
 r = 0;
 break;
 }

Why did you drop kvm_vcpu_ioctl_nmi()?

Please add (and document) a KVM_CAP flag that lets userspace know the new behaviour is supported.

From: Kenji Kaneshige <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, inject-nmi request is handled as follows.

- When in-kernel irqchip is disabled, inject LINT1 instead of NMI interrupt.
- When in-kernel irqchip is enabled, send nmi event to kernel as the current code does. LINT1 should be emulated in kernel.

Signed-off-by Kenji Kaneshige <[email protected]>

Tested-by Lai Jiangshan <[email protected]>

---
 hw/apic.c |   16 ++++++++++++++++
 hw/apic.h |    1 +
 monitor.c |    5 ++---
 3 files changed, 19 insertions(+), 3 deletions(-)

Index: qemu-kvm/hw/apic.c
===================================================================

hw/apic.c +2 -0

@@ -205,6 +205,22 @@ void apic_deliver_pic_intr(DeviceState *
     }
 }
 
+void apic_deliver_nmi(CPUState *env)
+{

Quoted 6 lines

From: Kenji Kaneshige<[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this

This is targeting uq/master?

Please make sure your patch passes checkpatch.pl

From: Kenji Kaneshige <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, inject-nmi request is handled as follows.

- When in-kernel irqchip is disabled, inject LINT1 instead of NMI interrupt.
- When in-kernel irqchip is enabled, send nmi event to kernel as the current code does. LINT1 should be emulated in kernel.

(laijs) changed from v1:
use KVM_CAP_LAPIC_NMI
adjust the pic_deliver_nmi() API

Signed-off-by Kenji Kaneshige <[email protected]>

Tested-by Lai Jiangshan <[email protected]>

---
 hw/apic.c |   26 ++++++++++++++++++++++++++
 hw/apic.h |    1 +
 monitor.c |    6 +++++-
 3 files changed, 32 insertions(+), 1 deletions(-)

hw/apic.c +0 -0

index 69d6ac5..76e8208 100644

As explained in some other mail, we could then emulate the missing kernel feature by reading out the current in-kernel APIC state, testing if LINT1 is unmasked, and then delivering the NMI directly.

Quoted 6 lines

As explained in some other mail, we could then emulate the missing kernel feature by reading out the current in-kernel APIC state, testing if LINT1 is unmasked, and then delivering the NMI directly.

Only the thread of the VCPU can safely get the in-kernel LAPIC states, so this approach will cause some troubles.

Quoted 7 lines · Lai Jiangshan

Quoted 6 lines

As explained in some other mail, we could then emulate the missing kernel feature by reading out the current in-kernel APIC state, testing if LINT1 is unmasked, and then delivering the NMI directly.

Quoted 2 lines

Only the thread of the VCPU can safely get the in-kernel LAPIC states, so this approach will cause some troubles.

run_on_cpu() can help.

Jan

Quoted 8 lines · Jan Kiszka

Quoted 7 lines · Lai Jiangshan

Quoted 6 lines

As explained in some other mail, we could then emulate the missing kernel feature by reading out the current in-kernel APIC state, testing if LINT1 is unmasked, and then delivering the NMI directly.

Quoted 7 lines

Quoted 2 lines

Only the thread of the VCPU can safely get the in-kernel LAPIC states, so this approach will cause some troubles.

run_on_cpu() can help.

Jan

Ah, I forgot it, Thanks.

From: Lai Jiangshan <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, inject-nmi request is handled as follows.

- When in-kernel irqchip is disabled, deliver LINT1 instead of NMI interrupt.
- When in-kernel irqchip is enabled, get the in-kernel LAPIC states and test the APIC_LVT_MASKED, if LINT1 is unmasked, and then delivering the NMI directly. (Suggested by Jan Kiszka)

Changed from old version:
re-implement it by the Jan's suggestion.

Signed-off-by Lai Jiangshan <[email protected]>

Reported-by Kenji Kaneshige <[email protected]>

---
 hw/apic.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 hw/apic.h |    1 +
 monitor.c |    6 +++++-
 3 files changed, 54 insertions(+), 1 deletions(-)

hw/apic.c +0 -0

index 69d6ac5..9a40129 100644

hw/apic.c +12 -0

+}
+
+void apic_deliver_nmi(DeviceState *d)
+{
+    APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+    if (kvm_irqchip_in_kernel()) {
+        struct kvm_get_remote_lapic_params p = {.env = s->cpu_env,};
+        uint32_t lvt;
+
+        run_on_cpu(s->cpu_env, kvm_get_remote_lapic, &p);
+        lvt = kapic_reg(&p.klapic, 0x32 + APIC_LVT_LINT1);

Quoted 51 lines · Lai Jiangshan

Quoted 8 lines · Jan Kiszka

Quoted 7 lines · Lai Jiangshan

Quoted 6 lines

As explained in some other mail, we could then emulate the missing kernel feature by reading out the current in-kernel APIC state, testing if LINT1 is unmasked, and then delivering the NMI directly.

Quoted 7 lines

Quoted 2 lines

Only the thread of the VCPU can safely get the in-kernel LAPIC states, so this approach will cause some troubles.

run_on_cpu() can help.

Jan

Ah, I forgot it, Thanks.

From: Lai Jiangshan <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, inject-nmi request is handled as follows.

- When in-kernel irqchip is disabled, deliver LINT1 instead of NMI interrupt.
- When in-kernel irqchip is enabled, get the in-kernel LAPIC states and test the APIC_LVT_MASKED, if LINT1 is unmasked, and then delivering the NMI directly. (Suggested by Jan Kiszka)

Changed from old version:
re-implement it by the Jan's suggestion.

Signed-off-by Lai Jiangshan <[email protected]>

Reported-by Kenji Kaneshige <[email protected]>

---
 hw/apic.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 hw/apic.h |    1 +
 monitor.c |    6 +++++-
 3 files changed, 54 insertions(+), 1 deletions(-)

hw/apic.c +0 -0

index 69d6ac5..9a40129 100644

hw/apic.c void apic_deliver_pic_intr(DeviceState *d, int level) · line 208

Quoted 4 lines

hw/apic.c +1 -0

@@ -205,6 +205,54 @@ void apic_deliver_pic_intr(DeviceState *d, int level)
     }
 }
 
+#ifdef KVM_CAP_IRQCHIP

Again, this is always defined on x86 thus pointless to test.

hw/apic.c void apic_deliver_pic_intr(DeviceState *d, int level) · line 220

Quoted 12 lines

hw/apic.c +12 -0

+static inline uint32_t kapic_reg(struct kvm_lapic_state *kapic, int reg_id);
+
+struct kvm_get_remote_lapic_params {
+    CPUState *env;
+    struct kvm_lapic_state klapic;
+};
+
+static void kvm_get_remote_lapic(void *p)
+{
+    struct kvm_get_remote_lapic_params *params = p;
+
+    kvm_get_lapic(params->env, &params->klapic);

When you already interrupted that vcpu, why not inject from here? Avoids one further ping-pong round.

Quoted 19 lines

hw/apic.c +18 -0

+}
+
+void apic_deliver_nmi(DeviceState *d)
+{
+    APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+    if (kvm_irqchip_in_kernel()) {
+        struct kvm_get_remote_lapic_params p = {.env = s->cpu_env,};
+        uint32_t lvt;
+
+        run_on_cpu(s->cpu_env, kvm_get_remote_lapic, &p);
+        lvt = kapic_reg(&p.klapic, 0x32 + APIC_LVT_LINT1);
+
+        if (lvt & APIC_LVT_MASKED) {
+            return;
+        }
+
+        if (((lvt >> 8) & 7) != APIC_DM_NMI) {

hw/apic.c void apic_deliver_pic_intr(DeviceState *d, int level) · line 242

Quoted 4 lines

hw/apic.c +4 -0

+            return;
+        }
+
+        cpu_interrupt(s->cpu_env, CPU_INTERRUPT_NMI);

Err, aren't you introducing KVM_CAP_LAPIC_NMI that allows to test if this workaround is needed? Oh, your latest kernel patch is missing this again - requires fixing as well.

Quoted 59 lines · Jan Kiszka

Quoted 51 lines · Lai Jiangshan

Quoted 8 lines · Jan Kiszka

Quoted 7 lines · Lai Jiangshan

Quoted 6 lines

As explained in some other mail, we could then emulate the missing kernel feature by reading out the current in-kernel APIC state, testing if LINT1 is unmasked, and then delivering the NMI directly.

Quoted 7 lines

Quoted 2 lines

Only the thread of the VCPU can safely get the in-kernel LAPIC states, so this approach will cause some troubles.

run_on_cpu() can help.

Jan

Ah, I forgot it, Thanks.

From: Lai Jiangshan <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, inject-nmi request is handled as follows.

- When in-kernel irqchip is disabled, deliver LINT1 instead of NMI interrupt.
- When in-kernel irqchip is enabled, get the in-kernel LAPIC states and test the APIC_LVT_MASKED, if LINT1 is unmasked, and then delivering the NMI directly. (Suggested by Jan Kiszka)

Changed from old version:
re-implement it by the Jan's suggestion.

Signed-off-by Lai Jiangshan <[email protected]>

Reported-by Kenji Kaneshige <[email protected]>

---
 hw/apic.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 hw/apic.h |    1 +
 monitor.c |    6 +++++-
 3 files changed, 54 insertions(+), 1 deletions(-)

hw/apic.c +0 -0

index 69d6ac5..9a40129 100644

hw/apic.c void apic_deliver_pic_intr(DeviceState *d, int level) · line 208

Quoted 4 lines

hw/apic.c +1 -0

@@ -205,6 +205,54 @@ void apic_deliver_pic_intr(DeviceState *d, int level)
     }
 }
 
+#ifdef KVM_CAP_IRQCHIP

Again, this is always defined on x86 thus pointless to test.

Quoted 15 lines

hw/apic.c void apic_deliver_pic_intr(DeviceState *d, int level) · line 220

Quoted 12 lines

hw/apic.c +12 -0

+static inline uint32_t kapic_reg(struct kvm_lapic_state *kapic, int reg_id);
+
+struct kvm_get_remote_lapic_params {
+    CPUState *env;
+    struct kvm_lapic_state klapic;
+};
+
+static void kvm_get_remote_lapic(void *p)
+{
+    struct kvm_get_remote_lapic_params *params = p;
+
+    kvm_get_lapic(params->env, &params->klapic);

When you already interrupted that vcpu, why not inject from here? Avoids one further ping-pong round.

get_remote_lapic and inject nmi are two different things,
so I don't inject nmi from here. I didn't notice this ping-pond overhead. Thank you.

Quoted 20 lines

Quoted 20 lines

hw/apic.c +18 -0

+}
+
+void apic_deliver_nmi(DeviceState *d)
+{
+    APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+    if (kvm_irqchip_in_kernel()) {
+        struct kvm_get_remote_lapic_params p = {.env = s->cpu_env,};
+        uint32_t lvt;
+
+        run_on_cpu(s->cpu_env, kvm_get_remote_lapic, &p);
+        lvt = kapic_reg(&p.klapic, 0x32 + APIC_LVT_LINT1);
+
+        if (lvt & APIC_LVT_MASKED) {
+            return;
+        }
+
+        if (((lvt >> 8) & 7) != APIC_DM_NMI) {

Quoted 9 lines

hw/apic.c void apic_deliver_pic_intr(DeviceState *d, int level) · line 242

Quoted 4 lines

hw/apic.c +4 -0

+            return;
+        }
+
+        cpu_interrupt(s->cpu_env, CPU_INTERRUPT_NMI);

Err, aren't you introducing KVM_CAP_LAPIC_NMI that allows to test if this workaround is needed? Oh, your latest kernel patch is missing this again - requires fixing as well.

Kernel site patch is dropped with this v4 patch.

Did you mean you want KVM_CAP_SET_LINT1 + KVM_SET_LINT1 patches? I have made them.

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is masked in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, we introduce introduce KVM_SET_LINT1, and we can use KVM_SET_LINT1 to correctly emulate NMI button without change the old KVM_NMI behavior.

Signed-off-by Lai Jiangshan <[email protected]>

Reported-by Kenji Kaneshige <[email protected]>

---
 arch/x86/include/asm/kvm.h |    1 +
 arch/x86/kvm/irq.h         |    1 +
 arch/x86/kvm/lapic.c       |    7 +++++++
 arch/x86/kvm/x86.c         |    8 ++++++++
 include/linux/kvm.h        |    5 +++++
 5 files changed, 22 insertions(+), 0 deletions(-)

arch/x86/include/asm/kvm.h +1 -0

index 4d8dcbd..88d0ac3 100644
@@ -24,6 +24,7 @@
 #define __KVM_HAVE_DEBUGREGS
 #define __KVM_HAVE_XSAVE
 #define __KVM_HAVE_XCRS
+#define __KVM_HAVE_SET_LINT1
 
 /* Architectural interrupt line count. */
 #define KVM_NR_INTERRUPTS 256

arch/x86/kvm/irq.h +1 -0

index 53e2d08..0c96315 100644
@@ -95,6 +95,7 @@ void kvm_pic_reset(struct kvm_kpic_state *s);
 void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
 void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
 void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu);
+void kvm_apic_lint1_deliver(struct kvm_vcpu *vcpu);
 void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu);
 void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu);
 void __kvm_migrate_timers(struct kvm_vcpu *vcpu);

arch/x86/kvm/lapic.c +7 -0

index 57dcbd4..87fe36a 100644
@@ -1039,6 +1039,13 @@ void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu)
 		kvm_apic_local_deliver(apic, APIC_LVT0);
 }
 
+void kvm_apic_lint1_deliver(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;
+
+	kvm_apic_local_deliver(apic, APIC_LVT1);
+}
+
 static struct kvm_timer_ops lapic_timer_ops = {
 	.is_periodic = lapic_is_periodic,
 };

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

index 84a28ea..fccd094 100644
@@ -2077,6 +2077,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_XSAVE:
 	case KVM_CAP_ASYNC_PF:
 	case KVM_CAP_GET_TSC_KHZ:
+	case KVM_CAP_SET_LINT1:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:

@@ -3264,6 +3265,13 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 
 		goto out;
 	}
+	case KVM_SET_LINT1: {
+		r = -EINVAL;
+		if (!irqchip_in_kernel(vcpu->kvm))
+			goto out;
+		r = 0;
+		kvm_apic_lint1_deliver(vcpu);
+	}
 	default:
 		r = -EINVAL;
 	}

include/linux/kvm.h +0 -0

index aace6b8..3a10572 100644

Quoted 94 lines · Lai Jiangshan

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is masked in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, we introduce introduce KVM_SET_LINT1, and we can use KVM_SET_LINT1 to correctly emulate NMI button without change the old KVM_NMI behavior.

Signed-off-by Lai Jiangshan <[email protected]>

Reported-by Kenji Kaneshige <[email protected]>

---
 arch/x86/include/asm/kvm.h |    1 +
 arch/x86/kvm/irq.h         |    1 +
 arch/x86/kvm/lapic.c       |    7 +++++++
 arch/x86/kvm/x86.c         |    8 ++++++++
 include/linux/kvm.h        |    5 +++++
 5 files changed, 22 insertions(+), 0 deletions(-)

arch/x86/include/asm/kvm.h +1 -0

index 4d8dcbd..88d0ac3 100644
@@ -24,6 +24,7 @@
 #define __KVM_HAVE_DEBUGREGS
 #define __KVM_HAVE_XSAVE
 #define __KVM_HAVE_XCRS
+#define __KVM_HAVE_SET_LINT1
 
 /* Architectural interrupt line count. */
 #define KVM_NR_INTERRUPTS 256

arch/x86/kvm/irq.h +1 -0

index 53e2d08..0c96315 100644
@@ -95,6 +95,7 @@ void kvm_pic_reset(struct kvm_kpic_state *s);
 void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
 void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
 void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu);
+void kvm_apic_lint1_deliver(struct kvm_vcpu *vcpu);
 void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu);
 void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu);
 void __kvm_migrate_timers(struct kvm_vcpu *vcpu);

arch/x86/kvm/lapic.c +7 -0

index 57dcbd4..87fe36a 100644
@@ -1039,6 +1039,13 @@ void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu)
 		kvm_apic_local_deliver(apic, APIC_LVT0);
 }
 
+void kvm_apic_lint1_deliver(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;
+
+	kvm_apic_local_deliver(apic, APIC_LVT1);
+}
+
 static struct kvm_timer_ops lapic_timer_ops = {
 	.is_periodic = lapic_is_periodic,
 };

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

index 84a28ea..fccd094 100644
@@ -2077,6 +2077,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_XSAVE:
 	case KVM_CAP_ASYNC_PF:
 	case KVM_CAP_GET_TSC_KHZ:
+	case KVM_CAP_SET_LINT1:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:

@@ -3264,6 +3265,13 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 
 		goto out;
 	}
+	case KVM_SET_LINT1: {
+		r = -EINVAL;
+		if (!irqchip_in_kernel(vcpu->kvm))
+			goto out;
+		r = 0;
+		kvm_apic_lint1_deliver(vcpu);
+	}
 	default:
 		r = -EINVAL;
 	}

include/linux/kvm.h +0 -0

index aace6b8..3a10572 100644
@@ -554,6 +554,9 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_PPC_SMT 64

include/linux/kvm.h struct kvm_ppc_pvinfo { · line 559

Quoted 5 lines

include/linux/kvm.h +3 -0

 #define KVM_CAP_PPC_RMA	65
 #define KVM_CAP_S390_GMAP 71
+#ifdef __KVM_HAVE_SET_LINT1
+#define KVM_CAP_SET_LINT1 72
+#endif

Actually, there is no need for __KVM_HAVE_SET_LINT1 and #ifdef. User land will just do a runtime check.

Quoted 95 lines · Jan Kiszka

Quoted 94 lines · Lai Jiangshan

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is masked in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, we introduce introduce KVM_SET_LINT1, and we can use KVM_SET_LINT1 to correctly emulate NMI button without change the old KVM_NMI behavior.

Signed-off-by Lai Jiangshan <[email protected]>

Reported-by Kenji Kaneshige <[email protected]>

---
 arch/x86/include/asm/kvm.h |    1 +
 arch/x86/kvm/irq.h         |    1 +
 arch/x86/kvm/lapic.c       |    7 +++++++
 arch/x86/kvm/x86.c         |    8 ++++++++
 include/linux/kvm.h        |    5 +++++
 5 files changed, 22 insertions(+), 0 deletions(-)

arch/x86/include/asm/kvm.h +1 -0

index 4d8dcbd..88d0ac3 100644
@@ -24,6 +24,7 @@
 #define __KVM_HAVE_DEBUGREGS
 #define __KVM_HAVE_XSAVE
 #define __KVM_HAVE_XCRS
+#define __KVM_HAVE_SET_LINT1
 
 /* Architectural interrupt line count. */
 #define KVM_NR_INTERRUPTS 256

arch/x86/kvm/irq.h +1 -0

index 53e2d08..0c96315 100644
@@ -95,6 +95,7 @@ void kvm_pic_reset(struct kvm_kpic_state *s);
 void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
 void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
 void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu);
+void kvm_apic_lint1_deliver(struct kvm_vcpu *vcpu);
 void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu);
 void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu);
 void __kvm_migrate_timers(struct kvm_vcpu *vcpu);

arch/x86/kvm/lapic.c +7 -0

index 57dcbd4..87fe36a 100644
@@ -1039,6 +1039,13 @@ void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu)
 		kvm_apic_local_deliver(apic, APIC_LVT0);
 }
 
+void kvm_apic_lint1_deliver(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;
+
+	kvm_apic_local_deliver(apic, APIC_LVT1);
+}
+
 static struct kvm_timer_ops lapic_timer_ops = {
 	.is_periodic = lapic_is_periodic,
 };

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

index 84a28ea..fccd094 100644
@@ -2077,6 +2077,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_XSAVE:
 	case KVM_CAP_ASYNC_PF:
 	case KVM_CAP_GET_TSC_KHZ:
+	case KVM_CAP_SET_LINT1:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:

@@ -3264,6 +3265,13 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 
 		goto out;
 	}
+	case KVM_SET_LINT1: {
+		r = -EINVAL;
+		if (!irqchip_in_kernel(vcpu->kvm))
+			goto out;
+		r = 0;
+		kvm_apic_lint1_deliver(vcpu);
+	}
 	default:
 		r = -EINVAL;
 	}

include/linux/kvm.h +0 -0

index aace6b8..3a10572 100644
@@ -554,6 +554,9 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_PPC_SMT 64

Quoted 10 lines

include/linux/kvm.h struct kvm_ppc_pvinfo { · line 559

Quoted 5 lines

include/linux/kvm.h +3 -0

 #define KVM_CAP_PPC_RMA	65
 #define KVM_CAP_S390_GMAP 71
+#ifdef __KVM_HAVE_SET_LINT1
+#define KVM_CAP_SET_LINT1 72
+#endif

Actually, there is no need for __KVM_HAVE_SET_LINT1 and #ifdef. User land will just do a runtime check.

There is not bad result brought by __KVM_HAVE_SET_LINT1 and help for compile time check.

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is masked in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, we introduce introduce KVM_SET_LINT1, and we can use KVM_SET_LINT1 to correctly emulate NMI button without change the old KVM_NMI behavior.

Quoted 11 lines · Lai Jiangshan

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is masked in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, we introduce introduce KVM_SET_LINT1, and we can use KVM_SET_LINT1 to correctly emulate NMI button without change the old KVM_NMI behavior.

include/linux/kvm.h struct kvm_clock_data { · line 768

Quoted 8 lines

include/linux/kvm.h +2 -0

@@ -759,6 +762,8 @@ struct kvm_clock_data {
 #define KVM_CREATE_SPAPR_TCE	  _IOW(KVMIO,  0xa8, struct kvm_create_spapr_tce)
 /* Available with KVM_CAP_RMA */
 #define KVM_ALLOCATE_RMA	  _IOR(KVMIO,  0xa9, struct kvm_allocate_rma)
+/* Available with KVM_CAP_SET_LINT1 for x86 */
+#define KVM_SET_LINT1		  _IO(KVMIO,   0xaa)
 
 

LINT1 may have been programmed as a level -triggered interrupt instead of edge triggered (NMI or interrupt). We can use the ioctl argument for the level (and pressing the NMI button needs to pulse the level to 1 and back to 0).

Quoted 12 lines · Avi Kivity

Quoted 11 lines · Lai Jiangshan

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is masked in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, we introduce introduce KVM_SET_LINT1, and we can use KVM_SET_LINT1 to correctly emulate NMI button without change the old KVM_NMI behavior.

Quoted 14 lines

include/linux/kvm.h struct kvm_clock_data { · line 768

Quoted 8 lines

include/linux/kvm.h +2 -0

@@ -759,6 +762,8 @@ struct kvm_clock_data {
 #define KVM_CREATE_SPAPR_TCE	  _IOW(KVMIO,  0xa8, struct kvm_create_spapr_tce)
 /* Available with KVM_CAP_RMA */
 #define KVM_ALLOCATE_RMA	  _IOR(KVMIO,  0xa9, struct kvm_allocate_rma)
+/* Available with KVM_CAP_SET_LINT1 for x86 */
+#define KVM_SET_LINT1		  _IO(KVMIO,   0xaa)
 
 

LINT1 may have been programmed as a level -triggered interrupt instead of edge triggered (NMI or interrupt). We can use the ioctl argument for the level (and pressing the NMI button needs to pulse the level to 1 and back to 0).

Hi, Avi,

How to handle level=0 in the kernel?
Or just ignore it?

Quoted 28 lines · Lai Jiangshan

Quoted 12 lines · Avi Kivity

Quoted 11 lines · Lai Jiangshan

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is masked in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, we introduce introduce KVM_SET_LINT1, and we can use KVM_SET_LINT1 to correctly emulate NMI button without change the old KVM_NMI behavior.

Quoted 14 lines

include/linux/kvm.h struct kvm_clock_data { · line 768

Quoted 8 lines

include/linux/kvm.h +2 -0

@@ -759,6 +762,8 @@ struct kvm_clock_data {
 #define KVM_CREATE_SPAPR_TCE	  _IOW(KVMIO,  0xa8, struct kvm_create_spapr_tce)
 /* Available with KVM_CAP_RMA */
 #define KVM_ALLOCATE_RMA	  _IOR(KVMIO,  0xa9, struct kvm_allocate_rma)
+/* Available with KVM_CAP_SET_LINT1 for x86 */
+#define KVM_SET_LINT1		  _IO(KVMIO,   0xaa)
 
 

LINT1 may have been programmed as a level -triggered interrupt instead of edge triggered (NMI or interrupt). We can use the ioctl argument for the level (and pressing the NMI button needs to pulse the level to 1 and back to 0).

Quoted 4 lines

Hi, Avi,

How to handle level=0 in the kernel?
Or just ignore it?

It needs to be handled according to the delivery mode, polarity, and trigger mode bits in the LVT.

For example, a Fixed delivery mode with polarity 1 and level trigger mode will post the interrupt as long as it is in level 0 and not masked by the ISR. __apic_accept_irq() should handle this.

Quoted 11 lines · Lai Jiangshan

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is masked in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, we introduce introduce KVM_SET_LINT1, and we can use KVM_SET_LINT1 to correctly emulate NMI button without change the old KVM_NMI behavior.

include/linux/kvm.h struct kvm_clock_data { · line 768

Quoted 8 lines

include/linux/kvm.h +2 -0

@@ -759,6 +762,8 @@ struct kvm_clock_data {
 #define KVM_CREATE_SPAPR_TCE	  _IOW(KVMIO,  0xa8, struct kvm_create_spapr_tce)
 /* Available with KVM_CAP_RMA */
 #define KVM_ALLOCATE_RMA	  _IOR(KVMIO,  0xa9, struct kvm_allocate_rma)
+/* Available with KVM_CAP_SET_LINT1 for x86 */
+#define KVM_SET_LINT1		  _IO(KVMIO,   0xaa)
 
 

LINT1 may have been programmed as a level -triggered interrupt instead of edge triggered (NMI or interrupt). We can use the ioctl argument for the level (and pressing the NMI button needs to pulse the level to 1 and back to 0).

Quoted 7 lines

Quoted 1 line

LINT1 may have been programmed as a level -triggered interrupt instead of edge triggered (NMI or interrupt). We can use the ioctl argument for the level (and pressing the NMI button needs to pulse the level to 1 and back to 0).

Hi, Avi, Jan,

Which approach you prefer to?
I need to know the result before wasting too much time to respin the approach.

1) Fix KVM_NMI emulation approach (which is v3 patchset)
- It directly fixes the problem and matches the real hard ware more, but it changes KVM_NMI bahavior.
- Require both kernel-site and userspace-site fix.

2) Get the LAPIC state from kernel irqchip, and inject NMI if it is allowed (which is v4 patchset)
- Simple, don't changes any kernel behavior.
- Only need the userspace-site fix

3) Add KVM_SET_LINT1 approach (which is v5 patchset)
- don't changes the kernel's KVM_NMI behavior.
- much complex
- Require both kernel-site and userspace-site fix.
- userspace-site should also handle the !KVM_SET_LINT1 condition, it uses all the 2) approach' code. it means this approach equals the 2) approach + KVM_SET_LINT1 ioctl.

This is an urgent bug of us, we need to settle it down soo

Quoted 8 lines · Lai Jiangshan

Quoted 7 lines

Quoted 1 line

LINT1 may have been programmed as a level -triggered interrupt instead of edge triggered (NMI or interrupt). We can use the ioctl argument for the level (and pressing the NMI button needs to pulse the level to 1 and back to 0).

Quoted 5 lines

Hi, Avi, Jan,

Which approach you prefer to?
I need to know the result before wasting too much time to respin the approach.

Yes, sorry about the slow and sometimes conflicting feedback.

Quoted 18 lines

1) Fix KVM_NMI emulation approach (which is v3 patchset)
- It directly fixes the problem and matches the real hard ware more, but it changes KVM_NMI bahavior.
- Require both kernel-site and userspace-site fix.

2) Get the LAPIC state from kernel irqchip, and inject NMI if it is allowed (which is v4 patchset)
- Simple, don't changes any kernel behavior.
- Only need the userspace-site fix

3) Add KVM_SET_LINT1 approach (which is v5 patchset)
- don't changes the kernel's KVM_NMI behavior.
- much complex
- Require both kernel-site and userspace-site fix.
- userspace-site should also handle the !KVM_SET_LINT1 condition, it uses all the 2) approach' code. it means this approach equals the 2) approach + KVM_SET_LINT1 ioctl.

Quoted 1 line

This is an urgent bug of us, we need to settle it down soo

While (1) is simple, it overloads a single ioctl with two meanings, that's not so good.

Whether we do (1) or (3), we need (2) as well, for older kernels.

So I recommend first focusing on (2) and merging it, then doing (3).

(note an additional issue with 3 is whether to make it a vm or vcpu ioctl - we've been assuming vcpu ioctl but it's not necessarily the best choice).

Quoted 40 lines · Avi Kivity

Quoted 8 lines · Lai Jiangshan

Quoted 7 lines

Quoted 1 line

LINT1 may have been programmed as a level -triggered interrupt instead of edge triggered (NMI or interrupt). We can use the ioctl argument for the level (and pressing the NMI button needs to pulse the level to 1 and back to 0).

Quoted 5 lines

Hi, Avi, Jan,

Which approach you prefer to?
I need to know the result before wasting too much time to respin the approach.

Yes, sorry about the slow and sometimes conflicting feedback.

Quoted 18 lines

1) Fix KVM_NMI emulation approach (which is v3 patchset)
- It directly fixes the problem and matches the real hard ware more, but it changes KVM_NMI bahavior.
- Require both kernel-site and userspace-site fix.

2) Get the LAPIC state from kernel irqchip, and inject NMI if it is allowed (which is v4 patchset)
- Simple, don't changes any kernel behavior.
- Only need the userspace-site fix

3) Add KVM_SET_LINT1 approach (which is v5 patchset)
- don't changes the kernel's KVM_NMI behavior.
- much complex
- Require both kernel-site and userspace-site fix.
- userspace-site should also handle the !KVM_SET_LINT1 condition, it uses all the 2) approach' code. it means this approach equals the 2) approach + KVM_SET_LINT1 ioctl.

Quoted 1 line

This is an urgent bug of us, we need to settle it down soo

While (1) is simple, it overloads a single ioctl with two meanings, that's not so good.

Quoted 8 lines

Whether we do (1) or (3), we need (2) as well, for older kernels.

So I recommend first focusing on (2) and merging it, then doing (3).

(note an additional issue with 3 is whether to make it a vm or vcpu ioctl - we've been assuming vcpu ioctl but it's not necessarily the best choice).

It is the 2) approach.
It only changes the user space site, the kernel site is not touched. It is changed from previous v4 patch, fixed problems found by Jan.
----------------------------

From: Lai Jiangshan <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, inject-nmi request is handled as follows.

- When in-kernel irqchip is disabled, deliver LINT1 instead of NMI interrupt.
- When in-kernel irqchip is enabled, get the in-kernel LAPIC states and test the APIC_LVT_MASKED, if LINT1 is unmasked, and then delivering the NMI directly. (Suggested by Jan Kiszka)

Changed from old version:
re-implement it by the Jan's suggestion.
fix the race found by Jan.

Signed-off-by Lai Jiangshan <[email protected]>

Reported-by Kenji Kaneshige <[email protected]>

---
 hw/apic.c |   33 +++++++++++++++++++++++++++++++++
 hw/apic.h |    1 +
 monitor.c |    6 +++++-
 3 files changed, 39 insertions(+), 1 deletions(-)

hw/apic.c +33 -0

index 69d6ac5..922796a 100644
@@ -205,6 +205,39 @@ void apic_deliver_pic_intr(DeviceState *d, int level)
     }
 }
 
+static inline uint32_t kapic_reg(struct kvm_lapic_state *kapic, int reg_id);
+
+static void kvm_irqchip_deliver_nmi(void *p)
+{
+    APICState *s = p;
+    struct kvm_lapic_state klapic;
+    uint32_t lvt;
+
+    kvm_get_lapic(s->cpu_env, &klapic);
+    lvt = kapic_reg(&klapic, 0x32 + APIC_LVT_LINT1);
+
+    if (lvt & APIC_LVT_MASKED) {
+        return;
+    }
+
+    if (((lvt >> 8) & 7) != APIC_DM_NMI) {
+        return;
+    }
+
+    kvm_vcpu_ioctl(s->cpu_env, KVM_NMI);
+}
+
+void apic_deliver_nmi(DeviceState *d)
+{
+    APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+    if (kvm_irqchip_in_kernel()) {
+        run_on_cpu(s->cpu_env, kvm_irqchip_deliver_nmi, s);
+    } else {
+        apic_local_deliver(s, APIC_LVT_LINT1);
+    }
+}
+
 #define foreach_apic(apic, deliver_bitmask, code) \
 {\
     int __i, __j, __mask;\

hw/apic.h +1 -0

index c857d52..3a4be0a 100644
@@ -10,6 +10,7 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode,
                              uint8_t trigger_mode);
 int apic_accept_pic_intr(DeviceState *s);
 void apic_deliver_pic_intr(DeviceState *s, int level);
+void apic_deliver_nmi(DeviceState *d);
 int apic_get_interrupt(DeviceState *s);
 void apic_reset_irq_delivered(void);
 int apic_get_irq_delivered(void);

monitor.c +0 -0

index cb485bf..0b81f17 100644
@@ -2616,7 +2616,11 @@ static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
     CPUState *env;
 

Quoted 155 lines · Lai Jiangshan

Quoted 40 lines · Avi Kivity

Quoted 8 lines · Lai Jiangshan

Quoted 7 lines

Quoted 1 line

LINT1 may have been programmed as a level -triggered interrupt instead of edge triggered (NMI or interrupt). We can use the ioctl argument for the level (and pressing the NMI button needs to pulse the level to 1 and back to 0).

Quoted 5 lines

Hi, Avi, Jan,

Which approach you prefer to?
I need to know the result before wasting too much time to respin the approach.

Yes, sorry about the slow and sometimes conflicting feedback.

Quoted 18 lines

1) Fix KVM_NMI emulation approach (which is v3 patchset)
- It directly fixes the problem and matches the real hard ware more, but it changes KVM_NMI bahavior.
- Require both kernel-site and userspace-site fix.

2) Get the LAPIC state from kernel irqchip, and inject NMI if it is allowed (which is v4 patchset)
- Simple, don't changes any kernel behavior.
- Only need the userspace-site fix

3) Add KVM_SET_LINT1 approach (which is v5 patchset)
- don't changes the kernel's KVM_NMI behavior.
- much complex
- Require both kernel-site and userspace-site fix.
- userspace-site should also handle the !KVM_SET_LINT1 condition, it uses all the 2) approach' code. it means this approach equals the 2) approach + KVM_SET_LINT1 ioctl.

Quoted 1 line

This is an urgent bug of us, we need to settle it down soo

While (1) is simple, it overloads a single ioctl with two meanings, that's not so good.

Quoted 8 lines

Whether we do (1) or (3), we need (2) as well, for older kernels.

So I recommend first focusing on (2) and merging it, then doing (3).

(note an additional issue with 3 is whether to make it a vm or vcpu ioctl - we've been assuming vcpu ioctl but it's not necessarily the best choice).

It is the 2) approach.
It only changes the user space site, the kernel site is not touched. It is changed from previous v4 patch, fixed problems found by Jan.
----------------------------

From: Lai Jiangshan <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, inject-nmi request is handled as follows.

- When in-kernel irqchip is disabled, deliver LINT1 instead of NMI interrupt.
- When in-kernel irqchip is enabled, get the in-kernel LAPIC states and test the APIC_LVT_MASKED, if LINT1 is unmasked, and then delivering the NMI directly. (Suggested by Jan Kiszka)

Changed from old version:
re-implement it by the Jan's suggestion.
fix the race found by Jan.

Signed-off-by Lai Jiangshan <[email protected]>

Reported-by Kenji Kaneshige <[email protected]>

---
 hw/apic.c |   33 +++++++++++++++++++++++++++++++++
 hw/apic.h |    1 +
 monitor.c |    6 +++++-
 3 files changed, 39 insertions(+), 1 deletions(-)

hw/apic.c +33 -0

index 69d6ac5..922796a 100644
@@ -205,6 +205,39 @@ void apic_deliver_pic_intr(DeviceState *d, int level)
     }
 }
 
+static inline uint32_t kapic_reg(struct kvm_lapic_state *kapic, int reg_id);
+
+static void kvm_irqchip_deliver_nmi(void *p)
+{
+    APICState *s = p;
+    struct kvm_lapic_state klapic;
+    uint32_t lvt;
+
+    kvm_get_lapic(s->cpu_env, &klapic);
+    lvt = kapic_reg(&klapic, 0x32 + APIC_LVT_LINT1);
+
+    if (lvt & APIC_LVT_MASKED) {
+        return;
+    }
+
+    if (((lvt >> 8) & 7) != APIC_DM_NMI) {
+        return;
+    }
+
+    kvm_vcpu_ioctl(s->cpu_env, KVM_NMI);
+}
+
+void apic_deliver_nmi(DeviceState *d)
+{
+    APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+    if (kvm_irqchip_in_kernel()) {
+        run_on_cpu(s->cpu_env, kvm_irqchip_deliver_nmi, s);
+    } else {
+        apic_local_deliver(s, APIC_LVT_LINT1);
+    }
+}
+
 #define foreach_apic(apic, deliver_bitmask, code) \
 {\
     int __i, __j, __mask;\

hw/apic.h +1 -0

index c857d52..3a4be0a 100644
@@ -10,6 +10,7 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode,
                              uint8_t trigger_mode);
 int apic_accept_pic_intr(DeviceState *s);
 void apic_deliver_pic_intr(DeviceState *s, int level);
+void apic_deliver_nmi(DeviceState *d);
 int apic_get_interrupt(DeviceState *s);
 void apic_reset_irq_delivered(void);
 int apic_get_irq_delivered(void);

monitor.c +5 -1

index cb485bf..0b81f17 100644
@@ -2616,7 +2616,11 @@ static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
     CPUState *env;
 
     for (env = first_cpu; env != NULL; env = env->next_cpu) {
-        cpu_interrupt(env, CPU_INTERRUPT_NMI);
+        if (!env->apic_state) {
+            cpu_interrupt(env, CPU_INTERRUPT_NMI);
+        } else {
+            apic_deliver_nmi(env->apic_state);
+        }
     }
 

monitor.c static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data) · line 2626

Quoted 1 line

monitor.c +0 -0

     return 0;

Looks OK to me.

Please don't forget to bake a qemu-only patch for those bits that apply to upstream as well (ie. the user space APIC path).

Jan

Quoted 160 lines · Jan Kiszka

Quoted 146 lines · Lai Jiangshan

Quoted 40 lines · Avi Kivity

Quoted 8 lines · Lai Jiangshan

Quoted 7 lines

Quoted 1 line

LINT1 may have been programmed as a level -triggered interrupt instead of edge triggered (NMI or interrupt). We can use the ioctl argument for the level (and pressing the NMI button needs to pulse the level to 1 and back to 0).

Quoted 5 lines

Hi, Avi, Jan,

Which approach you prefer to?
I need to know the result before wasting too much time to respin the approach.

Yes, sorry about the slow and sometimes conflicting feedback.

Quoted 18 lines

1) Fix KVM_NMI emulation approach (which is v3 patchset)
- It directly fixes the problem and matches the real hard ware more, but it changes KVM_NMI bahavior.
- Require both kernel-site and userspace-site fix.

2) Get the LAPIC state from kernel irqchip, and inject NMI if it is allowed (which is v4 patchset)
- Simple, don't changes any kernel behavior.
- Only need the userspace-site fix

3) Add KVM_SET_LINT1 approach (which is v5 patchset)
- don't changes the kernel's KVM_NMI behavior.
- much complex
- Require both kernel-site and userspace-site fix.
- userspace-site should also handle the !KVM_SET_LINT1 condition, it uses all the 2) approach' code. it means this approach equals the 2) approach + KVM_SET_LINT1 ioctl.

Quoted 1 line

This is an urgent bug of us, we need to settle it down soo

While (1) is simple, it overloads a single ioctl with two meanings, that's not so good.

Quoted 8 lines

Whether we do (1) or (3), we need (2) as well, for older kernels.

So I recommend first focusing on (2) and merging it, then doing (3).

(note an additional issue with 3 is whether to make it a vm or vcpu ioctl - we've been assuming vcpu ioctl but it's not necessarily the best choice).

It is the 2) approach.
It only changes the user space site, the kernel site is not touched. It is changed from previous v4 patch, fixed problems found by Jan.
----------------------------

From: Lai Jiangshan <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, inject-nmi request is handled as follows.

- When in-kernel irqchip is disabled, deliver LINT1 instead of NMI interrupt.
- When in-kernel irqchip is enabled, get the in-kernel LAPIC states and test the APIC_LVT_MASKED, if LINT1 is unmasked, and then delivering the NMI directly. (Suggested by Jan Kiszka)

Changed from old version:
re-implement it by the Jan's suggestion.
fix the race found by Jan.

Signed-off-by Lai Jiangshan <[email protected]>

Reported-by Kenji Kaneshige <[email protected]>

---
 hw/apic.c |   33 +++++++++++++++++++++++++++++++++
 hw/apic.h |    1 +
 monitor.c |    6 +++++-
 3 files changed, 39 insertions(+), 1 deletions(-)

hw/apic.c +33 -0

index 69d6ac5..922796a 100644
@@ -205,6 +205,39 @@ void apic_deliver_pic_intr(DeviceState *d, int level)
     }
 }
 
+static inline uint32_t kapic_reg(struct kvm_lapic_state *kapic, int reg_id);
+
+static void kvm_irqchip_deliver_nmi(void *p)
+{
+    APICState *s = p;
+    struct kvm_lapic_state klapic;
+    uint32_t lvt;
+
+    kvm_get_lapic(s->cpu_env, &klapic);
+    lvt = kapic_reg(&klapic, 0x32 + APIC_LVT_LINT1);
+
+    if (lvt & APIC_LVT_MASKED) {
+        return;
+    }
+
+    if (((lvt >> 8) & 7) != APIC_DM_NMI) {
+        return;
+    }
+
+    kvm_vcpu_ioctl(s->cpu_env, KVM_NMI);
+}
+
+void apic_deliver_nmi(DeviceState *d)
+{
+    APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+    if (kvm_irqchip_in_kernel()) {
+        run_on_cpu(s->cpu_env, kvm_irqchip_deliver_nmi, s);
+    } else {
+        apic_local_deliver(s, APIC_LVT_LINT1);
+    }
+}
+
 #define foreach_apic(apic, deliver_bitmask, code) \
 {\
     int __i, __j, __mask;\

hw/apic.h +1 -0

index c857d52..3a4be0a 100644
@@ -10,6 +10,7 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode,
                              uint8_t trigger_mode);
 int apic_accept_pic_intr(DeviceState *s);
 void apic_deliver_pic_intr(DeviceState *s, int level);
+void apic_deliver_nmi(DeviceState *d);
 int apic_get_interrupt(DeviceState *s);
 void apic_reset_irq_delivered(void);
 int apic_get_irq_delivered(void);

monitor.c +0 -0

index cb485bf..0b81f17 100644
@@ -2616,7 +2616,11 @@ static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
     CPUState *env;
 

monitor.c static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data) · line 2626

Quoted 10 lines

monitor.c +5 -1

     for (env = first_cpu; env != NULL; env = env->next_cpu) {
-        cpu_interrupt(env, CPU_INTERRUPT_NMI);
+        if (!env->apic_state) {
+            cpu_interrupt(env, CPU_INTERRUPT_NMI);
+        } else {
+            apic_deliver_nmi(env->apic_state);
+        }
     }
 
     return 0;

Looks OK to me.

Quoted 5 lines

Please don't forget to bake a qemu-only patch for those bits that apply to upstream as well (ie. the user space APIC path).

Jan

I did forget it.
Did you mean we need to add "#ifdef KVM_CAP_IRQCHIP" back?

Quoted 11 lines

Quoted 8 lines

Looks OK to me.

Please don't forget to bake a qemu-only patch for those bits that apply to upstream as well (ie. the user space APIC path).

Jan

I did forget it.
Did you mean we need to add "#ifdef KVM_CAP_IRQCHIP" back?

No. I meant basically your patch minus the kvm_in_kernel_irqchip code paths, applicable against current qemu.git. Those paths will be re-added (slightly differently) when upstream gains that support. I'm working on a basic version an will incorporate the logic if your qemu patch is already available.

Jan

Looks OK to me.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 4 lines · Jan Kiszka

Looks OK to me.

Same here.

- --
I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJOnpiNAAoJEI7yEDeUysxllqUP/3K9oPbz9OxbqH3+9G1W9cUy 49hKR0DtLyf5WH0hoSq3/jA2T00PWR6fLIo6itth76x/TqnIuimjln6Nrj/T2nhO PPvwJB4OE/9ahSlm3JOVsE/JYwDx6h3u9eouN5BqVoQax8S3mnhxSGLxZOp8wvar ol6vDj2U8JbigV3fCsFheiP9tTZWZgH66qCdCUzuNUnYWUW5m9repdsXflTp6YyW id30xzuZETnQ/0RFU0hnhrfQ/vvm1dJeK6Y2bPKowoDCp+CFNi/CnJYDAZA18FSQ V5096U8cj8/m/Hr8fPLpyZzDonPz0KfMPvtfV9rVHEtqvf04Ym+gcdfwo+2U4LQs 16RNGWwsF6qIAcyevK9xCpcU9g00v6m0fyj3eQgD+JT+pV+m8QCzNnQyDqDlEUEl ub0WR7ilnl3/NIa6FTKHqZ5Wct8f9mO6wcCtJKXDTcHo/2uB5+kHzqJsLE2UCaXm ptaiyFGZgGNpUocO+tYxeORWm4kNMoZRAaYmiU0RWaoIkQMY0P/m/Ghy+nZBUexM vdH1lQ8DQoqQQxiC38MoO717rBOHDgxPoUGVPyPtU7qPhI2sSMYa2r+Uwi/Pmsm/ /dbKMbQs9q9pVkESBsmpkSLMVOrLQE/ju3h7iikZmY5RVrm+pI8fyOo9e20+/mKG aO5IT5IDaHXAVk8jjAWB
=rMf/
-----END PGP SIGNATURE-----

From: Lai Jiangshan <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, inject-nmi request is handled as follows.

- When in-kernel irqchip is disabled, deliver LINT1 instead of NMI interrupt.
- When in-kernel irqchip is enabled, get the in-kernel LAPIC states and test the APIC_LVT_MASKED, if LINT1 is unmasked, and then delivering the NMI directly. (Suggested by Jan Kiszka)

Changed from old version:
re-implement it by the Jan's suggestion.
fix the race found by Jan.

Quoted 17 lines · Lai Jiangshan

From: Lai Jiangshan <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, inject-nmi request is handled as follows.

- When in-kernel irqchip is disabled, deliver LINT1 instead of NMI interrupt.
- When in-kernel irqchip is enabled, get the in-kernel LAPIC states and test the APIC_LVT_MASKED, if LINT1 is unmasked, and then delivering the NMI directly. (Suggested by Jan Kiszka)

Quoted 3 lines

Changed from old version:
re-implement it by the Jan's suggestion.
fix the race found by Jan.

This patch fell through the cracks, sorry. Now applied.

Quoted 18 lines · Avi Kivity

Quoted 17 lines · Lai Jiangshan

From: Lai Jiangshan <[email protected]>

Currently, NMI interrupt is blindly sent to all the vCPUs when NMI button event happens. This doesn't properly emulate real hardware on which NMI button event triggers LINT1. Because of this, NMI is sent to the processor even when LINT1 is maskied in LVT. For example, this causes the problem that kdump initiated by NMI sometimes doesn't work on KVM, because kdump assumes NMI is masked on CPUs other than CPU0.

With this patch, inject-nmi request is handled as follows.

- When in-kernel irqchip is disabled, deliver LINT1 instead of NMI interrupt.
- When in-kernel irqchip is enabled, get the in-kernel LAPIC states and test the APIC_LVT_MASKED, if LINT1 is unmasked, and then delivering the NMI directly. (Suggested by Jan Kiszka)

Quoted 5 lines

Quoted 3 lines

Changed from old version:
re-implement it by the Jan's suggestion.
fix the race found by Jan.

This patch fell through the cracks, sorry. Now applied.

Lai, what is the state of a corresponding QEMU upstream patch? I'd like to build on top of it for my upstream irqchip series.