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

[David Gibson]

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 <address@hidden>
> >>> ---
> >>>  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.

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.

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

signature.asc
Description: PGP signature