From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] KVM: nVMX: Fix direct injection of interrupts from L0 to L2 Date: Sun, 17 Feb 2013 18:01:05 +0100 Message-ID: <51210CD1.3010208@web.de> References: <511FBD76.8010307@web.de> <20130217150721.GU9817@redhat.com> <5120F7CE.6050905@web.de> <20130217162617.GW9817@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="----enig2SPKOIKNSQQCILQLGNASV" Cc: Marcelo Tosatti , kvm , Nadav Har'El , "Nakajima, Jun" To: Gleb Natapov Return-path: Received: from mout.web.de ([212.227.15.3]:58054 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751155Ab3BQRBO (ORCPT ); Sun, 17 Feb 2013 12:01:14 -0500 In-Reply-To: <20130217162617.GW9817@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2SPKOIKNSQQCILQLGNASV Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2013-02-17 17:26, Gleb Natapov wrote: > On Sun, Feb 17, 2013 at 04:31:26PM +0100, Jan Kiszka wrote: >> On 2013-02-17 16:07, Gleb Natapov wrote: >>> On Sat, Feb 16, 2013 at 06:10:14PM +0100, Jan Kiszka wrote: >>>> From: Jan Kiszka >>>> >>>> If L1 does not set PIN_BASED_EXT_INTR_MASK, we incorrectly skipped >>>> vmx_complete_interrupts on L2 exits. This is required because, with >>>> direct interrupt injection from L0 to L2, L0 has to update its pendi= ng >>>> events. >>>> >>>> Also, we need to allow vmx_cancel_injection when entering L2 in we l= eft >>>> to L0. This condition is indirectly derived from the absence of vali= d >>>> vectoring info in vmcs12. We no explicitly clear it if we find out t= hat >>>> the L2 exit is not targeting L1 but L0. >>>> >>> We really need to overhaul how interrupt injection is emulated in nes= ted >>> VMX. Why not put pending events into event queue instead of >>> get_vmcs12(vcpu)->idt_vectoring_info_field and inject them in usual w= ay. >> >> I was thinking about the same step but felt unsure so far if >> vmx_complete_interrupts & Co. do not include any assumptions about the= >> vmcs configuration that won't match what L1 does. So I went for a >> different path first, specifically to avoid impact on these hairy bits= >> for non-nested mode. >> > Assumption made by those functions should be still correct since guest > VMCS configuration is not applied directly to real HW, but we should be= > careful of course. For instance interrupt queues should be cleared > during nested vmexit and event transfered back to idt_vectoring_info_fi= eld. > IIRC this is how nested SVM works BTW. Checking __vmx_complete_interrupts, the first issue I find is that type 5 (privileged software exception) is not decoded, thus will be lost if L2 leaves this way. That's a reason why it might be better to re-inject the content of vmcs12 if it is valid. VMX is a bit more hairy than SVM, I guess. >=20 > And with you patch you did a half of the job already :) When exiting to= > L0 you transfer event information from get_vmcs12(vcpu)->idt_vectoring_= info_field > to our internal event queues anyway. Hmm, but you do not clear the queu= e > during nested vmexit. So what happens if L2 exits to L0 with an excepti= on > in idt_vectoring_info_field. Now interrupt is delivered so nested vm ex= it > is done, but exception is left in internal queue. I think it will be > delivered into L1 during next vmentry. Indeed. The queue is only cleared on L2->L0 exits (via the late vmx_complete_interrupts). It should be cleared on L2->L1 exists as well. Will fix. Jan ------enig2SPKOIKNSQQCILQLGNASV 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 Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlEhDNUACgkQitSsb3rl5xSGBwCfeZ4VAmKACz54DrZFfqdav7aw 2uoAoN3Ba1vu8ed8RvN+VXq7T4od91Zq =cT/i -----END PGP SIGNATURE----- ------enig2SPKOIKNSQQCILQLGNASV--