From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45885) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKz9V-0003dZ-Sp for qemu-devel@nongnu.org; Wed, 06 Jul 2016 22:34:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bKz9S-0007ZP-K7 for qemu-devel@nongnu.org; Wed, 06 Jul 2016 22:34:57 -0400 Date: Thu, 7 Jul 2016 12:08:28 +1000 From: David Gibson Message-ID: <20160707020828.GN14675@voom.fritz.box> References: <1467693772-7391-1-git-send-email-bharata@linux.vnet.ibm.com> <1467693772-7391-3-git-send-email-bharata@linux.vnet.ibm.com> <20160705091551.544e1ae1@172-15-179-184.lightspeed.austtx.sbcglobal.net> <20160705124322.GC25522@in.ibm.com> <20160706072544.4c379034@172-15-179-184.lightspeed.austtx.sbcglobal.net> <20160706075546.GD25522@in.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="cs5saTBZh7UZl2eX" Content-Disposition: inline In-Reply-To: <20160706075546.GD25522@in.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v0 2/5] cpu: Optionally use arch_id instead of cpu_index in cpu vmstate_register() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: Igor Mammedov , qemu-devel@nongnu.org, qemu-ppc@nongnu.org, groug@kaod.org, nikunj@linux.vnet.ibm.com, pbonzini@redhat.com --cs5saTBZh7UZl2eX Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 06, 2016 at 01:55:25PM +0530, Bharata B Rao wrote: > On Wed, Jul 06, 2016 at 07:25:44AM +0200, Igor Mammedov wrote: > > On Tue, 5 Jul 2016 18:13:22 +0530 > > Bharata B Rao wrote: > >=20 > > > On Tue, Jul 05, 2016 at 09:15:51AM +0200, Igor Mammedov wrote: > > > > On Tue, 5 Jul 2016 10:12:49 +0530 > > > > Bharata B Rao wrote: > > > >=20 > > > > > Introduce CPUState.prefer_arch_id_over_cpu_index and > > > > > MachineClass.prefer_arch_id_over_cpu_index that allow target > > > > > machines to optionally switch to using arch_id instead of > > > > > cpu_index as instance_id in vmstate_register(). This will help > > > > > allow successful migration in cases where holes are introduced in > > > > > cpu_index range after CPU hot removals. > > > > >=20 > > > > > Whether to use arch_id or cpu_index is based on machine type > > > > > version and hence added > > > > > MachineClass.prefer_arch_id_over_cpu_index. However the > > > > > enforcement is via and during CPU creation and hence added > > > > > CPUState.prefer_arch_id_over_cpu_index. So it becomes a two step > > > > > process for the target to enable the use of arch_id: > > > > >=20 > > > > > 1. Set MachineClass.prefer_arch_id_over_cpu_index. > > > > > 2. Ensure CPUState.prefer_arch_id_over_cpu_index is set for all > > > > > CPUs based on 1. above. > > > > >=20 > > > > > Suggested-by: Igor Mammedov > > > > > Signed-off-by: Bharata B Rao > > > > > --- > > > > > exec.c | 10 ++++++++-- > > > > > include/hw/boards.h | 1 + > > > > > include/qom/cpu.h | 4 ++++ > > > > > 3 files changed, 13 insertions(+), 2 deletions(-) > > > > >=20 > > > > > diff --git a/exec.c b/exec.c > > > > > index 8ce8e90..7cc1d06 100644 > > > > > --- a/exec.c > > > > > +++ b/exec.c > > > > > @@ -616,15 +616,21 @@ static void cpu_release_index(CPUState *cpu) > > > > > bitmap_clear(cpu_index_map, cpu->cpu_index, 1); > > > > > } > > > > > =20 > > > > > +/* > > > > > + * TODO: cpu_index and instance_id are of type int > > > > > while .get_arch_id()is > > > > > + * of type int64_t. What is the consequence of changing > > > > > instance_id to int64_t ? > > > > > + */ > > > > ARM potentially could have get_arch_id() =3D=3D MPIDR (return 64bit= int) > > > > that would get truncated in this case. > > > >=20 > > > > I wonder if we could add "int CPUState::migration_id" and let > > > > machine code set it to what it uses for stable cpu id and then > > > > cpu_realize could call vmstate_register with it. > > >=20 > > > Hmm how would that help ? instance_id argument to vmstate_register() > > > is being treated as 32 bit integer as pointed by Greg earlier. > > >=20 > > > If we want to use 64 bit arch_id/migration_id as instance_id arg in > > > vmstate_register, shouldn't we be changing instance_id to 64 bit ? > > Trying to extend instance_id from 32bit to 64bit, looks hard to me as > > it's on wire format so it's not possible to change without breaking > > compatibility. > >=20 > > That's why dedicated migration_id should also be 32bit integer to keep > > it compatible with vmstate_foo() magic. On x86 and ARM that id will be > > in range [0..maxcpus), it's stable index in possible_cpus array. >=20 > Since we were planning to use arch_id (aka device tree id) as the stable > index for CPU cores on PowerPC, we would have to use int64_t arch_id > itself as int instance_id I suppose. The range could be > [0...maxcpus*8] for us. Huh? I don't see how this comment relates to the one before. --=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 --cs5saTBZh7UZl2eX Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXfbmbAAoJEGw4ysog2bOSxvwQAJcagAj95Yoe7Zeex5ZiVlNA 1xhNWy+9UV5YTTPw3C1WOtgUNlvNUpJzAlAbRN3A2MfgJrRlPJz1fU4ldWU+XyCq wSXnYbUtQ7+l8M1xUqYTrn/XCBK2T7+C2swczmYNLlZ9SECKe+DkCZEebs75OwV3 Dbu7AOn39zvooDXNz30UsRTGnFwoWmLnyMRvMowaLNvEXUPkM3NKdaSCyCfh3lNY bTlpOCx0Bu+RoCXdymwM8niDOXs9WjnWX3hy1VtQxuuQGO/lquTrSR5GVmcjvj+9 g1tSSSG6Y4LbOLJs4GoYxOUisxPm6iewDeWWZ6DAoZioPGGwIIIgWKpaUXusOpTG Fsu0NoG8+YxX5BavkSU4+oi0BplUEo9B3tGx1NbomqgysIoRvOxkxiGyy100Qp9U Ri/HXVC0sSjOwdyycgj3VStOEwGtPUsEeiqr+FOUQaqWEszKf3tp3lIFpcMKt2Vb q+YZbyAorFQq6R1NroKw56C7KxUjNrscM6QyReoOg+/bIir7lUTsrR8REtYOfsnr Un9uyraYChWWSVXvGBnwOVK0k88eWitnt+S4oZcSI4dTBSFFdrCJNB/TcDKIPJvQ DwohKatZ9DbzQj5KZIugF18OXWKstk9mOP5TYfg9NVLdXiL6m+jgeO5PSWhB1Bxb ZRrY6m6xcOTnbSJjWoUV =FP0I -----END PGP SIGNATURE----- --cs5saTBZh7UZl2eX--