From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liran Alon Subject: Re: [PATCH] KVM: nVMX: Fix nested APICv Secondary CPU Controls when apicv disabled Date: Mon, 13 Nov 2017 10:51:13 +0200 Message-ID: <5A095D01.2030104@ORACLE.COM> References: <20171112163102.139877-1-arbel.moshe@oracle.com> <4c68d179-b4c4-84c2-6f64-bf55de8f065f@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: idan.brown@ORACLE.COM, Krish Sadhukhan To: Paolo Bonzini , Arbel Moshe , rkrcmar@redhat.com, kvm@vger.kernel.org Return-path: Received: from userp1040.oracle.com ([156.151.31.81]:25250 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751585AbdKMIvU (ORCPT ); Mon, 13 Nov 2017 03:51:20 -0500 In-Reply-To: <4c68d179-b4c4-84c2-6f64-bf55de8f065f@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 13/11/17 10:36, Paolo Bonzini wrote: > On 12/11/2017 17:31, Arbel Moshe wrote: >> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES | >> SECONDARY_EXEC_DESC | >> SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE | >> - SECONDARY_EXEC_APIC_REGISTER_VIRT | >> - SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY | >> SECONDARY_EXEC_WBINVD_EXITING; >> >> + if (kvm_vcpu_apicv_active(&vmx->vcpu)) { >> + vmx->nested.nested_vmx_secondary_ctls_high |= >> + (SECONDARY_EXEC_APIC_REGISTER_VIRT | >> + SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY); >> + } >> + > > I think kvm_vcpu_apicv_active may change after > nested_vmx_setup_ctls_msrs is called. You need to clear the bits in > refresh_apicv_exec_ctrl. Agreed. Seems this is called from kvm_vcpu_deactivate_apicv() which is only called from kvm_hv_activate_synic() which enables Hyper-V SynIC. However, in case Hyper-V SynIC is not enabled, QEMU will never issue ioctl that invokes kvm_vcpu_deactivate_apicv() and therefore refresh_apicv_exec_ctrl() won't be called. Therefore, we suggest the following: 1. Keeping the code Arbel added to nested_vmx_setup_ctls_msrs(). 2. Adding clearing of relevant bits also to refresh_apicv_exec_ctrl(). 3. Fix bug of not also clearing PIN_BASED_POSTED_INTR from the VMCS & nested_vmx_pinbased_ctls_high in refresh_apicv_exec_ctrl(). Arbel will fix these in v2 of this series. Thanks. -Liran > > Thanks, > > Paolo >