[PATCH v3 0/7] hw/nmi: Disconnect of vCPU and disentangle Monitor code

[PATCH v3 0/7] hw/nmi: Disconnect of vCPU and disentangle Monitor code

[Philippe Mathieu-Daudé]

- Have s390x always deliver NMI to the first CPU.
- Remove the @cpu_index / @errp arguments from handler.
- Rename API as nmi_trigger() (since not monitor specific).
- Only deliver NMI once

Rationale described by Peter in v1 [*]:

> The current hw/core/nmi.c code is a bit odd because it's partly
> working with a cpu_index and partly not: the code passes cpu_index
> around, but in practice for the QMP command the user can't set
> which CPU to operate on, and for everything except s390 the
> implementation doesn't care anyway. My impression from the IRC
> discussion is that it's not really necessary for the S390 that
> the monitor user be able to specify which CPU to NMI (and in any
> case you can only do that from the HMP command, not the QMP
> command, AIUI), so getting rid of that weird inconsistency makes
> sense to me: and that's what this patchset is doing.

[*] https://lore.kernel.org/qemu-devel/CAFEAcA_0qUFW0MewHC+v+pSOisE-kQDt9Wv4F3RafEkyQ0DGJA@mail.gmail.com/:

Philippe Mathieu-Daudé (7):
  hw/nmi: Use object_child_foreach_recursive() in nmi_children()
  hw/s390x/virtio-ccw: Always deliver NMI to first CPU
  hw/nmi: Remove @cpu_index argument from
    NMIClass::nmi_monitor_handler()
  hw/nmi: Remove @cpu_index argument from nmi_trigger()
  hw/nmi: Rename nmi_monitor_handler() -> deliver_nmi()
  hw/nmi: Remove unused @errp argument from deliver_nmi()
  hw/nmi: Deliver NMI only once

 qapi/run-state.json        |  6 +++--
 include/hw/core/nmi.h      | 32 +++++++++++++++++++++++++--
 hw/core/nmi.c              | 45 ++++++++++----------------------------
 hw/hppa/machine.c          |  4 ++--
 hw/i386/x86.c              |  4 ++--
 hw/intc/m68k_irqc.c        |  4 ++--
 hw/ipmi/ipmi.c             |  3 +--
 hw/m68k/q800-glue.c        |  4 ++--
 hw/misc/macio/gpio.c       |  4 ++--
 hw/ppc/pnv.c               |  4 ++--
 hw/ppc/spapr.c             |  4 ++--
 hw/s390x/s390-virtio-ccw.c |  8 +++----
 hw/watchdog/watchdog.c     |  2 +-
 system/cpus.c              |  2 +-
 hmp-commands.hx            |  2 +-
 15 files changed, 66 insertions(+), 62 deletions(-)

-- 
2.53.0

[PATCH v3 1/7] hw/nmi: Use object_child_foreach_recursive() in nmi_children()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <philmd@linaro.org>

Replace object_child_foreach() and recursion by a single
object_child_foreach_recursive() call.
Propagate the returned value so callers can check it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 hw/core/nmi.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/core/nmi.c b/hw/core/nmi.c
index 4b447e126ba..22724046575 100644
--- a/hw/core/nmi.c
+++ b/hw/core/nmi.c
@@ -31,8 +31,6 @@ struct do_nmi_s {
     bool handled;
 };
 
-static void nmi_children(Object *o, struct do_nmi_s *ns);
-
 static int do_nmi(Object *o, void *opaque)
 {
     struct do_nmi_s *ns = opaque;
@@ -47,14 +45,13 @@ static int do_nmi(Object *o, void *opaque)
             return -1;
         }
     }
-    nmi_children(o, ns);
 
     return 0;
 }
 
-static void nmi_children(Object *o, struct do_nmi_s *ns)
+static int nmi_children(Object *o, struct do_nmi_s *ns)
 {
-    object_child_foreach(o, do_nmi, ns);
+    return object_child_foreach_recursive(o, do_nmi, ns);
 }
 
 void nmi_monitor_handle(int cpu_index, Error **errp)
@@ -65,10 +62,9 @@ void nmi_monitor_handle(int cpu_index, Error **errp)
         .handled = false
     };
 
-    nmi_children(object_get_root(), &ns);
-    if (ns.handled) {
+    if (nmi_children(object_get_root(), &ns)) {
         error_propagate(errp, ns.err);
-    } else {
+    } else if (!ns.handled) {
         error_setg(errp, "machine does not provide NMIs");
     }
 }
-- 
2.53.0

[PATCH v3 2/7] hw/s390x/virtio-ccw: Always deliver NMI to first CPU

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <philmd@linaro.org>

We can trigger NMI from HMP or QMP.

QEMU maps the NMI to the s390x per-CPU 'RESTART' interrupt.
Linux guests usually setup this interrupt to trigger kdump
or crash. Such crashdump can be triggered in QEMU by HMP
"nmi" or QMP "inject-nmi" commands.

