Re: [Qemu-devel] [PATCH v8] s390x/cpu: expose the guest crash information

2018-02-13 Thread Cornelia Huck
On Fri,  9 Feb 2018 12:25:43 +
Christian Borntraeger  wrote:

> 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.
(...)
> Co-authored-by: Jing Liu 
> Signed-off-by: Christian Borntraeger 
> ---
>  qapi/run-state.json   | 55 
> +--
>  target/s390x/cpu.c| 45 +
>  target/s390x/cpu.h|  2 ++
>  target/s390x/helper.c |  5 -
>  target/s390x/kvm.c| 15 +++---
>  vl.c  | 11 +--
>  6 files changed, 120 insertions(+), 13 deletions(-)

Thanks, applied.



Re: [Qemu-devel] [PATCH v8] s390x/cpu: expose the guest crash information

2018-02-12 Thread Cornelia Huck
On Mon, 12 Feb 2018 11:51:37 -0600
Eric Blake  wrote:

> On 02/09/2018 06:25 AM, Christian Borntraeger wrote:
> > 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.
> >   
> ...
> > 
> > Co-authored-by: Jing Liu 
> > Signed-off-by: Christian Borntraeger 
> > ---  
> 
> > +##
> > +# @GuestPanicInformationS390:
> > +#
> > +# S390 specific guest panic information (PSW)
> > +#
> > +# @core: core id of the CPU that crashed
> > +# @psw-mask: control fields of guest PSW
> > +# @psw-addr: guest instruction address
> > +# @reason: guest crash reason in human readable form  
> 
> This description is stale, now that it is an enum (and thus also 
> machine-readable).  I'd shorten it to just
> 
> # @reason: guest crash reason

Agreed, folded in.

