Re: [RFC, PATCH 08/12] KVM: x86/tdp_mmu: Add phys_prepare() and phys_cleanup() to kvm_x86_ops

[Chao Gao <chao.gao@intel.com>]

>+static int tdp_mmu_install_spte(struct kvm_vcpu *vcpu,
>+				struct tdp_iter *iter,
>+				u64 spte)
>+{
>+	kvm_pfn_t pfn = 0;
>+	int ret = 0;
>+
>+	if (is_mirror_sptep(iter->sptep) && !is_frozen_spte(spte)) {
>+		pfn = spte_to_pfn(spte);
>+		ret = static_call(kvm_x86_phys_prepare)(vcpu, pfn);

nit: kvm is using kvm_x86_call() in most of cases, e.g.,

		ret = kvm_x86_call(phys_prepare)(vcpu, pfn);

>+	}

>+	if (ret)
>+		return ret;

fold this chunk into the if() statement above to align with tdp_mmu_link_sp()
below?

I'm concerned about handling phys_prepare() failures. Such failures may not be
recoverable. ...

>+	ret = tdp_mmu_set_spte_atomic(vcpu->kvm, iter, spte);
>+	if (pfn && ret)
>+		static_call(kvm_x86_phys_cleanup)(pfn);
>+
>+	return ret;
>+}
>+
> /*
>  * Installs a last-level SPTE to handle a TDP page fault.
>  * (NPT/EPT violation/misconfiguration)
>@@ -1170,7 +1190,7 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu,
> 
> 	if (new_spte == iter->old_spte)
> 		ret = RET_PF_SPURIOUS;
>-	else if (tdp_mmu_set_spte_atomic(vcpu->kvm, iter, new_spte))
>+	else if (tdp_mmu_install_spte(vcpu, iter, new_spte))
> 		return RET_PF_RETRY;

if RET_FP_RETRY is returned here, it could potentially cause an infinite loop.

I think we need a KVM_BUG_ON() somewhere.