在 2017/9/19 21:04, Eric Blake 写道:
On 09/19/2017 02:43 AM, Christian Borntraeger wrote:
From: Jing Liu <liuj...@linux.vnet.ibm.com>
This patch is the s390 implementation of guest crash information, similar
to commit d187e08dc4 ("i386/cpu: add crash-information QOM property") and
the related commits. We will detect several crash reasons, with the
"disabled wait" being the most important one, since this is used by all
s390 guests as a "panic like" notification.
Demonstrate the these ways with examples as follows.
Signed-off-by: Jing Liu <liuj...@linux.vnet.ibm.com>
Reviewed-by: Christian Borntraeger <borntrae...@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntrae...@de.ibm.com>
[minor fixes due to upstream feedback]
---
V1->V2:
- rename kvm-s390 to s390 in all places
- add "loop" to the crash reasons where appropriate
- use "-" instead of "_" for qapi
qapi/run-state.json | 19 ++++++++++++++++--
target/s390x/cpu.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++
target/s390x/cpu.h | 6 ++++++
target/s390x/kvm.c | 29 +++++++++++++++++++++------
vl.c | 6 ++++++
5 files changed, 109 insertions(+), 8 deletions(-)
diff --git a/qapi/run-state.json b/qapi/run-state.json
index d36ff49..4567510 100644
--- a/qapi/run-state.json
+
+##
+# @GuestPanicInformationS390:
+#
+# S390 specific guest panic information (PSW)
+#
+# Since: 2.11
+##
+{'struct': 'GuestPanicInformationS390',
+ 'data': { 'psw-mask': 'uint64',
+ 'psw-addr': 'uint64',
+ 'reason': 'str' } }
Missing documentation of the three fields; in particular, whether
I didn't get your point, do you mean we need to add comments
for the three fields? But I don't see the comments for Hyper-V either.
Thanks
'reason' is for human consumption only (presumably the case) rather than
for machine parsing.
yes, the 'reason' is for human understanding and also investigation but
psw-*
could be used to parse later.
+ cpu_synchronize_state(cs);
+ panic_info = g_malloc0(sizeof(GuestPanicInformation));
+
+ panic_info->type = GUEST_PANIC_INFORMATION_TYPE_S390;
+ panic_info->u.s390.psw_mask = cpu->env.psw.mask;
+ panic_info->u.s390.psw_addr = cpu->env.psw.addr;
+
+ switch (cs->exception_index) {
+ case EXCP_CRASH_PGM:
+ panic_info->u.s390.reason = g_strdup("program interrupt loop");
+ break;
+ case EXCP_CRASH_EXT:
+ panic_info->u.s390.reason = g_strdup("external interrupt loop");
+ break;
+ case EXCP_CRASH_WAITPSW:
+ panic_info->u.s390.reason = g_strdup("disabled wait");
+ break;
+ case EXCP_CRASH_OPEREXC:
+ panic_info->u.s390.reason = g_strdup("operation exception loop");
+ break;
+ default:
+ panic_info->u.s390.reason = g_strdup("unknown crash reason");
+ break;
Is it worth a QAPI enum type to expose the reason as one of a finite set
of known strings, or is that information not needed beyond the
human-only string that you are setting here?
Actually we had considered to use enum type for 'reason' instead of str,
the awkwardness
is that then qemu_system_guest_panicked has to parse the 'reason' before
it prints it, but
qemu_system_guest_panicked is a common function and we don't want to do
more arch
related handling there.
Thanks!
--
Regards
QingFeng Hao
