Hi Paolo: Thanks for review. On 7/18/2018 8:01 PM, Paolo Bonzini wrote: > On 09/07/2018 11:02, Tianyu Lan wrote: >> + /* >> + * Call kvm_arch_hv_tlb_remote first and go back old way when >> + * return failure. >> + */ >> + if (!kvm_arch_hv_flush_remote_tlb(kvm)) >> + return; >> + >> /* >> * Read tlbs_dirty before setting KVM_REQ_TLB_FLUSH in >> * kvm_make_all_cpus_request. >> */ >> - long dirty_count = smp_load_acquire(&kvm->tlbs_dirty); >> + dirty_count = smp_load_acquire(&kvm->tlbs_dirty); > > Also, the call to kvm_arch_flush_remote_tlbs should not replace the > whole function. It should be something like: > > long dirty_count = smp_load_acquire(&kvm->tlbs_dirty); > > /* > * We want to publish modifications to the page tables before reading > * mode. Pairs with a memory barrier in arch-specific code. > * - x86: smp_mb__after_srcu_read_unlock in vcpu_enter_guest > * and smp_mb in walk_shadow_page_lockless_begin/end. > * - powerpc: smp_mb in kvmppc_prepare_to_enter. > * > * There is already an smp_mb__after_atomic() before > * kvm_make_all_cpus_request() reads vcpu->mode. We reuse that > * barrier here. > */ > if (!kvm_arch_hv_flush_remote_tlb(kvm) || > kvm_make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH)) > ++kvm->stat.remote_tlb_flush; > cmpxchg(&kvm->tlbs_dirty, dirty_count, 0); > > Otherwise, kvm_mmu_notifier_invalidate_range_start will incorrectly skip > a TLB flush. Nice catch. Will update in the next version. > > Thanks, > > Paolo >
