From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [RFC][PATCH] qemu-kvm: Introduce writeback scope for cpu_synchronize_state Date: Tue, 17 Nov 2009 09:14:08 +0100 Message-ID: <4B025B50.4070505@web.de> References: <4B018542.3020602@siemens.com> <4B01A487.3020808@redhat.com> <4B01C2B0.3000205@web.de> <4B02592C.6060004@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigF529D15ABC41288CADD55206" Cc: Marcelo Tosatti , kvm , Gleb Natapov To: Avi Kivity Return-path: Received: from fmmailgate02.web.de ([217.72.192.227]:35218 "EHLO fmmailgate02.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752983AbZKQIOX (ORCPT ); Tue, 17 Nov 2009 03:14:23 -0500 In-Reply-To: <4B02592C.6060004@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigF529D15ABC41288CADD55206 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Avi Kivity wrote: > On 11/16/2009 11:22 PM, Jan Kiszka wrote: >> Avi Kivity wrote: >> =20 >>> On 11/16/2009 07:00 PM, Jan Kiszka wrote: >>> =20 >>>> This patch aims at addressing the mp_state writeback issue in a clea= ner >>>> fashion. >>>> =20 >>> What's the issue? the fact that mp_state is updated whenever state i= s >>> synchronized, while it could be simultaneously updated from other vcp= us >>> (which latter updates are then lost)? >>> =20 >> Right, the issue b8a7857071 addressed. But that approach spreads more >> kvm_* fragments in unrelated qemu code, e.g. the monitor, and fails to= >> update other parts (gdbstub). And it doesn't care about what happens i= f >> kvm is off at build or runtime. Such things are better addressed in >> upstream by encapsulating kvm calls in synchronization points. >> =20 >=20 > Note we have the same issue with nmi and the sipi vector - any vcpu Good point. > state that is updated outside the vcpu thread. These are particularly > bad since we can't exclude them from updates without excluding other > state as well. We easily can, using the very same mechanism: No need to overwrite any of the kvm_vcpu_events during runtime, only on reset/vmload). >=20 > The whole issue is tricky. I'm inclined to pretend we never meant any > vcpu state (outside lapic) to be asynchronous and declare the whole > thing a bug. We could fix it by modeling external changes to state > (INIT, SIPI, NMI) as messages queued to the vcpu, to be processed in th= e > vcpu thread. The queue would be drained before running the vcpu or > before reading state from userspace, so the message queue contents can > never be observed and never lost. >=20 > Of course, we can't really implement this as a queue (SIGSTOP vcpu > thread -> overflow), but a word is sufficient. INIT writes the word, > everything else uses compare-and-swap or set_bit to raise events (e.g. > SIPI =3D do { oldq =3D vcpu->queue; newq =3D (oldq & ~SIPI_MASK) | sipi= _vector > | RUNNING; } while (!cas(&vcpu->queue, oldq, newq))) >=20 I do not yet see why we need this complication, why the proposed model isn't enough. Jan --------------enigF529D15ABC41288CADD55206 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.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAksCW1QACgkQitSsb3rl5xTiQACgrKMvDfFEj0dCSyrQJ4mm59Rb OVsAniGM9bvl7wgj4YocljoOrRokAfhS =E+I7 -----END PGP SIGNATURE----- --------------enigF529D15ABC41288CADD55206--