linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Marc Zyngier <maz@kernel.org>,
	Huacai Chen <chenhuacai@kernel.org>,
	Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>,
	Anup Patel <anup@brainfault.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Christian Borntraeger <borntraeger@linux.ibm.com>,
	Janosch Frank <frankja@linux.ibm.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>,
	Matthew Rosato <mjrosato@linux.ibm.com>,
	Eric Farman <farman@linux.ibm.com>,
	Sean Christopherson <seanjc@google.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Paul Durrant <paul@xen.org>
Cc: "James Morse" <james.morse@arm.com>,
	"Alexandru Elisei" <alexandru.elisei@arm.com>,
	"Suzuki K Poulose" <suzuki.poulose@arm.com>,
	"Oliver Upton" <oliver.upton@linux.dev>,
	"Atish Patra" <atishp@atishpatra.org>,
	"David Hildenbrand" <david@redhat.com>,
	kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.linux.dev, kvmarm@lists.cs.columbia.edu,
	linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org,
	linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Yuan Yao" <yuan.yao@intel.com>,
	"Cornelia Huck" <cohuck@redhat.com>,
	"Isaku Yamahata" <isaku.yamahata@intel.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Fabiano Rosas" <farosas@linux.ibm.com>,
	"Michael Ellerman" <mpe@ellerman.id.au>,
	"Kai Huang" <kai.huang@intel.com>,
	"Chao Gao" <chao.gao@intel.com>,
	"Thomas Gleixner" <tglx@linutronix.de>
Subject: [PATCH v2 47/50] KVM: Make hardware_enable_failed a local variable in the "enable all" path
Date: Wed, 30 Nov 2022 23:09:31 +0000	[thread overview]
Message-ID: <20221130230934.1014142-48-seanjc@google.com> (raw)
In-Reply-To: <20221130230934.1014142-1-seanjc@google.com>

From: Isaku Yamahata <isaku.yamahata@intel.com>

Rework detecting hardware enabling errors to use a local variable in the
"enable all" path to track whether or not enabling was successful across
all CPUs.  Using a global variable complicates paths that enable hardware
only on the current CPU, e.g. kvm_resume() and kvm_online_cpu().

