On 20/3/24 14:34, Peter Maydell wrote:
On Tue, 20 Feb 2024 at 15:09, Philippe Mathieu-Daudé <[email protected]> wrote:
nmi_monitor_handle() is not related to the monitor,
rename it as nmi_trigger().
Return boolean value
indicating success / failure. The 'cpu_index' argument
is not used, remove it.
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
---
include/hw/nmi.h | 13 ++++++++++++-
hw/core/nmi.c | 9 ++++-----
hw/ipmi/ipmi.c | 3 +--
hw/watchdog/watchdog.c | 2 +-
system/cpus.c | 2 +-
5 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/include/hw/nmi.h b/include/hw/nmi.h
index c70db941c9..32b27067f2 100644
--- a/include/hw/nmi.h
+++ b/include/hw/nmi.h
@@ -49,6 +49,17 @@ struct NMIClass {
bool (*nmi_handler)(NMIState *n, Error **errp);
};
-void nmi_monitor_handle(int cpu_index, Error **errp);
+/**
+ * nmi_trigger: Trigger a NMI.
+ *
+ * @errp: pointer to error object
+ *
+ * Iterate over all objects implementing the TYPE_NMI interface
+ * and deliver NMI to them.
I think I would document this something like;
* nmi_trigger: Trigger an NMI, in a machine-specific way
*
* 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.
Thanks!
(In an ideal world we would also document per-board what
the NMI handling is, in the user-facing board docs...)
+ *
+ * On success, return %true.
+ * On failure, store an error through @errp and return %false.
+ */
+bool nmi_trigger(Error **errp);
Why return a bool here? None of the callsites looks at the
return value.
Indeed, but again this is the style *recommended* by the Error API
since commit e3fe3988d7 ("error: Document Error API usage rules").
Callers providing a non-NULL errp should check the return value,
but the QMP handlers generated by gen_command_decl() like
qmp_inject_nmi() here don't return anything.
Markus, what would be your preference?
