From patchwork Mon Oct 1 21:25:33 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sean Christopherson X-Patchwork-Id: 10622757 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 044FD17E0 for ; Mon, 1 Oct 2018 21:25:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E6921285CE for ; Mon, 1 Oct 2018 21:25:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DA5B7285D3; Mon, 1 Oct 2018 21:25:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 61BC3285DA for ; Mon, 1 Oct 2018 21:25:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726365AbeJBEFZ (ORCPT ); Tue, 2 Oct 2018 00:05:25 -0400 Received: from mga02.intel.com ([134.134.136.20]:31354 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725994AbeJBEFZ (ORCPT ); Tue, 2 Oct 2018 00:05:25 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Oct 2018 14:25:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,328,1534834800"; d="scan'208";a="262009990" Received: from sjchrist-coffee.jf.intel.com ([10.54.74.193]) by orsmga005.jf.intel.com with ESMTP; 01 Oct 2018 14:25:38 -0700 From: Sean Christopherson To: Paolo Bonzini , =?utf-8?b?UmFkaW0gS3LEjW3DocWZ?= Cc: kvm@vger.kernel.org, Jim Mattson , Gerhard Wiesinger Subject: [PATCH 1/2] KVM: VMX: remove kvm-intel.flexpriority module parameter Date: Mon, 1 Oct 2018 14:25:33 -0700 Message-Id: <20181001212534.20073-2-sean.j.christopherson@intel.com> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20181001212534.20073-1-sean.j.christopherson@intel.com> References: <20181001212534.20073-1-sean.j.christopherson@intel.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Many moons ago, commit 4c9fc8ef5017 ("KVM: VMX: Add module option to disable flexpriority") introduced kvm-intel.flexpriority as it was "Useful for debugging". At that time, kvm-intel.flexpriority only determined whether or not KVM would enable VIRTUALIZE_APIC_ACCESSES. In short, it was intended as a way to disable virtualization of APIC accesses for debug purposes. Nowadays, kvm-intel.flexpriority is a haphazard param that is inconsistently honored by KVM, e.g. it still controls VIRTUALIZE_APIC_ACCESSES but not TPR_SHADOW, CR8-exiting or VIRTUALIZE_X2APIC_MODE, and only for non-nested guests. Disabling the param also announces support for KVM_TPR_ACCESS_REPORTING (via KVM_CAP_VAPIC), which may be functionally desirable, e.g. Qemu uses KVM_TPR_ACCESS_REPORTING only to patch MMIO APIC access, but isn't exactly accurate given its name since KVM may not intercept/report TPR accesses via CR8 or MSR. Remove kvm-intel.flexpriority as its usefulness for debug is dubious given the current code base, while its existence is confusing and can complicate code changes and/or lead to new bugs being introduced. For example, as of commit 8d860bbeedef ("kvm: vmx: Basic APIC virtualization controls have three settings"), KVM will disable VIRTUALIZE_APIC_ACCESSES when a nested guest writes APIC_BASE MSR and kvm-intel.flexpriority=0, whereas previously KVM would allow a nested guest to enable VIRTUALIZE_APIC_ACCESSES so long as it's supported in hardware. I.e. KVM now advertises VIRTUALIZE_APIC_ACCESSES to a guest but doesn't (always) allow setting it when kvm-intel.flexpriority=0, and may even initially allow the control and then clear it when the nested guest writes APIC_BASE MSR, which is decidedly odd even if it doesn't cause functional issues. Fixes: 8d860bbeedef ("kvm: vmx: Basic APIC virtualization controls have three settings") Cc: Jim Mattson Cc: stable@vger.kernel.org Signed-off-by: Sean Christopherson Reviewed-by: Jim Mattson --- .../admin-guide/kernel-parameters.txt | 4 ---- arch/x86/kvm/vmx.c | 19 ++++--------------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 9871e649ffef..670f0b0c6806 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1983,10 +1983,6 @@ [KVM,Intel] Enable emulation of invalid guest states Default is 0 (disabled) - kvm-intel.flexpriority= - [KVM,Intel] Disable FlexPriority feature (TPR shadow). - Default is 1 (enabled) - kvm-intel.nested= [KVM,Intel] Enable VMX nesting (nVMX). Default is 0 (disabled) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 1d26f3c4985b..e1b8ea9a2bc4 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -78,9 +78,6 @@ module_param_named(vpid, enable_vpid, bool, 0444); static bool __read_mostly enable_vnmi = 1; module_param_named(vnmi, enable_vnmi, bool, S_IRUGO); -static bool __read_mostly flexpriority_enabled = 1; -module_param_named(flexpriority, flexpriority_enabled, bool, S_IRUGO); - static bool __read_mostly enable_ept = 1; module_param_named(ept, enable_ept, bool, S_IRUGO); @@ -1857,7 +1854,7 @@ static inline bool cpu_has_vmx_basic_inout(void) static inline bool cpu_need_virtualize_apic_accesses(struct kvm_vcpu *vcpu) { - return flexpriority_enabled && lapic_in_kernel(vcpu); + return cpu_has_vmx_flexpriority() && lapic_in_kernel(vcpu); } static inline bool cpu_has_vmx_vpid(void) @@ -1924,11 +1921,6 @@ static bool vmx_umip_emulated(void) SECONDARY_EXEC_DESC; } -static inline bool report_flexpriority(void) -{ - return flexpriority_enabled; -} - static inline unsigned nested_cpu_vmx_misc_cr3_count(struct kvm_vcpu *vcpu) { return vmx_misc_cr3_count(to_vmx(vcpu)->nested.msrs.misc_low); @@ -7895,9 +7887,6 @@ static __init int hardware_setup(void) if (!cpu_has_vmx_unrestricted_guest() || !enable_ept) enable_unrestricted_guest = 0; - if (!cpu_has_vmx_flexpriority()) - flexpriority_enabled = 0; - if (!cpu_has_virtual_nmis()) enable_vnmi = 0; @@ -7906,7 +7895,7 @@ static __init int hardware_setup(void) * page upon invalidation. No need to do anything if not * using the APIC_ACCESS_ADDR VMCS field. */ - if (!flexpriority_enabled) + if (!cpu_has_vmx_flexpriority()) kvm_x86_ops->set_apic_access_page_addr = NULL; if (!cpu_has_vmx_tpr_shadow()) @@ -10233,7 +10222,7 @@ static void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu) case LAPIC_MODE_DISABLED: break; case LAPIC_MODE_XAPIC: - if (flexpriority_enabled) { + if (cpu_has_vmx_flexpriority()) { sec_exec_control |= SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES; vmx_flush_tlb(vcpu, true); @@ -14007,7 +13996,7 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = { .check_processor_compatibility = vmx_check_processor_compat, .hardware_enable = hardware_enable, .hardware_disable = hardware_disable, - .cpu_has_accelerated_tpr = report_flexpriority, + .cpu_has_accelerated_tpr = cpu_has_vmx_flexpriority, .has_emulated_msr = vmx_has_emulated_msr, .vm_init = vmx_vm_init,