Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755827AbcJGMa6 (ORCPT ); Fri, 7 Oct 2016 08:30:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36378 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751222AbcJGMar (ORCPT ); Fri, 7 Oct 2016 08:30:47 -0400 Date: Fri, 7 Oct 2016 14:30:43 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Paolo Bonzini Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Wanpeng Li Subject: Re: [PATCH v2 2/3] KVM: VMX: join functions that disable x2apic msr intercepts Message-ID: <20161007123043.GA20011@potion> References: <20160929204133.1259-1-rkrcmar@redhat.com> <20160929204133.1259-3-rkrcmar@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 07 Oct 2016 12:30:47 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4847 Lines: 142 2016-09-30 10:29+0200, Paolo Bonzini: > On 29/09/2016 22:41, Radim Krčmář wrote: >> for (msr = 0x800; msr <= 0x8ff; msr++) { >> if (msr == 0x839 /* TMCCT */) >> continue; >> - vmx_disable_intercept_msr_read_x2apic(msr, true); >> + vmx_disable_intercept_msr_x2apic(msr, MSR_TYPE_R, true); >> } >> >> /* TPR */ >> - vmx_disable_intercept_msr_write_x2apic(0x808, true); >> + vmx_disable_intercept_msr_x2apic(0x808, MSR_TYPE_W, true); >> /* EOI */ >> - vmx_disable_intercept_msr_write_x2apic(0x80b, true); >> + vmx_disable_intercept_msr_x2apic(0x80b, MSR_TYPE_W, true); >> /* SELF-IPI */ >> - vmx_disable_intercept_msr_write_x2apic(0x83f, true); >> + vmx_disable_intercept_msr_x2apic(0x83f, MSR_TYPE_W, true); >> >> /* >> * (enable_apicv && !kvm_vcpu_apicv_active()) || >> * !enable_apicv >> */ >> /* TPR */ >> - vmx_disable_intercept_msr_read_x2apic(0x808, false); >> - vmx_disable_intercept_msr_write_x2apic(0x808, false); >> + vmx_disable_intercept_msr_x2apic(0x808, MSR_TYPE_R | MSR_TYPE_W, false); > > Alternatively you could place the two function calls for 0x808 together: > > for (msr = 0x800; msr <= 0x8ff; msr++) > vmx_disable_intercept_msr_x2apic(msr, MSR_TYPE_R, true); > > /* > * TPR reads and writes can be virtualized even if virtual interrupt delivery > * is not in use. > */ > vmx_disable_intercept_msr_x2apic(0x808, MSR_TYPE_R | MSR_TYPE_W, false); > vmx_disable_intercept_msr_x2apic(0x808, MSR_TYPE_W, true); > > /* EOI */ > vmx_disable_intercept_msr_x2apic(0x80b, MSR_TYPE_W, true); > > /* SELF-IPI */ > vmx_disable_intercept_msr_x2apic(0x83f, MSR_TYPE_W, true); I have performed this change, thanks, and applied to kvm/queue; please check the complete patch below: ---8<--- vmx_disable_intercept_msr_read_x2apic() and vmx_disable_intercept_msr_write_x2apic() differed only in the type. Pass the type to a new function. [Ordered and commented TPR intercept according to Paolo's suggestion.] Signed-off-by: Radim Krčmář --- arch/x86/kvm/vmx.c | 51 +++++++++++++++------------------------------------ 1 file changed, 15 insertions(+), 36 deletions(-) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 8c83268f6f4c..69ff1be3db7b 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -4656,33 +4656,18 @@ static void vmx_disable_intercept_for_msr(u32 msr, bool longmode_only) msr, MSR_TYPE_R | MSR_TYPE_W); } -static void vmx_disable_intercept_msr_read_x2apic(u32 msr, bool apicv_active) +static void vmx_disable_intercept_msr_x2apic(u32 msr, int type, bool apicv_active) { if (apicv_active) { __vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy_x2apic_apicv, - msr, MSR_TYPE_R); + msr, type); __vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode_x2apic_apicv, - msr, MSR_TYPE_R); + msr, type); } else { __vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy_x2apic, - msr, MSR_TYPE_R); + msr, type); __vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode_x2apic, - msr, MSR_TYPE_R); - } -} - -static void vmx_disable_intercept_msr_write_x2apic(u32 msr, bool apicv_active) -{ - if (apicv_active) { - __vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy_x2apic_apicv, - msr, MSR_TYPE_W); - __vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode_x2apic_apicv, - msr, MSR_TYPE_W); - } else { - __vmx_disable_intercept_for_msr(vmx_msr_bitmap_legacy_x2apic, - msr, MSR_TYPE_W); - __vmx_disable_intercept_for_msr(vmx_msr_bitmap_longmode_x2apic, - msr, MSR_TYPE_W); + msr, type); } } @@ -6463,29 +6448,23 @@ static __init int hardware_setup(void) set_bit(0, vmx_vpid_bitmap); /* 0 is reserved for host */ - /* - * enable_apicv && kvm_vcpu_apicv_active() - */ for (msr = 0x800; msr <= 0x8ff; msr++) { if (msr == 0x839 /* TMCCT */) continue; - vmx_disable_intercept_msr_read_x2apic(msr, true); + vmx_disable_intercept_msr_x2apic(msr, MSR_TYPE_R, true); } - /* TPR */ - vmx_disable_intercept_msr_write_x2apic(0x808, true); - /* EOI */ - vmx_disable_intercept_msr_write_x2apic(0x80b, true); - /* SELF-IPI */ - vmx_disable_intercept_msr_write_x2apic(0x83f, true); - /* - * (enable_apicv && !kvm_vcpu_apicv_active()) || - * !enable_apicv + * TPR reads and writes can be virtualized even if virtual interrupt + * delivery is not in use. */ - /* TPR */ - vmx_disable_intercept_msr_read_x2apic(0x808, false); - vmx_disable_intercept_msr_write_x2apic(0x808, false); + vmx_disable_intercept_msr_x2apic(0x808, MSR_TYPE_W, true); + vmx_disable_intercept_msr_x2apic(0x808, MSR_TYPE_R | MSR_TYPE_W, false); + + /* EOI */ + vmx_disable_intercept_msr_x2apic(0x80b, MSR_TYPE_W, true); + /* SELF-IPI */ + vmx_disable_intercept_msr_x2apic(0x83f, MSR_TYPE_W, true); if (enable_ept) { kvm_mmu_set_mask_ptes(VMX_EPT_READABLE_MASK,