David Gibson <da...@gibson.dropbear.id.au> writes:
> [ Unknown signature status ]
> On Thu, Sep 22, 2016 at 02:34:19PM +0530, Nikunj A Dadhania wrote:
>> Benjamin Herrenschmidt <b...@kernel.crashing.org> writes:
>>
>> > On Thu, 2016-09-22 at 11:45 +0530, Bharata B Rao wrote:
>> >> On Thu, Sep 22, 2016 at 04:07:21PM +1000, Benjamin Herrenschmidt wrote:
>> >> >
>> >> > On Thu, 2016-09-22 at 10:51 +0530, Bharata B Rao wrote:
>> >> > >
>> >> > > The flag values are expected to remain same for a machine version for
>> >> > > the migration to succeed, but this expectation is broken now. Should
>> >> > > we make the addition of these flags conditional on machine type
>> >> > > version ?
>> >> > > But these flags are part of POWER8 CPU definition which is common for
>> >> > > both pseries and upcoming powernv.
>> >> >
>> >> > Does this affect KVM ? (And if yes why on earth would KVM give a flying
>> >> > f*** about the TCG instruction flags ?) ... If not, then I think we can
>> >> > safely not care.
>> >>
>> >> Yes, KVM migration is broken.
>> >
>> > Argh then ... stupid design in QEMU. We can't fix anything without
>> > breaking migration, yay !
>>
>> Looking back in the history of the code:
>>
>> commit: a90db1584a00dc1d1439dc7729d99674b666b85e (target-ppc: Convert
>> ppc cpu savevm to VMStateDescription) added this:
>>
>> + /* Sanity checking */
>> + VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
>> + VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
>>
>> These flags weren't part of vmstate, I am not sure what was the reason
>> behind adding it though. Its a bit old, Alexey do you remember?
>>
>> > I don't know what to do to fix that to be honest. Do we have a way to
>> > filter
>> > what flags actually matter and filter things out when KVM is enabled ?
>>
>> 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()
>> },
>
> This looks like the right solution to me. AFAICT this was just a
> sanity check that wasn't thought through well enough.
>
>> TCG migration still remains broken with this.
>
> Uh.. why?
Didn't debug it yet, reported on the other thread
qemu: fatal: Trying to deliver HV exception 4 with no HV support
NIP c0000000000795c8 LR d00000000074407c CTR c000000000079544 XER
0000000000000000 CPU#0
MSR 8000000000009032 HID0 0000000000000000 HF 8000000000000030 iidx 1
didx 1
TB 00000007 32202510341 DECR 00596259
Once it just hung, without any messages.
Regards
Nikunj