Using QMP, since we can not select a particular CPU, the first
CPU is used (CPU#0). See the documentation from commit 795dc6e4
("watchdog: Add new Virtual Watchdog action INJECT-NMI"):

  @inject-nmi: a non-maskable interrupt is injected into the
               first VCPU (all VCPUS on x86) (since 2.4)

While we can select a particular CPU on HMP, the guest behavior
is expected to be the same if using CPU #N or CPU #0. Since
always using CPU#0 simplifies API maintainance, update s390_nmi()
to deliver NMI to the first CPU.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 qapi/run-state.json        | 6 ++++--
 hw/s390x/s390-virtio-ccw.c | 4 +---
 hmp-commands.hx            | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/qapi/run-state.json b/qapi/run-state.json
index a5771ad4681..e4fdead1e0b 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -317,8 +317,10 @@
 #
 # @none: nothing is done
 #
-# @inject-nmi: a non-maskable interrupt is injected into the first
-#     VCPU (all VCPUS on x86) (since 2.4)
+# @inject-nmi: a non-maskable interrupt is injected (machine
+#     specific: for example on s390x CCW only the first vCPU
+#     receives the NMI, but on x86 machines all vCPUs receive
+#     it).  (since 2.4)
 #
 # Since: 2.1
 ##
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 25a9fa49554..4f7861dbbb7 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -727,9 +727,7 @@ static HotplugHandler *s390_get_hotplug_handler(MachineState *machine,
 
 static void s390_nmi(NMIState *n, int cpu_index, Error **errp)
 {
-    CPUState *cs = qemu_get_cpu(cpu_index);
-
-    s390_cpu_restart(S390_CPU(cs));
+    s390_cpu_restart(S390_CPU(first_cpu));
 }
 
 static inline bool machine_get_aes_key_wrap(Object *obj, Error **errp)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 7ae2468a3d9..3aa159f11ce 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -870,7 +870,7 @@ ERST
     },
 SRST
 ``nmi`` *cpu*
-  Inject an NMI on the default CPU (x86/s390) or all CPUs (ppc64).
+  Inject an NMI.
 ERST
 
     {
-- 
2.53.0

[PATCH v3 3/7] hw/nmi: Remove @cpu_index argument from NMIClass::nmi_monitor_handler()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <philmd@linaro.org>

Only s390x was using the 'cpu_index' argument, but since the
previous commit it isn't anymore (it use the first cpu).
Since this argument is now completely unused, remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 include/hw/core/nmi.h      | 8 +++++++-
 hw/core/nmi.c              | 2 +-
 hw/hppa/machine.c          | 2 +-
 hw/i386/x86.c              | 2 +-
 hw/intc/m68k_irqc.c        | 2 +-
 hw/m68k/q800-glue.c        | 2 +-
 hw/misc/macio/gpio.c       | 2 +-
 hw/ppc/pnv.c               | 2 +-
 hw/ppc/spapr.c             | 2 +-
 hw/s390x/s390-virtio-ccw.c | 2 +-
 10 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/include/hw/core/nmi.h b/include/hw/core/nmi.h
index fff41bebc69..4c4ce79071d 100644
--- a/include/hw/core/nmi.h
+++ b/include/hw/core/nmi.h
@@ -37,7 +37,13 @@ typedef struct NMIState NMIState;
 struct NMIClass {
     InterfaceClass parent_class;
 
-    void (*nmi_monitor_handler)(NMIState *n, int cpu_index, Error **errp);
+    /**
+     * nmi_monitor_handler: Callback to handle NMI notifications.
+     *
+     * @ns: Class #NMIState state
+     * @errp: pointer to error object
+     */
+    void (*nmi_monitor_handler)(NMIState *ns, Error **errp);
 };
 
 void nmi_monitor_handle(int cpu_index, Error **errp);
diff --git a/hw/core/nmi.c b/hw/core/nmi.c
index 22724046575..ff6454437c1 100644
--- a/hw/core/nmi.c
+++ b/hw/core/nmi.c
@@ -40,7 +40,7 @@ static int do_nmi(Object *o, void *opaque)
         NMIClass *nc = NMI_GET_CLASS(n);
 
         ns->handled = true;
-        nc->nmi_monitor_handler(n, ns->cpu_index, &ns->err);
+        nc->nmi_monitor_handler(n, &ns->err);
         if (ns->err) {
             return -1;
         }
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index d762163ddf5..2b44debc388 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -829,7 +829,7 @@ static void hppa_machine_reset(MachineState *ms, ResetType type)
     cpu[0]->env.cmdline_or_bootorder = 'c';
 }
 
-static void hppa_nmi(NMIState *n, int cpu_index, Error **errp)
+static void hppa_nmi(NMIState *ns, Error **errp)
 {
     CPUState *cs;
 
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index dc7f0d56b01..0ea5453403d 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -147,7 +147,7 @@ static const CPUArchIdList *x86_possible_cpu_arch_ids(MachineState *ms)
     return ms->possible_cpus;
 }
 
-static void x86_nmi(NMIState *n, int cpu_index, Error **errp)
+static void x86_nmi(NMIState *ns, Error **errp)
 {
     /* cpu index isn't used */
     CPUState *cs;
diff --git a/hw/intc/m68k_irqc.c b/hw/intc/m68k_irqc.c
index 68ddb5351be..bc41b44d455 100644
--- a/hw/intc/m68k_irqc.c
+++ b/hw/intc/m68k_irqc.c
@@ -70,7 +70,7 @@ static void m68k_irqc_instance_init(Object *obj)
     qdev_init_gpio_in(DEVICE(obj), m68k_set_irq, M68K_IRQC_LEVEL_NUM);
 }
 
-static void m68k_nmi(NMIState *n, int cpu_index, Error **errp)
+static void m68k_nmi(NMIState *n, Error **errp)
 {
     m68k_set_irq(n, M68K_IRQC_LEVEL_7, 1);
 }
diff --git a/hw/m68k/q800-glue.c b/hw/m68k/q800-glue.c
index ac9062c6488..b13e5ef95f1 100644
--- a/hw/m68k/q800-glue.c
+++ b/hw/m68k/q800-glue.c
@@ -159,7 +159,7 @@ static void glue_auxmode_set_irq(void *opaque, int irq, int level)
     s->auxmode = level;
 }
 
-static void glue_nmi(NMIState *n, int cpu_index, Error **errp)
+static void glue_nmi(NMIState *n, Error **errp)
 {
     GLUEState *s = GLUE(n);
 
diff --git a/hw/misc/macio/gpio.c b/hw/misc/macio/gpio.c
index 1a7c534d652..1bdca728f97 100644
--- a/hw/misc/macio/gpio.c
+++ b/hw/misc/macio/gpio.c
@@ -188,7 +188,7 @@ static void macio_gpio_reset(DeviceState *dev)
     macio_set_gpio(s, 1, true);
 }
 
