On Tue, Aug 20, 2024 at 10:06:00AM -0700, Sean Christopherson wrote:
> On Tue, Aug 20, 2024, Will Deacon wrote:
> > On Tue, Aug 20, 2024 at 09:07:22AM -0700, Sean Christopherson wrote:
> > > On Tue, Aug 20, 2024, Will Deacon wrote:
> > > > handler could do the invalidation as part of its page-table walk (for
> > > > example, it could use information about the page-table structure such
> > > > as the level of the leaves to optimise the invalidation further), but
> > > > this does at least avoid zapping the whole VMID on CPUs with range
> > > > support.
> > > >
> > > > My only slight concern is that, should clear_flush_young() be extended
> > > > to operate on more than a single page-at-a-time in future, this will
> > > > silently end up invalidating the entire VMID for each memslot unless we
> > > > teach kvm_arch_flush_remote_tlbs_range() to return !0 in that case.
> > >
> > > I'm not sure I follow the "entire VMID for each memslot" concern. Are you
> > > worried about kvm_arch_flush_remote_tlbs_range() failing and triggering a VM-wide
> > > flush?
> >
> > The arm64 implementation of kvm_arch_flush_remote_tlbs_range()
> > unconditionally returns 0, so we could end up over-invalidating pretty
> > badly if that doesn't change. It should be straightforward to fix, but
> > I just wanted to point it out because it would be easy to miss too!
>
> Sorry, I'm still not following. 0==success, and gfn_range.{start,end} is scoped
> precisely to the overlap between the memslot and hva range. Regardless of the
> number of pages that are passed into clear_flush_young(), KVM should naturally
> flush only the exact range being aged. The only hiccup would be if the hva range
> straddles multiple memslots, but if userspace creates multiple memslots for a
> single vma, then that's a userspace problem.
Fair enough, but it's not a lot of effort to fix this (untested diff
below) and if the code were to change in future so that
__kvm_handle_hva_range() was more commonly used to span multiple
memslots we probably wouldn't otherwise notice the silent
over-invalidation for a while.
Will
--->8
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 6981b1bc0946..1e34127f79b0 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -175,6 +175,9 @@ int kvm_arch_flush_remote_tlbs(struct kvm *kvm)
int kvm_arch_flush_remote_tlbs_range(struct kvm *kvm,
gfn_t gfn, u64 nr_pages)
{
+ if (!system_supports_tlb_range())
+ return -EOPNOTSUPP;
+
kvm_tlb_flush_vmid_range(&kvm->arch.mmu,
gfn << PAGE_SHIFT, nr_pages << PAGE_SHIFT);
return 0;
