On Wed, Feb 03, 2021, Paolo Bonzini wrote: > On 03/02/21 18:46, Ben Gardon wrote: > > enum kvm_mmu_lock_mode lock_mode = > > get_mmu_lock_mode_for_root(vcpu->kvm, vcpu->arch.mmu->root_hpa); > > .... > > kvm_mmu_lock_for_mode(lock_mode); > > > > Not sure if either of those are actually clearer, but the latter > > trends in the direction the RCF took, having an enum to capture > > read/write and whether or not yo yield in a lock mode parameter. > > Could be a possibility. Also: > > enum kvm_mmu_lock_mode lock_mode = > kvm_mmu_lock_for_root(vcpu->kvm, vcpu->arch.mmu->root_hpa); > > kvm_mmu_unlock(vcpu->kvm, lock_mode); > > Anyway it can be done on top. Maybe go with a literal name, unless we expect additional usage? E.g. kvm_mmu_(un)lock_for_page_fault() isn't terrible. I'm not a fan of the kvm_mmu_lock_for_root() variants. "for_root" doesn't have an obvious connection to the page fault handler or to the read/shared mode of the TDP. But, the name is also specific enough to pique my curiosity and make me wonder what's it's doing.