-static void macio_gpio_nmi(NMIState *n, int cpu_index, Error **errp)
+static void macio_gpio_nmi(NMIState *n, Error **errp)
 {
     macio_set_gpio(MACIO_GPIO(n), 9, true);
     macio_set_gpio(MACIO_GPIO(n), 9, false);
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index c0cb45dbfbc..8676be7bbb1 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -3552,7 +3552,7 @@ static void pnv_cpu_do_nmi(PnvChip *chip, PowerPCCPU *cpu, void *opaque)
     async_run_on_cpu(CPU(cpu), pnv_cpu_do_nmi_on_cpu, RUN_ON_CPU_HOST_INT(0));
 }
 
-static void pnv_nmi(NMIState *n, int cpu_index, Error **errp)
+static void pnv_nmi(NMIState *ns, Error **errp)
 {
     PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
     int i;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b79828b4e90..189334b9f0a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3550,7 +3550,7 @@ void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg)
     }
 }
 
-static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
+static void spapr_nmi(NMIState *ns, Error **errp)
 {
     CPUState *cs;
 
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4f7861dbbb7..5d8a73a63df 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -725,7 +725,7 @@ static HotplugHandler *s390_get_hotplug_handler(MachineState *machine,
     return NULL;
 }
 
-static void s390_nmi(NMIState *n, int cpu_index, Error **errp)
+static void s390_nmi(NMIState *ns, Error **errp)
 {
     s390_cpu_restart(S390_CPU(first_cpu));
 }
-- 
2.53.0

[PATCH v3 4/7] hw/nmi: Remove @cpu_index argument from nmi_trigger()

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <philmd@linaro.org>

nmi_monitor_handle() is not related to the monitor, rename
it as nmi_trigger().

Return a boolean value indicating success / failure as
recommended by the Error API since commit e3fe3988d7
("error: Document Error API usage rules").

The 'cpu_index' argument is not used, remove it.

Document nmi_trigger() as suggested by Peter Maydell in
https://lore.kernel.org/qemu-devel/CAFEAcA-yALySmCJLbitCmYpiZKUXJNOavGJG9RYeo8fKqz7gcw@mail.gmail.com/.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 include/hw/core/nmi.h  | 25 ++++++++++++++++++++++++-
 hw/core/nmi.c          |  9 ++++-----
 hw/ipmi/ipmi.c         |  3 +--
 hw/watchdog/watchdog.c |  2 +-
 system/cpus.c          |  2 +-
 5 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/include/hw/core/nmi.h b/include/hw/core/nmi.h
index 4c4ce79071d..e77c382452a 100644
--- a/include/hw/core/nmi.h
+++ b/include/hw/core/nmi.h
@@ -46,6 +46,29 @@ struct NMIClass {
     void (*nmi_monitor_handler)(NMIState *ns, Error **errp);
 };
 
-void nmi_monitor_handle(int cpu_index, Error **errp);
+/**
+ * nmi_trigger: Trigger an NMI, in a machine-specific way
+ * @errp: pointer to error object
+ *
+ * This function triggers an NMI, in a machine-specific way. The
+ * intention is that this should typically trigger a guest kernel
+ * dump or reboot, and might happen as a result of user request
+ * from the monitor, watchdog timeouts, and similar events.
+ * (For example on the x86 PC it triggers an NMI on all CPUs,
+ * and on s390 it triggers the RESTART interrupt on the first CPU.)
+ *
+ * The NMI is triggered by looking for QOM objects which
+ * implement the TYPE_NMI interface, and calling their nmi_handler
+ * method. Usually it is the machine model class that implements
+ * this interface.
+ *
+ * Not all machines implement NMI handling; this function
+ * will return an error if used on a machine which does not
+ * implement NMIs.
+ *
+ * On success, return %true.
+ * On failure, store an error through @errp and return %false.
+ */
+bool nmi_trigger(Error **errp);
 
 #endif /* NMI_H */
diff --git a/hw/core/nmi.c b/hw/core/nmi.c
index ff6454437c1..662f2051123 100644
--- a/hw/core/nmi.c
+++ b/hw/core/nmi.c
@@ -22,11 +22,8 @@
 #include "qemu/osdep.h"
 #include "hw/core/nmi.h"
 #include "qapi/error.h"
-#include "qemu/module.h"
-#include "monitor/monitor.h"
 
 struct do_nmi_s {
-    int cpu_index;
     Error *err;
     bool handled;
 };
@@ -54,19 +51,21 @@ static int nmi_children(Object *o, struct do_nmi_s *ns)
     return object_child_foreach_recursive(o, do_nmi, ns);
 }
 
