From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41794) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bVYuI-0000Gc-Lt for qemu-devel@nongnu.org; Fri, 05 Aug 2016 02:47:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bVYuD-0001wg-DP for qemu-devel@nongnu.org; Fri, 05 Aug 2016 02:46:57 -0400 Date: Fri, 5 Aug 2016 12:16:42 +1000 From: David Gibson Message-ID: <20160805021641.GI9189@voom.fritz.box> References: <1470201951-19288-1-git-send-email-david@gibson.dropbear.id.au> <1470201951-19288-2-git-send-email-david@gibson.dropbear.id.au> <20160804104453.GB13652@in.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="kH8JNVvasRCCW1Oz" Content-Disposition: inline In-Reply-To: <20160804104453.GB13652@in.ibm.com> Subject: Re: [Qemu-devel] [PULL 1/2] spapr: Error out when CPU hotplug is attempted on older pseries machines List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: peter.maydell@linaro.org, agraf@suse.de, pbonzini@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org --kH8JNVvasRCCW1Oz Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Aug 04, 2016 at 04:14:53PM +0530, Bharata B Rao wrote: > On Wed, Aug 03, 2016 at 03:25:50PM +1000, David Gibson wrote: > > From: Bharata B Rao > >=20 > > CPU hotplug and coldplug aren't supported prior to pseries-2.7. Furthe= r, > > earlier machine types don't use CPU core objects at all. These mean th= at > > query-hotpluggable-cpus and coldplug on older pseries machines will cra= sh > > QEMU. It also means that hotpluggable_cpus flag in query-machines will > > be incorrectly set to true for pseries < 2.7, since it is based on the > > presence of the query_hotpluggable_cpus hook. > >=20 > > - Don't assign the query_hotpluggable_cpus hook for pseries < 2.7 > > - query_hotpluggable_cpus should therefore never be called on pseries < > > 2.7, so add an assert > > - spapr_core_pre_plug() should fail hot/cold plug attempts for pseries < > > 2.7, since core objects are never used there > > - spapr_core_plug() should therefore never be called for pseries < 2.7,= so > > add an assert. > >=20 > > Signed-off-by: Bharata B Rao > > [dwg: Change from query_hotpluggable_cpus returning NULL for pseries < = 2.7 > > to not being called at all, reword commit message for accuracy] > > Signed-off-by: David Gibson > > --- > > hw/ppc/spapr.c | 7 ++++++- > > hw/ppc/spapr_cpu_core.c | 19 ++++++------------- > > 2 files changed, 12 insertions(+), 14 deletions(-) > >=20 > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index fbbd051..bce2371 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -2376,8 +2376,11 @@ static HotpluggableCPUList *spapr_query_hotplugg= able_cpus(MachineState *machine) > > int i; > > HotpluggableCPUList *head =3D NULL; > > sPAPRMachineState *spapr =3D SPAPR_MACHINE(machine); > > + sPAPRMachineClass *smc =3D SPAPR_MACHINE_GET_CLASS(machine); > > int spapr_max_cores =3D max_cpus / smp_threads; > >=20 > > + g_assert(smc->dr_cpu_enabled); > > + > > for (i =3D 0; i < spapr_max_cores; i++) { > > HotpluggableCPUList *list_item =3D g_new0(typeof(*list_item), = 1); > > HotpluggableCPU *cpu_item =3D g_new0(typeof(*cpu_item), 1); > > @@ -2432,7 +2435,9 @@ static void spapr_machine_class_init(ObjectClass = *oc, void *data) > > hc->plug =3D spapr_machine_device_plug; > > hc->unplug =3D spapr_machine_device_unplug; > > mc->cpu_index_to_socket_id =3D spapr_cpu_index_to_socket_id; > > - mc->query_hotpluggable_cpus =3D spapr_query_hotpluggable_cpus; > > + if (smc->dr_cpu_enabled) { > > + mc->query_hotpluggable_cpus =3D spapr_query_hotpluggable_cpus; > > + } > >=20 > > smc->dr_lmb_enabled =3D true; > > smc->dr_cpu_enabled =3D true; >=20 > Unfortunately smc->dr_cpu_enabled is always false when you set > mc->query_hotpluggable_cpus and will be set to true immediately > afterwards as seen in above hunk. >=20 > This leads to query-hotpluggable-cpus being unavailable for all > machine type versions. Your first version of setting > mc->query_hotpluggable_cpus to NULL explicitly for 2.6 and downwards > was correct. *facepalm* I can't believe I forgot to check that. In fact.. we could just replace dr_lmb_enabled with the NULL or not-NULL status of query_hotpluggable_cpus. I'll send a fix shortly. --=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 --kH8JNVvasRCCW1Oz Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXo/cJAAoJEGw4ysog2bOSizQP/jNAFKZJC5Y1Gc6Ff/7aWRF7 SlcEyzvaDg69B/6ObHQBolk1ay3781ntrS9xOJnttDuZcHqxiAMkhoXwQnUAMLw6 JewzJcQEHFLUrH4bCJilvI2OLOcmZU9VATAQWfnB0VcIxfkuGqwDpNUD7h71lRrt bd8k95/iW+uMz49cVU/KvHXs+hnWla3U1GCqPMN0XinmtUkYCJ3psUzHR2jTzyn2 ynYMxQ4aK6HJYaoLXkbs5CZRqOfJVXX8kTh54RuxniS2PYa7ZRUHBuDGozHiEX+T 4yMegzvgztiJeDrQKXBG7m6FQtDqpzHN+KteFaxdTQ8vZpk2odu3AKDPK2ISPA+G Wow7IBdjn1tTEQcK7YDqUsceZvRwb3/Jy09lP4U9QjpBUkzrX3sS+3sZzl9r9BWi Prq0HSfd0T8X9tB9Cv/RUqnVOtpKHAYAxmnSAKwt1oLnj22CEUQhT/42ghatWKFL 9VjxUe5CeSt0DJazs9uxgdq+g84UoITZEpK+f0aVeGR1pYq1oxYACWiIdtk5mbWR lt57juY4LTn81gL+KuxPhyZPqLuTTO7j4H0aLuq3W8QdNh9ryT7J6DcO/R3bSzGR J973LxymzG7ukr14W8nbVXta1BeAxHy01eOcNgKICwCJ3L8NI2It6J4mRwpl2yDU Ofhlj/3TJ/mfYWvehpa4 =9edX -----END PGP SIGNATURE----- --kH8JNVvasRCCW1Oz--