[PATCH] KVM: x86: Temporarily drop kvm->srcu when uninitialized vCPU is blocking

[PATCH] KVM: x86: Temporarily drop kvm->srcu when uninitialized vCPU is blocking

[Sean Christopherson]

Temporarily drop kvm->srcu before invoking kvm_vcpu_block() on a vCPU
that hasn't yet been initialized.  Best case scenario, blocking while
holding kvm->srcu will degrade guest performance.  Worst case scenario,
the vCPU will never get a wake event and the VM's tasks will hang
indefinitely on synchronize_srcu(), e.g. when trying update memslots.

E.g. running the "apic" KVM unit test often results in the test hanging

  ==> 6409/stack <==
  [<0>] __synchronize_srcu.part.0+0x7a/0xa0
  [<0>] kvm_swap_active_memslots+0x141/0x180
  [<0>] kvm_set_memslot+0x2f9/0x470
  [<0>] kvm_set_memory_region+0x29/0x40
  [<0>] kvm_vm_ioctl+0x2c3/0xd70
  [<0>] __x64_sys_ioctl+0x83/0xb0
  [<0>] do_syscall_64+0x3b/0xc0
  [<0>] entry_SYSCALL_64_after_hwframe+0x44/0xae

  ==> 6410/stack <==
  [<0>] kvm_vcpu_block+0x36/0x80
  [<0>] kvm_arch_vcpu_ioctl_run+0x17b1/0x1f50
  [<0>] kvm_vcpu_ioctl+0x279/0x690
  [<0>] __x64_sys_ioctl+0x83/0xb0
  [<0>] do_syscall_64+0x3b/0xc0
  [<0>] entry_SYSCALL_64_after_hwframe+0x44/0xae

While it's tempting to never acquire kvm->srcu for an uninitialized vCPU,
practically speaking there's no penalty to acquiring kvm->srcu "early"
as the KVM_MP_STATE_UNINITIALIZED path is a one-time thing per vCPU.  On
the other hand, seemingly innocuous helpers like kvm_apic_accept_events()
and sync_regs() can theoretically reach code that might access
SRCU-protected data structures, e.g. sync_regs() can trigger forced
existing of nested mode via kvm_vcpu_ioctl_x86_set_vcpu_events().

Fixes: 5d8d2bfc5e65 ("KVM: x86: pull kvm->srcu read-side to kvm_arch_vcpu_ioctl_run")
Cc: Like Xu <like.xu.linux@gmail.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/x86.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e55de9b48d1a..8fd60887f38d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10411,7 +10411,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
 		 * use before KVM has ever run the vCPU.
 		 */
 		WARN_ON_ONCE(kvm_lapic_hv_timer_in_use(vcpu));
+
+		srcu_read_unlock(&kvm->srcu, vcpu->srcu_idx);
 		kvm_vcpu_block(vcpu);
+		vcpu->srcu_idx = srcu_read_lock(&kvm->srcu);
+
 		if (kvm_apic_accept_events(vcpu) < 0) {
 			r = 0;
 			goto out;

base-commit: 991f988b43c5ee82ef681907bfe979bee93a55c2
-- 
2.35.1.574.g5d30c73bfb-goog

Re: [PATCH] KVM: x86: Temporarily drop kvm->srcu when uninitialized vCPU is blocking

[Sean Christopherson]

On Thu, Feb 24, 2022, Paolo Bonzini wrote:
> I had just found the same issue and dropped the patch. If it's okay, I will
> squash this and also include your reverts in the next pull request.

That's fine, I got a giggle out of seeing kvm/master force updated :-)

In case you haven't seen it, this needs to be squashed too:

https://lore.kernel.org/all/20220224190609.3464071-1-seanjc@google.com

Re: [PATCH] KVM: x86: Temporarily drop kvm->srcu when uninitialized vCPU is blocking

[Paolo Bonzini]

Queued, thanks.

Paolo

Re: [PATCH] KVM: x86: Temporarily drop kvm->srcu when uninitialized vCPU is blocking

[Paolo Bonzini]

Queued, thanks.

Paolo