From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59732) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TVGdD-0008HR-SN for qemu-devel@nongnu.org; Mon, 05 Nov 2012 01:58:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TVGdC-0006Ig-HY for qemu-devel@nongnu.org; Mon, 05 Nov 2012 01:57:59 -0500 Received: from mout.web.de ([212.227.15.3]:53422) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TVGdC-0006Ic-7b for qemu-devel@nongnu.org; Mon, 05 Nov 2012 01:57:58 -0500 Message-ID: <5097636E.4010901@web.de> Date: Mon, 05 Nov 2012 07:57:50 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <1352093924-17598-1-git-send-email-qemulist@gmail.com> <1352093924-17598-8-git-send-email-qemulist@gmail.com> In-Reply-To: <1352093924-17598-8-git-send-email-qemulist@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig2F2F402363D3ADA096793834" Subject: Re: [Qemu-devel] [PATCH v6 7/8] memory: introduce tls context to trace nested mmio request issue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Liu Ping Fan Cc: Peter Maydell , Stefan Hajnoczi , Marcelo Tosatti , qemu-devel@nongnu.org, Avi Kivity , Anthony Liguori , Paolo Bonzini This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig2F2F402363D3ADA096793834 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable On 2012-11-05 06:38, Liu Ping Fan wrote: > From: Liu Ping Fan >=20 > After breaking down big lock, nested MMIO request which not targeting > at RAM can cause deadlock issue. Supposing the scene: dev_a,b with > fine-grain locks lockA/B, then ABBA dealock issue can be triggered. > We fix this by tracing and rejecting such request. >=20 > Signed-off-by: Liu Ping Fan > --- > exec.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > qemu-thread.h | 7 +++++++ > 2 files changed, 54 insertions(+), 0 deletions(-) >=20 > diff --git a/exec.c b/exec.c > index fa34ef9..1eb920d 100644 > --- a/exec.c > +++ b/exec.c > @@ -3442,6 +3442,48 @@ static bool address_space_section_lookup_ref(Add= ressSpace *as, > return safe_ref; > } > =20 > +typedef struct ThreadContext { > + DispatchType dispatch_type; > + unsigned int mmio_req_pending; > +} ThreadContext; > + > +static __thread ThreadContext thread_context =3D { ^^^^^^^^ Again, you will have to work on qemu-tls.h and then use DEFINE_TLS. The above is not portable. > + .dispatch_type =3D DISPATCH_INIT, > + .mmio_req_pending =3D 0 > +}; > + > +void qemu_thread_set_dispatch_type(DispatchType type) > +{ > + thread_context.dispatch_type =3D type; > +} > + > +void qemu_thread_reset_dispatch_type(void) > +{ > + thread_context.dispatch_type =3D DISPATCH_INIT; > +} > + > +static void address_space_check_inc_req_pending(MemoryRegionSection *s= ection) > +{ > + bool nested =3D false; > + > + /* currently, only mmio out of big lock, and need this to avoid de= ad lock */ > + if (thread_context.dispatch_type =3D=3D DISPATCH_MMIO) { > + nested =3D ++thread_context.mmio_req_pending > 1 ? true : fals= e; > + /* To fix, will filter iommu case */ > + if (nested && !memory_region_is_ram(section->mr)) { > + fprintf(stderr, "mmio: nested target not RAM is not suppor= t"); > + abort(); > + } > + } This should already take PIO into account, thus all scenarios: If we are dispatching MMIO or PIO, reject any further requests that are not targeting RAM. I don't think we need mmio_req_pending for this. We are not interested in differentiating between MMIO and PIO, both will be problematic. We just store the information if a request is going on in the TLS variable here, not before entering cpu_physical_memory_xxx. And then we can simply bail out if another non-RAM request is arriving, the nesting level will never be >1. And with bailing out I mean warn once + ignore request, not abort(). This would be a needless guest triggerable VM termination. Jan --------------enig2F2F402363D3ADA096793834 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iEYEARECAAYFAlCXY24ACgkQitSsb3rl5xS1LACglidNvgYp/ICkm9vMek57pj1a 8NYAoIfVItiUEhOrkkzXQVKgG8t5VKzZ =yBKV -----END PGP SIGNATURE----- --------------enig2F2F402363D3ADA096793834--