From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58643) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c5GKl-0000zE-1f for qemu-devel@nongnu.org; Fri, 11 Nov 2016 13:13:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c5GKh-0004RW-QB for qemu-devel@nongnu.org; Fri, 11 Nov 2016 13:13:51 -0500 Received: from 5.mo53.mail-out.ovh.net ([46.105.48.2]:33559) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c5GKh-0004Qj-KW for qemu-devel@nongnu.org; Fri, 11 Nov 2016 13:13:47 -0500 Received: from player158.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo53.mail-out.ovh.net (Postfix) with ESMTP id E30F041C94 for ; Fri, 11 Nov 2016 19:13:45 +0100 (CET) Date: Fri, 11 Nov 2016 19:13:34 +0100 From: Greg Kurz Message-ID: <20161111191334.3ce8161c@bahia> In-Reply-To: <20161108053108.GW28688@umbus.fritz.box> References: <1477825928-10803-1-git-send-email-david@gibson.dropbear.id.au> <1477825928-10803-17-git-send-email-david@gibson.dropbear.id.au> <57db31f3-e8b6-cdb4-da1e-e08f06bebd5c@ozlabs.ru> <20161108053108.GW28688@umbus.fritz.box> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; boundary="Sig_/w3oL=cxLtBPSWv6yWGn_fd6"; protocol="application/pgp-signature" Subject: Re: [Qemu-devel] [Qemu-ppc] [RFC 16/17] ppc: Remove counter-productive "sanity checks" in migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: Alexey Kardashevskiy , lvivier@redhat.com, thuth@redhat.com, mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org, qemu-ppc@nongnu.org --Sig_/w3oL=cxLtBPSWv6yWGn_fd6 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Tue, 8 Nov 2016 16:31:08 +1100 David Gibson wrote: > On Fri, Nov 04, 2016 at 04:52:39PM +1100, Alexey Kardashevskiy wrote: > > On 30/10/16 22:12, David Gibson wrote: =20 > > > When vmstate for the ppc cpu was introduced in a90db158 "target-ppc: > > > Convert ppc cpu savevm to VMStateDescription", several "sanity check" > > > fields were included, verifying that certain cpu parameters matched b= etween > > > source and destination. > > >=20 > > > This turns out not to have been a good idea. For one thing it's redu= ndant > > > with existing checks for a compatible cpu version at either end. But= more > > > importantly the insns_flags and insns_flags2 checks actively break th= ings: > > > they expose what's essentially an internal TCG implementation detail = in the > > > migration stream. That means that when new instruction classes are a= dded > > > or rearranged, migration can break. > > >=20 > > > This removes these ill-considered sanity checks. > > >=20 > > > Signed-off-by: David Gibson > > > --- > > > target-ppc/machine.c | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > >=20 > > > diff --git a/target-ppc/machine.c b/target-ppc/machine.c > > > index 62b9e94..453ef0a 100644 > > > --- a/target-ppc/machine.c > > > +++ b/target-ppc/machine.c > > > @@ -602,10 +602,10 @@ const VMStateDescription vmstate_ppc_cpu =3D { > > > /* FIXME: access_type? */ > > > =20 > > > /* Sanity checking */ > > > - VMSTATE_UINTTL_EQUAL(env.msr_mask, PowerPCCPU), > > > - VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU), > > > - VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU), > > > - VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU), > > > + VMSTATE_UNUSED(sizeof(target_ulong) /* msr_mask */ > > > + + sizeof(uint64_t) /* insns_flags */ > > > + + sizeof(uint64_t) /* insns_flags2 */ > > > + + sizeof(uint32_t)), /* nb_BATs */ =20 > >=20 > >=20 > > This breaks migration to older QEMU: > >=20 > > 25055@1478238734.537761:vmstate_load_field_error field "env.msr_mask" l= oad > > failed, ret =3D -22 =20 >=20 > Again, I don't think we generally support backwards migration. >=20 > That said, it would be nice here to do a "set to this field on > ourgoing migration, but ignore on incoming migration". Do you know a > way to do that? >=20 This doesn't exist in vmstate but it is certainly doable. An alternative would be to always send the fields, but have the destination to use pre_load and post_load callbacks to preserve its state. > a > >=20 > >=20 > > =20 > > > VMSTATE_END_OF_LIST() > > > }, > > > .subsections =3D (const VMStateDescription*[]) { > > > =20 > >=20 > > =20 >=20 --Sig_/w3oL=cxLtBPSWv6yWGn_fd6 Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAlgmCk8ACgkQAvw66wEB28KVpwCfVMzY0emRsWSrJe677PUNHUCG QJAAoIBm3F0oaarhId4SeS8WL27tIkOQ =Jh8Y -----END PGP SIGNATURE----- --Sig_/w3oL=cxLtBPSWv6yWGn_fd6--