-void nmi_monitor_handle(int cpu_index, Error **errp)
+bool nmi_trigger(Error **errp)
 {
     struct do_nmi_s ns = {
-        .cpu_index = cpu_index,
         .err = NULL,
         .handled = false
     };
 
     if (nmi_children(object_get_root(), &ns)) {
         error_propagate(errp, ns.err);
+        return false;
     } else if (!ns.handled) {
         error_setg(errp, "machine does not provide NMIs");
+        return false;
     }
+    return true;
 }
 
 static const TypeInfo nmi_info = {
diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
index 74818ff3cea..444af726d75 100644
--- a/hw/ipmi/ipmi.c
+++ b/hw/ipmi/ipmi.c
@@ -59,8 +59,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
         if (checkonly) {
             return 0;
         }
-        /* We don't care what CPU we use. */
-        nmi_monitor_handle(0, NULL);
+        nmi_trigger(NULL);
         return 0;
 
     case IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP:
diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index 0842fe373ae..026768e9fb2 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -81,7 +81,7 @@ void watchdog_perform_action(void)
 
     case WATCHDOG_ACTION_INJECT_NMI:
         qapi_event_send_watchdog(WATCHDOG_ACTION_INJECT_NMI);
-        nmi_monitor_handle(0, NULL);
+        nmi_trigger(NULL);
         break;
 
     default:
diff --git a/system/cpus.c b/system/cpus.c
index 97e5a5edee2..5c0a192c01e 100644
--- a/system/cpus.c
+++ b/system/cpus.c
@@ -926,6 +926,6 @@ exit:
 
 void qmp_inject_nmi(Error **errp)
 {
-    nmi_monitor_handle(monitor_get_cpu_index(monitor_cur()), errp);
+    nmi_trigger(errp);
 }
 
-- 
2.53.0

[PATCH v3 5/7] hw/nmi: Rename nmi_monitor_handler() -> deliver_nmi()

[Philippe Mathieu-Daudé]

nmi_monitor_handler() is not related to the monitor,
rename it as deliver_nmi().

Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 include/hw/core/nmi.h      | 4 ++--
 hw/core/nmi.c              | 2 +-
 hw/hppa/machine.c          | 2 +-
 hw/i386/x86.c              | 2 +-
 hw/intc/m68k_irqc.c        | 2 +-
 hw/m68k/q800-glue.c        | 2 +-
 hw/misc/macio/gpio.c       | 2 +-
 hw/ppc/pnv.c               | 2 +-
 hw/ppc/spapr.c             | 2 +-
 hw/s390x/s390-virtio-ccw.c | 2 +-
 10 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/hw/core/nmi.h b/include/hw/core/nmi.h
index e77c382452a..d6bd2a9fceb 100644
--- a/include/hw/core/nmi.h
+++ b/include/hw/core/nmi.h
@@ -38,12 +38,12 @@ struct NMIClass {
     InterfaceClass parent_class;
 
     /**
-     * nmi_monitor_handler: Callback to handle NMI notifications.
+     * deliver_nmi: Callback to handle NMI notifications.
      *
      * @ns: Class #NMIState state
      * @errp: pointer to error object
      */
-    void (*nmi_monitor_handler)(NMIState *ns, Error **errp);
+    void (*deliver_nmi)(NMIState *ns, Error **errp);
 };
 
 /**
diff --git a/hw/core/nmi.c b/hw/core/nmi.c
index 662f2051123..02c0ff2c21d 100644
--- a/hw/core/nmi.c
+++ b/hw/core/nmi.c
@@ -37,7 +37,7 @@ static int do_nmi(Object *o, void *opaque)
         NMIClass *nc = NMI_GET_CLASS(n);
 
         ns->handled = true;
-        nc->nmi_monitor_handler(n, &ns->err);
+        nc->deliver_nmi(n, &ns->err);
         if (ns->err) {
             return -1;
         }
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 2b44debc388..47e0302bac8 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -851,7 +851,7 @@ static void hppa_machine_common_class_init(ObjectClass *oc, const void *data)
     mc->default_ram_id = "hppa.ram";
     mc->default_nic = "tulip";
 
-    nc->nmi_monitor_handler = hppa_nmi;
+    nc->deliver_nmi = hppa_nmi;
 }
 
 static void HP_B160L_machine_init_class_init(ObjectClass *oc, const void *data)
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 0ea5453403d..48f7575895a 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -389,7 +389,7 @@ static void x86_machine_class_init(ObjectClass *oc, const void *data)
     mc->get_default_cpu_node_id = x86_get_default_cpu_node_id;
     mc->possible_cpu_arch_ids = x86_possible_cpu_arch_ids;
     mc->kvm_type = x86_kvm_type;
-    nc->nmi_monitor_handler = x86_nmi;
+    nc->deliver_nmi = x86_nmi;
 
     object_class_property_add(oc, X86_MACHINE_SMM, "OnOffAuto",
         x86_machine_get_smm, x86_machine_set_smm,
diff --git a/hw/intc/m68k_irqc.c b/hw/intc/m68k_irqc.c
index bc41b44d455..281542e3997 100644
--- a/hw/intc/m68k_irqc.c
+++ b/hw/intc/m68k_irqc.c
@@ -97,7 +97,7 @@ static void m68k_irqc_class_init(ObjectClass *oc, const void *data)
     InterruptStatsProviderClass *ic = INTERRUPT_STATS_PROVIDER_CLASS(oc);
 
     device_class_set_props(dc, m68k_irqc_properties);
-    nc->nmi_monitor_handler = m68k_nmi;
+    nc->deliver_nmi = m68k_nmi;
     device_class_set_legacy_reset(dc, m68k_irqc_reset);
     dc->vmsd = &vmstate_m68k_irqc;
     ic->get_statistics = m68k_irqc_get_statistics;
diff --git a/hw/m68k/q800-glue.c b/hw/m68k/q800-glue.c
index b13e5ef95f1..b75f7f44d3c 100644
--- a/hw/m68k/q800-glue.c
+++ b/hw/m68k/q800-glue.c
@@ -237,7 +237,7 @@ static void glue_class_init(ObjectClass *klass, const void *data)
     dc->vmsd = &vmstate_glue;
     device_class_set_props(dc, glue_properties);
     rc->phases.hold = glue_reset_hold;
-    nc->nmi_monitor_handler = glue_nmi;
+    nc->deliver_nmi = glue_nmi;
 }
 
 static const TypeInfo glue_info_types[] = {
diff --git a/hw/misc/macio/gpio.c b/hw/misc/macio/gpio.c
index 1bdca728f97..086d90f20b6 100644
--- a/hw/misc/macio/gpio.c
+++ b/hw/misc/macio/gpio.c
@@ -201,7 +201,7 @@ static void macio_gpio_class_init(ObjectClass *oc, const void *data)
 
     device_class_set_legacy_reset(dc, macio_gpio_reset);
     dc->vmsd = &vmstate_macio_gpio;
-    nc->nmi_monitor_handler = macio_gpio_nmi;
+    nc->deliver_nmi = macio_gpio_nmi;
 }
 
 static const TypeInfo macio_gpio_init_info = {
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 8676be7bbb1..bfb1d45d058 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -3583,7 +3583,7 @@ static void pnv_machine_class_init(ObjectClass *oc, const void *data)
     mc->default_ram_size = 1 * GiB;
     mc->default_ram_id = "pnv.ram";
     ispc->print_info = pnv_pic_print_info;
-    nc->nmi_monitor_handler = pnv_nmi;
+    nc->deliver_nmi = pnv_nmi;
 
     object_class_property_add_bool(oc, "hb-mode",
                                    pnv_machine_get_hb, pnv_machine_set_hb);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 189334b9f0a..ed545acf3c0 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4652,7 +4652,7 @@ static void spapr_machine_class_init(ObjectClass *oc, const void *data)
     mc->nvdimm_supported = true;
     smc->resize_hpt_default = SPAPR_RESIZE_HPT_ENABLED;
     fwc->get_dev_path = spapr_get_fw_dev_path;
-    nc->nmi_monitor_handler = spapr_nmi;
+    nc->deliver_nmi = spapr_nmi;
     vhc->cpu_in_nested = spapr_cpu_in_nested;
     vhc->deliver_hv_excp = spapr_exit_nested;
     vhc->hypercall = emulate_spapr_hypercall;
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 5d8a73a63df..f9e8ecb4fe0 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -830,7 +830,7 @@ static void ccw_machine_class_init(ObjectClass *oc, const void *data)
     hc->plug = s390_machine_device_plug;
     hc->unplug_request = s390_machine_device_unplug_request;
     hc->unplug = s390_machine_device_unplug;
-    nc->nmi_monitor_handler = s390_nmi;
+    nc->deliver_nmi = s390_nmi;
     mc->default_ram_id = "s390.ram";
     mc->default_nic = "virtio-net-ccw";
     dsi->qmp_dump_skeys = s390_qmp_dump_skeys;
-- 
2.53.0

[PATCH v3 6/7] hw/nmi: Remove unused @errp argument from deliver_nmi()

[Philippe Mathieu-Daudé]

Not a single handler update @errp. The single user is
nmi_trigger() filling with "machine does not provide NMIs".
Remove the unused argument from the deliver_nmi() callback,
simplifying the methods in hw/core/nmi.c.

Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 include/hw/core/nmi.h      |  3 +--
 hw/core/nmi.c              | 32 ++++++--------------------------
 hw/hppa/machine.c          |  2 +-
 hw/i386/x86.c              |  2 +-
 hw/intc/m68k_irqc.c        |  2 +-
 hw/m68k/q800-glue.c        |  2 +-
 hw/misc/macio/gpio.c       |  2 +-
 hw/ppc/pnv.c               |  2 +-
 hw/ppc/spapr.c             |  2 +-
 hw/s390x/s390-virtio-ccw.c |  2 +-
 10 files changed, 15 insertions(+), 36 deletions(-)

diff --git a/include/hw/core/nmi.h b/include/hw/core/nmi.h
index d6bd2a9fceb..b0f58542bcc 100644
--- a/include/hw/core/nmi.h
+++ b/include/hw/core/nmi.h
@@ -41,9 +41,8 @@ struct NMIClass {
      * deliver_nmi: Callback to handle NMI notifications.
      *
      * @ns: Class #NMIState state
-     * @errp: pointer to error object
      */
-    void (*deliver_nmi)(NMIState *ns, Error **errp);
+    void (*deliver_nmi)(NMIState *ns);
 };
 
 /**
diff --git a/hw/core/nmi.c b/hw/core/nmi.c
index 02c0ff2c21d..8eea75ad530 100644
--- a/hw/core/nmi.c
+++ b/hw/core/nmi.c
@@ -23,45 +23,25 @@
 #include "hw/core/nmi.h"
 #include "qapi/error.h"
 
-struct do_nmi_s {
-    Error *err;
-    bool handled;
-};
-
 static int do_nmi(Object *o, void *opaque)
 {
-    struct do_nmi_s *ns = opaque;
+    bool *handled = opaque;
     NMIState *n = (NMIState *) object_dynamic_cast(o, TYPE_NMI);
 
     if (n) {
-        NMIClass *nc = NMI_GET_CLASS(n);
-
-        ns->handled = true;
-        nc->deliver_nmi(n, &ns->err);
-        if (ns->err) {
-            return -1;
-        }
+        *handled = true;
+        NMI_GET_CLASS(n)->deliver_nmi(n);
     }
 
     return 0;
 }
 
-static int nmi_children(Object *o, struct do_nmi_s *ns)
-{
-    return object_child_foreach_recursive(o, do_nmi, ns);
-}
-
 bool nmi_trigger(Error **errp)
 {
-    struct do_nmi_s ns = {
-        .err = NULL,
-        .handled = false
-    };
+    bool handled = false;
 
-    if (nmi_children(object_get_root(), &ns)) {
-        error_propagate(errp, ns.err);
-        return false;
-    } else if (!ns.handled) {
+    object_child_foreach_recursive(object_get_root(), do_nmi, &handled);
+    if (!handled) {
         error_setg(errp, "machine does not provide NMIs");
         return false;
     }
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 47e0302bac8..51ecdabdbd5 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -829,7 +829,7 @@ static void hppa_machine_reset(MachineState *ms, ResetType type)
     cpu[0]->env.cmdline_or_bootorder = 'c';
 }
 
-static void hppa_nmi(NMIState *ns, Error **errp)
+static void hppa_nmi(NMIState *ns)
 {
     CPUState *cs;
 
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 48f7575895a..8d82bd890da 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -147,7 +147,7 @@ static const CPUArchIdList *x86_possible_cpu_arch_ids(MachineState *ms)
     return ms->possible_cpus;
 }
 
-static void x86_nmi(NMIState *ns, Error **errp)
+static void x86_nmi(NMIState *ns)
 {
     /* cpu index isn't used */
     CPUState *cs;
