Re: [Qemu-devel] pseries-2.6 migration from QEMU-2.6 to QEMU-2.7 broken

[Nikunj A Dadhania]

Cédric Le Goater <address@hidden> writes:

> On 09/22/2016 01:07 PM, Nikunj A Dadhania wrote:
>> Benjamin Herrenschmidt <address@hidden> writes:
>> 
>>> On Thu, 2016-09-22 at 14:34 +0530, Nikunj A Dadhania wrote:
>>>> Something like this works for KVM:
>>>>
>>>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
>>>> index 4820f22..1cf3779 100644
>>>> --- a/target-ppc/machine.c
>>>> +++ b/target-ppc/machine.c
>>>> @@ -563,8 +563,8 @@ const VMStateDescription vmstate_ppc_cpu = {
>>>>  
>>>>          /* Sanity checking */
>>>>          VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
>>>> -        VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
>>>> -        VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
>>>> +        VMSTATE_UNUSED(sizeof(target_ulong)), /* was 
>>>> _EQUAL(env.insns_flags) */
>>>> +        VMSTATE_UNUSED(sizeof(target_ulong)), /* was 
>>>> _EQUAL(env.insns_flags2) */
>>>>          VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU),
>>>>          VMSTATE_END_OF_LIST()
>>>>      },
>>>>
>>>> TCG migration still remains broken with this.
>>>
>>> Can we have conditionally present flags and a post-load that does some
>>> matching ?
>> 
>> I think its possible like this:
>> 
>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
>> index 4820f22..dc4704e 100644
>> --- a/target-ppc/machine.c
>> +++ b/target-ppc/machine.c
>> @@ -528,6 +528,42 @@ static const VMStateDescription vmstate_tlbmas = {
>>      }
>>  };
>>  
>> +static bool ppc_kvm_enabled(void *opaque, int version_id)
>> +{
>> +    printf("%s: is kvm enabled %d\n", __func__, kvm_enabled());
>> +    return !kvm_enabled();
>> +}
>> +
>> +static int get_insns_equal(QEMUFile *f, void *pv, size_t size)
>> +{
>> +    uint64_t *v = pv;
>> +    uint64_t v2;
>> +    qemu_get_be64s(f, &v2);
>> +
>> +    printf("%s: \n", __func__);
>> +
>> +    if (*v == v2) {
>> +        return 0;
>> +    }
>> +    printf("Did not match, ignore %" PRIu64 " != %" PRIu64 "\n", *v, v2);
>> +    return 0;
>> +}
>> +
>> +static void put_insns(QEMUFile *f, void *pv, size_t size)
>> +{
>> +    uint64_t *v = pv;
>> +    qemu_put_be64s(f, v);
>> +}
>> +
>> +const VMStateInfo vmstate_info_insns_equal = {
>> +    .name = "insns equal",
>> +    .get  = get_insns_equal,
>> +    .put  = put_insns,
>> +};
>> +
>> +#define VMSTATE_INSNS_EQUAL(_f, _s, _t)                                 \
>> +    VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_info_insns_equal, uint64_t)
>> +
>>  const VMStateDescription vmstate_ppc_cpu = {
>>      .name = "cpu",
>>      .version_id = 5,
>> @@ -563,8 +599,8 @@ const VMStateDescription vmstate_ppc_cpu = {
>>  
>>          /* Sanity checking */
>>          VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU),
>> -        VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
>> -        VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
>> +        VMSTATE_INSNS_EQUAL(env.insns_flags, PowerPCCPU, ppc_kvm_enabled),
>> +        VMSTATE_INSNS_EQUAL(env.insns_flags2, PowerPCCPU, ppc_kvm_enabled),
>>          VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU),
>>          VMSTATE_END_OF_LIST()
>>      },
>> 
>> 
>> TCG migration succeeds and proceeds ahead. But fails somewhere ahead in
>> powerpc exception handler:
>> 
>> [qemu]$ ./ppc64-softmmu/qemu-system-ppc64  -machine pseries-2.6,usb=off -vga 
>> none -nographic -m 2G   ../../imgs/guest.disk -monitor pty --incoming 
>> tcp:localhost:4444 
>> char device redirected to /dev/pts/5 (label compat_monitor0)
>> ppc_kvm_enabled: is kvm enabled 0
>> get_insns_equal: 
>> Did not match, ignore 9223477658187168481 != 9223477658187151905
>> ppc_kvm_enabled: is kvm enabled 0
>> get_insns_equal: 
>> Did not match, ignore 331702 != 69558
>> Cannot open font file True
>> Cannot open font file True
>> qemu: fatal: Trying to deliver HV exception 4 with no HV support
>
> hmm, this is because we added MSR_HVB in msr_mask AFAICT. we should have 
> a similar vmstate op for it I think

Not sure how will vmstate op help here. As vmstate is migrated
successfully. Do we need to copy msr features of source ?

Regards
Nikunj