[v4,14/19] KVM: SVM: Check that the current CPU supports SVM in kvm_is_svm_supported()
Message ID | 20230721201859.2307736-15-seanjc@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9010:0:b0:3e4:2afc:c1 with SMTP id l16csp467708vqg; Fri, 21 Jul 2023 14:13:54 -0700 (PDT) X-Google-Smtp-Source: APBJJlFFvR/GDRtTTCpLZahDm3+R/nGipiK9ebDPxPrSn2VcnFLJ15CyGKz2CXEztYjhqhEGyWo2 X-Received: by 2002:a17:906:2216:b0:989:3e0d:89fb with SMTP id s22-20020a170906221600b009893e0d89fbmr2548112ejs.45.1689974034167; Fri, 21 Jul 2023 14:13:54 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689974034; cv=none; d=google.com; s=arc-20160816; b=r0TGUC+XTuo3okifnF5Ulpywpkeq6+Jc6BXugYuO+toUJdMfcVAubLJgLTnTfMHU72 9rC0m6TN/vV4UsWysAgRNf5rPMMfvodEYhECg0pJ8i0Y4RWJJQ6J/V0xlXGNhkdrPY5Y DY7uyJGBsgJpSWHF5EQrnwTFgxS4eEpqlDBpB/+ApZolU+0MnldDHN9tnfFqyqFIHcl7 q1aMJBvD7+/8AyCkDBcFrss2cwjgX1ZkhXuFZLg3weq+am4pXAyoMWnIWcG6X2Jh/k0V yH0ZCXVoMoZwHM7Xh3cDUTmFyIgG/tP5ISg0akcSmhkCnqsoQnNQyBgJJjmqEv9O/E9G sLFg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:reply-to:dkim-signature; bh=uhoBWFPCBOgYgQs9Wbxf7anLd8UMxgDqXbWpH+bX2KY=; fh=GitwJflddDxrBBVhnTGEzIZYOBTU7RKSqL+OyCDv/0M=; b=Ta7o9RiUgQ/731rvnnZCPt3EoH53EXb73Rdbe7eUhzCGLztASSnpxqptp+Cpujyr9J VEUvP0kFXrqU4PUFrzaB1CbRlHgHFKxDNP6vCw0cafy6PE7zHdvA86yqLsYDeOej1CG8 Q3iD5tfeAfknVKZwueQJMlvpnfDYzGzjOj2jvYCHQuRl8QRZ8htj30+mnXcGTJbj7Oyo IRrpu8h71fqa0kB6v9Q9ud6q0qCf3IfS32l/llP8yVmAt2QIUT0Fs9AuXLZk1L7SQUip iAxT9NKb+fqL2YyxD9fGmhAqPetZK8bPQk9HNis1J483J3eDnjePcX3MOefIxAA+4E9F 6tqg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b=BqJBwjEk; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s11-20020a170906168b00b009942c859e77si2602604ejd.193.2023.07.21.14.13.28; Fri, 21 Jul 2023 14:13:54 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b=BqJBwjEk; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231167AbjGUUUd (ORCPT <rfc822;assdfgzxcv4@gmail.com> + 99 others); Fri, 21 Jul 2023 16:20:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39200 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231439AbjGUUUF (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 21 Jul 2023 16:20:05 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1F5664216 for <linux-kernel@vger.kernel.org>; Fri, 21 Jul 2023 13:19:33 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id 3f1490d57ef6-c5a479bc2d4so2186020276.1 for <linux-kernel@vger.kernel.org>; Fri, 21 Jul 2023 13:19:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1689970772; x=1690575572; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=uhoBWFPCBOgYgQs9Wbxf7anLd8UMxgDqXbWpH+bX2KY=; b=BqJBwjEk+/zhFp4v5DrxQNReQVSoNlh/Bn0hUbsO8ZiEO/etbAIosjlClY7b6ifBBy wi3HDJfJE+jUUceINpJHN6wSTWbqUmyfzZi1k/JQKHiIJGwA7j6u5reIne4DWb5D1UeB p7pcAuNqF4vndeC9wIz/8uWHcddezeQhI8tep358HE0tpXUoAcue+Ghl6YEpRZEzbO/U J3+qH1Zc8yvKpVjIjnIx/75G7nRlmV863pOmIuyo9J3xyzA50nbAtEa0Myyb9v3ipxpH kDIyKNilgiU1a4xzVdhXo9++dIWuAlATjktwg3RONaxNWlAzAQafqJVzTHbU6vE4FJ1e d34g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689970772; x=1690575572; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=uhoBWFPCBOgYgQs9Wbxf7anLd8UMxgDqXbWpH+bX2KY=; b=ioPM0KVZdVzr750WNG+NuaMelMGwhJd6klo0H6CLm7AtcHPPs49o1WIXUuD5ax9uiH anZML967yc8GhY297JQpXwuEBVZZezLGyQOkEb8BaCBtG5cLF9CVw2OapMyXe7eOz5XL 6L1+HNeMYuhot2dt8Gp+Y7IDQZxMpSz75qmaGVC5tk4piVE9P/5eIgVDx3dBVmupoWsX qsZqHS67nExZfNZXEwRZJcCtq1iBJTFK4kspH2F6Mo+VBggVvwgM9agWAQeW1oz+62KU DJLBmryrZsjxQ69N9SEYuslGFR8A+q4Kagvcl3vUvMFuwGx5pqoZAUTnt8OqpyFJhQSB Z1Bw== X-Gm-Message-State: ABy/qLbSm38lnt9zS73T52AvEvPT8+sa/m2sKB83tjTuJtuxTxXvKLDZ RkyYRe3ENsbFRo+bQT+RVSMKmoXjiLg= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a25:40cf:0:b0:cc7:b850:7f2 with SMTP id n198-20020a2540cf000000b00cc7b85007f2mr19598yba.5.1689970772756; Fri, 21 Jul 2023 13:19:32 -0700 (PDT) Reply-To: Sean Christopherson <seanjc@google.com> Date: Fri, 21 Jul 2023 13:18:54 -0700 In-Reply-To: <20230721201859.2307736-1-seanjc@google.com> Mime-Version: 1.0 References: <20230721201859.2307736-1-seanjc@google.com> X-Mailer: git-send-email 2.41.0.487.g6d72f3e995-goog Message-ID: <20230721201859.2307736-15-seanjc@google.com> Subject: [PATCH v4 14/19] KVM: SVM: Check that the current CPU supports SVM in kvm_is_svm_supported() From: Sean Christopherson <seanjc@google.com> To: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org, Sean Christopherson <seanjc@google.com>, Paolo Bonzini <pbonzini@redhat.com> Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Andrew Cooper <Andrew.Cooper3@citrix.com>, Kai Huang <kai.huang@intel.com>, Chao Gao <chao.gao@intel.com> Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1772066212938188462 X-GMAIL-MSGID: 1772066212938188462 |
Series |
x86/reboot: KVM: Clean up "emergency" virt code
|
|
Commit Message
Sean Christopherson
July 21, 2023, 8:18 p.m. UTC
Check "this" CPU instead of the boot CPU when querying SVM support so that the per-CPU checks done during hardware enabling actually function as intended, i.e. will detect issues where SVM isn't support on all CPUs. Disable migration for the use from svm_init() mostly so that the standard accessors for the per-CPU data can be used without getting yelled at by CONFIG_DEBUG_PREEMPT=y sanity checks. Preventing the "disabled by BIOS" error message from reporting the wrong CPU is largely a bonus, as ensuring a stable CPU during module load is a non-goal for KVM. Link: https://lore.kernel.org/all/ZAdxNgv0M6P63odE@google.com Cc: Kai Huang <kai.huang@intel.com> Cc: Chao Gao <chao.gao@intel.com> Reviewed-by: Kai Huang <kai.huang@intel.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/svm/svm.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
Comments
On Fri, Jul 21, 2023 at 01:18:54PM -0700, Sean Christopherson wrote: > Check "this" CPU instead of the boot CPU when querying SVM support so that > the per-CPU checks done during hardware enabling actually function as > intended, i.e. will detect issues where SVM isn't support on all CPUs. Is that a realistic concern?
On Mon, Jul 24, 2023, Peter Zijlstra wrote: > On Fri, Jul 21, 2023 at 01:18:54PM -0700, Sean Christopherson wrote: > > Check "this" CPU instead of the boot CPU when querying SVM support so that > > the per-CPU checks done during hardware enabling actually function as > > intended, i.e. will detect issues where SVM isn't support on all CPUs. > > Is that a realistic concern? It's not a concern in the sense that it should never happen, but I know of at least one example where VMX on Intel completely disappeared[1]. The "compatibility" checks are really more about the entire VMX/SVM feature set, the base VMX/SVM support check is just an easy and obvious precursor to the full compatibility checks. Of course, SVM doesn't currently have compatibility checks on the full SVM feature set, but that's more due to lack of a forcing function than a desire to _not_ have them. Intel CPUs have a pesky habit of bugs, ucode updates, and/or in-field errors resulting in VMX features randomly appearing or disappearing. E.g. there's an ongoing buzilla (sorry) issue[2] where a user is only able to load KVM *after* a suspend+resume cycle, because TSC scaling only shows up on one socket immediately after boot, which is then somehow resolved by suspend+resume. [1] 009bce1df0bb ("x86/split_lock: Don't write MSR_TEST_CTRL on CPUs that aren't whitelisted") [2] https://bugzilla.kernel.org/show_bug.cgi?id=217574
Hi Sean, On Fri, Jul 21, 2023 at 01:18:54PM -0700, Sean Christopherson wrote: > +static bool kvm_is_svm_supported(void) > +{ > + bool supported; > + > + migrate_disable(); > + supported = __kvm_is_svm_supported(); > + migrate_enable(); I am typically very wary of the constructs like this, as the value returned is obsolete the moment migrate_enable() happens. Is value of "svm was supported at some time in the past but may or may not be supported right now" useful and if it is then could you add comment why? Thanks.
On Mon, Jul 24, 2023, Dmitry Torokhov wrote: > Hi Sean, > > On Fri, Jul 21, 2023 at 01:18:54PM -0700, Sean Christopherson wrote: > > +static bool kvm_is_svm_supported(void) > > +{ > > + bool supported; > > + > > + migrate_disable(); > > + supported = __kvm_is_svm_supported(); > > + migrate_enable(); > > I am typically very wary of the constructs like this, as the value > returned is obsolete the moment migrate_enable() happens. Yeah, I don't like this code, but there's no great solution in this case. Or at least, none that I've found. At some point KVM has to enable migration/preemption before "is KVM supported?" is ultimately consumed, as the real consumer is userspace. > Is value of "svm was supported at some time in the past but may or may not be > supported right now" useful and if it is then could you add comment why? No, because barring fatal silicon/ucode/kernel bugs, SVM support isn't expected to disappear or (re)appear). KVM defends against the "disappear" case as much as can be reasonably expected. It's ugly, but functionally ok (not perfect, but ok). KVM doesn't actually care which CPU does the initial support check, because KVM will do fully protected support checks on all CPUs before actually letting userspace create VMs. This is why the changelog states that ensuring a stable CPU is a non-goal, and also why the inner helpers don't use the raw accessors. The "(re)appear" case doesn't need to be handled, because userspace could simply retry if it really wanted to (but that would be quite insane/nonsensical, and just asking for problems). I didn't add a comment because VMX uses the exact same pattern, and I didn't want to copy+paste a non-trivial comment. And this is a single use local helper, so I'm not terribly concerned about it being misused. That said, I'll see if I can find a common, intuitive location to document this.
On Mon, Jul 24, 2023 at 02:40:03PM -0700, Sean Christopherson wrote: > On Mon, Jul 24, 2023, Peter Zijlstra wrote: > > On Fri, Jul 21, 2023 at 01:18:54PM -0700, Sean Christopherson wrote: > > > Check "this" CPU instead of the boot CPU when querying SVM support so that > > > the per-CPU checks done during hardware enabling actually function as > > > intended, i.e. will detect issues where SVM isn't support on all CPUs. > > > > Is that a realistic concern? > > It's not a concern in the sense that it should never happen, but I know of at > least one example where VMX on Intel completely disappeared[1]. The "compatibility" > checks are really more about the entire VMX/SVM feature set, the base VMX/SVM > support check is just an easy and obvious precursor to the full compatibility > checks. > > Of course, SVM doesn't currently have compatibility checks on the full SVM feature > set, but that's more due to lack of a forcing function than a desire to _not_ have > them. Intel CPUs have a pesky habit of bugs, ucode updates, and/or in-field errors > resulting in VMX features randomly appearing or disappearing. E.g. there's an > ongoing buzilla (sorry) issue[2] where a user is only able to load KVM *after* a > suspend+resume cycle, because TSC scaling only shows up on one socket immediately > after boot, which is then somehow resolved by suspend+resume. > > [1] 009bce1df0bb ("x86/split_lock: Don't write MSR_TEST_CTRL on CPUs that aren't whitelisted") > [2] https://bugzilla.kernel.org/show_bug.cgi?id=217574 Is that using late loading of ucode? Anything that changes *any* feature flag must be early ucode load, there is no other possible way since einux does feature enumeration early, and features are fixed thereafter. This is one of the many reasons late loading is a trainwreck. Doing suspend/resume probably re-loads the firmware and re-does the feature enumeration -- I didn't check. Also, OMG don't you just love computers :/
On Tue, Jul 25, 2023, Peter Zijlstra wrote: > On Mon, Jul 24, 2023 at 02:40:03PM -0700, Sean Christopherson wrote: > > On Mon, Jul 24, 2023, Peter Zijlstra wrote: > > > On Fri, Jul 21, 2023 at 01:18:54PM -0700, Sean Christopherson wrote: > > > > Check "this" CPU instead of the boot CPU when querying SVM support so that > > > > the per-CPU checks done during hardware enabling actually function as > > > > intended, i.e. will detect issues where SVM isn't support on all CPUs. > > > > > > Is that a realistic concern? > > > > It's not a concern in the sense that it should never happen, but I know of at > > least one example where VMX on Intel completely disappeared[1]. The "compatibility" > > checks are really more about the entire VMX/SVM feature set, the base VMX/SVM > > support check is just an easy and obvious precursor to the full compatibility > > checks. > > > > Of course, SVM doesn't currently have compatibility checks on the full SVM feature > > set, but that's more due to lack of a forcing function than a desire to _not_ have > > them. Intel CPUs have a pesky habit of bugs, ucode updates, and/or in-field errors > > resulting in VMX features randomly appearing or disappearing. E.g. there's an > > ongoing buzilla (sorry) issue[2] where a user is only able to load KVM *after* a > > suspend+resume cycle, because TSC scaling only shows up on one socket immediately > > after boot, which is then somehow resolved by suspend+resume. > > > > [1] 009bce1df0bb ("x86/split_lock: Don't write MSR_TEST_CTRL on CPUs that aren't whitelisted") > > [2] https://bugzilla.kernel.org/show_bug.cgi?id=217574 > > Is that using late loading of ucode? Not sure, though I don't think that is relevant for this particular bug. > Anything that changes *any* feature flag must be early ucode load, there is > no other possible way since einux does feature enumeration early, and > features are fixed thereafter. > > This is one of the many reasons late loading is a trainwreck. > > Doing suspend/resume probably re-loads the firmware Ya, it does. > and re-does the feature enumeration -- I didn't check. The reported ucode revision is the same before and after resume, and is consistent across all CPUs. KVM does the per-CPU feature enumeration (for sanity checks) everytime userspace attempts to load KVM (the module), so the timing of the ucode patch load _shouldn't_ matter. The user is running quite old ucode for their system, so the current theory is that old buggy ucode is to blame.
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index ff6c769aafb2..9e449167e71b 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -518,18 +518,20 @@ static void svm_init_osvw(struct kvm_vcpu *vcpu) vcpu->arch.osvw.status |= 1; } -static bool kvm_is_svm_supported(void) +static bool __kvm_is_svm_supported(void) { - int cpu = raw_smp_processor_id(); + int cpu = smp_processor_id(); + struct cpuinfo_x86 *c = &cpu_data(cpu); + u64 vm_cr; - if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD && - boot_cpu_data.x86_vendor != X86_VENDOR_HYGON) { + if (c->x86_vendor != X86_VENDOR_AMD && + c->x86_vendor != X86_VENDOR_HYGON) { pr_err("CPU %d isn't AMD or Hygon\n", cpu); return false; } - if (!boot_cpu_has(X86_FEATURE_SVM)) { + if (!cpu_has(c, X86_FEATURE_SVM)) { pr_err("SVM not supported by CPU %d\n", cpu); return false; } @@ -548,9 +550,20 @@ static bool kvm_is_svm_supported(void) return true; } +static bool kvm_is_svm_supported(void) +{ + bool supported; + + migrate_disable(); + supported = __kvm_is_svm_supported(); + migrate_enable(); + + return supported; +} + static int svm_check_processor_compat(void) { - if (!kvm_is_svm_supported()) + if (!__kvm_is_svm_supported()) return -EIO; return 0;