From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54327) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c4Rb3-00045U-8Z for qemu-devel@nongnu.org; Wed, 09 Nov 2016 07:03:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c4Rb0-00026C-3M for qemu-devel@nongnu.org; Wed, 09 Nov 2016 07:03:17 -0500 Date: Wed, 9 Nov 2016 17:40:41 +1100 From: David Gibson Message-ID: <20161109064041.GG427@umbus.fritz.box> References: <1477825928-10803-1-git-send-email-david@gibson.dropbear.id.au> <1477825928-10803-16-git-send-email-david@gibson.dropbear.id.au> <20161108052956.GV28688@umbus.fritz.box> <20161109042407.GE427@umbus.fritz.box> <74024fe8-dde4-07a3-201d-ce6249411ab3@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="WkfBGePaEyrk4zXB" Content-Disposition: inline In-Reply-To: <74024fe8-dde4-07a3-201d-ce6249411ab3@ozlabs.ru> Subject: Re: [Qemu-devel] [RFC 15/17] ppc: Check that CPU model stays consistent across migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: nikunj@linux.vnet.ibm.com, mdroth@linux.vnet.ibm.com, thuth@redhat.com, lvivier@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org --WkfBGePaEyrk4zXB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 "targe= t-ppc: > >>>>> Convert ppc cpu savevm to VMStateDescription"), a VMSTATE_EQUAL was= used > >>>>> to ensure that identical CPU models were used at source and destina= tion > >>>>> as based on the PVR (Processor Version Register). > >>>>> > >>>>> However this was a problem for HV KVM, where due to hardware limita= tions > >>>>> 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 w= as > >>>>> 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 migr= ation. > >>>>> 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 ru= nning > >>>>> in "raw" mode, we verify that the destination has the same PVR valu= e. > >>>>> > >>>>> Signed-off-by: David Gibson > >>>>> --- > >>>>> 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 ve= rsion_id) > >>>>> target_ulong msr; > >>>>> =20 > >>>>> /* > >>>>> - * We always ignore the source PVR. The user or management > >>>>> - * software has to take care of running QEMU in a compatible m= ode. > >>>>> + * 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 exa= ctly > >>>>> + * the same CPU model as the source. > >>>>> */ > >>>>> - env->spr[SPR_PVR] =3D env->spr_cb[SPR_PVR].default_value; > >>>>> =20 > >>>>> #if defined(TARGET_PPC64) > >>>>> if (cpu->compat_pvr) { > >>>>> @@ -188,8 +190,13 @@ static int cpu_post_load(void *opaque, int ver= sion_id) > >>>>> error_free(local_err); > >>>>> return -1; > >>>>> } > >>>>> - } > >>>>> + } else > >>>>> #endif > >>>>> + { > >>>>> + if (env->spr[SPR_PVR] !=3D env->spr_cb[SPR_PVR].default_va= lue) { > >>>>> + return -1; > >>>>> + } > >>>>> + } > >>>> > >>>> This should break migration from host with PVR=3D004d0200 to host wi= th > >>>> PVR=3D004d0201, 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=3D004B0201 (POWER8) to one with PVR=3D00201400 > >>> (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. > >=20 > > 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. > >=20 > > 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. >=20 > So the management stack (libvirt) will need to know that if it is HV KVM, > then -cpu host,compat=3Dxxxx; if it is PR KVM, then -cpu XXXX and no comp= at. > That was really annoying when we had exact PVR matching. >=20 >=20 > > 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. >=20 > 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. > >=20 >=20 >=20 --=20 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 --WkfBGePaEyrk4zXB Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYIsTmAAoJEGw4ysog2bOSOW4P/1UGHGD0ok8IR2DrVgwpO7LQ ldKAGLAT7NHRFbUZLfQ92Oj3L+AC8CmQUj0pYqzaSDbc8dzriTnwv6W+Hnpzz1cW GJd+1aemAG+Mm/YKWv6G7TcylhFnGa+wqC3dp3a0zTfvRKr+IiilLKDqy7a7VUNw dJw+oeDFXpPMDwR3AARlXMRaGiZ6bC1YAlyXAooOPGyrA9iDMJLEIXdkBGDFBVZe 6sYYVzmnF7A5avUSxO1X4rqd+PnWJQPbLdnMS3u+G1kI1e1qMqlzj7IyTcb4jA2M JHFy3epSeKcA4oj8rNpqc929a+LGs6i0mIQlEn6M+goIDSsL/I8Axgqe7khKDBFq CSPqoxVVn4+xQi+Y1XZwvob3HEYah+FLXB07+wYCL1ImsDB9uk/mCc49AjhaOoqp a+qc6pr65SaL7qbr/ABb6qahDsgpriOsZy0AGzKvbCYzs8gssrwp6QqweHd3+KLG GIR1frxqqMPu8eycocXypMjUdbMKEALo2vHzo73hRMF2gA0eYf1LE28Y8vfes8tr W1C5T6wP7RSQOChYxAJYuY03Hz32RM08lKuzJVDa05Up1ru36q1Og7U+vHTs3HNx aWbimLdmH1JQTINcR6gBTZGyPLddBJcvIOa8h6n2R7bXTCBTm8QWSNj8zQeA/zUu +TNFOq3usHXbIfaujmxs =G+lQ -----END PGP SIGNATURE----- --WkfBGePaEyrk4zXB--