> 
> > +#
> > +# Since: 2.12
> > +##
> > +{'struct': 'GuestPanicInformationS390',
> > + 'data': { 'core': 'uint32',
> > +   'psw-mask': 'uint64',
> > +   'psw-addr': 'uint64',
> > +   'reason': 'S390CrashReason' } }  
> 
> > +static GuestPanicInformation *s390_cpu_get_crash_info(CPUState *cs)
> > +{
> > +GuestPanicInformation *panic_info;
> > +S390CPU *cpu = S390_CPU(cs);
> > +
> > +cpu_synchronize_state(cs);
> > +panic_info = g_malloc0(sizeof(GuestPanicInformation));
> > +
> > +panic_info->type = GUEST_PANIC_INFORMATION_TYPE_S390;
> > +#if !defined(CONFIG_USER_ONLY)
> > +panic_info->u.s390.core = cpu->env.core_id;
> > +#else
> > +panic_info->u.s390.core = 0; /* sane default for non system emulation 
> > */  
> 
> Redundant assignment thanks to the g_malloc0() above, but the comment 
> makes it explicit why you did that, so I can live with it.

I actually prefer it this way, as it preempts questions about user-only
handling.

> 
> With the qapi comment tweak,
> Reviewed-by: Eric Blake 

Thanks!



Re: [Qemu-devel] [PATCH v8] s390x/cpu: expose the guest crash information

2018-02-12 Thread Eric Blake

On 02/09/2018 06:25 AM, Christian Borntraeger wrote:

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.


...


Co-authored-by: Jing Liu 
Signed-off-by: Christian Borntraeger 
---



+##
+# @GuestPanicInformationS390:
+#
+# S390 specific guest panic information (PSW)
+#
+# @core: core id of the CPU that crashed
+# @psw-mask: control fields of guest PSW
+# @psw-addr: guest instruction address
+# @reason: guest crash reason in human readable form


This description is stale, now that it is an enum (and thus also 
machine-readable).  I'd shorten it to just


# @reason: guest crash reason


+#
+# Since: 2.12
+##
+{'struct': 'GuestPanicInformationS390',
+ 'data': { 'core': 'uint32',
+   'psw-mask': 'uint64',
+   'psw-addr': 'uint64',
+   'reason': 'S390CrashReason' } }



+static GuestPanicInformation *s390_cpu_get_crash_info(CPUState *cs)
+{
+GuestPanicInformation *panic_info;
+S390CPU *cpu = S390_CPU(cs);
+
+cpu_synchronize_state(cs);
+panic_info = g_malloc0(sizeof(GuestPanicInformation));
+
+panic_info->type = GUEST_PANIC_INFORMATION_TYPE_S390;
+#if !defined(CONFIG_USER_ONLY)
+panic_info->u.s390.core = cpu->env.core_id;
+#else
+panic_info->u.s390.core = 0; /* sane default for non system emulation */


Redundant assignment thanks to the g_malloc0() above, but the comment 
makes it explicit why you did that, so I can live with it.


With the qapi comment tweak,
Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



Re: [Qemu-devel] [PATCH v8] s390x/cpu: expose the guest crash information

2018-02-12 Thread Cornelia Huck
On Fri,  9 Feb 2018 12:25:43 +
Christian Borntraeger  wrote:

> 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 these ways with examples as follows.
> 
>   1. crash-information QOM property;
> 
>   Run qemu with -qmp unix:qmp-sock,server, then use utility "qmp-shell"
>   to execute "qom-get" command, and might get the result like,
> 
>   (QEMU) (QEMU) qom-get path=/machine/unattached/device[0] \
>   property=crash-information
>   {"return": {"core": 0, "reason": "disabled-wait", "psw-mask": 
> 562956395872256, \
>   "type": "s390", "psw-addr": 1102832}}
> 
>   2. GUEST_PANICKED event reporting;
> 
>   Run qemu with a socket option, and telnet or nc to that,
>   -chardev socket,id=qmp,port=,host=localhost,server \
>   -mon chardev=qmp,mode=control,pretty=on \
>   Negotiating the mode by { "execute": "qmp_capabilities" }, and the crash
>   information will be reported on a guest crash event like,
> 
>   {
> "timestamp": {
> "seconds": 1518004739,
> "microseconds": 552563
> },
> "event": "GUEST_PANICKED",
> "data": {
> "action": "pause",
> "info": {
> "core": 0,
> "psw-addr": 1102832,
> "reason": "disabled-wait",
> "psw-mask": 562956395872256,
> "type": "s390"
> }
> }
>   }
> 
>   3. log;
> 
>   Run qemu with the parameters: -D  -d guest_errors, to
>   specify the logfile and log item. The results might be,
> 
>   Guest crashed on cpu 0: disabled-wait
>   PSW: 0x000200018000 0x0010d3f0
> 
> Co-authored-by: Jing Liu 
> Signed-off-by: Christian Borntraeger 
> ---
>  qapi/run-state.json   | 55 
> +--
>  target/s390x/cpu.c| 45 +
>  target/s390x/cpu.h|  2 ++
>  target/s390x/helper.c |  5 -
>  target/s390x/kvm.c| 15 +++---
>  vl.c  | 11 +--
>  6 files changed, 120 insertions(+), 13 deletions(-)

Looks good to me and I want to queue it; however, I'd still like an ack
or two, especially for the qapi part.



[Qemu-devel] [PATCH v8] s390x/cpu: expose the guest crash information

2018-02-09 Thread Christian Borntraeger
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 these ways with examples as follows.

  1. crash-information QOM property;

  Run qemu with -qmp unix:qmp-sock,server, then use utility "qmp-shell"
  to execute "qom-get" command, and might get the result like,

  (QEMU) (QEMU) qom-get path=/machine/unattached/device[0] \
  property=crash-information
  {"return": {"core": 0, "reason": "disabled-wait", "psw-mask": 
562956395872256, \
  "type": "s390", "psw-addr": 1102832}}

  2. GUEST_PANICKED event reporting;

  Run qemu with a socket option, and telnet or nc to that,
  -chardev socket,id=qmp,port=,host=localhost,server \
  -mon chardev=qmp,mode=control,pretty=on \
  Negotiating the mode by { "execute": "qmp_capabilities" }, and the crash
  information will be reported on a guest crash event like,

  {
"timestamp": {
"seconds": 1518004739,
"microseconds": 552563
},
"event": "GUEST_PANICKED",
"data": {
"action": "pause",
"info": {
"core": 0,
"psw-addr": 1102832,
"reason": "disabled-wait",
"psw-mask": 562956395872256,
"type": "s390"
}
}
  }

  3. log;

  Run qemu with the parameters: -D  -d guest_errors, to
  specify the logfile and log item. The results might be,

  Guest crashed on cpu 0: disabled-wait
  PSW: 0x000200018000 0x0010d3f0

Co-authored-by: Jing Liu 
Signed-off-by: Christian Borntraeger 
---
 qapi/run-state.json   | 55 +--
 target/s390x/cpu.c| 45 +
 target/s390x/cpu.h|  2 ++
 target/s390x/helper.c |  5 -
 target/s390x/kvm.c| 15 +++---
 vl.c  | 11 +--
 6 files changed, 120 insertions(+), 13 deletions(-)

diff --git a/qapi/run-state.json b/qapi/run-state.json
index bca46a8785..4bd15ae54f 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -320,22 +320,29 @@
 #
 # An enumeration of the guest panic information types
 #
+# @hyper-v: hyper-v guest panic information type
+#
+# @s390: s390 guest panic information type (Since: 2.12)
+#
 # Since: 2.9
 ##
 { 'enum': 'GuestPanicInformationType',
-  'data': [ 'hyper-v'] }
+  'data': [ 'hyper-v', 's390' ] }
 
 ##
 # @GuestPanicInformation:
 #
 # Information about a guest panic
 #
+# @type: Crash type that defines the hypervisor specific information
+#
 # Since: 2.9
 ##
 {'union': 'GuestPanicInformation',
  'base': {'type': 'GuestPanicInformationType'},
  'discriminator': 'type',
- 'data': { 'hyper-v': 'GuestPanicInformationHyperV' } }
+ 'data': { 'hyper-v': 'GuestPanicInformationHyperV',
+   's390': 'GuestPanicInformationS390' } }
 
 ##
 # @GuestPanicInformationHyperV:
