Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756585AbdLTVSJ (ORCPT ); Wed, 20 Dec 2017 16:18:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:13515 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756602AbdLTVRa (ORCPT ); Wed, 20 Dec 2017 16:17:30 -0500 Subject: Re: [PATCH 2/3] KVM: vmx: simplify MSR bitmap setup To: Jim Mattson Cc: LKML , kvm list , David Hildenbrand References: <1513771538-41693-1-git-send-email-pbonzini@redhat.com> <1513771538-41693-3-git-send-email-pbonzini@redhat.com> From: Paolo Bonzini Message-ID: Date: Wed, 20 Dec 2017 22:13:46 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Wed, 20 Dec 2017 21:17:29 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2166 Lines: 51 On 20/12/2017 20:40, Jim Mattson wrote: > This doesn't look right to me. Without APIC-register virtualization, > the only X2APIC MSR intercept that should be disabled is TPR. Of course... The bitmap that has to be outside the "if" is *_x2apic_apicv, not *_x2apic. I sent the wrong version of the series (and this was the only difference, together s/superset/subset/ in the commit message). Will resend tomorrow. Paolo > On Wed, Dec 20, 2017 at 4:05 AM, Paolo Bonzini wrote: >> The APICv-enabled MSR bitmap is a superset of the APICv-disabled bitmap. >> Make that obvious in vmx_disable_intercept_msr_x2apic. >> >> Signed-off-by: Paolo Bonzini >> --- >> arch/x86/kvm/vmx.c | 8 ++------ >> 1 file changed, 2 insertions(+), 6 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 9f9c3194440f..905aaa778306 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -5263,12 +5263,9 @@ static void vmx_disable_intercept_msr_x2apic(u32 msr, int type, bool apicv_activ >> msr, type); >> __vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode_x2apic_apicv, >> msr, type); >> - } else { >> - __vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy_x2apic, >> - msr, type); >> - __vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode_x2apic, >> - msr, type); >> } >> + __vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy_x2apic, msr, type); >> + __vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode_x2apic, msr, type); >> } >> >> static bool vmx_get_enable_apicv(struct kvm_vcpu *vcpu) >> @@ -7148,7 +7145,6 @@ static __init int hardware_setup(void) >> * TPR reads and writes can be virtualized even if virtual interrupt >> * delivery is not in use. >> */ >> - vmx_disable_intercept_msr_x2apic(0x808, MSR_TYPE_W, true); >> vmx_disable_intercept_msr_x2apic(0x808, MSR_TYPE_R | MSR_TYPE_W, false); >> >> /* EOI */ >> -- >> 1.8.3.1 >> >>