| Message ID | 20240220150833.13674-5-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/nmi: Remove @cpu_index argument | expand |
On Tue, 20 Feb 2024 at 15:09, Philippe Mathieu-Daudé <philmd@linaro.org> 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é <philmd@linaro.org> > --- > 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. (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. thanks -- PMM
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. + * + * 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 409164d445..a740f39c98 100644 --- a/hw/core/nmi.c +++ b/hw/core/nmi.c @@ -22,11 +22,8 @@ #include "qemu/osdep.h" #include "hw/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; }; @@ -53,19 +50,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 bbb07b151e..45e36a7492 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 955046161b..d324c761aa 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 68d161d96b..f11ee3d404 100644 --- a/system/cpus.c +++ b/system/cpus.c @@ -856,6 +856,6 @@ exit: void qmp_inject_nmi(Error **errp) { - nmi_monitor_handle(monitor_get_cpu_index(monitor_cur()), errp); + nmi_trigger(errp); }
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é <philmd@linaro.org> --- 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(-)