From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49034) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1byrol-0002gs-Qh for qemu-devel@nongnu.org; Mon, 24 Oct 2016 22:50:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1byrok-0006of-Iq for qemu-devel@nongnu.org; Mon, 24 Oct 2016 22:50:23 -0400 Date: Tue, 25 Oct 2016 13:47:13 +1100 From: David Gibson Message-ID: <20161025024713.GP11052@umbus.fritz.box> References: <87a8dtkdwx.fsf@linaro.org> <878ttdkdse.fsf@linaro.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ftQmbtOmUf2cr8rB" Content-Disposition: inline In-Reply-To: <878ttdkdse.fsf@linaro.org> Subject: Re: [Qemu-devel] Holding the BQL for emulate_ppc_hypercall List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex =?iso-8859-1?Q?Benn=E9e?= Cc: Nikunj A Dadhania , Bharata B Rao , qemu-ppc@nongnu.org, Qemu Developers --ftQmbtOmUf2cr8rB Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Oct 24, 2016 at 03:44:01PM +0100, Alex Benn=E9e wrote: >=20 > Alex Benn=E9e writes: >=20 > > Hi, > > > > In the MTTCG patch set one of the big patches is to remove the > > requirement to hold the BQL while running code: > > > > tcg: drop global lock during TCG code execution > > > > And this broke the PPC code because emulate_ppc_hypercall can cause > > changes to the global state. This function just calls spapr_hypercall() > > and puts the results into the TCG register file. Normally > > spapr_hypercall() is called under the BQL in KVM as > > kvm_arch_handle_exit() does things with the BQL held. > > > > I blithely wrapped the called in a lock/unlock pair only to find the > > ppc64 check builds failed as the hypercall was made during the > > cc->do_interrupt() code which also holds the BQL. > > > > I'm a little confused by the nature of PPC hypercalls in TCG? Are they > > not all detectable at code generation time? What is the case that causes > > an exception to occur rather than the helper function doing the > > hypercall? > > > > I guess it comes down to can I avoid doing: > > > > /* If we come via cc->do_interrupt BQL may already be held */ > > if (!qemu_mutex_iothread_locked()) { > > g_mutex_lock_iothread(); > > env->gpr[3] =3D spapr_hypercall(cpu, env->gpr[3], &env->gpr[4]); > > g_muetx_unlock_iothread(); > > } else { > > env->gpr[3] =3D spapr_hypercall(cpu, env->gpr[3], &env->gpr[4]); > > } >=20 > Of course I mean: >=20 > /* If we come via cc->do_interrupt BQL may already be held */ > if (!qemu_mutex_iothread_locked()) { > qemu_mutex_lock_iothread(); > env->gpr[3] =3D spapr_hypercall(cpu, env->gpr[3], &env->gpr[4]); > qemu_mutex_unlock_iothread(); > } else { > env->gpr[3] =3D spapr_hypercall(cpu, env->gpr[3], &env->gpr[4]); > } >=20 > > Any thoughts? So, I understand why the hypercall is being called from exception code and therefore with the BQL held. On Power, the hypercall instruction is the same as the guest-level system call instruction, just with a flag bit set. System calls are, of course, treated as exceptions, because they change the CPU's privilege mode. Likewise if we were implementing a full host system (like the upcoming 'powernv' machine type) we'd need to treat hypercalls as exceptions for the same reason. We could detect hypercalls at translation time, but at present we don't: we go into the exception path, then detect that it's a "level 1" (i.e. hypervisor) sc instruction and branch off to the hypercall emulation code if that's been set up. It just seemed the simplet approach at the time. What I *don't* understand is how the hypercall code is ever being invoked *without* the BQL. I grepped through and the only entry paths I can see are the one in the exception handling and KVM. Could you try to get a backtrace from the case where we're entering the hypercall without the BQL? --=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 --ftQmbtOmUf2cr8rB Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYDsewAAoJEGw4ysog2bOSlRAP/RxPVoW9k55FM9AcrJ4PHfpQ vOUyKqauua7T43Fd38eGqlHzMNaKjtbmwj7xfySHyq/0RBUrQ6qpvcXADRFqxiPA 0qNXZbjcLS93HK5XLr7etLdKu5GWRK56HH9ZVLcpYiUINMDguToQhHMdzj0VbZe+ bXrm3ARlGN5+Me6qHaQw1UhY6BStt87IE28DCjSdE39sagDB3Q/wfEWjo+esqcOa jh6XyjndXLDVvXdGr2nmR/YEavVLT40NEL+Ol3XyUadrIg6xFpqHzPZ/JZXP4ETs KvgdsQGa05YLyf23I7CYNno9cZbOAQwCVM1rjFuUuSJ+yao5V3oMfuxscJqsHEOD Wog/xCk2YvgxZtxome38kQRtwYxfYMdTiU6vRmUQDYiXVrhN0+uGuY4feWpJR7p+ pPAJwqQ0TFV5ouMOsq+WAShPykb3NlXVtcud57fFHoIRr6V8V7K72jeXKOZdUIqC ZGrSnDphXLv/+ZseWsNSIwA+9KKlBR0xe7PESyH4kSgDS8oe8IFeFIWIqeU/7FJO llv9Cpi1p69o9Vow/DsOVz+33j8cKsiVRvPgm7uWCA9txW5Z65QxIs6mEtl2YAPW MdL+OH7pIOoN1QUoyrWyGr5r6pkwGne4O6nO1liKUJTNaVNe80s7j53cahmIOY/8 9HHGmBknC2OBkrjgMWxA =1S3P -----END PGP SIGNATURE----- --ftQmbtOmUf2cr8rB--