[Qemu-devel] [PATCH 0/4] Minor nmi cleanups

[Qemu-devel] [PATCH 0/4] Minor nmi cleanups

[Bandan Das]

The primary change is a arch specific x86 nmi function
which can be called by the core nmi handler.

Bandan Das (4):
  target-i386: add a generic x86 nmi handler
  nmi: remove x86 specific nmi handling
  nmi: add errp function parameter to inject_nmi()
  cpus: call the core nmi injection function

 cpus.c                 | 16 +---------------
 hw/core/nmi.c          | 23 ++---------------------
 hw/i386/pc.c           | 20 ++++++++++++++++++++
 hw/watchdog/watchdog.c |  2 +-
 include/hw/nmi.h       |  2 +-
 5 files changed, 25 insertions(+), 38 deletions(-)

-- 
2.5.5

[Qemu-devel] [PATCH 1/4] target-i386: add a generic x86 nmi handler

[Bandan Das]

Instead of having x86 ifdefs in core nmi code, this
change adds a arch specific handler that the nmi common
code can call.

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 hw/i386/pc.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 99437e0..e29ccc8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -67,6 +67,7 @@
 #include "qapi/visitor.h"
 #include "qapi-visit.h"
 #include "qom/cpu.h"
+#include "hw/nmi.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
@@ -1963,11 +1964,28 @@ static CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine)
     return list;
 }
 
+static void x86_nmi(NMIState *n, int cpu_index, Error **errp)
+{
+    /* cpu index isn't used */
+    CPUState *cs;
+
+    CPU_FOREACH(cs) {
+        X86CPU *cpu = X86_CPU(cs);
+
+        if (!cpu->apic_state) {
+            cpu_interrupt(cs, CPU_INTERRUPT_NMI);
+        } else {
+            apic_deliver_nmi(cpu->apic_state);
+        }
+    }
+}
+
 static void pc_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
     PCMachineClass *pcmc = PC_MACHINE_CLASS(oc);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
+    NMIClass *nc = NMI_CLASS(oc);
 
     pcmc->get_hotplug_handler = mc->get_hotplug_handler;
     pcmc->pci_enabled = true;
@@ -1993,6 +2011,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     hc->plug = pc_machine_device_plug_cb;
     hc->unplug_request = pc_machine_device_unplug_request_cb;
     hc->unplug = pc_machine_device_unplug_cb;
+    nc->nmi_monitor_handler = x86_nmi;
 }
 
 static const TypeInfo pc_machine_info = {
@@ -2005,6 +2024,7 @@ static const TypeInfo pc_machine_info = {
     .class_init = pc_machine_class_init,
     .interfaces = (InterfaceInfo[]) {
          { TYPE_HOTPLUG_HANDLER },
+         { TYPE_NMI },
          { }
     },
 };
-- 
2.5.5

[Qemu-devel] [PATCH 2/4] nmi: remove x86 specific nmi handling

[Bandan Das]

nmi_monitor_handle is wired to call the x86 nmi
handler now. So, we can call it here. Note that
this also reverts changes made by commit 33c11879

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 hw/core/nmi.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/hw/core/nmi.c b/hw/core/nmi.c
index f616a79..cc47025 100644
--- a/hw/core/nmi.c
+++ b/hw/core/nmi.c
@@ -20,16 +20,11 @@
  */
 
 #include "qemu/osdep.h"
-#include "qom/cpu.h"
 #include "hw/nmi.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
 #include "monitor/monitor.h"
 
-#if defined(TARGET_I386)
-#include "cpu.h"
-#endif
-
 struct do_nmi_s {
     int cpu_index;
     Error *err;
@@ -80,21 +75,7 @@ void nmi_monitor_handle(int cpu_index, Error **errp)
 
 void inject_nmi(void)
 {
-#if defined(TARGET_I386)
-    CPUState *cs;
-
-    CPU_FOREACH(cs) {
-        X86CPU *cpu = X86_CPU(cs);
-
-        if (!cpu->apic_state) {
-            cpu_interrupt(cs, CPU_INTERRUPT_NMI);
-        } else {
-            apic_deliver_nmi(cpu->apic_state);
-        }
-    }
-#else
     nmi_monitor_handle(0, NULL);
-#endif
 }
 
 static const TypeInfo nmi_info = {
-- 
2.5.5

[Qemu-devel] [PATCH 3/4] nmi: add errp function parameter to inject_nmi()

[Bandan Das]

If caller has errp, set it appropriately in case of
an error and pass it along.

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 hw/core/nmi.c          | 4 ++--
 hw/watchdog/watchdog.c | 2 +-
 include/hw/nmi.h       | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/core/nmi.c b/hw/core/nmi.c
index cc47025..40324a9 100644
--- a/hw/core/nmi.c
+++ b/hw/core/nmi.c
@@ -73,9 +73,9 @@ void nmi_monitor_handle(int cpu_index, Error **errp)
     }
 }
 
-void inject_nmi(void)
+void inject_nmi(Error **errp)
 {
-    nmi_monitor_handle(0, NULL);
+    nmi_monitor_handle(0, errp);
 }
 
 static const TypeInfo nmi_info = {
diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index bbf3646..c3f2c87 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -143,7 +143,7 @@ void watchdog_perform_action(void)
     case WDT_NMI:
         qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_INJECT_NMI,
                                  &error_abort);
-        inject_nmi();
+        inject_nmi(NULL);
         break;
     }
 }
