On Tue, May 06, 2025 at 05:05:50PM -0700, Sean Christopherson wrote:
> > + if ((old_flags ^ new_flags) & KVM_MEM_USERFAULT &&
> > + (change == KVM_MR_FLAGS_ONLY)) {
> > + if (old_flags & KVM_MEM_USERFAULT)
> > + kvm_mmu_recover_huge_pages(kvm, new);
> > + else
> > + kvm_arch_flush_shadow_memslot(kvm, old);
>
> The call to kvm_arch_flush_shadow_memslot() should definitely go in common code.
> The fancy recovery logic is arch specific, but blasting the memslot when userfault
> is toggled on is not.
Not like anything in KVM is consistent but sprinkling translation
changes / invalidations between arch and generic code feels
error-prone. Especially if there isn't clear ownership of a particular
flag, e.g. 0 -> 1 transitions happen in generic code and 1 -> 0 happens
in arch code.
Even in the case of KVM_MEM_USERFAULT, an architecture could potentially
preserve the stage-2 translations but reap access permissions without
modifying page tables / TLBs.
I'm happy with arch interfaces that clearly express intent (make this
memslot inaccessible), then the architecture can make an informed
decision about how to best achieve that. Otherwise we're always going to
use the largest possible hammer potentially overinvalidate.
Thanks,
Oliver
