| Message ID | 1402506183-29736-2-git-send-email-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
On 06/11/2014 11:03 AM, Alexey Kardashevskiy wrote: > This introduces an NMI (Non Maskable Interrupt) interface with > a single nmi_monitor_handler() method. A machine or a device can > implement it. This searches for an QOM object which supports the interface > and if it is implemented , calls it. The callback implements an action > required to cause debug crash dump on in-kernel debugger invocation. > The callback returns Error**. > > This adds support for it in qmp_inject_nmi(). Since no architecture > supports it at the moment, there is no change in behaviour. > > This changes inject-nmi command description for HMP and QMP. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > +++ b/hw/core/nmi.c > @@ -0,0 +1,81 @@ > +/* > + * NMI monitor handler class and helpers. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, > + * or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, see <http://www.gnu.org/licenses/>. > + */ For the GPL license to work, there has to be a Copyright line. > +static int do_nmi(Object *o, void *opaque) > +{ > + struct do_nmi_s *ns = opaque; > + NMI *n = (NMI *) object_dynamic_cast(o, TYPE_NMI); Is the cast to (NMI *) necessary, or does object_dynamic_cast() already return something that can be assigned to an arbitrary pointer type? > +++ b/include/hw/nmi.h > @@ -0,0 +1,46 @@ > +/* > + * NMI monitor handler class and helpers definitions. Same comment about Copyright. > +#define TYPE_NMI "nmi" > + > +#define NMI_CLASS(klass) \ > + OBJECT_CLASS_CHECK(NMIClass, (klass), TYPE_NMI) > +#define NMI_GET_CLASS(obj) \ > + OBJECT_GET_CLASS(NMIClass, (obj), TYPE_NMI) > +#define NMI(obj) \ > + INTERFACE_CHECK(NMI, (obj), TYPE_NMI) > + > +typedef struct NMI { > + Object parent_obj; > +} NMI; Eww. This is declaring both 'NMI' as a type, and 'NMI()' as a macro. Is that going to trip people up? Is 'NMIState' a better name? > +++ b/qapi-schema.json > @@ -1116,13 +1116,13 @@ > ## > # @inject-nmi: > # > -# Injects an Non-Maskable Interrupt into all guest's VCPUs. > +# Injects an Non-Maskable Interrupt into the default CPU (x86/s390) or all CPUs (ppc64). s/an Non/a Non/ > +++ b/qmp-commands.hx > @@ -477,7 +477,7 @@ SQMP > inject-nmi > ---------- > > -Inject an NMI on guest's CPUs. > +Inject an NMI on the default CPU (x86/s390) or all CPUs (ppc64). Oh, I see the problem. The acronym 'NMI' is pronounced with a leading vowel (an "en-em-eye"), but the expansion is pronounced with a consonant (a non-maskable interrupt), so the correct article depends on whether you are spelling things out.
On 06/12/2014 03:21 AM, Eric Blake wrote: > On 06/11/2014 11:03 AM, Alexey Kardashevskiy wrote: >> This introduces an NMI (Non Maskable Interrupt) interface with >> a single nmi_monitor_handler() method. A machine or a device can >> implement it. This searches for an QOM object which supports the interface >> and if it is implemented , calls it. The callback implements an action >> required to cause debug crash dump on in-kernel debugger invocation. >> The callback returns Error**. >> >> This adds support for it in qmp_inject_nmi(). Since no architecture >> supports it at the moment, there is no change in behaviour. >> >> This changes inject-nmi command description for HMP and QMP. >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- > >> +++ b/hw/core/nmi.c >> @@ -0,0 +1,81 @@ >> +/* >> + * NMI monitor handler class and helpers. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, >> + * or (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, see <http://www.gnu.org/licenses/>. >> + */ > > For the GPL license to work, there has to be a Copyright line. Hm. Ok. include/hw/fw-path-provider.h (where I copied this from) does not have one and it was ok. > >> +static int do_nmi(Object *o, void *opaque) >> +{ >> + struct do_nmi_s *ns = opaque; >> + NMI *n = (NMI *) object_dynamic_cast(o, TYPE_NMI); > > Is the cast to (NMI *) necessary, or does object_dynamic_cast() already > return something that can be assigned to an arbitrary pointer type? All the macros from include/qom/object.h do this cast and so do I. object_dynamic_cast returns Object*. >> +++ b/include/hw/nmi.h >> @@ -0,0 +1,46 @@ >> +/* >> + * NMI monitor handler class and helpers definitions. > > Same comment about Copyright. > > >> +#define TYPE_NMI "nmi" >> + >> +#define NMI_CLASS(klass) \ >> + OBJECT_CLASS_CHECK(NMIClass, (klass), TYPE_NMI) >> +#define NMI_GET_CLASS(obj) \ >> + OBJECT_GET_CLASS(NMIClass, (obj), TYPE_NMI) >> +#define NMI(obj) \ >> + INTERFACE_CHECK(NMI, (obj), TYPE_NMI) >> + >> +typedef struct NMI { >> + Object parent_obj; >> +} NMI; > > Eww. This is declaring both 'NMI' as a type, and 'NMI()' as a macro. > Is that going to trip people up? Is 'NMIState' a better name? My bad, I'll fix. I'll probably get rid of the struct. > > >> +++ b/qapi-schema.json >> @@ -1116,13 +1116,13 @@ >> ## >> # @inject-nmi: >> # >> -# Injects an Non-Maskable Interrupt into all guest's VCPUs. >> +# Injects an Non-Maskable Interrupt into the default CPU (x86/s390) or all CPUs (ppc64). > > s/an Non/a Non/ Yep, here and in other places I read it as "en-em-eye" but it is not correct right here :) I'll fix, thanks. >> +++ b/qmp-commands.hx >> @@ -477,7 +477,7 @@ SQMP >> inject-nmi >> ---------- >> >> -Inject an NMI on guest's CPUs. >> +Inject an NMI on the default CPU (x86/s390) or all CPUs (ppc64). > > Oh, I see the problem. The acronym 'NMI' is pronounced with a leading > vowel (an "en-em-eye"), but the expansion is pronounced with a consonant > (a non-maskable interrupt), so the correct article depends on whether > you are spelling things out.
On 06/11/2014 06:10 PM, Alexey Kardashevskiy wrote: >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License as published by >>> + * the Free Software Foundation; either version 2 of the License, >>> + * or (at your option) any later version. >> >> For the GPL license to work, there has to be a Copyright line. > > Hm. Ok. include/hw/fw-path-provider.h (where I copied this from) does not > have one and it was ok. Just because there's existing bad practice in the code base doesn't mean new files should perpetuate it :) > >> >>> +static int do_nmi(Object *o, void *opaque) >>> +{ >>> + struct do_nmi_s *ns = opaque; >>> + NMI *n = (NMI *) object_dynamic_cast(o, TYPE_NMI); >> >> Is the cast to (NMI *) necessary, or does object_dynamic_cast() already >> return something that can be assigned to an arbitrary pointer type? > > > All the macros from include/qom/object.h do this cast and so do I. > > object_dynamic_cast returns Object*. Okay, then the cast is needed. (I wonder if object_dynamic_cast() should return void* to avoid the need for casting, but that probably invites more errors than what it saves from the eliminated casts).
diff --git a/cpus.c b/cpus.c index dd7ac13..da1f6a9 100644 --- a/cpus.c +++ b/cpus.c @@ -38,6 +38,7 @@ #include "qemu/main-loop.h" #include "qemu/bitmap.h" #include "qemu/seqlock.h" +#include "hw/nmi.h" #ifndef _WIN32 #include "qemu/compatfd.h" @@ -1495,6 +1496,6 @@ void qmp_inject_nmi(Error **errp) } } #else - error_set(errp, QERR_UNSUPPORTED); + nmi(monitor_get_cpu_index(), errp); #endif } diff --git a/hmp-commands.hx b/hmp-commands.hx index 2e462c0..c25f88f 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -830,19 +830,17 @@ The values that can be specified here depend on the machine type, but are the same that can be specified in the @code{-boot} command line option. ETEXI -#if defined(TARGET_I386) || defined(TARGET_S390X) { .name = "nmi", .args_type = "", .params = "", - .help = "inject an NMI on all guest's CPUs", + .help = "inject an NMI", .mhandler.cmd = hmp_inject_nmi, }, -#endif STEXI @item nmi @var{cpu} @findex nmi -Inject an NMI (x86) or RESTART (s390x) on the given CPU. +Inject an NMI on the default CPU (x86/s390) or all CPUs (ppc64). ETEXI diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs index 5377d05..17845df 100644 --- a/hw/core/Makefile.objs +++ b/hw/core/Makefile.objs @@ -4,6 +4,7 @@ common-obj-y += fw-path-provider.o # irq.o needed for qdev GPIO handling: common-obj-y += irq.o common-obj-y += hotplug.o +common-obj-y += nmi.o common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o common-obj-$(CONFIG_XILINX_AXI) += stream.o diff --git a/hw/core/nmi.c b/hw/core/nmi.c new file mode 100644 index 0000000..e81ebab --- /dev/null +++ b/hw/core/nmi.c @@ -0,0 +1,81 @@ +/* + * NMI monitor handler class and helpers. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, + * or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see <http://www.gnu.org/licenses/>. + */ + +#include "hw/nmi.h" +#include "qapi/qmp/qerror.h" + +struct do_nmi_s { + int cpu_index; + Error *errp; + 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; + NMI *n = (NMI *) object_dynamic_cast(o, TYPE_NMI); + + if (n) { + NMIClass *nc = NMI_GET_CLASS(n); + + ns->handled = true; + nc->nmi_monitor_handler(n, ns->cpu_index, &ns->errp); + if (ns->errp) { + return -1; + } + } + nmi_children(o, ns); + + return 0; +} + +void nmi_children(Object *o, struct do_nmi_s *ns) +{ + object_child_foreach(o, do_nmi, ns); +} + +void nmi(int cpu_index, Error **errp) +{ + Object *root = object_get_root(); + struct do_nmi_s ns = { + .cpu_index = cpu_index, + .errp = NULL, + .handled = false + }; + + nmi_children(root, &ns); + if (ns.handled) { + error_propagate(errp, ns.errp); + } else { + error_set(errp, QERR_UNSUPPORTED); + } +} + +static const TypeInfo nmi_info = { + .name = TYPE_NMI, + .parent = TYPE_INTERFACE, + .class_size = sizeof(NMIClass), +}; + +static void nmi_register_types(void) +{ + type_register_static(&nmi_info); +} + +type_init(nmi_register_types) diff --git a/include/hw/nmi.h b/include/hw/nmi.h new file mode 100644 index 0000000..c2d8206 --- /dev/null +++ b/include/hw/nmi.h @@ -0,0 +1,46 @@ +/* + * NMI monitor handler class and helpers definitions. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, + * or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see <http://www.gnu.org/licenses/>. + */ + +#ifndef NMI_H +#define NMI_H 1 + +#include "qemu-common.h" +#include "qom/object.h" + +#define TYPE_NMI "nmi" + +#define NMI_CLASS(klass) \ + OBJECT_CLASS_CHECK(NMIClass, (klass), TYPE_NMI) +#define NMI_GET_CLASS(obj) \ + OBJECT_GET_CLASS(NMIClass, (obj), TYPE_NMI) +#define NMI(obj) \ + INTERFACE_CHECK(NMI, (obj), TYPE_NMI) + +typedef struct NMI { + Object parent_obj; +} NMI; + +typedef struct NMIClass { + InterfaceClass parent_class; + + void (*nmi_monitor_handler)(NMI *n, int cpu_index, Error **errp); +} NMIClass; + +void nmi(int cpu_index, Error **errp); + +#endif /* NMI_H */ + diff --git a/qapi-schema.json b/qapi-schema.json index 14b498b..8bf9849 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1116,13 +1116,13 @@ ## # @inject-nmi: # -# Injects an Non-Maskable Interrupt into all guest's VCPUs. +# Injects an Non-Maskable Interrupt into the default CPU (x86/s390) or all CPUs (ppc64). # # Returns: If successful, nothing # # Since: 0.14.0 # -# Notes: Only x86 Virtual Machines support this command. +# Note: prior to 2.1, this command was only supported for x86 and s390 VMs ## { 'command': 'inject-nmi' } diff --git a/qmp-commands.hx b/qmp-commands.hx index d8aa4ed..0028d96 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -477,7 +477,7 @@ SQMP inject-nmi ---------- -Inject an NMI on guest's CPUs. +Inject an NMI on the default CPU (x86/s390) or all CPUs (ppc64). Arguments: None. @@ -487,7 +487,6 @@ Example: <- { "return": {} } Note: inject-nmi fails when the guest doesn't support injecting. - Currently, only x86 (NMI) and s390x (RESTART) guests do. EQMP
This introduces an NMI (Non Maskable Interrupt) interface with a single nmi_monitor_handler() method. A machine or a device can implement it. This searches for an QOM object which supports the interface and if it is implemented , calls it. The callback implements an action required to cause debug crash dump on in-kernel debugger invocation. The callback returns Error**. This adds support for it in qmp_inject_nmi(). Since no architecture supports it at the moment, there is no change in behaviour. This changes inject-nmi command description for HMP and QMP. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- Changes: v6: * NMI is an interface again v5: * s/given guest's (CPU|VCPU)/default CPU/ * nmi_monitor_handler() now returns Error** v4: * s/\<nmi\>/nmi_monitor_handler/ v3: * actual nmi() enablement moved from last patch to first patch * changed description for QMP command too --- cpus.c | 3 +- hmp-commands.hx | 6 ++-- hw/core/Makefile.objs | 1 + hw/core/nmi.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/hw/nmi.h | 46 +++++++++++++++++++++++++++++ qapi-schema.json | 4 +-- qmp-commands.hx | 3 +- 7 files changed, 135 insertions(+), 9 deletions(-) create mode 100644 hw/core/nmi.c create mode 100644 include/hw/nmi.h