linux-kernel - Re: [PATCHv2 08/12] KVM: TDX: Handle PAMT allocation in fault path

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 for Android: free password hash cracker in your pocket

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

Message-ID: <ypsnjhvy2odzedxtujzaumvz4mxe3dcci7biepsjrate7pzw7d@6h7qwy3xcqld>
Date: Thu, 10 Jul 2025 11:45:26 +0300
From: "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
Cc: "pbonzini@...hat.com" <pbonzini@...hat.com>, 
	"seanjc@...gle.com" <seanjc@...gle.com>, "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>, 
	"Gao, Chao" <chao.gao@...el.com>, "bp@...en8.de" <bp@...en8.de>, 
	"Huang, Kai" <kai.huang@...el.com>, "x86@...nel.org" <x86@...nel.org>, 
	"mingo@...hat.com" <mingo@...hat.com>, "Zhao, Yan Y" <yan.y.zhao@...el.com>, 
	"tglx@...utronix.de" <tglx@...utronix.de>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>, 
	"linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>, "Yamahata, Isaku" <isaku.yamahata@...el.com>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv2 08/12] KVM: TDX: Handle PAMT allocation in fault path

On Thu, Jul 10, 2025 at 01:33:41AM +0000, Edgecombe, Rick P wrote:
> On Mon, 2025-06-09 at 22:13 +0300, Kirill A. Shutemov wrote:
> >  int tdx_sept_set_private_spte(struct kvm *kvm, gfn_t gfn,
> >  			      enum pg_level level, kvm_pfn_t pfn)
> >  {
> > +	struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
> 
> This is unfortunate. In practice, all of the callers will be in a vCPU context,
> but __tdp_mmu_set_spte_atomic() can be called for zap's which is why there is no
> vCPU.

IIUC, __tdp_mmu_set_spte_atomic() to zap, only for shared case which is
!is_mirror_sptep() and will not get us here. !shared case get to
tdx_sept_remove_private_spte().

> We don't want to split the tdp mmu calling code to introduce a variant that has
> a vCPU. 
> 
> What about a big comment? Or checking for NULL and returning -EINVAL like
> PG_LEVEL_4K below? I guess in this case a NULL pointer will be plenty loud. So
> probably a comment is enough.

Yes, comment is helpful here

> Hmm, the only reason we need the vCPU here is to get at the the per-vCPU pamt
> page cache. This is also the reason for the strange callback scheme I was
> complaining about in the other patch. It kind of seems like there are two
> friction points in this series:
> 1. How to allocate dpamt pages
> 2. How to serialize the global DPAMT resource inside a read lock
> 
> I'd like to try to figure out a better solution for (1). (2) seems good. But I'm
> still processing.

I tried few different approached to address the problem. See phys_prepare
and phys_cleanup in v1.

> 
> >  	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
> >  	struct page *page = pfn_to_page(pfn);
> > +	int ret;
> > +
> > +	ret = tdx_pamt_get(page, level, tdx_alloc_pamt_page_atomic, vcpu);
> > +	if (ret)
> > +		return ret;
> 

-- 
  Kiryl Shutsemau / Kirill A. Shutemov