From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59227) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aavSf-0008HF-H8 for qemu-devel@nongnu.org; Tue, 01 Mar 2016 20:20:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aavSb-0003vi-Dw for qemu-devel@nongnu.org; Tue, 01 Mar 2016 20:20:21 -0500 Date: Wed, 2 Mar 2016 11:53:30 +1100 From: David Gibson Message-ID: <20160302005330.GN5427@voom.redhat.com> References: <1456417362-20652-1-git-send-email-bharata@linux.vnet.ibm.com> <1456417362-20652-5-git-send-email-bharata@linux.vnet.ibm.com> <20160226035141.GG20657@voom.fritz.box> <20160229044210.GA5756@in.ibm.com> <20160301075856.GH5756@in.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="McpcKDxJRrEJVmOH" Content-Disposition: inline In-Reply-To: <20160301075856.GH5756@in.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v0 4/6] spapr: CPU hotplug support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: mjrosato@linux.vnet.ibm.com, agraf@suse.de, thuth@redhat.com, pkrempa@redhat.com, ehabkost@redhat.com, aik@ozlabs.ru, qemu-devel@nongnu.org, armbru@redhat.com, borntraeger@de.ibm.com, qemu-ppc@nongnu.org, pbonzini@redhat.com, imammedo@redhat.com, afaerber@suse.de, mdroth@linux.vnet.ibm.com --McpcKDxJRrEJVmOH Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 01, 2016 at 01:28:56PM +0530, Bharata B Rao wrote: > On Mon, Feb 29, 2016 at 10:12:10AM +0530, Bharata B Rao wrote: > > > > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > > > > index b7c5ebd..cc0369e 100644 > > > > --- a/hw/ppc/spapr_rtas.c > > > > +++ b/hw/ppc/spapr_rtas.c > > > > @@ -34,6 +34,7 @@ > > > > =20 > > > > #include "hw/ppc/spapr.h" > > > > #include "hw/ppc/spapr_vio.h" > > > > +#include "hw/ppc/ppc.h" > > > > #include "qapi-event.h" > > > > #include "hw/boards.h" > > > > =20 > > > > @@ -161,6 +162,27 @@ static void rtas_query_cpu_stopped_state(Power= PCCPU *cpu_, > > > > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > > > > } > > > > =20 > > > > +/* > > > > + * Set the timebase offset of the CPU to that of first CPU. > > > > + * This helps hotplugged CPU to have the correct timebase offset. > > > > + */ > > > > +static void spapr_cpu_update_tb_offset(PowerPCCPU *cpu) > > > > +{ > > > > + PowerPCCPU *fcpu =3D POWERPC_CPU(first_cpu); > > > > + > > > > + cpu->env.tb_env->tb_offset =3D fcpu->env.tb_env->tb_offset; > > > > +} > > > > + > > > > +static void spapr_cpu_set_endianness(PowerPCCPU *cpu) > > > > +{ > > > > + PowerPCCPU *fcpu =3D POWERPC_CPU(first_cpu); > > > > + PowerPCCPUClass *pcc =3D POWERPC_CPU_GET_CLASS(fcpu); > > > > + > > > > + if (!pcc->interrupts_big_endian(fcpu)) { > > > > + cpu->env.spr[SPR_LPCR] |=3D LPCR_ILE; > > > > + } > > > > +} > > > > + > > >=20 > > > Any particular reason for doing these things at rtas_start_cpu() time, > > > but other initialization at plug time? Could you consolidate it to > > > one place or the other? > >=20 > > Those board specific things that are needed to be done have been consol= idated > > into spapr_cpu_init() which will be called from the plug handler. We ha= ve > > discussed this earlier at: > >=20 > > https://lists.nongnu.org/archive/html/qemu-devel/2015-02/msg04399.html > >=20 > > It has been a while but there was a good reason why setting endianness > > here rather than in plug handler is necessary. W/o this LE hotplug on g= uests > > wouldn't work, I will dig up and come back on what exactly necessiated > > this change. >=20 > If we set LPCR_ILE in cpu->env.spr[SPR_LPCR] at plug time > (from spapr_cpu_init()), there are at least two places later where it gets > over-written. One is spapr_cpu_reset() and the other one when > kvm_cpu_synchronize_state() is called from rtas_start_cpu(). We could > probably issue a kvm_arch_put_registers(), but I found rtas_start_cpu() > as a place where this change is guaranteed to get reflected. Ok, makes sense. In that case can we move all, or nearly all, of the PAPR specific thread initialization to rtas_start_cpu()? Obviously we'd need a separate call to the same stuff for the boot cpu. --=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 --McpcKDxJRrEJVmOH Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW1jmKAAoJEGw4ysog2bOSbwoQANw/J50Hq3gRy/W/vk1btHOE 1yTzyKUe3hIyNrTHk1xbeahBMZ8zbdqK0KWOB7YXkKnLHi3tlu7UImPBmOuIFQxe UIjIlRlSWiolspFZvE9UGk3k9aDz8reeZ0zA8zh7VlWo/vQ2oYayYGVtjFdur//Q irMUsPawuTyi4TJStQA9fzFULY5y+ZsCe6+4PGq7EpQkU0q076DY1NqeaCnhfCis gxSyXW+5cqDaqFAWMg8fUAnuXW3XL4JDm7lcJuTZ46o3Pm+Ao1xgL56THlxb8zMp jg272Df0s1A38P140cX53JzeTkqLg3WkTPRLZQ/vByLzOpD2zUjvKIP0tB0yey4X CV6ll1pvmxlzi/czTz7i3hqqEA4YS5dOSr0IAsTOupAri6LlbPlTFVgxJBWPsU6w wQL/s5m8xViRESqfjBQGyM0b6eAPHrbEDyZDvcwZMXvNgK6vW9nE4jIl/5upwxbW yLzK89m018dwN9k32qxQjhbzacs7LouSh4DVuD8PZ6AG0dsNo+0kgesEDo2KT4/t 1QjWoXrqeKk1B6AjVdIYheJXjow6XROc4bu/lVefWDVvNF0x/O2VBdVqCfxFfVT5 wmS1B6LT5D1hKvIihqzKwHXAAvAJrJtzThILZYI9QeiErprXcSSPqtja78mxUn5B IkZfNebJPldQYS8d0cGl =yh68 -----END PGP SIGNATURE----- --McpcKDxJRrEJVmOH--