@@ -350,3 +357,47 @@
'arg3': 'uint64',
'arg4': 'uint64',
'arg5': 'uint64' } }
+
+##
+# @S390CrashReason:
+#
+# Reason why the CPU is in a crashed state.
+#
+# @unknown: no crash reason was set
+#
+# @disabled-wait: the CPU has entered a disabled wait state
+#
+# @extint-loop: clock comparator or cpu timer interrupt with new PSW enabled
+#  for external interrupts
+#
+# @pgmint-loop: program interrupt with BAD new PSW
+#
+# @opint-loop: operation exception interrupt with invalid code at the program
+# interrupt new PSW
+#
+# Since: 2.12
+##
+{ 'enum': 'S390CrashReason',
+  'data': [ 'unknown',
+'disabled-wait',
+'extint-loop',
+'pgmint-loop',
+'opint-loop' ] }
+
+##
+# @GuestPanicInformationS390:
+#
+# S390 specific guest panic information (PSW)
+#
+# @core: core id of the CPU that crashed
+# @psw-mask: control fields of guest PSW
+# @psw-addr: guest instruction address
+# @reason: guest crash reason in human readable form
+#
+# Since: 2.12
+##
+{'struct': 'GuestPanicInformationS390',
+ 'data': { 'core': 'uint32',
+   'psw-mask': 'uint64',
+   'psw-addr': 'uint64',
+   'reason': 'S390CrashReason' } }
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 979469dc3c..2a6df4e4bd 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -35,6 +35,8 @@
 #include "qemu/error-report.h"
 #include "trace.h"
 #include "qapi/visitor.h"
+#include "qapi-visit.h"
+#include "sysemu/hw_accel.h"
 #include "exec/exec-all.h"
 #include "hw/qdev-properties.h"
 #ifndef CONFIG_USER_ONLY
@@ -237,6 +239,46 @@ out:
 error_propagate(errp, err);
 }
 
+static GuestPanicInformation *s390_cpu_get_crash_info(CPUState *cs)
+{
+GuestPanicInformation *panic_info;
+S390CPU *cpu = S390_CPU(cs);
+
+cpu_synchronize_state(cs);
+panic_info =