Re: [PATCH V2 2/5] KVM: Add tlb remote flush callback in kvm_x86_ops.

[Paolo Bonzini]

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.

Thanks,

Paolo