diff --git a/hw/intc/m68k_irqc.c b/hw/intc/m68k_irqc.c
index 281542e3997..b78ca5ca11b 100644
--- a/hw/intc/m68k_irqc.c
+++ b/hw/intc/m68k_irqc.c
@@ -70,7 +70,7 @@ static void m68k_irqc_instance_init(Object *obj)
     qdev_init_gpio_in(DEVICE(obj), m68k_set_irq, M68K_IRQC_LEVEL_NUM);
 }
 
-static void m68k_nmi(NMIState *n, Error **errp)
+static void m68k_nmi(NMIState *n)
 {
     m68k_set_irq(n, M68K_IRQC_LEVEL_7, 1);
 }
diff --git a/hw/m68k/q800-glue.c b/hw/m68k/q800-glue.c
index b75f7f44d3c..8be7696777e 100644
--- a/hw/m68k/q800-glue.c
+++ b/hw/m68k/q800-glue.c
@@ -159,7 +159,7 @@ static void glue_auxmode_set_irq(void *opaque, int irq, int level)
     s->auxmode = level;
 }
 
-static void glue_nmi(NMIState *n, Error **errp)
+static void glue_nmi(NMIState *n)
 {
     GLUEState *s = GLUE(n);
 
diff --git a/hw/misc/macio/gpio.c b/hw/misc/macio/gpio.c
index 086d90f20b6..ba4df62c674 100644
--- a/hw/misc/macio/gpio.c
+++ b/hw/misc/macio/gpio.c
@@ -188,7 +188,7 @@ static void macio_gpio_reset(DeviceState *dev)
     macio_set_gpio(s, 1, true);
 }
 
