On Sat, Nov 12, 2022 at 12:43:07AM +0000, Sean Christopherson wrote: > On Sat, Nov 12, 2022, Yan Zhao wrote: > > And I'm also not sure if a slots_arch_lock is required for > > kvm_slot_page_track_add_page() and kvm_slot_page_track_remove_page(). > > It's not required. slots_arch_lock protects interaction between memslot updates In kvm_slot_page_track_add_page() and kvm_slot_page_track_remove_page(), slot->arch.gfn_track[mode][index] is updated in update_gfn_track(), do you know which lock is used to protect it? Thanks Yan > mmu_first_shadow_root_alloc(). When CONFIG_KVM_EXTERNAL_WRITE_TRACKING=y, then > the mmu_first_shadow_root_alloc() doesn't touch the memslots because everything > is pre-allocated: > > bool kvm_page_track_write_tracking_enabled(struct kvm *kvm) > { > return IS_ENABLED(CONFIG_KVM_EXTERNAL_WRITE_TRACKING) || > !tdp_enabled || kvm_shadow_root_allocated(kvm); > } > > int kvm_page_track_create_memslot(struct kvm *kvm, > struct kvm_memory_slot *slot, > unsigned long npages) > { > if (!kvm_page_track_write_tracking_enabled(kvm)) <== always true > return 0; > > return __kvm_page_track_write_tracking_alloc(slot, npages); > } > > Though now that you point it out, it's tempting to #ifdef out some of those hooks > so that's basically impossible for mmu_first_shadow_root_alloc() to cause problems. > Not sure the extra #ideffery would be worth while though. > > slots_arch_lock also protects shadow_root_allocated, but that's a KVM-internal > detail that isn't relevant to the page-tracking machinery when > CONFIG_KVM_EXTERNAL_WRITE_TRACKING=y.