On 02/19/2016 07:51 PM, Paolo Bonzini wrote:
On 14/02/2016 12:31, Xiao Guangrong wrote:
+void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new, + int bytes) +{ + struct kvm_page_track_notifier_head *head; + struct kvm_page_track_notifier_node *n; + int idx; + + head = &vcpu->kvm->arch.track_notifier_head;Please check outside SRCU if the notifier list is empty. If so, there is no need to do the (relatively) expensive srcu_read_lock/unlock.
Good to me. I will check it by calling hlist_empty() first before holding the srcu read lock.
Paolo+ idx = srcu_read_lock(&head->track_srcu); + hlist_for_each_entry_rcu(n, &head->track_notifier_list, node) + if (n->track_write) + n->track_write(vcpu, gpa, new, bytes); + srcu_read_unlock(&head->track_srcu, idx); +} diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index e25ebb7..98019b6 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4370,6 +4370,7 @@ int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa, if (ret < 0) return 0;A kvm_vcpu_mark_page_dirty is missing here, isn't it? I can take care of it, but it would be great if you double-checked this. If so, that should be fixed in stable kernels too.
No. It's already been handled in emulator_write_phys() -> kvm_vcpu_write_guest() -> kvm_vcpu_write_guest_page() -> __kvm_write_guest_page().
Can you add a kvm_vcpu_note_page_write(vcpu, gpa, val, bytes) function that takes care of calling kvm_vcpu_mark_page_dirty, kvm_mmu_pte_write and kvm_page_track-write?
After this patchset, kvm_mmu_pte_write is only a static notifier callback called by kvm_page_track_write(). And the dirty tracking in emulator_write_phys() is handled in a public API (as my explanation above), in emulator_cmpxchg_emulated is handled by itself. So i think it is better to leaving dirty tracking to the separate paths, no? :)
