Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932805AbcJGXnf (ORCPT ); Fri, 7 Oct 2016 19:43:35 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:34068 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754882AbcJGXn0 (ORCPT ); Fri, 7 Oct 2016 19:43:26 -0400 MIME-Version: 1.0 In-Reply-To: <20161007123043.GA20011@potion> References: <20160929204133.1259-1-rkrcmar@redhat.com> <20160929204133.1259-3-rkrcmar@redhat.com> <20161007123043.GA20011@potion> From: Wanpeng Li Date: Sat, 8 Oct 2016 07:43:03 +0800 Message-ID: Subject: Re: [PATCH v2 2/3] KVM: VMX: join functions that disable x2apic msr intercepts To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Cc: Paolo Bonzini , "linux-kernel@vger.kernel.org" , kvm Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u97NhcGK028226 Content-Length: 6088 Lines: 149 2016-10-07 20:30 GMT+08:00 Radim Krčmář : > 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.] Looks good to me. :) Regards, Wanpeng Li > 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,