On 02/24/2011 02:33 AM, Markus Armbruster wrote:
Anthony Liguori<anthony@xxxxxxxxxxxxx> writes:
On 01/27/2011 02:20 AM, Lai Jiangshan wrote:
Make we can inject NMI via qemu-monitor-protocol.
We use "inject-nmi" for the qmp command name, the meaning is clearer.
Signed-off-by: Lai Jiangshan<laijs@xxxxxxxxxxxxxx>
---
diff --git a/hmp-commands.hx b/hmp-commands.hx
index ec1a4db..e763bf9 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -725,7 +725,8 @@ ETEXI
.params = "[cpu]",
.help = "Inject an NMI on all CPUs if no argument is given, "
"otherwise inject it on the specified CPU",
- .mhandler.cmd = do_inject_nmi,
+ .user_print = monitor_user_noop,
+ .mhandler.cmd_new = do_inject_nmi,
},
#endif
STEXI
diff --git a/monitor.c b/monitor.c
index 387b020..1b1c0ba 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2542,7 +2542,7 @@ static void do_wav_capture(Monitor *mon, const QDict *qdict)
#endif
#if defined(TARGET_I386)
-static void do_inject_nmi(Monitor *mon, const QDict *qdict)
+static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
CPUState *env;
int cpu_index;
@@ -2550,7 +2550,7 @@ static void do_inject_nmi(Monitor *mon, const QDict *qdict)
if (!qdict_haskey(qdict, "cpu-index")) {
for (env = first_cpu; env != NULL; env = env->next_cpu)
cpu_interrupt(env, CPU_INTERRUPT_NMI);
- return;
+ return 0;
}
cpu_index = qdict_get_int(qdict, "cpu-index");
@@ -2560,8 +2560,10 @@ static void do_inject_nmi(Monitor *mon, const QDict *qdict)
kvm_inject_interrupt(env, CPU_INTERRUPT_NMI);
else
cpu_interrupt(env, CPU_INTERRUPT_NMI);
- break;
+ return 0;
}
+
+ return -1;
}
#endif
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 56c4d8b..a887dd5 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -429,6 +429,34 @@ Example:
EQMP
+#if defined(TARGET_I386)
+ {
+ .name = "inject-nmi",
+ .args_type = "cpu-index:i?",
+ .params = "[cpu]",
+ .help = "Inject an NMI on all CPUs if no argument is given, "
+ "otherwise inject it on the specified CPU",
+ .user_print = monitor_user_noop,
+ .mhandler.cmd_new = do_inject_nmi,
+ },
+#endif
+SQMP
+inject-nmi
+----------
+
+Inject an NMI on all CPUs or the given CPU (x86 only).
+
+Arguments:
+
+- "cpu-index": the index of the CPU to be injected NMI (json-int, optional)
+
+Example:
+
+-> { "execute": "inject-nmi", "arguments": { "cpu-index": 0 } }
+<- { "return": {} }
Please describe all expected errors.
Quoting qmp-commands.hx:
3. Errors, in special, are not documented. Applications should NOT check
for specific errors classes or data (it's strongly recommended to only
check for the "error" key)
Indeed, not a single error is documented there. This is intentional.
Yeah, but we're not 0.14 anymore and for 0.15, we need to document
errors. If you are suggesting I send a patch to remove that section,
I'm more than happy to.
Once we have an error design in place that has a reasonable hope to
stand the test of time, and have errors documented for at least some of
the commands here, we can start to require proper error documentation
for new commands. But not now.
I'm quite happy with the error design we have today. The only problem
is that we don't propagate errors in a sane way but I've got that all
but fixed in my qapi tree.
Don't hide this command for
!defined(TARGET_I386), instead have it throw an error in the
implementation.
Works for me.
Don't have commands that multiple behavior based on the presence or
absence of arguments. Make it take a list of cpus if you want the
ability to inject the NMI to more than one CPU.
Having optional arguments is fine. It's good taste to give them
"default semantics", i.e. "no argument" is shorthand for one specific
argument value.
Luiz already pointed to the thread where we discussed this command
before. Executive summary:
* Real hardware's NMI button injects all CPUs. This is the primary use
case.
* Lai said injecting a single CPU can be useful for debugging. Was
deemed acceptable as secondary use case.
Lai also pointed out that the human monitor's nmi command injects a
single CPU. That was dismissed as irrelevant for QMP.
* No other use cases have been presented.
Therefore, the "list of CPUs" idea was shot down as overly general.
That's fine, then we should do two commands. Think of it from the
perspective of the client. This appears as:
in C:
qmp_inject_nmi(sess, false, 0, &err);
in Python:
sess.inject_nmi()
The first example doesn't tell you at all what's happening. The second
API does look really nice until you see the following later:
sess.inject_nmi(0)
What's the difference between these two functions? You might say this
is bad form and that an explicit named argument should be given but I
wouldn't count on it.
Having two commands, nmi_inject and nmi_inject_on_cpu, would result in a
much more readable API in both C and Python:
in C:
qmp_nmi_inject(sess, &err);
qmp_nmi_inject_on_cpu(sess, 3, &err);
in Python:
sess.nmi_inject()
sess.nmi_inject_on_cpu(3)
Regards,
Anthony Liguori
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