-static void macio_gpio_nmi(NMIState *n, Error **errp)
+static void macio_gpio_nmi(NMIState *n)
 {
     macio_set_gpio(MACIO_GPIO(n), 9, true);
     macio_set_gpio(MACIO_GPIO(n), 9, false);
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index bfb1d45d058..2ec0c198fa5 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -3552,7 +3552,7 @@ static void pnv_cpu_do_nmi(PnvChip *chip, PowerPCCPU *cpu, void *opaque)
     async_run_on_cpu(CPU(cpu), pnv_cpu_do_nmi_on_cpu, RUN_ON_CPU_HOST_INT(0));
 }
 
-static void pnv_nmi(NMIState *ns, Error **errp)
+static void pnv_nmi(NMIState *ns)
 {
     PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
     int i;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ed545acf3c0..477054d8c85 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3550,7 +3550,7 @@ void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg)
     }
 }
 
-static void spapr_nmi(NMIState *ns, Error **errp)
+static void spapr_nmi(NMIState *ns)
 {
     CPUState *cs;
 
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index f9e8ecb4fe0..255698fd7ed 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -725,7 +725,7 @@ static HotplugHandler *s390_get_hotplug_handler(MachineState *machine,
     return NULL;
 }
 
-static void s390_nmi(NMIState *ns, Error **errp)
+static void s390_nmi(NMIState *ns)
 {
     s390_cpu_restart(S390_CPU(first_cpu));
 }
-- 
2.53.0

[PATCH v3 7/7] hw/nmi: Deliver NMI only once

[Philippe Mathieu-Daudé]

Use the first handler available to deliver NMI only once.
If anothers handlers are available, no need to keep delivering.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 hw/core/nmi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/core/nmi.c b/hw/core/nmi.c
index 8eea75ad530..4fe5ee75bfc 100644
--- a/hw/core/nmi.c
+++ b/hw/core/nmi.c
@@ -31,6 +31,8 @@ static int do_nmi(Object *o, void *opaque)
     if (n) {
         *handled = true;
         NMI_GET_CLASS(n)->deliver_nmi(n);
+        /* We only need to deliver NMI once */
+        return 1;
     }
 
     return 0;
-- 
2.53.0

Re: [PATCH v3 4/7] hw/nmi: Remove @cpu_index argument from nmi_trigger()

[Marc-André Lureau]

Hi

On Tue, Aug 11, 2026 at 2:55 PM Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
>
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> nmi_monitor_handle() is not related to the monitor, rename
> it as nmi_trigger().
>
> Return a boolean value indicating success / failure as
> recommended by the Error API since commit e3fe3988d7
> ("error: Document Error API usage rules").
>
> The 'cpu_index' argument is not used, remove it.
>
> Document nmi_trigger() as suggested by Peter Maydell in
> https://lore.kernel.org/qemu-devel/CAFEAcA-yALySmCJLbitCmYpiZKUXJNOavGJG9RYeo8fKqz7gcw@mail.gmail.com/.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
>  include/hw/core/nmi.h  | 25 ++++++++++++++++++++++++-
>  hw/core/nmi.c          |  9 ++++-----
>  hw/ipmi/ipmi.c         |  3 +--
>  hw/watchdog/watchdog.c |  2 +-
>  system/cpus.c          |  2 +-
>  5 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/include/hw/core/nmi.h b/include/hw/core/nmi.h
> index 4c4ce79071d..e77c382452a 100644
> --- a/include/hw/core/nmi.h
> +++ b/include/hw/core/nmi.h
> @@ -46,6 +46,29 @@ struct NMIClass {
>      void (*nmi_monitor_handler)(NMIState *ns, Error **errp);
>  };
>
> -void nmi_monitor_handle(int cpu_index, Error **errp);
> +/**
> + * nmi_trigger: Trigger an NMI, in a machine-specific way
> + * @errp: pointer to error object
> + *
> + * This function triggers an NMI, in a machine-specific way. The
> + * intention is that this should typically trigger a guest kernel
> + * dump or reboot, and might happen as a result of user request
> + * from the monitor, watchdog timeouts, and similar events.
> + * (For example on the x86 PC it triggers an NMI on all CPUs,
> + * and on s390 it triggers the RESTART interrupt on the first CPU.)
> + *
> + * The NMI is triggered by looking for QOM objects which
> + * implement the TYPE_NMI interface, and calling their nmi_handler
> + * method. Usually it is the machine model class that implements
> + * this interface.
> + *
> + * Not all machines implement NMI handling; this function
> + * will return an error if used on a machine which does not
> + * implement NMIs.
> + *
> + * On success, return %true.
> + * On failure, store an error through @errp and return %false.
> + */
> +bool nmi_trigger(Error **errp);
>
>  #endif /* NMI_H */
> diff --git a/hw/core/nmi.c b/hw/core/nmi.c
> index ff6454437c1..662f2051123 100644
> --- a/hw/core/nmi.c
> +++ b/hw/core/nmi.c
> @@ -22,11 +22,8 @@
>  #include "qemu/osdep.h"
>  #include "hw/core/nmi.h"
>  #include "qapi/error.h"
> -#include "qemu/module.h"
> -#include "monitor/monitor.h"
>
>  struct do_nmi_s {
> -    int cpu_index;
>      Error *err;
>      bool handled;
>  };
> @@ -54,19 +51,21 @@ static int nmi_children(Object *o, struct do_nmi_s *ns)
>      return object_child_foreach_recursive(o, do_nmi, ns);
>  }
>
> -void nmi_monitor_handle(int cpu_index, Error **errp)
> +bool nmi_trigger(Error **errp)
>  {
>      struct do_nmi_s ns = {
> -        .cpu_index = cpu_index,
>          .err = NULL,
>          .handled = false
>      };
>
>      if (nmi_children(object_get_root(), &ns)) {
>          error_propagate(errp, ns.err);
> +        return false;
>      } else if (!ns.handled) {
>          error_setg(errp, "machine does not provide NMIs");
> +        return false;
>      }
> +    return true;
>  }
>
>  static const TypeInfo nmi_info = {
> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
> index 74818ff3cea..444af726d75 100644
> --- a/hw/ipmi/ipmi.c
> +++ b/hw/ipmi/ipmi.c
> @@ -59,8 +59,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
>          if (checkonly) {
>              return 0;
>          }
> -        /* We don't care what CPU we use. */
> -        nmi_monitor_handle(0, NULL);
> +        nmi_trigger(NULL);
>          return 0;
>
>      case IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP:
> diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
> index 0842fe373ae..026768e9fb2 100644
> --- a/hw/watchdog/watchdog.c
> +++ b/hw/watchdog/watchdog.c
> @@ -81,7 +81,7 @@ void watchdog_perform_action(void)
>
>      case WATCHDOG_ACTION_INJECT_NMI:
>          qapi_event_send_watchdog(WATCHDOG_ACTION_INJECT_NMI);
> -        nmi_monitor_handle(0, NULL);
> +        nmi_trigger(NULL);
>          break;
>
>      default:
> diff --git a/system/cpus.c b/system/cpus.c
> index 97e5a5edee2..5c0a192c01e 100644
> --- a/system/cpus.c
> +++ b/system/cpus.c
> @@ -926,6 +926,6 @@ exit:
>
>  void qmp_inject_nmi(Error **errp)
>  {
> -    nmi_monitor_handle(monitor_get_cpu_index(monitor_cur()), errp);
> +    nmi_trigger(errp);
>  }

This officially drops the current CPU for HMP command too. Just
acknowledge it in the commit message, and remove the "*cpu*"
documentation in hmp-commands.hx?

Also, can we use one word for "deliver", "trigger", "inject"? or is
there any distinction I am missing?

Re: [PATCH v3 3/7] hw/nmi: Remove @cpu_index argument from NMIClass::nmi_monitor_handler()

[Peter Maydell]

On Tue, 11 Aug 2026 at 11:56, Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
>
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Only s390x was using the 'cpu_index' argument, but since the
> previous commit it isn't anymore (it use the first cpu).
> Since this argument is now completely unused, remove it.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH v3 6/7] hw/nmi: Remove unused @errp argument from deliver_nmi()

