From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57612) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VE7fD-0002NP-9F for qemu-devel@nongnu.org; Mon, 26 Aug 2013 21:01:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VE7fB-0001r7-H3 for qemu-devel@nongnu.org; Mon, 26 Aug 2013 21:01:43 -0400 Date: Tue, 27 Aug 2013 01:29:02 +1000 From: David Gibson Message-ID: <20130826152902.GB9120@voom.fritz.box> References: <1377257435-27714-1-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="JP+T4n/bALQSJXh8" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH] spapr: support CPU hotplug List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Alexey Kardashevskiy , qemu-ppc@nongnu.org, Paul Mackerras , qemu-devel@nongnu.org --JP+T4n/bALQSJXh8 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Aug 26, 2013 at 02:24:49PM +0200, Alexander Graf wrote: >=20 > On 23.08.2013, at 13:30, Alexey Kardashevskiy wrote: >=20 > > PAPR+ requires two RTAS calls to be supported by the hypervisor in > > order to allow hotplugging VCPUs from the guest. The "start-cpu" RTAS > > call was already there but "stop-self" was not. > >=20 > > This adds the "stop-self" RTAS call. > >=20 > > Signed-off-by: Alexey Kardashevskiy > > --- > > hw/ppc/spapr_rtas.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > >=20 > > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > > index 394ce05..8a4cfa0 100644 > > --- a/hw/ppc/spapr_rtas.c > > +++ b/hw/ppc/spapr_rtas.c > > @@ -202,6 +202,19 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPR= Environment *spapr, > > rtas_st(rets, 0, -3); > > } > >=20 > > +static void rtas_stop_self(PowerPCCPU *cpu, sPAPREnvironment *spapr, > > + uint32_t token, uint32_t nargs, > > + target_ulong args, > > + uint32_t nret, target_ulong rets) > > +{ > > + CPUState *cs =3D CPU(cpu); > > + CPUPPCState *env =3D &cpu->env; > > + > > + cs->halted =3D 1; > > + env->msr =3D 0; >=20 > So this is here to make sure we don't accidentally get out of halted stat= e by an interrupt on that vcpu. Could you please somehow make that part obv= ious? Either by adding a comment or by only explicitly masking DEC and EE a= nd a comment :). >=20 > > + cs->exit_request =3D 1; >=20 > This should probably be qemu_cpu_kick_self(). Uh, no, I don't think so. This is there purely to make sure we exit the inner loop, and actually test cpu_can_run() which will test halted. AFAICT qemu_cpu_kick_self() won't do anything similar. --=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 --JP+T4n/bALQSJXh8 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) iQIcBAEBAgAGBQJSG3Q+AAoJEGw4ysog2bOSNosP/REieijWxWZSDJnjhs1BNX2e VaB4XXFKPi9JwxxszIUHdY7GUnx577HHAgR8MYRPWKL43z7BsUwjM/KaBwQ+2Ovo P3Z5ruLyqeJaiLpgwKbaff71iVkaQLB4Kg9b5q7ybT/fwevR3JGeO6JOQOKj8FR+ JziMdj8YGu6OfwvIa8KCePuXxDgjM2iwxERBb2TbZcDDq/kMVx4ANvWMjDsewKjF MPMdhH6nHKrBJIwXmsThIcMMxT9HteZVwz4TawhlMb6x2VG4uqiJ7ZYNr0l093Rq PSsVqg/0lJ7x4YanTVAdYA5ADs9ImlthcQge5PlDgtPuB4w2oeh9+nbkF6g7ceDN +m4rwW2inEUhjLgVkOzS57CZN+O3twV456zOLq7BlAn9b1rAbHbqP2RWxlRKMx4f lcwwx9jfvcPMlUmeJFT/RXEf8wkTLGfW7KGKgavNH6Ls5brX9Nr56swB2GZVlk7Y wTpr4N/4q2DA5yiCcUN9yx46c4sIMe+UZILcpFWsPJTO6lbGAN+yE75OQm5ugKvE VkA5n0RcPQh8ZWvHnICX9r1nTceB0DaEbmf1telDRev414aE7H5oQ7d4UgGfuzB2 fS1OlisAcKY/HF9FNuPlZLcyTfVVXdyPRzYJbkyp4VFWWlR4yFvnqfiUCf7ic8Eu Zeb9l8j8WZWAfv7LkiP5 =plq1 -----END PGP SIGNATURE----- --JP+T4n/bALQSJXh8--