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

[David Hildenbrand]

On 19.09.2017 15:56, Christian Borntraeger wrote:
> 
> 
> On 09/19/2017 03:14 PM, David Hildenbrand wrote:
>>
>>>      CPUState *cs = CPU(obj);
>>> @@ -291,6 +345,8 @@ static void s390_cpu_initfn(Object *obj)
>>>      cs->exception_index = EXCP_HLT;
>>>      object_property_add(OBJECT(cpu), "id", "int64_t", s390x_cpu_get_id,
>>>                          s390x_cpu_set_id, NULL, NULL, NULL);
>>> +    object_property_add(obj, "crash-information", "GuestPanicInformation",
>>> +                        s390x_cpu_get_crash_info_qom, NULL, NULL, NULL, 
>>> NULL);
>>>      s390_cpu_model_register_props(obj);
>>>  #if !defined(CONFIG_USER_ONLY)
>>>      qemu_get_timedate(&tm, 0);
>>> @@ -517,6 +573,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void 
>>> *data)
>>>      cc->do_interrupt = s390_cpu_do_interrupt;
>>>  #endif
>>>      cc->dump_state = s390_cpu_dump_state;
>>> +    cc->get_crash_info = s390x_cpu_get_crash_info;
>>>      cc->set_pc = s390_cpu_set_pc;
>>>      cc->gdb_read_register = s390_cpu_gdb_read_register;
>>>      cc->gdb_write_register = s390_cpu_gdb_write_register;
>>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>>> index 0bd97a5..dba32db 100644
>>> --- a/target/s390x/cpu.h
>>> +++ b/target/s390x/cpu.h
>>> @@ -396,6 +396,12 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState* 
>>> env, target_ulong *pc,
>>>  #define EXCP_IO  7 /* I/O interrupt */
>>>  #define EXCP_MCHK 8 /* machine check */
>>>  
>>> +/* Crash cases. */
>>> +#define EXCP_CRASH_PGM 9
>>> +#define EXCP_CRASH_EXT 10
>>> +#define EXCP_CRASH_WAITPSW 11
>>> +#define EXCP_CRASH_OPEREXC 12
>>> +
>>
>> Okay, looking at cpu_handle_exception() I am pretty sure this won't work
>> with TCG unless cpu->exception_index >= EXCP_INTERRUPT. Maybe define new
>> EXCP_CRASH or store this information somewhere else inside s390 CPU?
> 
> Just asking: 
> Would it be ok to define EXCP_CRASH as 0x20000 or so?
> 

I guess the main problem with EXCP_CRASH is that it will get lost during
migration (if I am not wrong). So cpu->exception_index should most
probably really only be used for "temporary" things, e.g. to exit the
main loop.

Apart from that, I guess introducing it next to the other common code
EXCP_ defines sould be fine!

-- 

Thanks,

David