From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32811) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyDrX-0000Pb-OE for qemu-devel@nongnu.org; Mon, 16 Nov 2015 02:06:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZyDrU-0000Qm-Hx for qemu-devel@nongnu.org; Mon, 16 Nov 2015 02:06:03 -0500 Date: Mon, 16 Nov 2015 16:45:35 +1100 From: David Gibson Message-ID: <20151116054535.GK2747@voom.fritz.box> References: <20151111171135.4328.41819.stgit@aravindap> <20151111171602.4328.34006.stgit@aravindap> <56444957.9080003@redhat.com> <56445E7B.5010904@redhat.com> <20151113015715.GJ4886@voom.redhat.com> <56458B57.3010407@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="x+RZeZVNR8VILNfK" Content-Disposition: inline In-Reply-To: <56458B57.3010407@redhat.com> Subject: Re: [Qemu-devel] [PATCH 4/4] target-ppc: Handle NMI guest exit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: benh@au1.ibm.com, aik@ozlabs.ru, agraf@suse.de, qemu-devel@nongnu.org, qemu-ppc@nongnu.org, sam.bobroff@au1.ibm.com, Aravinda Prasad , paulus@samba.org --x+RZeZVNR8VILNfK Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 13, 2015 at 08:03:51AM +0100, Thomas Huth wrote: > On 13/11/15 02:57, David Gibson wrote: > > On Thu, Nov 12, 2015 at 10:40:11AM +0100, Thomas Huth wrote: > >> On 12/11/15 09:09, Thomas Huth wrote: > >>> On 11/11/15 18:16, Aravinda Prasad wrote: > [...] > >>>> + qemu_mutex_lock(&spapr->mc_in_progress); > >>> > >>> Using a mutex here is definitely wrong. The kvm_arch_handle_exit() co= de > >>> is run under the Big QEMU Lock=E2=84=A2 (see qemu_mutex_lock_iothread= () in > >>> kvm_cpu_exec()), > [...] > >> Ok, I now had a look into the LoPAPR spec, and if I've got that right, > >> you really have to serialize the NMIs in case they happen at multiple > >> CPUs at the same time. So I guess the best thing you can do here is > >> something like: > >> > >> while (spapr->mc_in_progress) { > >> /* > >> * There is already another NMI in progress, thus we need > >> * to yield here to wait until it has been finsihed > >> */ > >> qemu_mutex_unlock_iothread(); > >> usleep(10); > >> qemu_mutex_lock_iothread(); > >> } > >> spapr->mc_in_progress =3D true; > [...] > > You should be able to avoid the nasty usleep by using a pthread > > condition variable. So here you'd have > >=20 > > while (spapr->mc_in_progress) { > > pthread_cond_wait(&mc_delivery_cond, &qemu_iothread_lock); > > } > > spapr->mc_in_progress =3D true; > >=20 > > Or.. there may be qemu wrappers around the pthread functions you > > should be using. Once delivery of a single MC is complete, you'd use > > pthread_cond_signal() to wake up any additional ones. > >=20 > > pthread_cond_wait automatically drops the specified mutex internally, > > so access to mc_in_progress itself is still protected by the iothread > > mutex. >=20 > That's a nice one, didn't know that function yet! And actually, there is > already a QEMU wrapper function: qemu_cond_wait() - so this should be > used instead since threads on Windows are working differently in QEMU as > far as I know. Ah, good point, you definitely need to use the qemu wrapper in order to get proper windows support. --=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 --x+RZeZVNR8VILNfK Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWSW1/AAoJEGw4ysog2bOS5SUP/3p5Q3kJr0k3NK8FrnzSkHxg GX2x0K40SGGCKtvq8//V+1MVPPYM3D9bzYFUuelKbhtLP2oBqI5GJva9n9KyspUj bGwH5XBIh6r9yw+WMDrYzu1diQQFpp2BvCGVzeHrx6SlzHkWh7qR2RCYjl769/ek SPDaJujlJ6rRlwsnPnE4qSNjhVw6tyncHngfXmcnCyocsQKctkr4nw0eFwgTHolv 6mqAb2SelG43OFM66QnAT6z+1LUUcwGgJx88B4407qBT4U/s9vFJbp6v/ZwSJGqs kwjzOX8EVXHoBxf1bdyhbCT1GJa6vjC0ImnZnASYJ/X+PcGPnTUv5qk9OlEwVOON Z3DuvwwgEIzYmy+B4PlldyZODxvSrOGLhVtvrLlyjhsIjvn5VvozjexMiz3IARVx HTXslqfckUmep+C+d1yBB2NcYm6PD3sh3TESLqFWwUWChAYIFmGY4kGDhlTD3Xgk XWHYHShzPUtQf+u/ppKY6/RB/BLd+W7/PsVTLi52PF5K/iAI6QDiymOxrFeFABxK Mo55nGqcmzBJ0AJjzNafkdBnL9MTPTJn7Xom2J6E2jMblmf6I2x2d13SXcq5ZHkU qL9bAjhcsJppf/zWUiviN7a7Xm1PkF02O9+oLsSFUy9e1jcvc7COEtFjhwYIQqkQ isBhq2+zfqw9qGVUpJ4B =1Lai -----END PGP SIGNATURE----- --x+RZeZVNR8VILNfK--