Re: [Qemu-ppc] POWER8 <-> POWER8E migration changed/broken in ppc-for.2.10. Intended?

[David Gibson]

On Thu, Jun 22, 2017 at 09:08:55AM +0200, Thomas Huth wrote:
> On 22.06.2017 05:22, Alexey Kardashevskiy wrote:
> > On 22/06/17 12:49, Alexey Kardashevskiy wrote:
> >> On 22/06/17 11:45, Alexey Kardashevskiy wrote:
> >>> On 22/06/17 07:19, Daniel Henrique Barboza wrote:
> >>>> Hi,
> >>>>
> >>>> Yesterday I noticed that the migration between POWER8 and POWER8E hosts
> >>>> isn't working
> >>>> as before. These are the machines that I've been using for DRC testing 
> >>>> and
> >>>> I discovered
> >>>> this behavior when trying to test the DRC cleanup series (v5).
> >>>>
> >>>> What happens in that the migration seems to be completed - no errors on
> >>>> both source and
> >>>> destination, destination status marked as 'running' - but the guest at 
> >>>> dest
> >>>> looks
> >>>> frozen/inactive. Migrating from POWER8 to POWER8E or the other way around
> >>>> presents the same behavior.
> >>>>
> >>>> This is the command line I'm executing:
> >>>>
> >>>> sudo ./qemu-system-ppc64 -name migrate_qemu -boot strict=on -device
> >>>> nec-usb-xhci,id=usb,bus=pci.0,addr=0xf -device
> >>>> spapr-vscsi,id=scsi0,reg=0x2000 -smp
> >>>> 1,maxcpus=4,sockets=4,cores=1,threads=1 --machine
> >>>> pseries,accel=kvm,kvm-type=HV,usb=off,dump-guest-core=off -m
> >>>> 4G,slots=32,maxmem=32G -drive
> >>>> file=/home/danielhb/vm_imgs/ubuntu1704.qcow2,format=qcow2,if=none,id=drive-virtio-disk0,cache=none
> >>>> -device
> >>>> virtio-blk-pci,scsi=off,bus=pci.0,addr=0x2,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
> >>>> -nographic
> >>>>
> >>>> I've tracked the problem down to the commit b0517f54ef7d, "ppc: Rework 
> >>>> CPU
> >>>> compatibility testing across migration". If I revert this commit, 
> >>>> migration
> >>>> works as before
> >>>> between these hosts.
> >>>
> >>> With b0517f54ef7d, CPU state is not marked dirty and SPRs (all registers 
> >>> in
> >>> fact) are not put to KVM at the end of migration, hence the freeze, this
> >>> has something to do with cpu_synchronize_state() called from 
> >>> ppc_set_compat().
> >>
> >> No, I was not right, please ignore.
> > 
> > 
> > This hack fixes it:
> > 
> > 
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index f2f7c531bc..7ec4146b2b 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -919,6 +919,7 @@ static int kvm_put_vpa(CPUState *cs)
> >      return 0;
> >  }
> >  #endif /* TARGET_PPC64 */
> > +static uint32_t mfpvr(void);
> > 
> >  int kvmppc_put_books_sregs(PowerPCCPU *cpu)
> >  {
> > @@ -927,7 +928,7 @@ int kvmppc_put_books_sregs(PowerPCCPU *cpu)
> >      int i;
> > 
> >      sregs.pvr = env->spr[SPR_PVR];
> > -
> > +    sregs.pvr = mfpvr();
> >      sregs.u.s.sdr1 = env->spr[SPR_SDR1];
> > 
> >      /* Sync SLB */
> 
> In case you consider to submit this patch: Please add a check for
> !kvmppc_is_pr() so that this is only done for KVM-HV.
> 
> But I think the right way to hack this is somewhere in machine.c
> instead, eg. in cpu_post_load().

Is the problem is just that we're not considering POWER8 to be
compatible enough with POWER8E, then we should be altering the
pvr_match function so it does.

-- 
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