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.
