On 02/08/2018 03:20 PM, Eric Blake wrote:
> On 02/08/2018 03:57 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 <liuj...@linux.vnet.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntrae...@de.ibm.com>
>> ---
>
>> +##
>> +# @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',
>
> Should core be optional,...
>
>
>> +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;
>> +#endif
>
> ...given that it is only conditionally assigned? If so, you'd also need to
> set panic_info->u.s390.has_core when you have a core id to expose.
Can we simply set it to 0 here and keep it mandatory? After all for
linux-user the qapi interface really makes not a lot of sense.
>
>
>> +
>> +static void s390_cpu_get_crash_info_qom(Object *obj, Visitor *v,
>> + const char *name, void *opaque,
>> + Error **errp)
>> +{
>> + CPUState *cs = CPU(obj);
>> + GuestPanicInformation *panic_info;
>> +
>> + if (!cs->crash_occurred) {
>> + error_setg(errp, "No crash occured");
>
> s/occured/occurred/
>
yes
