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);
