Re: [Qemu-devel] [RFC 15/17] ppc: Check that CPU model stays consistent across migration

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 6352 bytes --]

On Wed, Nov 09, 2016 at 05:06:23PM +1100, Alexey Kardashevskiy wrote:
> On 09/11/16 15:24, David Gibson wrote:
> > On Tue, Nov 08, 2016 at 05:03:49PM +1100, Alexey Kardashevskiy wrote:
> >> On 08/11/16 16:29, David Gibson wrote:
> >>> On Fri, Nov 04, 2016 at 06:54:48PM +1100, Alexey Kardashevskiy wrote:
> >>>> On 30/10/16 22:12, David Gibson wrote:
> >>>>> When a vmstate for the ppc cpu was first introduced (a90db15 "target-ppc:
> >>>>> Convert ppc cpu savevm to VMStateDescription"), a VMSTATE_EQUAL was used
> >>>>> to ensure that identical CPU models were used at source and destination
> >>>>> as based on the PVR (Processor Version Register).
> >>>>>
> >>>>> However this was a problem for HV KVM, where due to hardware limitations
> >>>>> we always need to use the real PVR of the host CPU.  So, to allow
> >>>>> migration between hosts with "similar enough" CPUs, the PVR check was
> >>>>> removed in 569be9f0 "target-ppc: Remove PVR check from migration".  This
> >>>>> left the onus on user / management to only attempt migration between
> >>>>> compatible CPUs.
> >>>>>
> >>>>> Now that we've reworked the handling of compatiblity modes, we have the
> >>>>> information to actually determine if we're making a compatible migration.
> >>>>> So this patch partially restores the PVR check.  If the source was running
> >>>>> in a compatibility mode, we just make sure that the destination cpu can
> >>>>> also run in that compatibility mode.  However, if the source was running
> >>>>> in "raw" mode, we verify that the destination has the same PVR value.
> >>>>>
> >>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >>>>> ---
> >>>>>  target-ppc/machine.c | 15 +++++++++++----
> >>>>>  1 file changed, 11 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> >>>>> index 5d87ff6..62b9e94 100644
> >>>>> --- a/target-ppc/machine.c
> >>>>> +++ b/target-ppc/machine.c
> >>>>> @@ -173,10 +173,12 @@ static int cpu_post_load(void *opaque, int version_id)
> >>>>>      target_ulong msr;
> >>>>>  
> >>>>>      /*
> >>>>> -     * We always ignore the source PVR. The user or management
> >>>>> -     * software has to take care of running QEMU in a compatible mode.
> >>>>> +     * If we're operating in compat mode, we should be ok as long as
> >>>>> +     * the destination supports the same compatiblity mode.
> >>>>> +     *
> >>>>> +     * Otherwise, however, we require that the destination has exactly
> >>>>> +     * the same CPU model as the source.
> >>>>>       */
> >>>>> -    env->spr[SPR_PVR] = env->spr_cb[SPR_PVR].default_value;
> >>>>>  
> >>>>>  #if defined(TARGET_PPC64)
> >>>>>      if (cpu->compat_pvr) {
> >>>>> @@ -188,8 +190,13 @@ static int cpu_post_load(void *opaque, int version_id)
> >>>>>              error_free(local_err);
> >>>>>              return -1;
> >>>>>          }
> >>>>> -    }
> >>>>> +    } else
> >>>>>  #endif
> >>>>> +    {
> >>>>> +        if (env->spr[SPR_PVR] != env->spr_cb[SPR_PVR].default_value) {
> >>>>> +            return -1;
> >>>>> +        }
> >>>>> +    }
> >>>>
> >>>> This should break migration from host with PVR=004d0200 to host with
> >>>> PVR=004d0201, what is the benefit of such limitation?
> >>>
> >>> There probably isn't one.  But the point is it also blocks migration
> >>> from a host with PVR=004B0201 (POWER8) to one with PVR=00201400
> >>> (403GCX) and *that* has a clear benefit.  I don't see a way to block
> >>> the second without the first, except by creating a huge compatibility
> >>> matrix table, which would require inordinate amounts of time to
> >>> research carefully.
> >>
> >>
> >> This is pcc->pvr_match() for this purpose.
> > 
> > Hmm.. thinking about this.  Obviously requiring an exactly matching
> > PVR is the architecturally "safest" approach.  For TCG and PR KVM, it
> > really should be sufficient - if you can select "close" PVRs at each
> > end, you should be able to select exactly matching ones just as well.
> > 
> > For HV KVM, we should generally be using compatibility modes to allow
> > migration between a relatively wide range of CPUs.  My intention was
> > basically to require moving to that model, rather than "approximate
> > matching" real PVRs.
> 
> So the management stack (libvirt) will need to know that if it is HV KVM,
> then -cpu host,compat=xxxx; if it is PR KVM, then -cpu XXXX and no compat.
> That was really annoying when we had exact PVR matching.
> 
> 
> > I'm still convinced using compat modes is the right way to go medium
> > to long term.  However, allowing the approximate matches could make
> > for a more forgiving transition, if people have existing hosts in
> > "raw" mode.
> 
> Within the family, CPUs behave exactly (not slightly but exactly) the same
> even though 3 of 4 bytes of the PVR value are different so enforcing PVR to
> match or enforcing compatibility (which as a feature was not a great idea
> from the day one) does not sound compelling.

Ah, ok.  pvr_match sounds reasonable then - I've already implemented
that.

> Does x86 have anything like this compatibility thingy?

It's better thought out on x86.  AIUI compatibility options are set in
the VM control block, so it is under host control even though the
guest is not para-virtualized.  I believe CPUID (unlike mfpvr) *is*
virtualized so the guest simply sees the compatible CPU, not something
else running in a compatibility mode.  So, there's no need for
compatibility modes as such, you just set the CPU to an older one, and
qemu and KVM between them make it appear that way to the guest.

It helps that compatibility is 1 dimensional on x86 - basically
any model can be run to be compatible with any older model.  That's
true for sufficiently recent Power server CPUs, but not across Power
in general.

Actually, I'm not sure where Atom and AMD vs. Intel fit into that
picture, but at any rate, I believe they're closer to 1 dimensional
compatibility than Power / PowerPC is.

> > Ok, I'll add pvr_match checking to this.
> > 
> 
> 




-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]