From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH v4] KVM: nVMX: Fully support of nested VMX preemption timer Date: Mon, 16 Sep 2013 09:18:35 +0200 Message-ID: <5236B0CB.80901@web.de> References: <1378433091-18233-1-git-send-email-yzt356@gmail.com> <5233481F.8060606@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="mtb1RkDKD7DENSQlMDwPPpiwOcCEtSq64" Cc: Paolo Bonzini , kvm , Gleb Natapov , Yang Zhang To: Arthur Chunqi Li Return-path: Received: from mout.web.de ([212.227.15.4]:51877 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750938Ab3IPHSk (ORCPT ); Mon, 16 Sep 2013 03:18:40 -0400 Received: from mchn199C.mchp.siemens.de ([95.157.58.223]) by smtp.web.de (mrweb002) with ESMTPSA (Nemesis) id 0Mf0W1-1VbCeF3QOv-00Oazj for ; Mon, 16 Sep 2013 09:18:39 +0200 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --mtb1RkDKD7DENSQlMDwPPpiwOcCEtSq64 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 2013-09-16 07:42, Arthur Chunqi Li wrote: > On Sat, Sep 14, 2013 at 1:15 AM, Paolo Bonzini wr= ote: >> Il 06/09/2013 04:04, Arthur Chunqi Li ha scritto: >>> 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. >>> >>> With this patch, nested VMX preemption timer features are fully >>> supported. >>> >>> Signed-off-by: Arthur Chunqi Li >>> --- >>> ChangeLog to v3: >>> Move nested_adjust_preemption_timer to the latest place just before v= menter. >>> Some minor changes. >>> >>> arch/x86/include/uapi/asm/msr-index.h | 1 + >>> arch/x86/kvm/vmx.c | 49 +++++++++++++++++++++++= ++++++++-- >>> 2 files changed, 48 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include= /uapi/asm/msr-index.h >>> index bb04650..b93e09a 100644 >>> --- a/arch/x86/include/uapi/asm/msr-index.h >>> +++ b/arch/x86/include/uapi/asm/msr-index.h >>> @@ -536,6 +536,7 @@ >>> >>> /* MSR_IA32_VMX_MISC bits */ >>> #define MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS (1ULL << 29) >>> +#define MSR_IA32_VMX_MISC_PREEMPTION_TIMER_SCALE 0x1F >>> /* AMD-V MSRs */ >>> >>> #define MSR_VM_CR 0xc0010114 >>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>> index 1f1da43..f364d16 100644 >>> --- a/arch/x86/kvm/vmx.c >>> +++ b/arch/x86/kvm/vmx.c >>> @@ -374,6 +374,8 @@ struct nested_vmx { >>> */ >>> struct page *apic_access_page; >>> u64 msr_ia32_feature_control; >>> + /* Set if vmexit is L2->L1 */ >>> + bool nested_vmx_exit; >>> }; >>> >>> #define POSTED_INTR_ON 0 >>> @@ -2204,7 +2206,17 @@ static __init void nested_vmx_setup_ctls_msrs(= void) >>> #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; >>> + if (!(nested_vmx_pinbased_ctls_high & >>> + PIN_BASED_VMX_PREEMPTION_TIMER) || >>> + !(nested_vmx_exit_ctls_high & >>> + VM_EXIT_SAVE_VMX_PREEMPTION_TIMER)) { >> >> Align this under the other "!". Also, I prefer to have one long line >> for the whole "!(... & ...) ||" (and likewise below), but I don't know= >> if Gleb agrees >> >>> + nested_vmx_exit_ctls_high &=3D >>> + (~VM_EXIT_SAVE_VMX_PREEMPTION_TIMER); >> >> Please remove parentheses around ~VM_EXIT_SAVE_VMX_PREEMPTION_TIMER, a= nd >> likewise elsewhere in the patch. >> >>> + nested_vmx_pinbased_ctls_high &=3D >>> + (~PIN_BASED_VMX_PREEMPTION_TIMER); >>> + } >>> nested_vmx_exit_ctls_high |=3D (VM_EXIT_ALWAYSON_WITHOUT_TRUE_M= SR | >>> VM_EXIT_LOAD_IA32_EFER); >>> >>> @@ -6707,6 +6719,24 @@ static void vmx_get_exit_info(struct kvm_vcpu = *vcpu, u64 *info1, u64 *info2) >>> *info2 =3D vmcs_read32(VM_EXIT_INTR_INFO); >>> } >>> >>> +static void nested_adjust_preemption_timer(struct kvm_vcpu *vcpu) >>> +{ >>> + u64 delta_tsc_l1; >>> + u32 preempt_val_l1, preempt_val_l2, preempt_scale; >> >> Should this exit immediately if the preemption timer pin-based control= >> is disabled? > Hi Paolo, > How can I get pin-based control here from "struct kvm_vcpu *vcpu"? You can find get_vmcs12(vcpu)->pin_based_vm_exec_control in existing code= =2E Jan --mtb1RkDKD7DENSQlMDwPPpiwOcCEtSq64 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/ iEYEARECAAYFAlI2sM4ACgkQitSsb3rl5xTjGQCeOJbiMsOCUxUwTtje9sxkWSzB lhsAn2v+kLTs/mDnWdU7o3pCIG29T1TY =g4q+ -----END PGP SIGNATURE----- --mtb1RkDKD7DENSQlMDwPPpiwOcCEtSq64--