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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aCQ05eUM+U9N2bZ7@intel.com>
Date: Wed, 14 May 2025 14:15:01 +0800
From: Chao Gao <chao.gao@...el.com>
To: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
CC: <pbonzini@...hat.com>, <seanjc@...gle.com>, <rick.p.edgecombe@...el.com>,
	<isaku.yamahata@...el.com>, <kai.huang@...el.com>, <yan.y.zhao@...el.com>,
	<tglx@...utronix.de>, <mingo@...hat.com>, <bp@...en8.de>,
	<dave.hansen@...ux.intel.com>, <kvm@...r.kernel.org>, <x86@...nel.org>,
	<linux-coco@...ts.linux.dev>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC, PATCH 08/12] KVM: x86/tdp_mmu: Add phys_prepare() and
 phys_cleanup() to kvm_x86_ops

>+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.