From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julian Stecklina Subject: Re: [PATCH] KVM: Don't mistreat edge-triggered INIT IPI as INIT de-assert. (LAPIC) Date: Fri, 13 Jan 2012 12:46:19 +0100 Message-ID: <1326455179.9160.7.camel@tabernacle> References: <1324257267-17200-1-git-send-email-js@alien8.de> <20111223104031.GB24308@amt.cnet> <1326388071.12267.13.camel@tabernacle> <20120113105237.GB25595@amt.cnet> Mime-Version: 1.0 Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-jM9y590Nl6oSoMxbKcey" Cc: kvm@vger.kernel.org To: Marcelo Tosatti Return-path: Received: from mail.skyhub.de ([78.46.96.112]:58635 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932068Ab2AMLqZ (ORCPT ); Fri, 13 Jan 2012 06:46:25 -0500 In-Reply-To: <20120113105237.GB25595@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: --=-jM9y590Nl6oSoMxbKcey Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Am Freitag, den 13.01.2012, 08:52 -0200 schrieb Marcelo Tosatti: > On Thu, Jan 12, 2012 at 06:07:51PM +0100, Julian Stecklina wrote: > > Am Freitag, den 23.12.2011, 08:40 -0200 schrieb Marcelo Tosatti: > > > On Mon, Dec 19, 2011 at 02:14:27AM +0100, Julian Stecklina wrote: > > > > If the guest programs an IPI with level=3D0 (de-assert) and trig_mo= de=3D0 (edge), > > > > it is erroneously treated as INIT de-assert and ignored, but to quo= te the > > > > spec: "For this delivery mode [INIT de-assert], the level flag must= be set to > > > > 0 and trigger mode flag to 1." > > >=20 > > > Yes, the implementation ignores INIT de-assert. Quoting the spec: > > >=20 > > > "(INIT Level De-assert) (Not supported in the Pentium 4 and Intel Xeo= n > > > processors.)" > > >=20 > > > Your patch below is not improving the implementation to be closer to = the > > > spec: it'll trigger the INIT state initialization with trig_mode =3D= =3D 0 > > > (which is not in accordance with your spec quote above). > >=20 > > After reading the spec again and consulting with the guy who wrote the > > code triggering this, it seems the whole "if (level)" in the code path > > below is superfluous.=20 >=20 > No. Look at whats inside "if (level)": the mp_state assignment is the > internal implementation of "delivers an INIT request to the target > processor". >=20 > According to the spec, the INIT level de-assert=20 >=20 > "Sends a synchronization message to all the local APICs in the system > to set their arbitration IDs (stored in their Arb ID registers) to the > values of their APIC IDs (see Section 10.7, =E2=80=9CSystem and APIC Bus > Arbitration=E2=80=9D)." >=20 > So if you remove the "if (level)" check, INIT de-assert will be emulated > as INIT! Newer processors don't support INIT level de-assert and will interpret this as INIT. Without the "if (level)" check, KVM would behave in the same way, thus not breaking code that actually runs on real processors. For processors that still supported INIT level de-assert: If you look into older specs (243192), you read: 101 (INIT) ... INIT is treated as an edge triggered interrupt even if programmed otherwise. 101 (INIT Level De-assert) The trigger mode must also be set to 1 and level mode to 0. This means that if you don't set trigger mode to 1, you will get an INIT instead of INIT level de-assert. This is where the current code in KVM is wrong. So with my original patch, KVM would behave like the old spec mandates (check for trigger mode). With the "if (level)" check removed, it would behave like recent processors. Either way, the current code is bogus. Regards, Julian --=-jM9y590Nl6oSoMxbKcey Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (GNU/Linux) iEYEABECAAYFAk8QGYsACgkQ2EtjUdW3H9nMswCgw7x6WcyMiQM29O2hD7a2BIIu SLoAoKkCoUj8/GN7VcgmaEJVgFGq0g9d =m33D -----END PGP SIGNATURE----- --=-jM9y590Nl6oSoMxbKcey--