From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH 2/5] KVM: nVMX: Enable nested virtualize x2apic mode. Date: Wed, 21 Jan 2015 11:25:10 +0100 Message-ID: <54BF7E86.60205@redhat.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "gleb@kernel.org" , "kvm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Wanpeng Li , Jan Kiszka To: Wincy Van , "Zhang, Yang Z" Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 21/01/2015 11:16, Wincy Van wrote: > On Wed, Jan 21, 2015 at 4:35 PM, Zhang, Yang Z wrote: >> Wincy Van wrote on 2015-01-16: >>> When L2 is using x2apic, we can use virtualize x2apic mode to gain higher >>> performance. >>> >>> This patch also introduces nested_vmx_check_apicv_controls for the nested >>> apicv patches. >>> >>> Signed-off-by: Wincy Van >> >> To enable x2apic, should you to consider the behavior changes to rdmsr and wrmsr. I didn't see your patch do it. Is it correct? > > Yes, indeed, I've not noticed that kvm handle nested msr bitmap > manually, the next version will fix this. > >> BTW, this patch has nothing to do with APICv, it's better to not use x2apic here and change to apicv in following patch. > > Do you mean that we should split this patch from the apicv patch set? I think it's okay to keep it in the same patchset, but you can put it first. Paolo > >> >>> --- >>> arch/x86/kvm/vmx.c | 49 >>> ++++++++++++++++++++++++++++++++++++++++++++++++- >>> 1 files changed, 48 insertions(+), 1 deletions(-) >>> >>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 954dd54..10183ee >>> 100644 >>> --- a/arch/x86/kvm/vmx.c >>> +++ b/arch/x86/kvm/vmx.c >>> @@ -1134,6 +1134,11 @@ static inline bool nested_cpu_has_xsaves(struct >>> vmcs12 *vmcs12) >>> vmx_xsaves_supported(); >>> } >>> >>> +static inline bool nested_cpu_has_virt_x2apic_mode(struct vmcs12 >>> +*vmcs12) { >>> + return nested_cpu_has2(vmcs12, >>> +SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE); >>> +} >>> + >>> static inline bool is_exception(u32 intr_info) { >>> return (intr_info & (INTR_INFO_INTR_TYPE_MASK | >>> INTR_INFO_VALID_MASK)) @@ -2426,6 +2431,7 @@ static void >>> nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx) >>> vmx->nested.nested_vmx_secondary_ctls_low = 0; >>> vmx->nested.nested_vmx_secondary_ctls_high &= >>> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES | >>> + SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE | >>> SECONDARY_EXEC_WBINVD_EXITING | >>> SECONDARY_EXEC_XSAVES; >>> >>> @@ -7333,6 +7339,9 @@ static bool nested_vmx_exit_handled(struct >>> kvm_vcpu *vcpu) >>> case EXIT_REASON_APIC_ACCESS: >>> return nested_cpu_has2(vmcs12, >>> >>> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES); >>> + case EXIT_REASON_APIC_WRITE: >>> + /* apic_write should exit unconditionally. */ >>> + return 1; >> >> APIC_WRITE vmexit is introduced by APIC register virtualization not virtualize x2apic. Move it to next patch. > > Agreed, will do. > > Thanks, > > Wincy >