[Peter Maydell]

On Tue, 11 Aug 2026 at 11:56, Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
>
> Not a single handler update @errp. The single user is
> nmi_trigger() filling with "machine does not provide NMIs".
> Remove the unused argument from the deliver_nmi() callback,
> simplifying the methods in hw/core/nmi.c.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH v3 7/7] hw/nmi: Deliver NMI only once

[Peter Maydell]

On Tue, 11 Aug 2026 at 11:55, Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
>
> Use the first handler available to deliver NMI only once.
> If anothers handlers are available, no need to keep delivering.
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
>  hw/core/nmi.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/core/nmi.c b/hw/core/nmi.c
> index 8eea75ad530..4fe5ee75bfc 100644
> --- a/hw/core/nmi.c
> +++ b/hw/core/nmi.c
> @@ -31,6 +31,8 @@ static int do_nmi(Object *o, void *opaque)
>      if (n) {
>          *handled = true;
>          NMI_GET_CLASS(n)->deliver_nmi(n);
> +        /* We only need to deliver NMI once */
> +        return 1;
>      }
>
>      return 0;

Looking back at my review comments on v1 of this series,
I think we could clarify the comment / commit message a bit.
Commit message:

We only expect one device in the system to implement the
TYPE_NMI interface (typically the machine, but in a few cases
for e.g. m68k and ppc this is an interrupt controller or
similar device); so we don't need to keep walking the whole
QOM tree once we've found it. As no machine type creates more
than one object implementing TYPE_NMI, this is not a behaviour
change.

and comment:

 /*
  * We expect only one object to implement TYPE_NMI, so once
  * we've asked it to deliver the NMI we can stop looking.
  */

With that

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH v3 4/7] hw/nmi: Remove @cpu_index argument from nmi_trigger()

[Philippe Mathieu-Daudé]

On 11/8/26 13:10, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Aug 11, 2026 at 2:55 PM Philippe Mathieu-Daudé
> <philmd@oss.qualcomm.com> wrote:
>>
>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> nmi_monitor_handle() is not related to the monitor, rename
>> it as nmi_trigger().
>>
>> Return a boolean value indicating success / failure as
>> recommended by the Error API since commit e3fe3988d7
>> ("error: Document Error API usage rules").
>>
>> The 'cpu_index' argument is not used, remove it.
>>
>> Document nmi_trigger() as suggested by Peter Maydell in
>> https://lore.kernel.org/qemu-devel/CAFEAcA-yALySmCJLbitCmYpiZKUXJNOavGJG9RYeo8fKqz7gcw@mail.gmail.com/.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
>> ---
>>   include/hw/core/nmi.h  | 25 ++++++++++++++++++++++++-
>>   hw/core/nmi.c          |  9 ++++-----
>>   hw/ipmi/ipmi.c         |  3 +--
>>   hw/watchdog/watchdog.c |  2 +-
>>   system/cpus.c          |  2 +-
>>   5 files changed, 31 insertions(+), 10 deletions(-)


