From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 3/3] KVM: nVMX: check for null vmcs12 when L1 does invept Date: Sat, 22 Mar 2014 12:38:54 +0100 Message-ID: <532D764E.20607@web.de> References: <1395286089-5406-1-git-send-email-bsd@redhat.com> <1395286089-5406-4-git-send-email-bsd@redhat.com> <532AB624.4020400@siemens.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="lO21lbjDslDnKfQg2hWVxUNOp8wput5SG" Cc: kvm@vger.kernel.org, Paolo Bonzini , Gleb Natapov To: Bandan Das Return-path: Received: from mout.web.de ([212.227.15.3]:59505 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750841AbaCVLjB (ORCPT ); Sat, 22 Mar 2014 07:39:01 -0400 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --lO21lbjDslDnKfQg2hWVxUNOp8wput5SG Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable On 2014-03-20 21:58, Bandan Das wrote: > Jan Kiszka writes: >=20 >> On 2014-03-20 04:28, Bandan Das wrote: >>> Some L1 hypervisors such as Xen seem to be calling invept after >>> vmclear or before vmptrld on L2. In this case, proceed with >>> falling through and syncing roots as a case where >>> context wide invalidation can't be supported >> >> Can we also base this behaviour on a statement in the SDM? But on firs= t >> glance, I do not find anything like this over there. >=20 > The SDM has nothing of this sort explicitly mentioned but 28.3.3.1=20 > "Operations that invalidate Cached Mappings" does mention that > the instruction may invalidate mappings associated with other > EP4TAs (even in single context). Yes, "may". So we are implementing undefined behavior in order to please a broken hypervisor that relies on it? Then please state this in the patch and probably also inform Xen about their issue. >=20 > Note that I based this on what we currently do for context invalidation= - > static inline void ept_sync_context(u64 eptp) > { > if (enable_ept) { > if (cpu_has_vmx_invept_context()) > __invept(VMX_EPT_EXTENT_CONTEXT, eptp, 0); > else > ept_sync_global(); > } > } Don't get your point. This test is about testing for the CPU support context invalidating, then falling back to global invalidation if there is no support. Jan >=20 > Seemed easier and cleaner than having a cached eptp after vmcs12 is=20 > long gone :) >=20 > If you prefer, I can modify the commit message to reflect this. >=20 >> Jan >> >>> >>> Signed-off-by: Bandan Das >>> --- >>> arch/x86/kvm/vmx.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>> index c707389..b407b3a 100644 >>> --- a/arch/x86/kvm/vmx.c >>> +++ b/arch/x86/kvm/vmx.c >>> @@ -6435,8 +6435,9 @@ static int handle_invept(struct kvm_vcpu *vcpu)= >>> =20 >>> switch (type) { >>> case VMX_EPT_EXTENT_CONTEXT: >>> - if ((operand.eptp & eptp_mask) !=3D >>> - (nested_ept_get_cr3(vcpu) & eptp_mask)) >>> + if (get_vmcs12(vcpu) && >>> + ((operand.eptp & eptp_mask) !=3D (nested_ept_get_cr3(vcpu) & >>> + eptp_mask))) >>> break; >>> case VMX_EPT_EXTENT_GLOBAL: >>> kvm_mmu_sync_roots(vcpu); >>> --lO21lbjDslDnKfQg2hWVxUNOp8wput5SG 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.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlMtdk4ACgkQitSsb3rl5xTzmACbBRjTd+tmPTFv7EyOnDIxUDIk wXEAnjdc9L3lSIhrkbetWtJpOwn8OrJb =oLDp -----END PGP SIGNATURE----- --lO21lbjDslDnKfQg2hWVxUNOp8wput5SG--