diff --git a/include/hw/nmi.h b/include/hw/nmi.h
index f4cec62..63794d9 100644
--- a/include/hw/nmi.h
+++ b/include/hw/nmi.h
@@ -45,6 +45,6 @@ typedef struct NMIClass {
 } NMIClass;
 
 void nmi_monitor_handle(int cpu_index, Error **errp);
-void inject_nmi(void);
+void inject_nmi(Error **errp);
 
 #endif /* NMI_H */
-- 
2.5.5

[Qemu-devel] [PATCH 4/4] cpus: call the core nmi injection function

[Bandan Das]

Now that the common functions are in place, we can call it
instead of duplicating code.

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 cpus.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/cpus.c b/cpus.c
index eb34b4f..ead4e3f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1693,21 +1693,7 @@ exit:
 
 void qmp_inject_nmi(Error **errp)
 {
-#if defined(TARGET_I386)
-    CPUState *cs;
-
-    CPU_FOREACH(cs) {
-        X86CPU *cpu = X86_CPU(cs);
-
-        if (!cpu->apic_state) {
-            cpu_interrupt(cs, CPU_INTERRUPT_NMI);
-        } else {
-            apic_deliver_nmi(cpu->apic_state);
-        }
-    }
-#else
-    nmi_monitor_handle(monitor_get_cpu_index(), errp);
-#endif
+    inject_nmi(errp);
 }
 
 void dump_drift_info(FILE *f, fprintf_function cpu_fprintf)
-- 
2.5.5

Re: [Qemu-devel] [PATCH 4/4] cpus: call the core nmi injection function

[Paolo Bonzini]

On 20/05/2016 00:15, Bandan Das wrote:
>  void qmp_inject_nmi(Error **errp)
>  {
> -#if defined(TARGET_I386)
> -    CPUState *cs;
> -
> -    CPU_FOREACH(cs) {
> -        X86CPU *cpu = X86_CPU(cs);
> -
> -        if (!cpu->apic_state) {
> -            cpu_interrupt(cs, CPU_INTERRUPT_NMI);
> -        } else {
> -            apic_deliver_nmi(cpu->apic_state);
> -        }
> -    }
> -#else
> -    nmi_monitor_handle(monitor_get_cpu_index(), errp);
> -#endif
> +    inject_nmi(errp);

This changes the first argument to nmi_monitor_handle in the !i386 case.
 It should just remove the #ifdef, which I think makes the third patch
unnecessary.  The first two patches are okay.

Paolo

Re: [Qemu-devel] [PATCH 4/4] cpus: call the core nmi injection function

[Paolo Bonzini]

On 20/05/2016 00:15, Bandan Das wrote:
>  void qmp_inject_nmi(Error **errp)
>  {
> -#if defined(TARGET_I386)
> -    CPUState *cs;
> -
> -    CPU_FOREACH(cs) {
> -        X86CPU *cpu = X86_CPU(cs);
> -
> -        if (!cpu->apic_state) {
> -            cpu_interrupt(cs, CPU_INTERRUPT_NMI);
> -        } else {
> -            apic_deliver_nmi(cpu->apic_state);
> -        }
> -    }
> -#else
> -    nmi_monitor_handle(monitor_get_cpu_index(), errp);
> -#endif
> +    inject_nmi(errp);

This changes the first argument to nmi_monitor_handle in the !i386 case.
 It should just remove the #ifdef, which I think makes the third patch
unnecessary.  The first two patches are okay.

Paolo

Re: [Qemu-devel] [PATCH 4/4] cpus: call the core nmi injection function

[Bandan Das]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 20/05/2016 00:15, Bandan Das wrote:
>>  void qmp_inject_nmi(Error **errp)
>>  {
>> -#if defined(TARGET_I386)
>> -    CPUState *cs;
>> -
>> -    CPU_FOREACH(cs) {
>> -        X86CPU *cpu = X86_CPU(cs);
>> -
>> -        if (!cpu->apic_state) {
>> -            cpu_interrupt(cs, CPU_INTERRUPT_NMI);
>> -        } else {
>> -            apic_deliver_nmi(cpu->apic_state);
>> -        }
>> -    }
>> -#else
>> -    nmi_monitor_handle(monitor_get_cpu_index(), errp);
>> -#endif
>> +    inject_nmi(errp);
>
> This changes the first argument to nmi_monitor_handle in the !i386 case.
>  It should just remove the #ifdef, which I think makes the third patch
> unnecessary.  The first two patches are okay.

Thanks for the review, I have sent a v2.

> Paolo