On Tue, Feb 02, 2021, Ben Gardon wrote:
> +static inline bool tdp_mmu_zap_spte_atomic(struct kvm *kvm,
> + struct tdp_iter *iter)
> +{
> + /*
> + * Freeze the SPTE by setting it to a special,
> + * non-present value. This will stop other threads from
> + * immediately installing a present entry in its place
> + * before the TLBs are flushed.
> + */
> + if (!tdp_mmu_set_spte_atomic(kvm, iter, REMOVED_SPTE))
> + return false;
> +
> + kvm_flush_remote_tlbs_with_address(kvm, iter->gfn,
> + KVM_PAGES_PER_HPAGE(iter->level));
> +
> + /*
> + * No other thread can overwrite the removed SPTE as they
> + * must either wait on the MMU lock or use
> + * tdp_mmu_set_spte_atomic which will not overrite the
> + * special removed SPTE value. No bookkeeping is needed
> + * here since the SPTE is going from non-present
> + * to non-present.
> + */
Can we expand these comments out to 80 chars before the final/official push?
> + WRITE_ONCE(*iter->sptep, 0);
> +
> + return true;
> +}
> +
>
> /*
> * __tdp_mmu_set_spte - Set a TDP MMU SPTE and handle the associated
> bookkeeping
> @@ -523,6 +562,15 @@ static inline void __tdp_mmu_set_spte(struct kvm *kvm,
> struct tdp_iter *iter,
>
> lockdep_assert_held_write(&kvm->mmu_lock);
>
> + /*
> + * No thread should be using this function to set SPTEs to the
> + * temporary removed SPTE value.
> + * If operating under the MMU lock in read mode, tdp_mmu_set_spte_atomic
> + * should be used. If operating under the MMU lock in write mode, the
> + * use of the removed SPTE should not be necessary.
> + */
> + WARN_ON(iter->old_spte == REMOVED_SPTE);
> +
> WRITE_ONCE(*rcu_dereference(iter->sptep), new_spte);
>
> __handle_changed_spte(kvm, as_id, iter->gfn, iter->old_spte, new_spte,
> @@ -790,12 +838,9 @@ int kvm_tdp_mmu_map(struct kvm_vcpu *vcpu, gpa_t gpa,
> u32 error_code,
> */
> if (is_shadow_present_pte(iter.old_spte) &&
> is_large_pte(iter.old_spte)) {
> - if (!tdp_mmu_set_spte_atomic(vcpu->kvm, &iter, 0))
> + if (!tdp_mmu_zap_spte_atomic(vcpu->kvm, &iter))
> break;
>
> - kvm_flush_remote_tlbs_with_address(vcpu->kvm, iter.gfn,
> - KVM_PAGES_PER_HPAGE(iter.level));
> -
> /*
> * The iter must explicitly re-read the spte here
> * because the new value informs the !present
> --
> 2.30.0.365.g02bc693789-goog
>
