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

[Christian Borntraeger]

On 09/19/2017 03:06 PM, David Hildenbrand wrote:
> 
>> +##
>> +# @GuestPanicInformationS390:
>> +#
>> +# S390 specific guest panic information (PSW)
>> +#
>> +# Since: 2.11
>> +##
>> +{'struct': 'GuestPanicInformationS390',
>> + 'data': { 'psw-mask': 'uint64',
>> +           'psw-addr': 'uint64',
>> +           'reason': 'str' } }
> 
> Wonder if we should rather use an enum for reason.

Eric asked a similar question. Since this is just a human readable "info"
I would like to keep it as str. This will allow QEMU to add new things without
the need to update libvirt. Ok?

> 
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index 74b3e4f..5b835fe 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"
>>  #ifndef CONFIG_USER_ONLY
>>  #include "hw/hw.h"
>> @@ -276,6 +278,58 @@ static void s390x_cpu_set_id(Object *obj, Visitor *v, 
>> const char *name,
>>      cpu->id = value;
>>  }
>>  
> [...]
>> +static void unmanageable_intercept(S390CPU *cpu, int32_t reason, int 
>> pswoffset)
>>  {
>>      CPUState *cs = CPU(cpu);
>> +    const char *str;
>>  
>> +    switch (reason) {
>> +    case EXCP_CRASH_PGM:
>> +        str = "program interrupt loop";
>> +        break;
>> +    case EXCP_CRASH_EXT:
>> +        str = "external interrupt loop";
>> +        break;
>> +    case EXCP_CRASH_OPEREXC:
>> +        str = "operation exception loop";
>> +        break;
>> +    default:
>> +        str = "unknown crash reason";
>> +        break;
>> +    }
>>      error_report("Unmanageable %s! CPU%i new PSW: 0x%016lx:%016lx",
>>                   str, cs->cpu_index, ldq_phys(cs->as, cpu->env.psa + 
>> pswoffset),
>>                   ldq_phys(cs->as, cpu->env.psa + pswoffset + 8));
>>      s390_cpu_halt(cpu);
>> -    qemu_system_guest_panicked(NULL);
>> +    cs->exception_index = reason;
> 
> Hmmm, this might work for KVM but most probably in the current form not
> for TCG. cpu_handle_exception() will most probably just clear it and
> then exit to the main loop.

I was hoping to avoid the need to add new state, but adding a new variable 
should
be doable.