Re: [PATCH v4 21/30] KVM: x86/mmu: Zap invalidated roots via asynchronous worker

[Sean Christopherson]

On Thu, Mar 03, 2022, Sean Christopherson wrote:
> On Thu, Mar 03, 2022, Paolo Bonzini wrote:
> > The only issue is that kvm_tdp_mmu_invalidate_all_roots() now assumes that
> > there is at least one reference in kvm->users_count; so if the VM is dying
> > just go through the slow path, as there is nothing to gain by using the fast
> > zapping.
> 
> This isn't actually implemented. :-)

Oh, and when you implement it (or copy paste), can you also add a lockdep and a
comment about the check being racy, but that the race is benign?  It took me a
second to realize why it's safe to use a work queue without holding a reference
to @kvm.

static void kvm_mmu_zap_all_fast(struct kvm *kvm)
{
	lockdep_assert_held(&kvm->slots_lock);

	/*
	 * Zap using the "slow" path if the VM is being destroyed.  The "slow"
	 * path isn't actually slower, it just just doesn't block vCPUs for an
	 * extended duration, which is irrelevant if the VM is dying.
	 *
	 * Note, this doesn't guarantee users_count won't go to '0' immediately
	 * after this check, but that race is benign as callers that don't hold
	 * a reference to @kvm must hold kvm_lock to prevent use-after-free.
	 */
	if (unlikely(refcount_read(&kvm->users_count)) {
		lockdep_assert_held(&kvm_lock);
		kvm_mmu_zap_all(kvm);
		return;
	}

	write_lock(&kvm->mmu_lock);
	trace_kvm_mmu_zap_all_fast(kvm);