From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] KVM: nVMX: Fully support of nested VMX preemption timer Date: Sun, 25 Aug 2013 08:44:10 +0200 Message-ID: <5219A7BA.8050602@web.de> References: <1377369850-18583-1-git-send-email-root@Blade1-01.Blade1-01> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="REciA4JDbqNgkgAVactAIEstbfKb1GScv" Cc: kvm@vger.kernel.org, gleb@redhat.com, pbonzini@redhat.com To: =?UTF-8?B?IuadjuaYpeWlhyA8QXJ0aHVyIENodW5xaSBMaT4i?= Return-path: Received: from mout.web.de ([212.227.15.4]:65474 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756040Ab3HYGoR (ORCPT ); Sun, 25 Aug 2013 02:44:17 -0400 Received: from mchn199C.mchp.siemens.de ([95.157.58.223]) by smtp.web.de (mrweb103) with ESMTPSA (Nemesis) id 0McnqH-1VUsLm2Qev-00HtRd for ; Sun, 25 Aug 2013 08:44:15 +0200 In-Reply-To: <1377369850-18583-1-git-send-email-root@Blade1-01.Blade1-01> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --REciA4JDbqNgkgAVactAIEstbfKb1GScv Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 2013-08-24 20:44, root wrote: > This patch contains the following two changes: > 1. Fix the bug in nested preemption timer support. If vmexit L2->L0 > with some reasons not emulated by L1, preemption timer value should > be save in such exits. > 2. Add support of "Save VMX-preemption timer value" VM-Exit controls > to nVMX. >=20 > With this patch, nested VMX preemption timer features are fully > supported. >=20 > Signed-off-by: Arthur Chunqi Li > --- > arch/x86/kvm/vmx.c | 30 +++++++++++++++++++++++++----- > 1 file changed, 25 insertions(+), 5 deletions(-) >=20 > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 57b4e12..9579409 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2204,7 +2204,8 @@ static __init void nested_vmx_setup_ctls_msrs(voi= d) > #ifdef CONFIG_X86_64 > VM_EXIT_HOST_ADDR_SPACE_SIZE | > #endif > - VM_EXIT_LOAD_IA32_PAT | VM_EXIT_SAVE_IA32_PAT; > + VM_EXIT_LOAD_IA32_PAT | VM_EXIT_SAVE_IA32_PAT | > + VM_EXIT_SAVE_VMX_PREEMPTION_TIMER; > nested_vmx_exit_ctls_high |=3D (VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR | > VM_EXIT_LOAD_IA32_EFER); In the absence of VM_EXIT_SAVE_VMX_PREEMPTION_TIMER, you need to hide PIN_BASED_VMX_PREEMPTION_TIMER from the guest as we cannot emulate its behavior properly in that case. > =20 > @@ -7578,9 +7579,14 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu= , struct vmcs12 *vmcs12) > (vmcs_config.pin_based_exec_ctrl | > vmcs12->pin_based_vm_exec_control)); > =20 > - if (vmcs12->pin_based_vm_exec_control & PIN_BASED_VMX_PREEMPTION_TIME= R) > - vmcs_write32(VMX_PREEMPTION_TIMER_VALUE, > - vmcs12->vmx_preemption_timer_value); > + if (vmcs12->pin_based_vm_exec_control & PIN_BASED_VMX_PREEMPTION_TIME= R) { > + if (vmcs12->vm_exit_controls & VM_EXIT_SAVE_VMX_PREEMPTION_TIMER) > + vmcs12->vmx_preemption_timer_value =3D > + vmcs_read32(VMX_PREEMPTION_TIMER_VALUE); > + else > + vmcs_write32(VMX_PREEMPTION_TIMER_VALUE, > + vmcs12->vmx_preemption_timer_value); > + } This is not correct. We still need to set the vmcs to vmx_preemption_timer_value. The difference is that, on exit from L2, vmx_preemption_timer_value has to be updated according to the saved hardware state. The corresponding code is missing in your patch so far. > =20 > /* > * Whether page-faults are trapped is determined by a combination of > @@ -7690,7 +7696,11 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu= , struct vmcs12 *vmcs12) > * we should use its exit controls. Note that VM_EXIT_LOAD_IA32_EFER > * bits are further modified by vmx_set_efer() below. > */ > - vmcs_write32(VM_EXIT_CONTROLS, vmcs_config.vmexit_ctrl); > + if (vmcs12->pin_based_vm_exec_control & PIN_BASED_VMX_PREEMPTION_TIME= R) > + vmcs_write32(VM_EXIT_CONTROLS, vmcs_config.vmexit_ctrl | > + VM_EXIT_SAVE_VMX_PREEMPTION_TIMER); > + else > + vmcs_write32(VM_EXIT_CONTROLS, vmcs_config.vmexit_ctrl); Let's prepare the value for VM_EXIT_CONTROLS in a local variable first, then write it to the vmcs. > =20 > /* vmcs12's VM_ENTRY_LOAD_IA32_EFER and VM_ENTRY_IA32E_MODE are > * emulated by vmx_set_efer(), below. > @@ -7912,6 +7922,16 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu,= bool launch) > } > =20 > /* > + * If L2 support PIN_BASED_VMX_PREEMPTION_TIMER, L0 must support > + * VM_EXIT_SAVE_VMX_PREEMPTION_TIMER. > + */ > + if ((vmcs12->pin_based_vm_exec_control & PIN_BASED_VMX_PREEMPTION_TIM= ER) && > + !(nested_vmx_exit_ctls_high & VM_EXIT_SAVE_VMX_PREEMPTION_TIMER)) {= > + nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_CONTROL_FIELD); > + return 1; > + } Nope, the guest is free to run the preemption timer without saving on exits. It may have a valid use case for this, e.g. that it will always reprogram it on entry. > + > + /* > * We're finally done with prerequisite checking, and can start with > * the nested entry. > */ >=20 Jan --REciA4JDbqNgkgAVactAIEstbfKb1GScv 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/ iEYEARECAAYFAlIZp74ACgkQitSsb3rl5xTDMQCg4giRbjKxFCBDLlvF1IItD5XM znsAmwcia9VNw0Slpb8Lu/soN5jK8wQT =SzRt -----END PGP SIGNATURE----- --REciA4JDbqNgkgAVactAIEstbfKb1GScv--