From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH v6 01/15] nEPT: Support LOAD_IA32_EFER entry/exit controls for L1 Date: Fri, 02 Aug 2013 09:33:27 +0200 Message-ID: <51FB60C7.1050301@web.de> References: <1375366117-9014-1-git-send-email-gleb@redhat.com> <1375366117-9014-2-git-send-email-gleb@redhat.com> <51FB5333.3040901@web.de> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="GDfCv1n1IrQGkqlr5GLQeV132DhkQQjno" Cc: Gleb Natapov , "kvm@vger.kernel.org" , Xiao Guangrong , "Nakajima, Jun" , "pbonzini@redhat.com" , =?UTF-8?B?IuadjuaYpeWlhyA8QQ==?= =?UTF-8?B?cnRodXIgQ2h1bnFpIExpPiI=?= To: "Zhang, Yang Z" Return-path: Received: from mout.web.de ([212.227.17.12]:54309 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755170Ab3HBHda (ORCPT ); Fri, 2 Aug 2013 03:33:30 -0400 Received: from mchn199C.mchp.siemens.de ([95.157.58.223]) by smtp.web.de (mrweb001) with ESMTPSA (Nemesis) id 0M8hhT-1U9E1j1NER-00wGBM for ; Fri, 02 Aug 2013 09:33:29 +0200 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --GDfCv1n1IrQGkqlr5GLQeV132DhkQQjno Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 2013-08-02 09:27, Zhang, Yang Z wrote: > Jan Kiszka wrote on 2013-08-02: >> On 2013-08-02 05:04, Zhang, Yang Z wrote: >>> Gleb Natapov wrote on 2013-08-01: >>>> From: Nadav Har'El >>>> >>>> Recent KVM, since >>>> http://kerneltrap.org/mailarchive/linux-kvm/2010/5/2/6261577 >>>> switch the EFER MSR when EPT is used and the host and guest have=20 >>>> different NX bits. So if we add support for nested EPT (L1 guest=20 >>>> using EPT to run L2) and want to be able to run recent KVM as L1, we= =20 >>>> need to allow L1 to use this EFER switching feature. >>>> >>>> To do this EFER switching, KVM uses VM_ENTRY/EXIT_LOAD_IA32_EFER if = >>>> available, and if it isn't, it uses the generic=20 >>>> VM_ENTRY/EXIT_MSR_LOAD. This patch adds support for the former (the = >>>> latter is still unsupported). >>>> >>>> Nested entry and exit emulation (prepare_vmcs_02 and=20 >>>> load_vmcs12_host_state, >>>> respectively) already handled VM_ENTRY/EXIT_LOAD_IA32_EFER correctly= =2E >>>> So all that's left to do in this patch is to properly advertise this= =20 >>>> feature to L1. >>>> >>>> Note that vmcs12's VM_ENTRY/EXIT_LOAD_IA32_EFER are emulated by L0, = >>>> by using vmx_set_efer (which itself sets one of several vmcs02=20 >>>> fields), so we always support this feature, regardless of whether=20 >>>> the host supports it. >>>> >>>> Reviewed-by: Orit Wasserman >>>> Signed-off-by: Nadav Har'El >>>> Signed-off-by: Jun Nakajima >>>> Signed-off-by: Xinhao Xu >>>> Signed-off-by: Yang Zhang >>>> Signed-off-by: Gleb Natapov >>>> --- >>>> arch/x86/kvm/vmx.c | 23 ++++++++++++++++------- >>>> 1 file changed, 16 insertions(+), 7 deletions(-) diff --git=20 >>>> a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index e999dc7..27efa6a=20 >>>> 100644 >>>> --- a/arch/x86/kvm/vmx.c >>>> +++ b/arch/x86/kvm/vmx.c >>>> @@ -2198,7 +2198,8 @@ static __init void >>>> nested_vmx_setup_ctls_msrs(void) >>>> #else >>>> nested_vmx_exit_ctls_high =3D 0; >>>> #endif >>>> - nested_vmx_exit_ctls_high |=3D >>>> VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR; >>>> + nested_vmx_exit_ctls_high |=3D >>>> (VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR | >>>> + VM_EXIT_LOAD_IA32_EFER); >>>> >>>> /* entry controls */ >>>> rdmsr(MSR_IA32_VMX_ENTRY_CTLS, >>>> @@ -2207,8 +2208,8 @@ static __init void >>>> nested_vmx_setup_ctls_msrs(void) >>>> nested_vmx_entry_ctls_low =3D VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR; >>>> nested_vmx_entry_ctls_high &=3D VM_ENTRY_LOAD_IA32_PAT | >>>> VM_ENTRY_IA32E_MODE; >>>> - nested_vmx_entry_ctls_high |=3D >>>> VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR; >>>> - >>>> + nested_vmx_entry_ctls_high |=3D >>>> (VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR | >>>> + VM_ENTRY_LOAD_IA32_EFER); >>> Just saw it, we didn't expose bit22 save VMX-preemption timer in=20 >>> vm-exit >> control but we already allowed guest to set active VMX-preemption=20 >> timer in pin based vm-execution conrols. This is wrong. >> >> Does the presence of preemption timer support imply that saving its=20 >> value is also supported? Then we could demand this combination (ie. do= =20 >> not expose preemption timer support at all to L1 if value saving is >> missing) and build our preemption timer emulation on top. >> > I don't see we saved the preemption timer value to vmcs12 in prepare_vm= cs12(). Will it be saved automatically? No. As I said, there is more broken with our preemption timer emulation. Jan >=20 >> There is more broken /wrt VMX preemption timer, patches are welcome. >> Arthur will also try to develop test cases for it. But that topic is=20 >> unrelated to this series. >> >> Jan >> >>> >>>> /* cpu-based controls */ >>>> rdmsr(MSR_IA32_VMX_PROCBASED_CTLS, >>>> nested_vmx_procbased_ctls_low, >>>> nested_vmx_procbased_ctls_high); >>>> @@ -7529,10 +7530,18 @@ static void prepare_vmcs02(struct kvm_vcpu=20 >>>> *vcpu, struct vmcs12 *vmcs12) >>>> vcpu->arch.cr0_guest_owned_bits &=3D ~vmcs12->cr0_guest_host_mask;= >>>> vmcs_writel(CR0_GUEST_HOST_MASK, >>>> ~vcpu->arch.cr0_guest_owned_bits); >>>> >>>> - /* Note: IA32_MODE, LOAD_IA32_EFER are modified by vmx_set_efer >>>> below */ >>>> - vmcs_write32(VM_EXIT_CONTROLS, >>>> - vmcs12->vm_exit_controls | vmcs_config.vmexit_ctrl); >>>> - vmcs_write32(VM_ENTRY_CONTROLS, vmcs12->vm_entry_controls | >>>> + /* L2->L1 exit controls are emulated - the hardware exit is to L0 = so >>>> + * we should use its exit controls. Note that VM_EXIT_LOAD_IA32_EF= ER >>>> + * bits are further modified by vmx_set_efer() below. >>>> + */ >>>> + vmcs_write32(VM_EXIT_CONTROLS, vmcs_config.vmexit_ctrl); >>> Should we mentioned that save vmx preemption bit must use host|guest,= =20 >>> not just host? >>> >>>> + >>>> + /* vmcs12's VM_ENTRY_LOAD_IA32_EFER and VM_ENTRY_IA32E_MODE >>>> are >>>> + * emulated by vmx_set_efer(), below. >>>> + */ >>>> + vmcs_write32(VM_ENTRY_CONTROLS, >>>> + (vmcs12->vm_entry_controls & ~VM_ENTRY_LOAD_IA32_EFER & >>>> + ~VM_ENTRY_IA32E_MODE) | >>>> (vmcs_config.vmentry_ctrl & ~VM_ENTRY_IA32E_MODE)); >>>> =20 >>>> if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_PAT) >>>> -- >>>> 1.7.10.4 >>> >>> Best regards, >>> Yang >> >=20 >=20 > Best regards, > Yang >=20 >=20 --GDfCv1n1IrQGkqlr5GLQeV132DhkQQjno 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/ iEYEARECAAYFAlH7YMcACgkQitSsb3rl5xQsSwCgzr6UVI43LFd0wk2B6uMPxKQk n0MAoKLz4Bdvuw+hhd/NaFJXVpgatt6p =ZsgP -----END PGP SIGNATURE----- --GDfCv1n1IrQGkqlr5GLQeV132DhkQQjno--