Opportunistically add a WARN if hardware enabling fails during
kvm_resume(), KVM is all kinds of hosed if CPU0 fails to enable hardware.
The WARN is largely futile in the current code, as KVM BUG()s on spurious
faults on VMX instructions, e.g. attempting to run a vCPU on CPU if
hardware enabling fails will explode.

  ------------[ cut here ]------------
  kernel BUG at arch/x86/kvm/x86.c:508!
  invalid opcode: 0000 [#1] SMP
  CPU: 3 PID: 1009 Comm: CPU 4/KVM Not tainted 6.1.0-rc1+ #11
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
  RIP: 0010:kvm_spurious_fault+0xa/0x10
  Call Trace:
   vmx_vcpu_load_vmcs+0x192/0x230 [kvm_intel]
   vmx_vcpu_load+0x16/0x60 [kvm_intel]
   kvm_arch_vcpu_load+0x32/0x1f0
   vcpu_load+0x2f/0x40
   kvm_arch_vcpu_ioctl_run+0x19/0x9d0
   kvm_vcpu_ioctl+0x271/0x660
   __x64_sys_ioctl+0x80/0xb0
   do_syscall_64+0x2b/0x50
   entry_SYSCALL_64_after_hwframe+0x46/0xb0

But, the WARN may provide a breadcrumb to understand what went awry, and
someday KVM may fix one or both of those bugs, e.g. by finding a way to
eat spurious faults no matter the context (easier said than done due to
side effects of certain operations, e.g. Intel's VMCLEAR).

Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
[sean: rebase, WARN on failure in kvm_resume()]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 virt/kvm/kvm_main.c | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index c1e48c18e2d9..674a9dab5411 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -104,7 +104,6 @@ LIST_HEAD(vm_list);
 
 static DEFINE_PER_CPU(bool, hardware_enabled);
 static int kvm_usage_count;
-static atomic_t hardware_enable_failed;
 
 static struct kmem_cache *kvm_vcpu_cache;
 
@@ -5025,19 +5024,25 @@ static struct miscdevice kvm_dev = {
 	&kvm_chardev_ops,
 };
 
-static void hardware_enable_nolock(void *junk)
+static int __hardware_enable_nolock(void)
 {
 	if (__this_cpu_read(hardware_enabled))
-		return;
+		return 0;
 
 	if (kvm_arch_hardware_enable()) {
-		atomic_inc(&hardware_enable_failed);
 		pr_info("kvm: enabling virtualization on CPU%d failed\n",
 			raw_smp_processor_id());
-		return;
+		return -EIO;
 	}
 
 	__this_cpu_write(hardware_enabled, true);
+	return 0;
+}
+
+static void hardware_enable_nolock(void *failed)
+{
+	if (__hardware_enable_nolock())
+		atomic_inc(failed);
 }
 
 static int kvm_online_cpu(unsigned int cpu)
@@ -5050,16 +5055,8 @@ static int kvm_online_cpu(unsigned int cpu)
 	 * errors when scheduled to this CPU.
 	 */
 	mutex_lock(&kvm_lock);
-	if (kvm_usage_count) {
-		WARN_ON_ONCE(atomic_read(&hardware_enable_failed));
-
-		hardware_enable_nolock(NULL);
-
-		if (atomic_read(&hardware_enable_failed)) {
-			atomic_set(&hardware_enable_failed, 0);
-			ret = -EIO;
-		}
-	}
+	if (kvm_usage_count)
+		ret = __hardware_enable_nolock();
 	mutex_unlock(&kvm_lock);
 	return ret;
 }
@@ -5107,6 +5104,7 @@ static void hardware_disable_all(void)
 
 static int hardware_enable_all(void)
 {
+	atomic_t failed = ATOMIC_INIT(0);
 	int r = 0;
 
 	/*
@@ -5122,10 +5120,9 @@ static int hardware_enable_all(void)
 
 	kvm_usage_count++;
 	if (kvm_usage_count == 1) {
-		atomic_set(&hardware_enable_failed, 0);
-		on_each_cpu(hardware_enable_nolock, NULL, 1);
+		on_each_cpu(hardware_enable_nolock, &failed, 1);
 
-		if (atomic_read(&hardware_enable_failed)) {
+		if (atomic_read(&failed)) {
 			hardware_disable_all_nolock();
 			r = -EBUSY;
 		}
@@ -5759,7 +5756,7 @@ static void kvm_resume(void)
 	lockdep_assert_irqs_disabled();
 
 	if (kvm_usage_count)
-		hardware_enable_nolock(NULL);
+		WARN_ON_ONCE(__hardware_enable_nolock());
 }
 
 static struct syscore_ops kvm_syscore_ops = {
-- 
2.38.1.584.g0f3c55d4c2-goog


  parent reply	other threads:[~2022-11-30 23:17 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-30 23:08 [PATCH v2 00/50] KVM: Rework kvm_init() and hardware enabling Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 01/50] KVM: Register /dev/kvm as the _very_ last thing during initialization Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 02/50] KVM: Initialize IRQ FD after arch hardware setup Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 03/50] KVM: Allocate cpus_hardware_enabled " Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 04/50] KVM: Teardown VFIO ops earlier in kvm_exit() Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 05/50] KVM: s390: Unwind kvm_arch_init() piece-by-piece() if a step fails Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 06/50] KVM: s390: Move hardware setup/unsetup to init/exit Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 07/50] KVM: x86: Do timer initialization after XCR0 configuration Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 08/50] KVM: x86: Move hardware setup/unsetup to init/exit Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 09/50] KVM: Drop arch hardware (un)setup hooks Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 10/50] KVM: VMX: Reset eVMCS controls in VP assist page during hardware disabling Sean Christopherson
2022-12-01 15:42   ` Vitaly Kuznetsov
2022-11-30 23:08 ` [PATCH v2 11/50] KVM: VMX: Don't bother disabling eVMCS static key on module exit Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 12/50] KVM: VMX: Move Hyper-V eVMCS initialization to helper Sean Christopherson
2022-12-01 15:22   ` Vitaly Kuznetsov
2022-11-30 23:08 ` [PATCH v2 13/50] KVM: x86: Move guts of kvm_arch_init() to standalone helper Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 14/50] KVM: VMX: Do _all_ initialization before exposing /dev/kvm to userspace Sean Christopherson
2022-11-30 23:08 ` [PATCH v2 15/50] KVM: x86: Serialize vendor module initialization (hardware setup) Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 16/50] KVM: arm64: Simplify the CPUHP logic Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 17/50] KVM: arm64: Free hypervisor allocations if vector slot init fails Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 18/50] KVM: arm64: Unregister perf callbacks if hypervisor finalization fails Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 19/50] KVM: arm64: Do arm/arch initialization without bouncing through kvm_init() Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 20/50] KVM: arm64: Mark kvm_arm_init() and its unique descendants as __init Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 21/50] KVM: MIPS: Hardcode callbacks to hardware virtualization extensions Sean Christopherson
2022-12-01 22:00   ` Philippe Mathieu-Daudé
2022-12-01 22:49     ` Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 22/50] KVM: MIPS: Setup VZ emulation? directly from kvm_mips_init() Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 23/50] KVM: MIPS: Register die notifier prior to kvm_init() Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 24/50] KVM: RISC-V: Do arch init directly in riscv_kvm_init() Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 25/50] KVM: RISC-V: Tag init functions and data with __init, __ro_after_init Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 26/50] KVM: PPC: Move processor compatibility check to module init Sean Christopherson
2022-12-01  5:21   ` Michael Ellerman
2022-12-01 16:38     ` Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 27/50] KVM: s390: Do s390 specific init without bouncing through kvm_init() Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 28/50] KVM: s390: Mark __kvm_s390_init() and its descendants as __init Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 29/50] KVM: Drop kvm_arch_{init,exit}() hooks Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 30/50] KVM: VMX: Make VMCS configuration/capabilities structs read-only after init Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 31/50] KVM: x86: Do CPU compatibility checks in x86 code Sean Christopherson
2022-12-02 12:16   ` Huang, Kai
2022-12-05 20:52   ` Isaku Yamahata
2022-12-05 21:12     ` Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 32/50] KVM: Drop kvm_arch_check_processor_compat() hook Sean Christopherson
2022-12-02 12:18   ` Huang, Kai
2022-11-30 23:09 ` [PATCH v2 33/50] KVM: x86: Use KBUILD_MODNAME to specify vendor module name Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 34/50] KVM: x86: Unify pr_fmt to use module name for all KVM modules Sean Christopherson
2022-12-01 10:43   ` Paul Durrant
2022-11-30 23:09 ` [PATCH v2 35/50] KVM: VMX: Use current CPU's info to perform "disabled by BIOS?" checks Sean Christopherson
2022-12-02 12:18   ` Huang, Kai
2022-11-30 23:09 ` [PATCH v2 36/50] KVM: x86: Do VMX/SVM support checks directly in vendor code Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 37/50] KVM: VMX: Shuffle support checks and hardware enabling code around Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 38/50] KVM: SVM: Check for SVM support in CPU compatibility checks Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 39/50] KVM: x86: Move CPU compat checks hook to kvm_x86_ops (from kvm_x86_init_ops) Sean Christopherson
2022-12-02 13:01   ` Huang, Kai
2022-12-05 21:04   ` Isaku Yamahata
2022-11-30 23:09 ` [PATCH v2 40/50] KVM: x86: Do compatibility checks when onlining CPU Sean Christopherson
2022-12-02 13:03   ` Huang, Kai
2022-12-02 13:36   ` Huang, Kai
2022-12-02 16:04     ` Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 41/50] KVM: Rename and move CPUHP_AP_KVM_STARTING to ONLINE section Sean Christopherson
2022-12-02 13:06   ` Huang, Kai
2022-12-02 16:08     ` Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 42/50] KVM: Disable CPU hotplug during hardware enabling/disabling Sean Christopherson
2022-12-02 12:59   ` Huang, Kai
2022-12-02 16:31     ` Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 43/50] KVM: Ensure CPU is stable during low level hardware enable/disable Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 44/50] KVM: Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 45/50] KVM: Remove on_each_cpu(hardware_disable_nolock) in kvm_exit() Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 46/50] KVM: Use a per-CPU variable to track which CPUs have enabled virtualization Sean Christopherson
2022-11-30 23:09 ` Sean Christopherson [this message]
2022-11-30 23:09 ` [PATCH v2 48/50] KVM: Register syscore (suspend/resume) ops early in kvm_init() Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 49/50] KVM: Opt out of generic hardware enabling on s390 and PPC Sean Christopherson
2022-11-30 23:09 ` [PATCH v2 50/50] KVM: Clean up error labels in kvm_init() Sean Christopherson
2022-12-02  8:02 ` [PATCH v2 00/50] KVM: Rework kvm_init() and hardware enabling Chao Gao
2022-12-27 13:02 ` Paolo Bonzini
2022-12-28 11:22   ` Marc Zyngier
2022-12-28 11:58     ` Paolo Bonzini
2022-12-29 20:52     ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221130230934.1014142-48-seanjc@google.com \
    --to=seanjc@google.com \
    --cc=aleksandar.qemu.devel@gmail.com \
    --cc=alexandru.elisei@arm.com \
    --cc=anup@brainfault.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=atishp@atishpatra.org \
    --cc=borntraeger@linux.ibm.com \
    --cc=chao.gao@intel.com \
    --cc=chenhuacai@kernel.org \
    --cc=cohuck@redhat.com \
    --cc=david@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=farman@linux.ibm.com \
    --cc=farosas@linux.ibm.com \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=isaku.yamahata@intel.com \
    --cc=james.morse@arm.com \
    --cc=kai.huang@intel.com \
    --cc=kvm-riscv@lists.infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maz@kernel.org \
    --cc=mjrosato@linux.ibm.com \
    --cc=mpe@ellerman.id.au \
    --cc=oliver.upton@linux.dev \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=paul@xen.org \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=suzuki.poulose@arm.com \
    --cc=tglx@linutronix.de \
    --cc=vkuznets@redhat.com \
    --cc=yuan.yao@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).