Re: [qemu-s390x] [PATCH v7] s390x/cpu: expose the guest crash information

[Christian Borntraeger]

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 <address@hidden>
>> Signed-off-by: Christian Borntraeger <address@hidden>
>> ---
> 
>> +##
>> +# @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.

So after some irc discussion with Daniel and Erik, I plan to add

#else
    panic_info->u.s390.core = 0;

for two reasons
- this interface is likely never used for linux-user
- linux-user does not know about CPU numbers, so using 0 is a good default

an optional item seems like overkill interface-wise.