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

[Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Nikunj A Dadhania (nikunj@linux.vnet.ibm.com) wrote:
>> Benjamin Herrenschmidt <benh@kernel.crashing.org> 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,
>> +};
>> +
>
> I'd prefer it if you can avoid adding qemu_get/put's unless
> really desperate; I'm trying to squash all the read/writing back into
> standard macros; but I understand it can be tricky.

Right, the above code is just experimental :-)

>
> I'd agree that a post_load is the nicest way; it can return
> an error value.
> (Oh and ideally use error_report)

Sure

Regards
Nikunj