[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2] s390x/cpu: expose the guest crash informatio

From: QingFeng Hao
Subject: Re: [Qemu-devel] [PATCH v2] s390x/cpu: expose the guest crash information
Date: Tue, 7 Nov 2017 19:00:33 +0800
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

在 2017/9/19 21:04, Eric Blake 写道:
On 09/19/2017 02:43 AM, Christian Borntraeger wrote:
From: Jing Liu <address@hidden>

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 <address@hidden>
Reviewed-by: Christian Borntraeger <address@hidden>
Signed-off-by: Christian Borntraeger <address@hidden>
[minor fixes due to upstream feedback]
        - 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.
'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;
+        panic_info->u.s390.reason = g_strdup("disabled wait");
+        break;
+        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.

QingFeng Hao

reply via email to

[Prev in Thread] Current Thread [Next in Thread]