>> -void nmi_monitor_handle(int cpu_index, Error **errp);
>> +/**
>> + * nmi_trigger: Trigger an NMI, in a machine-specific way
>> + * @errp: pointer to error object
>> + *
>> + * This function triggers an NMI, in a machine-specific way. The
>> + * intention is that this should typically trigger a guest kernel
>> + * dump or reboot, and might happen as a result of user request
>> + * from the monitor, watchdog timeouts, and similar events.
>> + * (For example on the x86 PC it triggers an NMI on all CPUs,
>> + * and on s390 it triggers the RESTART interrupt on the first CPU.)
>> + *
>> + * The NMI is triggered by looking for QOM objects which
>> + * implement the TYPE_NMI interface, and calling their nmi_handler
>> + * method. Usually it is the machine model class that implements
>> + * this interface.
>> + *
>> + * Not all machines implement NMI handling; this function
>> + * will return an error if used on a machine which does not
>> + * implement NMIs.
>> + *
>> + * On success, return %true.
>> + * On failure, store an error through @errp and return %false.
>> + */
>> +bool nmi_trigger(Error **errp);


>> -void nmi_monitor_handle(int cpu_index, Error **errp)
>> +bool nmi_trigger(Error **errp)
>>   {
>>       struct do_nmi_s ns = {
>> -        .cpu_index = cpu_index,
>>           .err = NULL,
>>           .handled = false
>>       };
>>
>>       if (nmi_children(object_get_root(), &ns)) {
>>           error_propagate(errp, ns.err);
>> +        return false;
>>       } else if (!ns.handled) {
>>           error_setg(errp, "machine does not provide NMIs");
>> +        return false;
>>       }
>> +    return true;
>>   }
>>
>>   static const TypeInfo nmi_info = {
>> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
>> index 74818ff3cea..444af726d75 100644
>> --- a/hw/ipmi/ipmi.c
>> +++ b/hw/ipmi/ipmi.c
>> @@ -59,8 +59,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
>>           if (checkonly) {
>>               return 0;
>>           }
>> -        /* We don't care what CPU we use. */
>> -        nmi_monitor_handle(0, NULL);
>> +        nmi_trigger(NULL);
>>           return 0;
>>
>>       case IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP:
>> diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
>> index 0842fe373ae..026768e9fb2 100644
>> --- a/hw/watchdog/watchdog.c
>> +++ b/hw/watchdog/watchdog.c
>> @@ -81,7 +81,7 @@ void watchdog_perform_action(void)
>>
>>       case WATCHDOG_ACTION_INJECT_NMI:
>>           qapi_event_send_watchdog(WATCHDOG_ACTION_INJECT_NMI);
>> -        nmi_monitor_handle(0, NULL);
>> +        nmi_trigger(NULL);
>>           break;
>>
>>       default:
>> diff --git a/system/cpus.c b/system/cpus.c
>> index 97e5a5edee2..5c0a192c01e 100644
>> --- a/system/cpus.c
>> +++ b/system/cpus.c
>> @@ -926,6 +926,6 @@ exit:
>>
>>   void qmp_inject_nmi(Error **errp)
>>   {
>> -    nmi_monitor_handle(monitor_get_cpu_index(monitor_cur()), errp);
>> +    nmi_trigger(errp);
>>   }
> 
> This officially drops the current CPU for HMP command too. Just
> acknowledge it in the commit message, and remove the "*cpu*"
> documentation in hmp-commands.hx?

Ack.

> 
> Also, can we use one word for "deliver", "trigger", "inject"? or is
> there any distinction I am missing?

Hmm indeed from the monitor perspective we "inject" a NMI.

Watchdog seems to also "inject" with WATCHDOG_ACTION_INJECT_NMI
(here my understanding is "raise", but whatever).

I suppose IPMI devices also "raise", but "inject" is acceptable.

$ git grep deliver_nmi
hw/i386/x86.c:159:            apic_deliver_nmi(cpu->apic_state);
hw/intc/apic_common.c:122:void apic_deliver_nmi(APICCommonState *s)
include/hw/i386/apic.h:10:void apic_deliver_nmi(APICCommonState *s);

The NMIClass handler then "route" or "deliver" it.


I'll rename as 'nmi_inject' I suppose, better to keep in pair with
the QMP command name.

Re: [PATCH v3 7/7] hw/nmi: Deliver NMI only once

[Philippe Mathieu-Daudé]

On 11/8/26 15:34, Peter Maydell wrote:
> On Tue, 11 Aug 2026 at 11:55, Philippe Mathieu-Daudé
> <philmd@oss.qualcomm.com> wrote:
>>
>> Use the first handler available to deliver NMI only once.
>> If anothers handlers are available, no need to keep delivering.
>>
>> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
>> ---
>>   hw/core/nmi.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/core/nmi.c b/hw/core/nmi.c
>> index 8eea75ad530..4fe5ee75bfc 100644
>> --- a/hw/core/nmi.c
>> +++ b/hw/core/nmi.c
>> @@ -31,6 +31,8 @@ static int do_nmi(Object *o, void *opaque)
>>       if (n) {
>>           *handled = true;
>>           NMI_GET_CLASS(n)->deliver_nmi(n);
>> +        /* We only need to deliver NMI once */
>> +        return 1;
>>       }
>>
>>       return 0;
> 
> Looking back at my review comments on v1 of this series,
> I think we could clarify the comment / commit message a bit.
> Commit message:
> 
> We only expect one device in the system to implement the
> TYPE_NMI interface (typically the machine, but in a few cases
> for e.g. m68k and ppc this is an interrupt controller or
> similar device); so we don't need to keep walking the whole
> QOM tree once we've found it. As no machine type creates more
> than one object implementing TYPE_NMI, this is not a behaviour
> change.
> 
> and comment:
> 
>   /*
>    * We expect only one object to implement TYPE_NMI, so once
>    * we've asked it to deliver the NMI we can stop looking.
>    */
> 
> With that
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Thanks for the wording help!