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 <a...@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. -- Alexey