On Wed, Mar 31, 2021 at 3:27 PM Sean Christopherson <[email protected]> wrote:
>
> On Wed, Mar 31, 2021, Ben Gardon wrote:
> > Provide a real mechanism for fast invalidation by marking roots as
> > invalid so that their reference count will quickly fall to zero
> > and they will be torn down.
> >
> > One negative side affect of this approach is that a vCPU thread will
> > likely drop the last reference to a root and be saddled with the work of
> > tearing down an entire paging structure. This issue will be resolved in
> > a later commit.
> >
> > Signed-off-by: Ben Gardon <[email protected]>
> > ---
>
> ...
>
> > +/*
> > + * This function depends on running in the same MMU lock cirical section as
> > + * kvm_reload_remote_mmus. Since this is in the same critical section, no
> > new
> > + * roots will be created between this function and the MMU reload signals
> > + * being sent.
>
> Eww. That goes beyond just adding a lockdep assertion here. I know you want
> to
> isolate the TDP MMU as much as possible, but this really feels like it should
> be
> open coded in kvm_mmu_zap_all_fast(). And assuming this lands after as_id is
> added to for_each_tdp_mmu_root(), it's probably easier to open code anyways,
> e.g.
> use list_for_each_entry() directly instead of bouncing through an iterator.
Yeah, that's fair. I'll open-code it here. I agree that it will remove
confusion from the function, though it would be nice to be able to use
for_each_tdp_mmu_root for the lockdep and rcu annotations.
>
> > + */
> > +void kvm_tdp_mmu_invalidate_roots(struct kvm *kvm)
> > +{
> > + struct kvm_mmu_page *root;
> > +
> > + for_each_tdp_mmu_root(kvm, root)
> > + root->role.invalid = true;
> > +}
