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: Thu, 12 Jan 2012 18:07:51 +0100 Message-ID: <1326388071.12267.13.camel@tabernacle> References: <1324257267-17200-1-git-send-email-js@alien8.de> <20111223104031.GB24308@amt.cnet> Mime-Version: 1.0 Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-Oi40kwVz3kAnK7R97oD6" Cc: kvm@vger.kernel.org To: Marcelo Tosatti Return-path: Received: from mail.skyhub.de ([78.46.96.112]:49233 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754367Ab2ALRH7 (ORCPT ); Thu, 12 Jan 2012 12:07:59 -0500 In-Reply-To: <20111223104031.GB24308@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: --=-Oi40kwVz3kAnK7R97oD6 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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_mode= =3D0 (edge), > > it is erroneously treated as INIT de-assert and ignored, but to quote t= he > > 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 Xeon > 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). 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. Regarding level my spec says: "This flag has no meaning in Pentium 4 and Intel Xeon processors, and will always be issued as a 1." Judging from Section 10.2 this means every CPU where the LAPICs communicate over the system bus (everything after and including the P4). Thus the if can be removed, since its condition is always true (regardless of what the programmer actually programmed into the level field). If there is no objection, I'll submit a revised patch. > > Signed-off-by: Julian Stecklina > > --- > > arch/x86/kvm/lapic.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > >=20 > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > > index a7f3e65..260770d 100644 > > --- a/arch/x86/kvm/lapic.c > > +++ b/arch/x86/kvm/lapic.c > > @@ -433,7 +433,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic= , int delivery_mode, > > break; > > =20 > > case APIC_DM_INIT: > > - if (level) { > > + if (!trig_mode || level) { > > result =3D 1; > > vcpu->arch.mp_state =3D KVM_MP_STATE_INIT_RECEIVED; > > kvm_make_request(KVM_REQ_EVENT, vcpu); > > --=20 > > 1.7.7.4 > >=20 Regards, Julian --=-Oi40kwVz3kAnK7R97oD6 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) iEYEABECAAYFAk8PE2cACgkQ2EtjUdW3H9ndoQCcDe6M5zUN3CXVkct379W5pl7u EbQAoKdrTU5/KK3GgeS0ZhAZwyl1wGdi =5g30 -----END PGP SIGNATURE----- --=-Oi40kwVz3kAnK7R97oD6--